Bug 1403349 - Fix wrong sourceTabId on webNavigation.onCreatedTarget event. draft
authorLuca Greco <lgreco@mozilla.com>
Wed, 27 Sep 2017 14:24:54 +0200
changeset 681613 5a2d4ceaeffdba44e63d82deace8ce8df89d2527
parent 681450 0d9c6250f99dc4b6aa1a94f5260737d046c52b1e
child 681614 4085e82040b2f64acd8eff681185d5c8eab5804d
push id84871
push userluca.greco@alcacoop.it
push dateTue, 17 Oct 2017 13:50:36 +0000
bugs1403349, 1355120
milestone58.0a1
Bug 1403349 - Fix wrong sourceTabId on webNavigation.onCreatedTarget event. This changes fixes the regression introduced by Bug 1355120 and adds a new test case which contains a browserAction popup which open and immediately close a new window and ensure that the received onCreatedNavigationTarget is the expected one. MozReview-Commit-ID: JIcVCpBTpxj
browser/components/extensions/test/browser/browser_ext_webNavigation_onCreatedNavigationTarget_window_open.js
toolkit/modules/addons/WebNavigation.jsm
toolkit/modules/addons/WebNavigationContent.js
--- a/browser/components/extensions/test/browser/browser_ext_webNavigation_onCreatedNavigationTarget_window_open.js
+++ b/browser/components/extensions/test/browser/browser_ext_webNavigation_onCreatedNavigationTarget_window_open.js
@@ -53,50 +53,50 @@ async function runTestCase({extension, o
   is(webNavMsg.sourceTabId, sourceTabId, "Got the expected sourceTabId property");
   is(webNavMsg.sourceFrameId, sourceFrameId, "Got the expected sourceFrameId property");
   is(webNavMsg.url, url, "Got the expected url property");
 
   is(completedNavMsg.tabId, createdTabId, "Got the expected webNavigation.onCompleted tabId property");
   is(completedNavMsg.url, url, "Got the expected webNavigation.onCompleted url property");
 }
 
-add_task(async function test_on_created_navigation_target_from_window_open() {
+add_task(async function test_window_open() {
   const tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, SOURCE_PAGE);
 
   gBrowser.selectedTab = tab1;
 
   const extension = ExtensionTestUtils.loadExtension({
     background,
     manifest: {
       permissions: ["webNavigation", "tabs", "<all_urls>"],
     },
   });
 
   await extension.startup();
 
   const expectedSourceTab = await extension.awaitMessage("expectedSourceTab");
 
-  info("open an url in a new tab from a window.open call");
+  info("open a url in a new tab from a window.open call");
 
   await runTestCase({
     extension,
     openNavTarget() {
       extension.sendMessage({
         type: "execute-contentscript",
         code: `window.open("${OPENED_PAGE}#new-tab-from-window-open"); true;`,
       });
     },
     expectedWebNavProps: {
       sourceTabId: expectedSourceTab.sourceTabId,
       sourceFrameId: 0,
       url: `${OPENED_PAGE}#new-tab-from-window-open`,
     },
   });
 
-  info("open an url in a new window from a window.open call");
+  info("open a url in a new window from a window.open call");
 
   await runTestCase({
     extension,
     openNavTarget() {
       extension.sendMessage({
         type: "execute-contentscript",
         code: `window.open("${OPENED_PAGE}#new-win-from-window-open", "_blank", "toolbar=0"); true;`,
       });
@@ -108,50 +108,50 @@ add_task(async function test_on_created_
     },
   });
 
   await BrowserTestUtils.removeTab(tab1);
 
   await extension.unload();
 });
 
-add_task(async function test_on_created_navigation_target_from_window_open_subframe() {
+add_task(async function test_window_open_from_subframe() {
   const tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, SOURCE_PAGE);
 
   gBrowser.selectedTab = tab1;
 
   const extension = ExtensionTestUtils.loadExtension({
     background,
     manifest: {
       permissions: ["webNavigation", "tabs", "<all_urls>"],
     },
   });
 
   await extension.startup();
 
   const expectedSourceTab = await extension.awaitMessage("expectedSourceTab");
 
-  info("open an url in a new tab from subframe window.open call");
+  info("open a url in a new tab from subframe window.open call");
 
   await runTestCase({
     extension,
     openNavTarget() {
       extension.sendMessage({
         type: "execute-contentscript",
         code: `document.querySelector('iframe').contentWindow.open("${OPENED_PAGE}#new-tab-from-window-open-subframe"); true;`,
       });
     },
     expectedWebNavProps: {
       sourceTabId: expectedSourceTab.sourceTabId,
       sourceFrameId: expectedSourceTab.sourceTabFrames[1].frameId,
       url: `${OPENED_PAGE}#new-tab-from-window-open-subframe`,
     },
   });
 
-  info("open an url in a new window from subframe window.open call");
+  info("open a url in a new window from subframe window.open call");
 
   await runTestCase({
     extension,
     openNavTarget() {
       extension.sendMessage({
         type: "execute-contentscript",
         code: `document.querySelector('iframe').contentWindow.open("${OPENED_PAGE}#new-win-from-window-open-subframe", "_blank", "toolbar=0"); true;`,
       });
@@ -162,8 +162,72 @@ add_task(async function test_on_created_
       url: `${OPENED_PAGE}#new-win-from-window-open-subframe`,
     },
   });
 
   await BrowserTestUtils.removeTab(tab1);
 
   await extension.unload();
 });
+
+add_task(async function test_window_open_close_from_browserAction_popup() {
+  const tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, SOURCE_PAGE);
+
+  gBrowser.selectedTab = tab1;
+
+  function popup() {
+    window.open("", "_self").close();
+
+    browser.test.sendMessage("browserAction_popup_executed");
+  }
+
+  const extension = ExtensionTestUtils.loadExtension({
+    background,
+    manifest: {
+      browser_action: {
+        default_popup: "popup.html",
+      },
+      permissions: ["webNavigation", "tabs", "<all_urls>"],
+    },
+    files: {
+      "popup.html": `<!DOCTYPE html>
+        <html>
+          <head>
+            <meta charset="utf-8">
+          </head>
+          <body>
+            <script src="popup.js"></script>
+          </body>
+        </html>
+      `,
+      "popup.js": popup,
+    },
+  });
+
+  await extension.startup();
+
+  const expectedSourceTab = await extension.awaitMessage("expectedSourceTab");
+
+  clickBrowserAction(extension);
+
+  await extension.awaitMessage("browserAction_popup_executed");
+
+  info("open a url in a new tab from a window.open call");
+
+  await runTestCase({
+    extension,
+    openNavTarget() {
+      extension.sendMessage({
+        type: "execute-contentscript",
+        code: `window.open("${OPENED_PAGE}#new-tab-from-window-open"); true;`,
+      });
+    },
+    expectedWebNavProps: {
+      sourceTabId: expectedSourceTab.sourceTabId,
+      sourceFrameId: 0,
+      url: `${OPENED_PAGE}#new-tab-from-window-open`,
+    },
+  });
+
+  await BrowserTestUtils.removeTab(tab1);
+
+  await extension.unload();
+});
--- a/toolkit/modules/addons/WebNavigation.jsm
+++ b/toolkit/modules/addons/WebNavigation.jsm
@@ -301,29 +301,36 @@ var Manager = {
       let where = ownerWin.whereToOpenLink(data);
       if (where == "current") {
         this.setRecentTabTransitionData({link: true});
       }
     }
   },
 
   onCreatedNavigationTarget(browser, data) {
-    const {isSourceTab, createdWindowId, sourceFrameId, url} = data;
+    const {
+      createdOuterWindowId,
+      isSourceTab,
+      sourceFrameId,
+      url,
+    } = data;
 
-    // We are going to potentially received two message manager messages for a single
-    // onCreatedNavigationTarget event that is happening in the child process,
-    // we are going to use the generate uuid to pair them together.
-    const pairedMessage = this.createdNavigationTargetByOuterWindowId.get(createdWindowId);
+    // We are going to receive two message manager messages for a single
+    // onCreatedNavigationTarget event related to a window.open that is happening
+    // in the child process (one from the source tab and one from the created tab),
+    // the unique createdWindowId (the outerWindowID of the created docShell)
+    // to pair them together.
+    const pairedMessage = this.createdNavigationTargetByOuterWindowId.get(createdOuterWindowId);
 
     if (!pairedMessage) {
-      this.createdNavigationTargetByOuterWindowId.set(createdWindowId, {browser, data});
+      this.createdNavigationTargetByOuterWindowId.set(createdOuterWindowId, {browser, data});
       return;
     }
 
-    this.createdNavigationTargetByOuterWindowId.delete(createdWindowId);
+    this.createdNavigationTargetByOuterWindowId.delete(createdOuterWindowId);
 
     let sourceTabBrowser;
     let createdTabBrowser;
 
     if (isSourceTab) {
       sourceTabBrowser = browser;
       createdTabBrowser = pairedMessage.browser;
     } else {
--- a/toolkit/modules/addons/WebNavigationContent.js
+++ b/toolkit/modules/addons/WebNavigationContent.js
@@ -5,16 +5,27 @@
 var Ci = Components.interfaces;
 
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 Components.utils.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "WebNavigationFrames",
                                   "resource://gre/modules/WebNavigationFrames.jsm");
 
+function getDocShellOuterWindowId(docShell) {
+  if (!docShell) {
+    return undefined;
+  }
+
+  return docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+                 .getInterface(Ci.nsIDOMWindow)
+                 .getInterface(Ci.nsIDOMWindowUtils)
+                 .outerWindowID;
+}
+
 function loadListener(event) {
   let document = event.target;
   let window = document.defaultView;
   let url = document.documentURI;
   let frameId = WebNavigationFrames.getFrameId(window);
   let parentFrameId = WebNavigationFrames.getParentFrameId(window);
   sendAsyncMessage("Extension:DOMContentLoaded", {frameId, parentFrameId, url});
 }
@@ -51,28 +62,29 @@ var CreatedNavigationTargetListener = {
       // if the createdNavigationTarget is not related to this docShell
       // (this docShell is not the newly created docShell, it is not the source docShell,
       // and the source docShell is not a descendant of it)
       // there is nothing to do here and return early.
       return;
     }
 
     const isSourceTab = docShell === sourceDocShell || isSourceTabDescendant;
+
     const sourceFrameId = WebNavigationFrames.getDocShellFrameId(sourceDocShell);
-    const createdWindowId = WebNavigationFrames.getDocShellFrameId(createdDocShell);
+    const createdOuterWindowId = getDocShellOuterWindowId(sourceDocShell);
 
     let url;
     if (props.hasKey("url")) {
       url = props.getPropertyAsACString("url");
     }
 
     sendAsyncMessage("Extension:CreatedNavigationTarget", {
       url,
       sourceFrameId,
-      createdWindowId,
+      createdOuterWindowId,
       isSourceTab,
     });
   },
 };
 
 var FormSubmitListener = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
                                          Ci.nsIFormSubmitObserver,