Bug 1287010 - Use minimal global scope for ext-*.js scripts draft
authorRob Wu <rob@robwu.nl>
Wed, 17 Aug 2016 20:28:19 -0700
changeset 405202 48db819893bf2424f75b74e6675b943796128a52
parent 404988 01748a2b1a463f24efd9cd8abad9ccfd76b037b8
child 405203 0c58d0c20220169ae7b3d5a67d5870d75da2419f
push id27432
push userbmo:rob@robwu.nl
push dateThu, 25 Aug 2016 02:36:24 +0000
bugs1287010
milestone51.0a1
Bug 1287010 - Use minimal global scope for ext-*.js scripts Currently there is a tight coupling between registered APIs because they share the same global scope, and the dependencies between the modules that use these globals are not explicit. Consequently, it would be possible for APIs to break when the registered APIs run in separate processes, because then there are separate global scopes. To mitigate this issue, this patch isolates the global namespaces of API registrations in different environments, starting with the "chrome" process. Content and addon processes will follow later. A new JSM is introduced to avoid hidden dependencies between ext-*.js and the script loader. ExtensionUtils.jsm would be a natural choice for this shared utility method, but cannot be used because its local `EventEmitter` implementation conflicts with the `EventEmitter` import in ext-tabs.js. So, this patch provides isolation of global variables declared through `globals.XXX = ...`, but does not provide isolation for `Cu.import`-ed logic. Ideally `Cu.import` should always use its second argument to prevent inadvertent namespace pollution. MozReview-Commit-ID: 1DTZaKOaeSE
browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
browser/components/extensions/test/browser/browser_ext_currentWindow.js
browser/components/extensions/test/browser/browser_ext_getViews.js
browser/components/extensions/test/browser/browser_ext_tabs_audio.js
browser/components/extensions/test/browser/browser_ext_tabs_duplicate.js
browser/components/extensions/test/browser/browser_ext_tabs_zoom.js
browser/components/extensions/test/browser/browser_ext_windows_events.js
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionGlobalScope.jsm
toolkit/components/extensions/ext-webRequest.js
toolkit/components/extensions/moz.build
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
@@ -205,17 +205,18 @@ add_task(function* testBrowserActionClic
 
     files: {
       "popup.html": `<!DOCTYPE html><html><head><meta charset="utf-8"></head></html>`,
     },
   });
 
   yield extension.startup();
 
-  const {GlobalManager, browserActionFor} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  const {GlobalManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  const {browserActionFor} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
   let ext = GlobalManager.extensionMap.get(extension.id);
   let browserAction = browserActionFor(ext);
 
   let widget = getBrowserActionWidget(extension).forWindow(window);
 
   // Test canceled click.
   EventUtils.synthesizeMouseAtCenter(widget.node, {type: "mousedown", button: 0}, window);
--- a/browser/components/extensions/test/browser/browser_ext_currentWindow.js
+++ b/browser/components/extensions/test/browser/browser_ext_currentWindow.js
@@ -85,17 +85,17 @@ add_task(function* () {
       "popup.js": genericChecker,
     },
 
     background: genericChecker,
   });
 
   yield Promise.all([extension.startup(), extension.awaitMessage("background-ready")]);
 
-  let {WindowManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  let {WindowManager} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
   let winId1 = WindowManager.getId(win1);
   let winId2 = WindowManager.getId(win2);
 
   function* checkWindow(kind, winId, name) {
     extension.sendMessage(kind + "-check-current1");
     is((yield extension.awaitMessage("result")), winId, `${name} is on top (check 1) [${kind}]`);
     extension.sendMessage(kind + "-check-current2");
--- a/browser/components/extensions/test/browser/browser_ext_getViews.js
+++ b/browser/components/extensions/test/browser/browser_ext_getViews.js
@@ -99,17 +99,17 @@ add_task(function* () {
 
     background: genericChecker,
   });
 
   yield Promise.all([extension.startup(), extension.awaitMessage("background-ready")]);
 
   info("started");
 
-  let {WindowManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  let {WindowManager} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
   let winId1 = WindowManager.getId(win1);
   let winId2 = WindowManager.getId(win2);
 
   function* openTab(winId) {
     extension.sendMessage("background-open-tab", winId);
     yield extension.awaitMessage("tab-ready");
   }
--- a/browser/components/extensions/test/browser/browser_ext_tabs_audio.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_audio.js
@@ -171,17 +171,17 @@ add_task(function* () {
     manifest: {
       "permissions": ["tabs"],
     },
 
     background,
   });
 
   extension.onMessage("change-tab", (tabId, attr, on) => {
-    let {TabManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+    let {TabManager} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
     let tab = TabManager.getTab(tabId);
 
     if (attr == "muted") {
       // Ideally we'd simulate a click on the tab audio icon for this, but the
       // handler relies on CSS :hover states, which are complicated and fragile
       // to simulate.
       if (tab.muted != on) {
--- a/browser/components/extensions/test/browser/browser_ext_tabs_duplicate.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_duplicate.js
@@ -93,17 +93,17 @@ add_task(function* testDuplicateTabLazil
     manifest: {
       "permissions": ["tabs"],
     },
 
     background,
   });
 
   extension.onMessage("duplicate-tab", tabId => {
-    let {TabManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+    let {TabManager} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
     let tab = TabManager.getTab(tabId);
     // This is a bit of a hack to load a tab in the background.
     let newTab = gBrowser.duplicateTab(tab, false);
 
     BrowserTestUtils.waitForEvent(newTab, "SSTabRestored", () => true).then(() => {
       extension.sendMessage("duplicate-tab-done", TabManager.getId(newTab));
     });
--- a/browser/components/extensions/test/browser/browser_ext_tabs_zoom.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_zoom.js
@@ -196,17 +196,17 @@ add_task(function* () {
     manifest: {
       "permissions": ["tabs"],
     },
 
     background,
   });
 
   extension.onMessage("msg", (id, msg, ...args) => {
-    let {TabManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+    let {TabManager} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
     let resp;
     if (msg == "get-zoom") {
       let tab = TabManager.getTab(args[0]);
       resp = ZoomManager.getZoomForBrowser(tab.linkedBrowser);
     } else if (msg == "set-zoom") {
       let tab = TabManager.getTab(args[0]);
       ZoomManager.setZoomForBrowser(tab.linkedBrowser);
--- a/browser/components/extensions/test/browser/browser_ext_windows_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_windows_events.js
@@ -48,17 +48,17 @@ add_task(function* testWindowsEvents() {
 
   let extension = ExtensionTestUtils.loadExtension({
     background: `(${background})()`,
   });
 
   yield extension.startup();
   yield extension.awaitMessage("ready");
 
-  let {WindowManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  let {WindowManager} = Cu.import("resource://gre/modules/ExtensionGlobalScope.jsm", {}).getGlobalForTesting("chrome");
 
   let currentWindow = window;
   let currentWindowId = WindowManager.getId(currentWindow);
   info(`Current window ID: ${currentWindowId}`);
 
   info(`Create browser window 1`);
   let win1 = yield BrowserTestUtils.openNewBrowserWindow();
   let win1Id = yield extension.awaitMessage("window-created");
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -54,16 +54,19 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
                                   "resource://gre/modules/Preferences.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Schemas",
                                   "resource://gre/modules/Schemas.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "loadExtScriptInScope",
+                                  "resource://gre/modules/ExtensionGlobalScope.jsm");
+
 XPCOMUtils.defineLazyGetter(this, "require", () => {
   let obj = {};
   Cu.import("resource://devtools/shared/Loader.jsm", obj);
   return obj.require;
 });
 
 Cu.import("resource://gre/modules/ExtensionContent.jsm");
 Cu.import("resource://gre/modules/ExtensionManagement.jsm");
@@ -87,18 +90,16 @@ var {
   BaseContext,
   EventEmitter,
   LocaleData,
   Messenger,
   instanceOf,
   flushJarCache,
 } = ExtensionUtils;
 
-XPCOMUtils.defineLazyGetter(this, "console", ExtensionUtils.getConsole);
-
 const LOGGER_ID_BASE = "addons.webextension.";
 const UUID_MAP_PREF = "extensions.webextensions.uuids";
 const LEAVE_STORAGE_PREF = "extensions.webextensions.keepStorageOnUninstall";
 const LEAVE_UUID_PREF = "extensions.webextensions.keepUuidOnUninstall";
 
 const COMMENT_REGEXP = new RegExp(String.raw`
     ^
     (
@@ -106,18 +107,16 @@ const COMMENT_REGEXP = new RegExp(String
         [^"] |
         " (?:[^"\\] | \\.)* "
       )*?
     )
 
     //.*
   `.replace(/\s+/g, ""), "gm");
 
-var scriptScope = this;
-
 var ExtensionContext, GlobalManager;
 
 // This object loads the ext-*.js scripts that define the extension API.
 var Management = {
   initialized: null,
   scopes: [],
   schemaApis: [],
   emitter: new EventEmitter(),
@@ -137,26 +136,17 @@ var Management = {
       }
       for (let url of schemaURLs) {
         promises.push(Schemas.load(url));
       }
       return Promise.all(promises);
     });
 
     for (let [/* name */, value] of XPCOMUtils.enumerateCategoryEntries(CATEGORY_EXTENSION_SCRIPTS)) {
-      let scope = {
-        get console() { return console; },
-        extensions: this,
-        global: scriptScope,
-        require,
-      };
-      Services.scriptloader.loadSubScript(value, scope, "UTF-8");
-
-      // Save the scope to avoid it being garbage collected.
-      this.scopes.push(scope);
+      loadExtScriptInScope(value, this);
     }
 
     this.initialized = promise;
     return this.initialized;
   },
 
   /**
    * Called by an ext-*.js script to register an API.
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/ExtensionGlobalScope.jsm
@@ -0,0 +1,95 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = ["loadExtScriptInScope"];
+
+/*
+ * This file provides the common global scope for all ext-*.js modules. Any
+ * variable declared here is automatically shared with the ext-*.js files, so
+ * try to keep the number of globals to a minimum.
+ *
+ * See loadExtScriptInScope below and ExtensionUtils.SchemaAPIManager for more
+ * information.
+ */
+
+const Ci = Components.interfaces;
+const Cc = Components.classes;
+const Cu = Components.utils;
+const Cr = Components.results;
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "ExtensionUtils",
+                                  "resource://gre/modules/ExtensionUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "require",
+                                  "resource://devtools/shared/Loader.jsm");
+
+let _internalGlobals = new WeakMap();
+
+/**
+ * Load an ext-*.js script, with the global namespace set up as follows:
+ * - (invisible root)  This JSM's global scope.
+ * +- global           The global shared by all scripts in an apiManager.
+ *  +- scope           The implied global, visible to the script as `this`.
+ *
+ * By default variable declarations stay within scope `scope`. If there is a
+ * need to share variables with other modules, use `global`.
+ *
+ * To ensure consistent behavior in SchemaAPIManagers regardless of whether an
+ * addon runs in the chrome process or separate addon process, the scope
+ * (invisible root) should be avoided. When an ext-*.js script uses `Cu.import`
+ * without a second argument, variables appear in (invisible root).
+ *
+ * @param {string} scriptUrl The local script to load.
+ * @param {SchemaAPIManager} apiManager The API manager that is shared with the
+ *     script.
+ */
+this.loadExtScriptInScope = (scriptUrl, apiManager) => {
+  // Different SchemaAPIManagers should have different globals.
+  let global = _internalGlobals.get(apiManager);
+  if (!global) {
+    global = Object.create(null);
+    global.extensions = apiManager;
+    global.global = global;
+    global._internalScriptScopes = [];
+    _internalGlobals.set(apiManager, global);
+  }
+
+  let scope = Object.create(global, {
+    console: {
+      get() { return ExtensionUtils.console; },
+    },
+  });
+
+  Services.scriptloader.loadSubScript(scriptUrl, scope, "UTF-8");
+
+  // Save the scope to avoid it being garbage collected.
+  global._internalScriptScopes.push(scope);
+};
+
+/**
+ * Retrieve the shared global for a given type. This enables unit tests to test
+ * variables that were explicitly shared via `global`. If the global is not
+ * found, an error is thrown.
+ *
+ * @param {*} type A description of the global.
+ *     For now, only "chrome" is supported to get the global from the chrome
+ *     process. In the future support for other process types may be added.
+ * @returns {object} The global for the given type.
+ */
+this.getGlobalForTesting = (type) => {
+  if (type === "chrome") {
+    let {Management} = Cu.import("resource://gre/modules/Extension.jsm", {});
+    let global = _internalGlobals.get(Management);
+    if (global) {
+      return global;
+    }
+    throw new Error("Global not found. Did you really load an ext- script?");
+  }
+  // For now just throw until we find a need for more types.
+  throw new Error(`getGlobalForTesting: Parameter not supported yet: ${type}`);
+};
--- a/toolkit/components/extensions/ext-webRequest.js
+++ b/toolkit/components/extensions/ext-webRequest.js
@@ -4,16 +4,17 @@ var {classes: Cc, interfaces: Ci, utils:
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "MatchPattern",
                                   "resource://gre/modules/MatchPattern.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequest",
                                   "resource://gre/modules/WebRequest.jsm");
 
+Cu.import("resource://gre/modules/ExtensionManagement.jsm");
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   SingletonEventManager,
   runSafeSync,
 } = ExtensionUtils;
 
 // EventManager-like class specifically for WebRequest. Inherits from
 // SingletonEventManager. Takes care of converting |details| parameter
--- a/toolkit/components/extensions/moz.build
+++ b/toolkit/components/extensions/moz.build
@@ -3,16 +3,17 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 EXTRA_JS_MODULES += [
     'Extension.jsm',
     'ExtensionAPI.jsm',
     'ExtensionContent.jsm',
+    'ExtensionGlobalScope.jsm',
     'ExtensionManagement.jsm',
     'ExtensionStorage.jsm',
     'ExtensionUtils.jsm',
     'MessageChannel.jsm',
     'NativeMessaging.jsm',
     'Schemas.jsm',
 ]