Bug 1277054 - Handle exited add-on actors after reload. r=ochameau
authorKumar McMillan <kumar.mcmillan@gmail.com>
Tue, 31 May 2016 15:40:19 -0500
changeset 339339 4874ff5d90f81aca0bb57d2f74586210cbee3cf6
parent 339338 86dca8a132b53174e19082ddad03ac806cb60f98
child 339340 0ef7cc6b42c72be6ef8d0786fb1f5371c11e9e32
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1277054
milestone49.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 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]