Bug 1302702 - Remove from ext-backgroundPage any code that uses the AddonManager object. r=kmag
☠☠ backed out by 98571411cda0 ☠ ☠
authorLuca Greco <lgreco@mozilla.com>
Tue, 21 Mar 2017 16:28:15 +0100
changeset 358787 9859873385bc5722792131a3dcb9caced44f0261
parent 358786 9b16857ca48d5057aba8240b41546e8592e990bb
child 358788 ef12ed2b464146caff867bc950a43d700c007526
push id42849
push userryanvm@gmail.com
push dateWed, 17 May 2017 17:09:54 +0000
treeherderautoland@2255bb2a4215 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1302702
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1302702 - Remove from ext-backgroundPage any code that uses the AddonManager object. r=kmag The background page do not need to use the AddonManager to set its preferred debugging global anymore (and it would not be able to use it from the extension child process). MozReview-Commit-ID: 2IAxvCjDKvl
toolkit/components/extensions/ext-backgroundPage.js
toolkit/components/extensions/test/mochitest/chrome.ini
toolkit/components/extensions/test/mochitest/test_chrome_ext_background_debug_global.html
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/components/extensions/ext-backgroundPage.js
+++ b/toolkit/components/extensions/ext-backgroundPage.js
@@ -13,17 +13,16 @@ var {
 
 // Responsible for the background_page section of the manifest.
 class BackgroundPage extends HiddenExtensionPage {
   constructor(extension, options) {
     super(extension, "background");
 
     this.page = options.page || null;
     this.isGenerated = !!options.scripts;
-    this.webNav = null;
 
     if (this.page) {
       this.url = this.extension.baseURI.resolve(this.page);
     } else if (this.isGenerated) {
       this.url = this.extension.baseURI.resolve("_generated_background_page.html");
     }
 
     if (!this.extension.isExtensionURL(this.url)) {
@@ -36,44 +35,27 @@ class BackgroundPage extends HiddenExten
     await this.createBrowserElement();
 
     extensions.emit("extension-browser-inserted", this.browser);
 
     this.browser.loadURI(this.url);
 
     let context = await promiseExtensionViewLoaded(this.browser);
 
-    if (this.browser.docShell) {
-      this.webNav = this.browser.docShell.QueryInterface(Ci.nsIWebNavigation);
-      let window = this.webNav.document.defaultView;
-
-      // Set the add-on's main debugger global, for use in the debugger
-      // console.
-      if (this.extension.addonData.instanceID) {
-        AddonManager.getAddonByInstanceID(this.extension.addonData.instanceID)
-                    .then(addon => addon && addon.setDebugGlobal(window));
-      }
-    }
-
     if (context) {
       // Wait until all event listeners registered by the script so far
       // to be handled.
       await Promise.all(context.listenerPromises);
       context.listenerPromises = null;
     }
 
     this.extension.emit("startup");
   }
 
   shutdown() {
-    if (this.extension.addonData.instanceID) {
-      AddonManager.getAddonByInstanceID(this.extension.addonData.instanceID)
-                  .then(addon => addon && addon.setDebugGlobal(null));
-    }
-
     super.shutdown();
   }
 }
 
 this.backgroundPage = class extends ExtensionAPI {
   onManifestEntry(entryName) {
     let {manifest} = this.extension;
 
--- a/toolkit/components/extensions/test/mochitest/chrome.ini
+++ b/toolkit/components/extensions/test/mochitest/chrome.ini
@@ -8,18 +8,16 @@ support-files =
   file_sample.html
   file_with_images.html
   webrequest_chromeworker.js
   webrequest_test.jsm
   oauth.html
   redirect_auto.sjs
 tags = webextensions in-process-webextensions
 
-[test_chrome_ext_background_debug_global.html]
-skip-if = (os == 'android') # android doesn't have devtools
 [test_chrome_ext_background_page.html]
 skip-if = (toolkit == 'android') # android doesn't have devtools
 [test_chrome_ext_contentscript_data_uri.html]
 [test_chrome_ext_contentscript_unrecognizedprop_warning.html]
 [test_chrome_ext_downloads_saveAs.html]
 [test_chrome_ext_eventpage_warning.html]
 [test_chrome_ext_hybrid_addons.html]
 [test_chrome_ext_idle.html]
deleted file mode 100644
--- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_background_debug_global.html
+++ /dev/null
@@ -1,166 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <title>WebExtension test</title>
-  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
-  <script src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"></script>
-  <script src="chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js"></script>
-  <script type="text/javascript" src="chrome_head.js"></script>
-  <script type="text/javascript" src="head.js"></script>
-  <link rel="stylesheet" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
-</head>
-<body>
-
-<script type="text/javascript">
-"use strict";
-
-Cu.import("resource://devtools/client/framework/ToolboxProcess.jsm");
-Cu.import("resource://gre/modules/AddonManager.jsm");
-
-const {
-  XPIProvider,
-} = Components.utils.import("resource://gre/modules/addons/XPIProvider.jsm", {});
-
-/**
- * This test is asserting that ext-backgroundPage.js successfully sets its
- * debug global in the AddonWrapper provided by XPIProvider.jsm
- *
- * It does _not_ test any functionality in devtools and does not guarantee
- * debugging is actually working correctly end-to-end.
- */
-
-function background() {
-  window.testThing = "test!";
-  browser.test.notifyPass("background script ran");
-}
-
-const ID = "debug@tests.mozilla.org";
-let extensionData = {
-  useAddonManager: "temporary",
-  background,
-  manifest: {
-    applications: {gecko: {id: ID}},
-  },
-};
-
-add_task(async function() {
-  let extension = ExtensionTestUtils.loadExtension(extensionData);
-  await extension.startup();
-
-  await extension.awaitFinish("background script ran");
-
-  await new Promise(function(resolve) {
-    window.BrowserToolboxProcess.emit("connectionchange", "opened", {
-      setAddonOptions(id, options) {
-        if (id === ID) {
-          let context = Cu.waiveXrays(options.global);
-          ok(context.chrome, "global context has a chrome object");
-          ok(context.browser, "global context has a browser object");
-          is("test!", context.testThing, "global context is the background script context");
-          resolve();
-        }
-      },
-    });
-  });
-
-  let addon = await new Promise((resolve, reject) => {
-    AddonManager.getAddonByID(ID, addon => addon ? resolve(addon) : reject());
-  });
-
-  ok(addon, `Got the addon wrapper for ${addon.id}`);
-
-  function waitForDebugGlobalChanges(times, initialAddonInstanceID) {
-    return new Promise((resolve) => {
-      AddonManager.addAddonListener({
-        count: 0,
-        notNullGlobalsCount: 0,
-        undefinedPrivateWrappersCount: 0,
-        lastAddonInstanceID: initialAddonInstanceID,
-        onPropertyChanged(newAddon, changedPropNames) {
-          if (newAddon.id != addon.id ||
-              !changedPropNames.includes("debugGlobal")) {
-            return;
-          }
-
-          ok(!(newAddon.setDebugGlobal) && !(newAddon.getDebugGlobal),
-             "The addon wrapper should not be a PrivateWrapper");
-
-          let activeAddon = XPIProvider.activeAddons.get(addon.id);
-
-          let addonInstanceID;
-
-          if (!activeAddon) {
-            // The addon has been disable, the preferred global should be null
-            addonInstanceID = this.lastAddonInstanceID;
-            delete this.lastAddonInstanceID;
-          } else {
-            addonInstanceID = activeAddon.instanceID;
-            this.lastAddonInstanceID = addonInstanceID;
-          }
-
-          ok(addonInstanceID, `Got the addon instanceID for ${addon.id}`);
-
-          AddonManager.getAddonByInstanceID(addonInstanceID).then((privateWrapper) => {
-            this.count += 1;
-
-            if (!privateWrapper) {
-              // The addon has been uninstalled
-              this.undefinedPrivateWrappersCount += 1;
-            } else {
-              ok((privateWrapper.getDebugGlobal), "Got the addon PrivateWrapper");
-
-              if (privateWrapper.getDebugGlobal()) {
-                this.notNullGlobalsCount += 1;
-              }
-            }
-
-            if (this.count == times) {
-              AddonManager.removeAddonListener(this);
-              resolve({
-                counters: {
-                  count: this.count,
-                  notNullGlobalsCount: this.notNullGlobalsCount,
-                  undefinedPrivateWrappersCount: this.undefinedPrivateWrappersCount,
-                },
-                lastAddonInstanceID: this.lastAddonInstanceID,
-              });
-            }
-          });
-        },
-      });
-    });
-  }
-
-  // two calls expected, one for the shutdown and one for the startup
-  // of the background page.
-  let waitForDebugGlobalChangesOnReload = waitForDebugGlobalChanges(2);
-
-  info("Addon reload...");
-  await addon.reload();
-
-  info("Addon completed startup after reload");
-
-  let {
-    counters: reloadCounters,
-    lastAddonInstanceID,
-  } = await waitForDebugGlobalChangesOnReload;
-
-  isDeeply(reloadCounters, {count: 2, notNullGlobalsCount: 1, undefinedPrivateWrappersCount: 0},
-           "Got the expected number of onPropertyChanged calls on reload");
-
-  // one more call expected for the shutdown.
-  let waitForDebugGlobalChangesOnShutdown = waitForDebugGlobalChanges(1, lastAddonInstanceID);
-
-  info("extension unloading...");
-  await extension.unload();
-  info("extension unloaded");
-
-  let {counters: unloadCounters} = await waitForDebugGlobalChangesOnShutdown;
-
-  isDeeply(unloadCounters, {count: 1, notNullGlobalsCount: 0, undefinedPrivateWrappersCount: 1},
-           "Got the expected number of onPropertyChanged calls on shutdown");
-});
-</script>
-
-</body>
-</html>
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -2226,17 +2226,17 @@ var AddonManagerInternal = {
   if (!aCallback || typeof aCallback != "function")
     throw Components.Exception("aCallback must be a function",
                                Cr.NS_ERROR_INVALID_ARG);
 
    this.getAddonByInstanceID(aInstanceID).then(wrapper => {
      if (!wrapper) {
        throw Error("No addon matching instanceID:", aInstanceID.toString());
      }
-     let addonId = wrapper.addonId();
+     let addonId = wrapper.id;
      logger.debug(`Registering upgrade listener for ${addonId}`);
      this.upgradeListeners.set(addonId, aCallback);
    });
   },
 
   /**
    * Removes an UpgradeListener if the listener is registered.
    *
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -4553,26 +4553,17 @@ this.XPIProvider = {
    */
    getAddonByInstanceID(aInstanceID) {
      if (!aInstanceID || typeof aInstanceID != "symbol")
        throw Components.Exception("aInstanceID must be a Symbol()",
                                   Cr.NS_ERROR_INVALID_ARG);
 
      for (let [id, val] of this.activeAddons) {
        if (aInstanceID == val.instanceID) {
-         if (val.safeWrapper) {
-           return Promise.resolve(val.safeWrapper);
-         }
-
-         return new Promise(resolve => {
-           this.getAddonByID(id, function(addon) {
-             val.safeWrapper = new PrivateWrapper(addon);
-             resolve(val.safeWrapper);
-           });
-         });
+         return new Promise(resolve => this.getAddonByID(id, resolve));
        }
      }
 
      return Promise.resolve(null);
    },
 
   /**
    * Removes an AddonInstall from the list of active installs.
@@ -4829,17 +4820,17 @@ this.XPIProvider = {
   },
 
   onDebugConnectionChange(aEvent, aWhat, aConnection) {
     if (aWhat != "opened")
       return;
 
     for (let [id, val] of this.activeAddons) {
       aConnection.setAddonOptions(
-        id, { global: val.debugGlobal || val.bootstrapScope });
+        id, { global: val.bootstrapScope });
     }
   },
 
   /**
    * Notified when a preference we're interested in has changed.
    *
    * @see nsIObserver
    */
@@ -5145,18 +5136,16 @@ this.XPIProvider = {
    * @param  hasEmbeddedWebExtension
    *         Boolean indicating whether the add-on has an embedded webextension.
    * @return a JavaScript scope
    */
   loadBootstrapScope(aId, aFile, aVersion, aType,
                                aMultiprocessCompatible, aRunInSafeMode,
                                aDependencies, hasEmbeddedWebExtension) {
     this.activeAddons.set(aId, {
-      debugGlobal: null,
-      safeWrapper: null,
       bootstrapScope: null,
       // a Symbol passed to this add-on, which it can use to identify itself
       instanceID: Symbol(aId),
     });
 
     // Mark the add-on as active for the crash reporter before loading
     this.addAddonsToCrashReporter();
 
@@ -8108,78 +8097,16 @@ AddonWrapper.prototype = {
     let addon = addonFor(this);
     if (!aPath)
       return Services.io.newFileURI(addon._sourceBundle);
 
     return getURIForResourceInFile(addon._sourceBundle, aPath);
   }
 };
 
-/**
- * The PrivateWrapper is used to expose certain functionality only when being
- * called with the add-on instanceID, disallowing other add-ons to access it.
- */
-function PrivateWrapper(aAddon) {
-  AddonWrapper.call(this, aAddon);
-}
-
-PrivateWrapper.prototype = Object.create(AddonWrapper.prototype);
-Object.assign(PrivateWrapper.prototype, {
-  addonId() {
-    return this.id;
-  },
-
-  /**
-   * Retrieves the preferred global context to be used from the
-   * add-on debugging window.
-   *
-   * @returns  global
-   *         The object set as global context. Must be a window object.
-   */
-  getDebugGlobal(global) {
-    let activeAddon = XPIProvider.activeAddons.get(this.id);
-    if (activeAddon) {
-      return activeAddon.debugGlobal;
-    }
-
-    return null;
-  },
-
-  /**
-   * Defines a global context to be used in the console
-   * of the add-on debugging window.
-   *
-   * @param  global
-   *         The object to set as global context. Must be a window object.
-   */
-  setDebugGlobal(global) {
-    if (!global) {
-      // If the new global is null, notify the listeners regardless
-      // from the current state of the addon.
-      // NOTE: this happen after the addon has been disabled and
-      // the global will never be set to null otherwise.
-      AddonManagerPrivate.callAddonListeners("onPropertyChanged",
-                                             addonFor(this),
-                                             ["debugGlobal"]);
-    } else {
-      let activeAddon = XPIProvider.activeAddons.get(this.id);
-      if (activeAddon) {
-        let globalChanged = activeAddon.debugGlobal != global;
-        activeAddon.debugGlobal = global;
-
-        if (globalChanged) {
-          AddonManagerPrivate.callAddonListeners("onPropertyChanged",
-                                                 addonFor(this),
-                                                 ["debugGlobal"]);
-        }
-      }
-    }
-  }
-});
-
 function chooseValue(aAddon, aObj, aProp) {
   let repositoryAddon = aAddon._repositoryAddon;
   let objValue = aObj[aProp];
 
   if (repositoryAddon && (aProp in repositoryAddon) &&
       (objValue === undefined || objValue === null)) {
     return [repositoryAddon[aProp], true];
   }