Bug 1276025: Stop using injectInDocShell to tag docShells with types. r=billm
authorKris Maglione <maglione.k@gmail.com>
Thu, 02 Jun 2016 20:53:41 -0700
changeset 300827 a7b31be0a19ae5413db3e10ad4e5251cac84b08d
parent 300826 b93f2b53838a689fd175ad4b986f43feb4a2546c
child 300828 1401fcd673699f8d6e431ccb643ab46e166da2a9
child 300907 33230ff64650d566863edb1891aaae227a580067
push id19597
push usermaglione.k@gmail.com
push dateWed, 08 Jun 2016 00:58:36 +0000
treeherderfx-team@a7b31be0a19a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1276025
milestone50.0a1
Bug 1276025: Stop using injectInDocShell to tag docShells with types. r=billm MozReview-Commit-ID: 7h5PI2birY4
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_incognito_popup.js
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ext-backgroundPage.js
toolkit/mozapps/extensions/test/xpcshell/test_webextension.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -52,19 +52,18 @@ var pageDataMap = new WeakMap();
 // ExtensionContext.
 extensions.on("page-load", (type, page, params, sender, delegate) => {
   if (params.type == "tab" || params.type == "popup") {
     let browser = params.docShell.chromeEventHandler;
 
     let parentWindow = browser.ownerDocument.defaultView;
     page.windowId = WindowManager.getId(parentWindow);
 
-    let tab = null;
-    if (params.type == "tab") {
-      tab = parentWindow.gBrowser.getTabForBrowser(browser);
+    let tab = parentWindow.gBrowser.getTabForBrowser(browser);
+    if (tab) {
       sender.tabId = TabManager.getId(tab);
       page.tabId = TabManager.getId(tab);
     }
 
     pageDataMap.set(page, {tab, parentWindow});
   }
 
   delegate.getSender = getSender;
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -186,22 +186,20 @@ class BasePopup {
     this.browserReady.then(() => {
       this.browser.removeEventListener("DOMWindowCreated", this, true);
       this.browser.removeEventListener("load", this, true);
       this.browser.removeEventListener("DOMTitleChanged", this, true);
       this.browser.removeEventListener("DOMWindowClose", this, true);
 
       this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
 
-      this.context.unload();
       this.browser.remove();
 
       this.browser = null;
       this.viewNode = null;
-      this.context = null;
     });
   }
 
   // Returns the name of the event fired on `viewNode` when the popup is being
   // destroyed. This must be implemented by every subclass.
   get DESTROY_EVENT() {
     throw new Error("Not implemented");
   }
@@ -247,16 +245,17 @@ class BasePopup {
     }
   }
 
   createBrowser(viewNode, popupURI) {
     let document = viewNode.ownerDocument;
     this.browser = document.createElementNS(XUL_NS, "browser");
     this.browser.setAttribute("type", "content");
     this.browser.setAttribute("disableglobalhistory", "true");
+    this.browser.setAttribute("webextension-view-type", "popup");
 
     // Note: When using noautohide panels, the popup manager will add width and
     // height attributes to the panel, breaking our resize code, if the browser
     // starts out smaller than 30px by 10px. This isn't an issue now, but it
     // will be if and when we popup debugging.
 
     // This overrides the content's preferred size when displayed in a
     // fixed-size, slide-in panel.
@@ -275,25 +274,17 @@ class BasePopup {
       this.browser.addEventListener("load", loadListener, true);
     }).then(() => {
       let {contentWindow} = this.browser;
 
       contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
                    .getInterface(Ci.nsIDOMWindowUtils)
                    .allowScriptsToClose();
 
-      this.context = new ExtensionContext(this.extension, {
-        type: "popup",
-        contentWindow,
-        uri: popupURI,
-        docShell: this.browser.docShell,
-      });
-
-      GlobalManager.injectInDocShell(this.browser.docShell, this.extension, this.context);
-      this.browser.setAttribute("src", this.context.uri.spec);
+      this.browser.setAttribute("src", popupURI.spec);
 
       this.browser.addEventListener("DOMWindowCreated", this, true);
       this.browser.addEventListener("load", this, true);
       this.browser.addEventListener("DOMTitleChanged", this, true);
       this.browser.addEventListener("DOMWindowClose", this, true);
     });
   }
 
--- a/browser/components/extensions/test/browser/browser.ini
+++ b/browser/components/extensions/test/browser/browser.ini
@@ -25,16 +25,17 @@ support-files =
 [browser_ext_commands_execute_page_action.js]
 [browser_ext_commands_getAll.js]
 [browser_ext_commands_onCommand.js]
 [browser_ext_contentscript_connect.js]
 [browser_ext_contextMenus.js]
 [browser_ext_currentWindow.js]
 [browser_ext_getViews.js]
 [browser_ext_history.js]
+[browser_ext_incognito_popup.js]
 [browser_ext_lastError.js]
 [browser_ext_optionsPage_privileges.js]
 [browser_ext_pageAction_context.js]
 [browser_ext_pageAction_popup.js]
 [browser_ext_pageAction_simple.js]
 [browser_ext_popup_api_injection.js]
 [browser_ext_runtime_openOptionsPage.js]
 [browser_ext_runtime_setUninstallURL.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_incognito_popup.js
@@ -0,0 +1,107 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(function* testIncognitoPopup() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["tabs"],
+      "browser_action": {
+        "default_popup": "popup.html",
+      },
+      "page_action": {
+        "default_popup": "popup.html",
+      },
+    },
+
+    background() {
+      let resolveMessage;
+      browser.runtime.onMessage.addListener(msg => {
+        if (resolveMessage && msg.message == "popup-details") {
+          resolveMessage(msg);
+        }
+      });
+
+      let awaitPopup = windowId => {
+        return new Promise(resolve => {
+          resolveMessage = resolve;
+        }).then(msg => {
+          browser.test.assertEq(windowId, msg.windowId, "Got popup message from correct window");
+          return msg;
+        });
+      };
+
+      let testWindow = window => {
+        return browser.tabs.query({active: true, windowId: window.id}).then(([tab]) => {
+          return browser.pageAction.show(tab.id);
+        }).then(() => {
+          browser.test.sendMessage("click-pageAction");
+
+          return awaitPopup(window.id);
+        }).then(msg => {
+          browser.test.assertEq(window.incognito, msg.incognito, "Correct incognito status in pageAction popup");
+
+          browser.test.sendMessage("click-browserAction");
+
+          return awaitPopup(window.id);
+        }).then(msg => {
+          browser.test.assertEq(window.incognito, msg.incognito, "Correct incognito status in browserAction popup");
+        });
+      };
+
+      const URL = "http://example.com/incognito";
+      let windowReady = new Promise(resolve => {
+        browser.tabs.onUpdated.addListener(function listener(tabId, changed, tab) {
+          if (changed.status == "complete" && tab.url == URL) {
+            browser.tabs.onUpdated.removeListener(listener);
+            resolve();
+          }
+        });
+      });
+
+      browser.windows.getCurrent().then(window => {
+        return testWindow(window);
+      }).then(() => {
+        return browser.windows.create({incognito: true, url: URL});
+      }).then(window => {
+        return windowReady.then(() => {
+          return testWindow(window);
+        }).then(() => {
+          return browser.windows.remove(window.id);
+        });
+      }).then(() => {
+        browser.test.notifyPass("incognito");
+      }).catch(error => {
+        browser.test.fail(`Error: ${error} :: ${error.stack}`);
+        browser.test.notifyFail("incognito");
+      });
+    },
+
+    files: {
+      "popup.html": '<html><head><meta charset="utf-8"><script src="popup.js"></script></head></html>',
+
+      "popup.js": function() {
+        browser.windows.getCurrent().then(win => {
+          browser.runtime.sendMessage({
+            message: "popup-details",
+            windowId: win.id,
+            incognito: browser.extension.inIncognitoContext,
+          });
+          window.close();
+        });
+      },
+    },
+  });
+
+  extension.onMessage("click-browserAction", () => {
+    clickBrowserAction(extension, Services.wm.getMostRecentWindow("navigator:browser"));
+  });
+
+  extension.onMessage("click-pageAction", () => {
+    clickPageAction(extension, Services.wm.getMostRecentWindow("navigator:browser"));
+  });
+
+  yield extension.startup();
+  yield extension.awaitFinish("incognito");
+  yield extension.unload();
+});
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -477,69 +477,56 @@ ParentAPIManager.init();
 
 // For extensions that have called setUninstallURL(), send an event
 // so the browser can display the URL.
 let UninstallObserver = {
   init: function() {
     AddonManager.addAddonListener(this);
   },
 
+  uninit: function() {
+    AddonManager.removeAddonListener(this);
+  },
+
   onUninstalling: function(addon) {
     let extension = GlobalManager.extensionMap.get(addon.id);
     if (extension) {
       Management.emit("uninstall", extension);
     }
   },
 };
 
 // Responsible for loading extension APIs into the right globals.
 GlobalManager = {
-  // Number of extensions currently enabled.
-  count: 0,
-
-  // Map[docShell -> {extension, context}] where context is an ExtensionContext.
-  docShells: new Map(),
-
   // Map[extension ID -> Extension]. Determines which extension is
   // responsible for content under a particular extension ID.
   extensionMap: new Map(),
 
   init(extension) {
-    if (this.count == 0) {
+    if (this.extensionMap.size == 0) {
       Services.obs.addObserver(this, "content-document-global-created", false);
       UninstallObserver.init();
     }
-    this.count++;
 
     this.extensionMap.set(extension.id, extension);
   },
 
   uninit(extension) {
-    this.count--;
-    if (this.count == 0) {
-      Services.obs.removeObserver(this, "content-document-global-created");
-    }
+    this.extensionMap.delete(extension.id);
 
-    for (let [docShell, data] of this.docShells) {
-      if (extension == data.extension) {
-        this.docShells.delete(docShell);
-      }
+    if (this.extensionMap.size == 0) {
+      Services.obs.removeObserver(this, "content-document-global-created");
+      UninstallObserver.uninit();
     }
-
-    this.extensionMap.delete(extension.id);
   },
 
   getExtension(extensionId) {
     return this.extensionMap.get(extensionId);
   },
 
-  injectInDocShell(docShell, extension, context) {
-    this.docShells.set(docShell, {extension, context});
-  },
-
   injectInObject(extension, context, defaultCallback, dest, namespaces = null) {
     let api = Management.generateAPIs(extension, context, Management.apis, namespaces);
     injectAPI(api, dest);
 
     let schemaApi = Management.generateAPIs(extension, context, Management.schemaApis, namespaces);
 
     // Add in any extra API namespaces which do not have implementations
     // outside of their schema file.
@@ -632,48 +619,45 @@ GlobalManager = {
     }
 
     // We don't inject privileged APIs if the addonId is null
     // or doesn't exist.
     if (!this.extensionMap.has(id)) {
       return;
     }
 
+
     let docShell = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
-                                .getInterface(Ci.nsIWebNavigation)
-                                .QueryInterface(Ci.nsIDocShellTreeItem)
-                                .sameTypeRootTreeItem
-                                .QueryInterface(Ci.nsIDocShell);
+                                .getInterface(Ci.nsIDocShell);
+
+    let parentDocument = docShell.parent.QueryInterface(Ci.nsIDocShell)
+                                 .contentViewer.DOMDocument;
 
-    if (this.docShells.has(docShell)) {
-      let {extension, context} = this.docShells.get(docShell);
-      if (context && extension.id == id) {
-        inject(extension, context);
-      }
-      return;
+    let browser = docShell.chromeEventHandler;
+    // If this is a sub-frame of the add-on manager, use that <browser>
+    // element rather than the top-level chrome event handler.
+    if (contentWindow.frameElement && parentDocument.documentURI == "about:addons") {
+      browser = contentWindow.frameElement;
     }
 
+    let type = "tab";
+    if (browser.hasAttribute("webextension-view-type")) {
+      type = browser.getAttribute("webextension-view-type");
+    } else if (browser.classList.contains("inline-options-browser")) {
+      // Options pages are currently displayed inline, but in Chrome
+      // and in our UI mock-ups for a later milestone, they're
+      // pop-ups.
+      type = "popup";
+    }
+
+
     let extension = this.extensionMap.get(id);
     let uri = contentWindow.document.documentURIObject;
     let incognito = PrivateBrowsingUtils.isContentWindowPrivate(contentWindow);
 
-    let browser = docShell.chromeEventHandler;
-
-    let type = "tab";
-    if (browser instanceof Ci.nsIDOMElement) {
-      if (browser.hasAttribute("webextension-view-type")) {
-        type = browser.getAttribute("webextension-view-type");
-      } else if (browser.classList.contains("inline-options-browser")) {
-        // Options pages are currently displayed inline, but in Chrome
-        // and in our UI mock-ups for a later milestone, they're
-        // pop-ups.
-        type = "popup";
-      }
-    }
-
     let context = new ExtensionContext(extension, {type, contentWindow, uri, docShell, incognito});
     inject(extension, context);
 
     let eventHandler = docShell.chromeEventHandler;
     let listener = event => {
       if (event.target != docShell.contentViewer.DOMDocument) {
         return;
       }
--- a/toolkit/components/extensions/ext-backgroundPage.js
+++ b/toolkit/components/extensions/ext-backgroundPage.js
@@ -33,44 +33,40 @@ BackgroundPage.prototype = {
       url = this.extension.baseURI.resolve("_blank.html");
     }
 
     if (!this.extension.isExtensionURL(url)) {
       this.extension.manifestError("Background page must be a file within the extension");
       url = this.extension.baseURI.resolve("_blank.html");
     }
 
-    let uri = Services.io.newURI(url, null, null);
+    let system = Services.scriptSecurityManager.getSystemPrincipal();
 
-    let system = Services.scriptSecurityManager.getSystemPrincipal();
-    let interfaceRequestor = chromeWebNav.QueryInterface(Ci.nsIInterfaceRequestor);
-    let chromeShell = interfaceRequestor.getInterface(Ci.nsIDocShell);
+    let chromeShell = chromeWebNav.QueryInterface(Ci.nsIInterfaceRequestor)
+                                  .getInterface(Ci.nsIDocShell);
     chromeShell.createAboutBlankContentViewer(system);
 
     let chromeDoc = chromeWebNav.document;
     const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
     let browser = chromeDoc.createElementNS(XUL_NS, "browser");
     browser.setAttribute("type", "content");
     browser.setAttribute("disableglobalhistory", "true");
+    browser.setAttribute("webextension-view-type", "background");
     chromeDoc.body.appendChild(browser);
 
     let frameLoader = browser.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader;
     let docShell = frameLoader.docShell;
 
-    this.context = new ExtensionContext(this.extension, {type: "background", docShell, uri});
-    GlobalManager.injectInDocShell(docShell, this.extension, this.context);
-
     let webNav = docShell.QueryInterface(Ci.nsIWebNavigation);
     this.webNav = webNav;
 
     webNav.loadURI(url, 0, null, null, null);
 
     let window = webNav.document.defaultView;
     this.contentWindow = window;
-    this.context.contentWindow = window;
 
     if (this.extension.addonData.instanceID) {
       AddonManager.getAddonByInstanceID(this.extension.addonData.instanceID)
                   .then(addon => addon.setDebugGlobal(window));
     }
 
     // TODO: Right now we run onStartup after the background page
     // finishes. See if this is what Chrome does.
@@ -87,17 +83,17 @@ BackgroundPage.prototype = {
             require("devtools/client/framework/devtools-browser");
             let hudservice = require("devtools/client/webconsole/hudservice");
             hudservice.toggleBrowserConsole().catch(Cu.reportError);
           } else {
             // the Browser Console was already open
             consoleWindow.focus();
           }
 
-          this.context.contentWindow.console.warn("alert() is not supported in background windows; please use console.log instead.");
+          this.contentWindow.console.warn("alert() is not supported in background windows; please use console.log instead.");
 
           alertDisplayedWarning = true;
         }
 
         window.console.log(text);
       };
       Components.utils.exportFunction(alertOverwrite, window, {
         defineAs: "alert",
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js
@@ -32,26 +32,25 @@ function promiseInstallWebExtension(aDat
     Services.obs.notifyObservers(addonFile, "flush-cache-entry", null);
     return promiseAddonStartup();
   }).then(() => {
     return promiseAddonByID(aData.id);
   });
 }
 
 add_task(function*() {
-  do_check_eq(GlobalManager.count, 0);
-  do_check_false(GlobalManager.extensionMap.has(ID));
+  equal(GlobalManager.extensionMap.size, 0);
 
   yield Promise.all([
     promiseInstallAllFiles([do_get_addon("webextension_1")], true),
     promiseAddonStartup()
   ]);
 
-  do_check_eq(GlobalManager.count, 1);
-  do_check_true(GlobalManager.extensionMap.has(ID));
+  equal(GlobalManager.extensionMap.size, 1);
+  ok(GlobalManager.extensionMap.has(ID));
 
   let chromeReg = AM_Cc["@mozilla.org/chrome/chrome-registry;1"].
                   getService(AM_Ci.nsIChromeRegistry);
   try {
     chromeReg.convertChromeURL(NetUtil.newURI("chrome://webex/content/webex.xul"));
     do_throw("Chrome manifest should not have been registered");
   }
   catch (e) {
@@ -72,24 +71,23 @@ add_task(function*() {
   let uri = do_get_addon_root_uri(profileDir, ID);
 
   do_check_eq(addon.iconURL, uri + "icon48.png");
   do_check_eq(addon.icon64URL, uri + "icon64.png");
 
   // Should persist through a restart
   yield promiseShutdownManager();
 
-  do_check_eq(GlobalManager.count, 0);
-  do_check_false(GlobalManager.extensionMap.has(ID));
+  equal(GlobalManager.extensionMap.size, 0);
 
   startupManager();
   yield promiseAddonStartup();
 
-  do_check_eq(GlobalManager.count, 1);
-  do_check_true(GlobalManager.extensionMap.has(ID));
+  equal(GlobalManager.extensionMap.size, 1);
+  ok(GlobalManager.extensionMap.has(ID));
 
   addon = yield promiseAddonByID(ID);
   do_check_neq(addon, null);
   do_check_eq(addon.version, "1.0");
   do_check_eq(addon.name, "Web Extension Name");
   do_check_true(addon.isCompatible);
   do_check_false(addon.appDisabled);
   do_check_true(addon.isActive);
@@ -102,28 +100,27 @@ add_task(function*() {
 
   uri = do_get_addon_root_uri(profileDir, ID);
 
   do_check_eq(addon.iconURL, uri + "icon48.png");
   do_check_eq(addon.icon64URL, uri + "icon64.png");
 
   addon.userDisabled = true;
 
-  do_check_eq(GlobalManager.count, 0);
-  do_check_false(GlobalManager.extensionMap.has(ID));
+  equal(GlobalManager.extensionMap.size, 0);
 
   addon.userDisabled = false;
   yield promiseAddonStartup();
 
-  do_check_eq(GlobalManager.count, 1);
-  do_check_true(GlobalManager.extensionMap.has(ID));
+  equal(GlobalManager.extensionMap.size, 1);
+  ok(GlobalManager.extensionMap.has(ID));
 
   addon.uninstall();
 
-  do_check_eq(GlobalManager.count, 0);
+  equal(GlobalManager.extensionMap.size, 0);
   do_check_false(GlobalManager.extensionMap.has(ID));
 
   yield promiseShutdownManager();
 });
 
 // Writing the manifest direct to the profile should work
 add_task(function*() {
   writeWebManifestForExtension({