Bug 1238313: Part 2 - Implement tabs.onReplaced event. r?billm draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 17 Feb 2016 11:47:48 -0800
changeset 331669 d39e8938e7444a3cf82d79fa0384ec8cbca0c67c
parent 331668 cf766413a25e77d31495f2b6e7d38e1d1bb40603
child 514421 efd0cd59cc6dfe4bb6fbef78d437cb9d8042ae8e
push id11026
push usermaglione.k@gmail.com
push dateWed, 17 Feb 2016 19:58:26 +0000
reviewersbillm
bugs1238313
milestone47.0a1
Bug 1238313: Part 2 - Implement tabs.onReplaced event. r?billm MozReview-Commit-ID: 67sgzNcxBK0
browser/components/extensions/ext-tabs.js
browser/components/extensions/ext-utils.js
browser/components/extensions/test/browser/browser.ini
browser/components/extensions/test/browser/browser_ext_tabs_events.js
browser/components/extensions/test/browser/browser_ext_tabs_events_e10s.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -11,17 +11,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 var {
   EventManager,
-  ignoreEvent,
 } = ExtensionUtils;
 
 // This function is pretty tightly tied to Extension.jsm.
 // Its job is to fill in the |tab| property of the sender.
 function getSender(context, target, sender) {
   // The message was sent from a content script to a <browser> element.
   // We can just get the |tab| from |target|.
   if (target instanceof Ci.nsIDOMXULElement) {
@@ -243,17 +242,33 @@ extensions.registerSchemaAPI("tabs", nul
         WindowListManager.addCloseListener(windowListener);
         AllWindowEvents.addListener("TabClose", tabListener);
         return () => {
           WindowListManager.removeCloseListener(windowListener);
           AllWindowEvents.removeListener("TabClose", tabListener);
         };
       }).api(),
 
-      onReplaced: ignoreEvent(context, "tabs.onReplaced"),
+      onReplaced: new EventManager(context, "tabs.onReplaced", fire => {
+        // We treat a browser remoteness change as a tab being replaced, since the
+        // effects are more or less the same.
+        let listener = event => {
+          let tab = event.originalTarget;
+
+          let removedTabId = TabManager.getId(tab);
+          TabManager.forgetTab(tab);
+
+          fire(TabManager.getId(tab), removedTabId);
+        };
+
+        AllWindowEvents.addListener("TabRemotenessChange", listener);
+        return () => {
+          AllWindowEvents.removeListener("TabRemotenessChange", listener);
+        };
+      }).api(),
 
       onMoved: new EventManager(context, "tabs.onMoved", fire => {
         // There are certain circumstances where we need to ignore a move event.
         //
         // Namely, the first time the tab is moved after it's created, we need
         // to report the final position as the initial position in the tab's
         // onAttached or onCreated event. This is because most tabs are inserted
         // in a temporary location and then moved after the TabOpen event fires,
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -525,16 +525,24 @@ global.TabManager = {
     }
     this.initListener();
 
     let id = this._nextId++;
     this._tabs.set(tab, id);
     return id;
   },
 
+  /**
+   * Drops the cached ID for the given tab. To be used when a tab is being
+   * changed in such a way that it needs a new logical ID.
+   */
+  forgetTab(tab) {
+    this._tabs.delete(tab);
+  },
+
   getBrowserId(browser) {
     let gBrowser = browser.ownerDocument.defaultView.gBrowser;
     // Some non-browser windows have gBrowser but not
     // getTabForBrowser!
     if (gBrowser && gBrowser.getTabForBrowser) {
       let tab = gBrowser.getTabForBrowser(browser);
       if (tab) {
         return this.getId(tab);
--- a/browser/components/extensions/test/browser/browser.ini
+++ b/browser/components/extensions/test/browser/browser.ini
@@ -20,16 +20,18 @@ support-files =
 [browser_ext_popup_api_injection.js]
 [browser_ext_contextMenus.js]
 [browser_ext_getViews.js]
 [browser_ext_lastError.js]
 [browser_ext_runtime_setUninstallURL.js]
 [browser_ext_tabs_audio.js]
 [browser_ext_tabs_captureVisibleTab.js]
 [browser_ext_tabs_events.js]
+[browser_ext_tabs_events_e10s.js]
+skip-if = !e10s
 [browser_ext_tabs_executeScript.js]
 [browser_ext_tabs_executeScript_good.js]
 [browser_ext_tabs_executeScript_bad.js]
 [browser_ext_tabs_insertCSS.js]
 [browser_ext_tabs_query.js]
 [browser_ext_tabs_getCurrent.js]
 [browser_ext_tabs_create.js]
 [browser_ext_tabs_duplicate.js]
@@ -37,9 +39,9 @@ support-files =
 [browser_ext_tabs_onUpdated.js]
 [browser_ext_tabs_sendMessage.js]
 [browser_ext_tabs_move.js]
 [browser_ext_tabs_move_window.js]
 [browser_ext_windows_create_tabId.js]
 [browser_ext_windows_update.js]
 [browser_ext_contentscript_connect.js]
 [browser_ext_tab_runtimeConnect.js]
-[browser_ext_webNavigation_getFrames.js]
\ No newline at end of file
+[browser_ext_webNavigation_getFrames.js]
--- a/browser/components/extensions/test/browser/browser_ext_tabs_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_events.js
@@ -4,16 +4,20 @@
 
 add_task(function* testTabEvents() {
   function background() {
     let events = [];
     browser.tabs.onCreated.addListener(tab => {
       events.push({type: "onCreated", tab});
     });
 
+    browser.tabs.onReplaced.addListener((addedTabId, removedTabId) => {
+      events.push({type: "onReplaced", addedTabId, removedTabId});
+    });
+
     browser.tabs.onAttached.addListener((tabId, info) => {
       events.push(Object.assign({type: "onAttached", tabId}, info));
     });
 
     browser.tabs.onDetached.addListener((tabId, info) => {
       events.push(Object.assign({type: "onDetached", tabId}, info));
     });
 
@@ -26,16 +30,18 @@ add_task(function* testTabEvents() {
     });
 
     function expectEvents(names) {
       browser.test.log(`Expecting events: ${names.join(", ")}`);
 
       return new Promise(resolve => {
         setTimeout(resolve, 0);
       }).then(() => {
+        browser.test.log(`Got events: ${events.map(event => event.type).join(", ")}`);
+
         browser.test.assertEq(names.length, events.length, "Got expected number of events");
         for (let [i, name] of names.entries()) {
           browser.test.assertEq(name, i in events && events[i].type,
                                 `Got expected ${name} event`);
         }
         return events.splice(0);
       });
     }
@@ -49,17 +55,17 @@ add_task(function* testTabEvents() {
       windowId = windows[0].id;
       let otherWindowId = windows[1].id;
       let initialTab;
 
       return expectEvents(["onCreated"]).then(([created]) => {
         initialTab = created.tab;
 
         browser.test.log("Create tab in window 1");
-        return browser.tabs.create({windowId, index: 0});
+        return browser.tabs.create({windowId, index: 0, url: "about:blank"});
       }).then(tab => {
         let oldIndex = tab.index;
         browser.test.assertEq(0, oldIndex, "Tab has the expected index");
 
         return expectEvents(["onCreated"]).then(([created]) => {
           browser.test.assertEq(tab.id, created.tab.id, "Got expected tab ID");
           browser.test.assertEq(oldIndex, created.tab.index, "Got expected tab index");
 
copy from browser/components/extensions/test/browser/browser_ext_tabs_events.js
copy to browser/components/extensions/test/browser/browser_ext_tabs_events_e10s.js
--- a/browser/components/extensions/test/browser/browser_ext_tabs_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_events_e10s.js
@@ -4,16 +4,20 @@
 
 add_task(function* testTabEvents() {
   function background() {
     let events = [];
     browser.tabs.onCreated.addListener(tab => {
       events.push({type: "onCreated", tab});
     });
 
+    browser.tabs.onReplaced.addListener((addedTabId, removedTabId) => {
+      events.push({type: "onReplaced", addedTabId, removedTabId});
+    });
+
     browser.tabs.onAttached.addListener((tabId, info) => {
       events.push(Object.assign({type: "onAttached", tabId}, info));
     });
 
     browser.tabs.onDetached.addListener((tabId, info) => {
       events.push(Object.assign({type: "onDetached", tabId}, info));
     });
 
@@ -26,140 +30,74 @@ add_task(function* testTabEvents() {
     });
 
     function expectEvents(names) {
       browser.test.log(`Expecting events: ${names.join(", ")}`);
 
       return new Promise(resolve => {
         setTimeout(resolve, 0);
       }).then(() => {
+        browser.test.log(`Got events: ${events.map(event => event.type).join(", ")}`);
+
         browser.test.assertEq(names.length, events.length, "Got expected number of events");
         for (let [i, name] of names.entries()) {
           browser.test.assertEq(name, i in events && events[i].type,
                                 `Got expected ${name} event`);
         }
         return events.splice(0);
       });
     }
 
-    browser.test.log("Create second browser window");
     let windowId;
-    Promise.all([
-      browser.windows.getCurrent(),
-      browser.windows.create({}),
-    ]).then(windows => {
-      windowId = windows[0].id;
-      let otherWindowId = windows[1].id;
-      let initialTab;
-
-      return expectEvents(["onCreated"]).then(([created]) => {
-        initialTab = created.tab;
-
-        browser.test.log("Create tab in window 1");
-        return browser.tabs.create({windowId, index: 0});
-      }).then(tab => {
-        let oldIndex = tab.index;
-        browser.test.assertEq(0, oldIndex, "Tab has the expected index");
-
-        return expectEvents(["onCreated"]).then(([created]) => {
-          browser.test.assertEq(tab.id, created.tab.id, "Got expected tab ID");
-          browser.test.assertEq(oldIndex, created.tab.index, "Got expected tab index");
-
-          browser.test.log("Move tab to window 2");
-          return browser.tabs.move([tab.id], {windowId: otherWindowId, index: 0});
-        }).then(() => {
-          return expectEvents(["onDetached", "onAttached"]);
-        }).then(([detached, attached]) => {
-          browser.test.assertEq(oldIndex, detached.oldPosition, "Expected old index");
-          browser.test.assertEq(windowId, detached.oldWindowId, "Expected old window ID");
-
-          browser.test.assertEq(0, attached.newPosition, "Expected new index");
-          browser.test.assertEq(otherWindowId, attached.newWindowId, "Expected new window ID");
-
-          browser.test.log("Move tab within the same window");
-          return browser.tabs.move([tab.id], {index: 1});
-        }).then(([moved]) => {
-          browser.test.assertEq(1, moved.index, "Expected new index");
+    browser.windows.getCurrent().then(win => {
+      windowId = win.id;
 
-          return expectEvents(["onMoved"]);
-        }).then(([moved]) => {
-          browser.test.assertEq(tab.id, moved.tabId, "Expected tab ID");
-          browser.test.assertEq(0, moved.fromIndex, "Expected old index");
-          browser.test.assertEq(1, moved.toIndex, "Expected new index");
-          browser.test.assertEq(otherWindowId, moved.windowId, "Expected window ID");
-
-          browser.test.log("Remove tab");
-          return browser.tabs.remove(tab.id);
-        }).then(() => {
-          return expectEvents(["onRemoved"]);
-        }).then(([removed]) => {
-          browser.test.assertEq(tab.id, removed.tabId, "Expected removed tab ID");
-          browser.test.assertEq(otherWindowId, removed.windowId, "Expected removed tab window ID");
-          // Note: We want to test for the actual boolean value false here.
-          browser.test.assertEq(false, removed.isWindowClosing, "Expected isWindowClosing value");
+      browser.test.log("Create tab");
+      return browser.tabs.create({windowId});
+    }).then(initialTab => {
+      return expectEvents(["onCreated"]).then(([created]) => {
+        browser.test.assertEq(initialTab.id, created.tab.id, "Expected tab ID");
 
-          browser.test.log("Close second window");
-          return browser.windows.remove(otherWindowId);
-        }).then(() => {
-          return expectEvents(["onRemoved"]);
-        }).then(([removed]) => {
-          browser.test.assertEq(initialTab.id, removed.tabId, "Expected removed tab ID");
-          browser.test.assertEq(otherWindowId, removed.windowId, "Expected removed tab window ID");
-          browser.test.assertEq(true, removed.isWindowClosing, "Expected isWindowClosing value");
-        });
-      });
-    }).then(() => {
-      browser.test.log("Create additional tab in window 1");
-      return browser.tabs.create({windowId, url: "about:blank"});
-    }).then(tab => {
-      return expectEvents(["onCreated"]).then(() => {
-        browser.test.log("Create a new window, adopting the new tab");
-
-        // We have to explicitly wait for the event here, since its timing is
-        // not predictable.
-        let promiseAttached = new Promise(resolve => {
-          browser.tabs.onAttached.addListener(function listener(tabId) {
-            browser.tabs.onAttached.removeListener(listener);
+        // Wait for an onReplaced event, since we can't rely on timing in this
+        // case.
+        let promise = new Promise(resolve => {
+          let listener = (addedTabId, removedTabId) => {
+            browser.tabs.onReplaced.removeListener(listener);
             resolve();
-          });
+          };
+          browser.tabs.onReplaced.addListener(listener);
         });
 
-        return Promise.all([
-          browser.windows.create({tabId: tab.id}),
-          promiseAttached,
-        ]);
-      }).then(([window]) => {
-        return expectEvents(["onDetached", "onAttached"]).then(([detached, attached]) => {
-          browser.test.assertEq(tab.id, detached.tabId, "Expected onDetached tab ID");
+        browser.test.log("Load a URL which requires a remote browser");
+        browser.tabs.update(initialTab.id, {url: "http://example.com/"});
+        return promise;
+      }).then(() => {
+        return expectEvents(["onReplaced"]);
+      }).then(([replaced]) => {
+        browser.test.assertEq(initialTab.id, replaced.removedTabId, "Tab was replaced");
+        browser.test.assertTrue(initialTab.addedTabId != replaced.removedTabId, "Tab has a new ID");
 
-          browser.test.assertEq(tab.id, attached.tabId, "Expected onAttached tab ID");
-          browser.test.assertEq(0, attached.newPosition, "Expected onAttached new index");
-          browser.test.assertEq(window.id, attached.newWindowId,
-                                "Expected onAttached new window id");
-
-          browser.test.log("Close the new window");
-          return browser.windows.remove(window.id);
-        });
+        return browser.tabs.remove(replaced.addedTabId);
+      }).then(() => {
+        browser.test.notifyPass("tabs-events-e10s");
       });
-    }).then(() => {
-      browser.test.notifyPass("tabs-events");
     }).catch(e => {
       try {
         browser.test.fail(`${e} :: ${e.stack}`);
       } catch (ex) {
         throw e;
       }
-      browser.test.notifyFail("tabs-events");
+      browser.test.notifyFail("tabs-events-e10s");
     });
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "permissions": ["tabs"],
     },
 
     background,
   });
 
   yield extension.startup();
-  yield extension.awaitFinish("tabs-events");
+  yield extension.awaitFinish("tabs-events-e10s");
   yield extension.unload();
 });