Bug 1268773 - Notify Addon Listeners when the preferred addon debug global is changed. r=aswan draft
authorLuca Greco <lgreco@mozilla.com>
Thu, 07 Jul 2016 16:29:36 +0200
changeset 392956 a4dd7f15d693ddbd91080a2b63e50ab512fcbbcb
parent 392955 74a222e39c90a0b3d3181c86e451d578a9e721f3
child 392957 39215aa3dfecff2746cd2e6c1be2054ae0c3daf0
push id24157
push userluca.greco@alcacoop.it
push dateTue, 26 Jul 2016 16:01:12 +0000
reviewersaswan
bugs1268773
milestone50.0a1
Bug 1268773 - Notify Addon Listeners when the preferred addon debug global is changed. r=aswan MozReview-Commit-ID: Hn7jb8Kni2J
toolkit/components/extensions/ext-backgroundPage.js
toolkit/components/extensions/test/mochitest/test_chrome_ext_background_debug_global.html
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/components/extensions/ext-backgroundPage.js
+++ b/toolkit/components/extensions/ext-backgroundPage.js
@@ -105,31 +105,31 @@ BackgroundPage.prototype = {
     // TODO(robwu): This implementation of onStartup is wrong, see
     // https://bugzil.la/1247435#c1
     if (this.extension.onStartup) {
       this.extension.onStartup();
     }
   }),
 
   shutdown() {
+    if (this.extension.addonData.instanceID) {
+      AddonManager.getAddonByInstanceID(this.extension.addonData.instanceID)
+                  .then(addon => addon.setDebugGlobal(null));
+    }
+
     // Navigate away from the background page to invalidate any
     // setTimeouts or other callbacks.
     if (this.webNav) {
       this.webNav.loadURI("about:blank", 0, null, null, null);
       this.webNav = null;
     }
 
     this.windowlessBrowser.loadURI("about:blank", 0, null, null, null);
     this.windowlessBrowser.close();
     this.windowlessBrowser = null;
-
-    if (this.extension.addonData.instanceID) {
-      AddonManager.getAddonByInstanceID(this.extension.addonData.instanceID)
-                  .then(addon => addon.setDebugGlobal(null));
-    }
   },
 };
 
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("manifest_background", (type, directive, extension, manifest) => {
   let bgPage = new BackgroundPage(manifest.background, extension);
   backgroundPagesMap.set(extension, bgPage);
   return bgPage.build();
--- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_background_debug_global.html
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_background_debug_global.html
@@ -10,16 +10,21 @@
   <link rel="stylesheet" href="chrome://mochikit/contents/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.
  */
@@ -34,17 +39,16 @@ let extensionData = {
   background: "(" + backgroundScript.toString() + ")()",
   manifest: {},
   files: {},
 };
 
 add_task(function* () {
   let extension = ExtensionTestUtils.loadExtension(extensionData);
   yield extension.startup();
-  info("extension loaded");
 
   yield extension.awaitFinish("background script ran");
 
   yield new Promise(function(resolve) {
     window.BrowserToolboxProcess.emit("connectionchange", "opened", {
       setAddonOptions(id, options) {
         if (id === extension.id) {
           let context = Cu.waiveXrays(options.global);
@@ -52,15 +56,109 @@ add_task(function* () {
           ok(context.browser, "global context has a browser object");
           is("test!", context.testThing, "global context is the background script context");
           resolve();
         }
       },
     });
   });
 
+  let addon = yield new Promise((resolve, reject) => {
+    AddonManager.getAddonByID(extension.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...");
+  yield addon.reload();
+
+  info("Addon completed startup after reload");
+
+  let {
+    counters: reloadCounters,
+    lastAddonInstanceID,
+  } = yield 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...");
   yield extension.unload();
   info("extension unloaded");
+
+  let {counters: unloadCounters} = yield 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/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -7538,32 +7538,64 @@ AddonWrapper.prototype = {
  * 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) {
-    let activeAddon = XPIProvider.activeAddons.get(this.id);
-    if (activeAddon) {
-      activeAddon.debugGlobal = 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];