Bug 1277054 - Handle exited add-on actors after reload. r=ochameau draft
authorKumar McMillan <kumar.mcmillan@gmail.com>
Tue, 31 May 2016 15:40:19 -0500
changeset 374474 597eb0e43a745886607754bbeb1f0cd7dec96cd2
parent 373507 aee75e50fbc15694bbe48724eb28dea184ef985e
child 375304 1a0f27fad3e423981ee22f4f7b8c2477cd831673
push id20036
push userbmo:kumar.mcmillan@gmail.com
push dateThu, 02 Jun 2016 16:19:25 +0000
reviewersochameau
bugs1277054
milestone49.0a1
Bug 1277054 - Handle exited add-on actors after reload. r=ochameau MozReview-Commit-ID: 6WFI51GJ3ea
devtools/server/actors/webbrowser.js
devtools/server/tests/unit/addons/web-extension/manifest.json
devtools/server/tests/unit/addons/web-extension2/manifest.json
devtools/server/tests/unit/head_dbg.js
devtools/server/tests/unit/test_addon_reload.js
devtools/server/tests/unit/test_addons_actor.js
devtools/server/tests/unit/test_addons_actor/web-extension/manifest.json
devtools/server/tests/unit/xpcshell.ini
--- a/devtools/server/actors/webbrowser.js
+++ b/devtools/server/actors/webbrowser.js
@@ -2311,36 +2311,54 @@ Object.defineProperty(BrowserAddonList.p
     return this._onListChanged;
   },
   set(v) {
     if (v !== null && typeof v != "function") {
       throw new Error(
         "onListChanged property may only be set to 'null' or a function");
     }
     this._onListChanged = v;
-    if (this._onListChanged) {
-      AddonManager.addAddonListener(this);
-    } else {
-      AddonManager.removeAddonListener(this);
-    }
+    this._adjustListener();
   }
 });
 
 BrowserAddonList.prototype.onInstalled = function (addon) {
   if (this._actorByAddonId.get(addon.id)) {
     // When an add-on gets upgraded or reloaded, it will not be uninstalled
     // so this step is necessary to clear the cache.
     this._actorByAddonId.delete(addon.id);
   }
-  this._onListChanged();
+  this._notifyListChanged();
+  this._adjustListener();
 };
 
 BrowserAddonList.prototype.onUninstalled = function (addon) {
   this._actorByAddonId.delete(addon.id);
-  this._onListChanged();
+  this._notifyListChanged();
+  this._adjustListener();
+};
+
+BrowserAddonList.prototype._notifyListChanged = function () {
+  if (this._onListChanged) {
+    this._onListChanged();
+  }
+};
+
+BrowserAddonList.prototype._adjustListener = function () {
+  if (this._onListChanged) {
+    // As long as the callback exists, we need to listen for changes
+    // so we can notify about add-on changes.
+    AddonManager.addAddonListener(this);
+  } else {
+    // When the callback does not exist, we only need to keep listening
+    // if the actor cache will need adjusting when add-ons change.
+    if (this._actorByAddonId.size === 0) {
+      AddonManager.removeAddonListener(this);
+    }
+  }
 };
 
 exports.BrowserAddonList = BrowserAddonList;
 
 /**
  * The DebuggerProgressListener object is an nsIWebProgressListener which
  * handles onStateChange events for the inspected browser. If the user tries to
  * navigate away from a paused page, the listener makes sure that the debuggee
rename from devtools/server/tests/unit/test_addons_actor/web-extension/manifest.json
rename to devtools/server/tests/unit/addons/web-extension/manifest.json
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/addons/web-extension2/manifest.json
@@ -0,0 +1,10 @@
+{
+  "manifest_version": 2,
+  "name": "Test Addons Actor 2",
+  "version": "1.0",
+  "applications": {
+    "gecko": {
+      "id": "test-addons-actor2@mozilla.org"
+    }
+  }
+}
--- a/devtools/server/tests/unit/head_dbg.js
+++ b/devtools/server/tests/unit/head_dbg.js
@@ -42,16 +42,31 @@ const { addDebuggerToGlobal } = Cu.impor
 
 const systemPrincipal = Cc["@mozilla.org/systemprincipal;1"].createInstance(Ci.nsIPrincipal);
 
 var loadSubScript = Cc[
   "@mozilla.org/moz/jssubscript-loader;1"
 ].getService(Ci.mozIJSSubScriptLoader).loadSubScript;
 
 /**
+ * Initializes any test that needs to work with add-ons.
+ */
+function startupAddonsManager() {
+  // Create a directory for extensions.
+  const profileDir = do_get_profile().clone();
+  profileDir.append("extensions");
+
+  const internalManager = Cc["@mozilla.org/addons/integration;1"]
+    .getService(Ci.nsIObserver)
+    .QueryInterface(Ci.nsITimerCallback);
+
+  internalManager.observe(null, "addons-startup", null);
+}
+
+/**
  * Create a `run_test` function that runs the given generator in a task after
  * having attached to a memory actor. When done, the memory actor is detached
  * from, the client is finished, and the test is finished.
  *
  * @param {GeneratorFunction} testGeneratorFunction
  *        The generator function is passed (DebuggerClient, MemoryFront)
  *        arguments.
  *
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_addon_reload.js
@@ -0,0 +1,71 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const protocol = require("devtools/shared/protocol");
+const {AddonManager} = require("resource://gre/modules/AddonManager.jsm");
+
+startupAddonsManager();
+
+function promiseAddonEvent(event) {
+  return new Promise(resolve => {
+    let listener = {
+      [event]: function (...args) {
+        AddonManager.removeAddonListener(listener);
+        resolve(args);
+      }
+    };
+
+    AddonManager.addAddonListener(listener);
+  });
+}
+
+function* findAddonInRootList(client, addonId) {
+  const result = yield client.listAddons();
+  const addonActor = result.addons.filter(addon => addon.id === addonId)[0];
+  ok(addonActor, `Found add-on actor for ${addonId}`);
+  return addonActor;
+}
+
+function* reloadAddon(client, addonActor) {
+  // The add-on will be re-installed after a successful reload.
+  const onInstalled = promiseAddonEvent("onInstalled");
+  yield client.request({to: addonActor.actor, type: "reload"});
+  yield onInstalled;
+}
+
+function getSupportFile(path) {
+  const allowMissing = false;
+  return do_get_file(path, allowMissing);
+}
+
+add_task(function* testReloadExitedAddon() {
+  const client = yield new Promise(resolve => {
+    get_chrome_actors(client => resolve(client));
+  });
+
+  // Install our main add-on to trigger reloads on.
+  const addonFile = getSupportFile("addons/web-extension");
+  const installedAddon = yield AddonManager.installTemporaryAddon(
+    addonFile);
+
+  // Install a decoy add-on.
+  const addonFile2 = getSupportFile("addons/web-extension2");
+  const installedAddon2 = yield AddonManager.installTemporaryAddon(
+    addonFile2);
+
+  let addonActor = yield findAddonInRootList(client, installedAddon.id);
+
+  yield reloadAddon(client, addonActor);
+
+  // Uninstall the decoy add-on, which should cause its actor to exit.
+  const onUninstalled = promiseAddonEvent("onUninstalled");
+  installedAddon2.uninstall();
+  const [uninstalledAddon] = yield onUninstalled;
+
+  // Try to re-list all add-ons after a reload.
+  // This was throwing an exception because of the exited actor.
+  const newAddonActor = yield findAddonInRootList(client, installedAddon.id);
+  equal(newAddonActor.id, addonActor.id);
+
+  yield close(client);
+});
--- a/devtools/server/tests/unit/test_addons_actor.js
+++ b/devtools/server/tests/unit/test_addons_actor.js
@@ -1,25 +1,15 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 const protocol = require("devtools/shared/protocol");
 const {AddonsActor} = require("devtools/server/actors/addons");
 const {AddonsFront} = require("devtools/shared/fronts/addons");
 
-function startupAddonsManager() {
-  const internalManager = Cc["@mozilla.org/addons/integration;1"]
-    .getService(Ci.nsIObserver)
-    .QueryInterface(Ci.nsITimerCallback);
-
-  internalManager.observe(null, "addons-startup", null);
-}
-
-const profileDir = do_get_profile().clone();
-profileDir.append("extensions");
 startupAddonsManager();
 
 function* connect() {
   const client = yield new Promise(resolve => {
     get_chrome_actors(client => resolve(client));
   });
   const root = yield listTabs(client);
   const addonsActor = root.addonsActor;
@@ -29,17 +19,17 @@ function* connect() {
   return [client, addons];
 }
 
 add_task(function* testSuccessfulInstall() {
   const [client, addons] = yield connect();
 
   const allowMissing = false;
   const usePlatformSeparator = true;
-  const addonPath = getFilePath("test_addons_actor/web-extension",
+  const addonPath = getFilePath("addons/web-extension",
                                 allowMissing, usePlatformSeparator);
   const installedAddon = yield addons.installTemporaryAddon(addonPath);
   equal(installedAddon.id, "test-addons-actor@mozilla.org");
   // The returned object is currently not a proper actor.
   equal(installedAddon.actor, false);
 
   const addonList = yield client.listAddons();
   ok(addonList && addonList.addons && addonList.addons.map(a => a.name),
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -25,18 +25,20 @@ support-files =
   setBreakpoint-on-column-with-no-offsets-at-end-of-line.js
   setBreakpoint-on-column-with-no-offsets-in-gcd-script.js
   setBreakpoint-on-line.js
   setBreakpoint-on-line-in-gcd-script.js
   setBreakpoint-on-line-with-multiple-offsets.js
   setBreakpoint-on-line-with-multiple-statements.js
   setBreakpoint-on-line-with-no-offsets.js
   setBreakpoint-on-line-with-no-offsets-in-gcd-script.js
-  test_addons_actor/web-extension/manifest.json
+  addons/web-extension/manifest.json
+  addons/web-extension2/manifest.json
 
+[test_addon_reload.js]
 [test_addons_actor.js]
 [test_animation_name.js]
 [test_animation_type.js]
 [test_ScriptStore.js]
 [test_actor-registry-actor.js]
 [test_nesting-01.js]
 [test_nesting-02.js]
 [test_nesting-03.js]