Bug 1404584 Part 3: Convert ExtensionPreferencesManager to use update and uninstall events, r?aswan r?jkt draft
authorBob Silverberg <bsilverberg@mozilla.com>
Wed, 01 Nov 2017 09:51:41 -0400
changeset 699249 ec7288dc70eadd7f96b5c252602f41b4374f567f
parent 699248 f53d21b1089e57c0e5ba42c60c9d13a06a2c6d36
child 740580 f170df3c2b476ca31b1d4f45a64f8ed3657accff
push id89511
push userbmo:bob.silverberg@gmail.com
push dateThu, 16 Nov 2017 21:45:06 +0000
reviewersaswan, jkt
bugs1404584
milestone59.0a1
Bug 1404584 Part 3: Convert ExtensionPreferencesManager to use update and uninstall events, r?aswan r?jkt This includes removing the "web-extension-preferences-replacing" and "web-extension-preferences-replaced" notifications as they are no longer needed. MozReview-Commit-ID: IjNf4BImgas
browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js
browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
browser/components/extensions/test/browser/head.js
toolkit/components/contextualidentity/ContextualIdentityService.jsm
toolkit/components/extensions/ExtensionPreferencesManager.jsm
--- a/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js
+++ b/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js
@@ -195,20 +195,19 @@ add_task(async function test_extension_s
 
   Services.prefs.clearUserPref(HOMEPAGE_URL_PREF);
 
   is(getHomePageURL(), defaultHomePage,
      "Home url should be the default");
 });
 
 add_task(async function test_disable() {
+  const ID = "id@tests.mozilla.org";
   let defaultHomePage = getHomePageURL();
 
-  const ID = "id@tests.mozilla.org";
-
   let ext1 = ExtensionTestUtils.loadExtension({
     manifest: {
       applications: {
         gecko: {
           id: ID,
         },
       },
       "chrome_settings_overrides": {
@@ -217,34 +216,36 @@ add_task(async function test_disable() {
     },
     useAddonManager: "temporary",
   });
 
   let prefPromise = promisePrefChangeObserved(HOMEPAGE_URL_PREF);
   await ext1.startup();
   await prefPromise;
 
-  ok(getHomePageURL().endsWith(HOME_URI_1),
+  is(getHomePageURL(), HOME_URI_1,
      "Home url should be overridden by the extension.");
 
   let addon = await AddonManager.getAddonByID(ID);
-  is(addon.id, ID);
+  is(addon.id, ID, "Found the correct add-on.");
 
+  let disabledPromise = awaitEvent("shutdown", ID);
   prefPromise = promisePrefChangeObserved(HOMEPAGE_URL_PREF);
   addon.userDisabled = true;
-  await prefPromise;
+  await Promise.all([disabledPromise, prefPromise]);
 
   is(getHomePageURL(), defaultHomePage,
      "Home url should be the default");
 
+  let enabledPromise = awaitEvent("ready", ID);
   prefPromise = promisePrefChangeObserved(HOMEPAGE_URL_PREF);
   addon.userDisabled = false;
-  await prefPromise;
+  await Promise.all([enabledPromise, prefPromise]);
 
-  ok(getHomePageURL().endsWith(HOME_URI_1),
+  is(getHomePageURL(), HOME_URI_1,
      "Home url should be overridden by the extension.");
 
   prefPromise = promisePrefChangeObserved(HOMEPAGE_URL_PREF);
   await ext1.unload();
   await prefPromise;
 
   is(getHomePageURL(), defaultHomePage,
      "Home url should be the default");
--- a/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
+++ b/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
@@ -1,39 +1,19 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 
 "use strict";
 
-XPCOMUtils.defineLazyGetter(this, "Management", () => {
-  const {Management} = Cu.import("resource://gre/modules/Extension.jsm", {});
-  return Management;
-});
-
 XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
                                   "resource://gre/modules/AddonManager.jsm");
 
 const EXTENSION1_ID = "extension1@mozilla.com";
 const EXTENSION2_ID = "extension2@mozilla.com";
 
-function awaitEvent(eventName, id) {
-  return new Promise(resolve => {
-    let listener = (_eventName, ...args) => {
-      let extension = args[0];
-      if (_eventName === eventName &&
-          extension.id == id) {
-        Management.off(eventName, listener);
-        resolve(...args);
-      }
-    };
-
-    Management.on(eventName, listener);
-  });
-}
-
 let defaultEngineName = Services.search.currentEngine.name;
 
 function restoreDefaultEngine() {
   let engine = Services.search.getEngineByName(defaultEngineName);
   Services.search.currentEngine = engine;
 }
 registerCleanupFunction(restoreDefaultEngine);
 
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -15,32 +15,38 @@
  *          openTabContextMenu closeTabContextMenu
  *          openToolsMenu closeToolsMenu
  *          imageBuffer imageBufferFromDataURI
  *          getListStyleImage getPanelForNode
  *          awaitExtensionPanel awaitPopupResize
  *          promiseContentDimensions alterContent
  *          promisePrefChangeObserved openContextMenuInFrame
  *          promiseAnimationFrame getCustomizableUIPanelID
+ *          awaitEvent
  */
 
 // There are shutdown issues for which multiple rejections are left uncaught.
 // This bug should be fixed, but for the moment this directory is whitelisted.
 //
 // NOTE: Entire directory whitelisting should be kept to a minimum. Normally you
 //       should use "expectUncaughtRejection" to flag individual failures.
 const {PromiseTestUtils} = Cu.import("resource://testing-common/PromiseTestUtils.jsm", {});
 PromiseTestUtils.whitelistRejectionsGlobally(/Message manager disconnected/);
 PromiseTestUtils.whitelistRejectionsGlobally(/No matching message handler/);
 PromiseTestUtils.whitelistRejectionsGlobally(/Receiving end does not exist/);
 
 const {AppConstants} = Cu.import("resource://gre/modules/AppConstants.jsm", {});
 const {CustomizableUI} = Cu.import("resource:///modules/CustomizableUI.jsm", {});
 const {Preferences} = Cu.import("resource://gre/modules/Preferences.jsm", {});
 
+XPCOMUtils.defineLazyGetter(this, "Management", () => {
+  const {Management} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  return Management;
+});
+
 // We run tests under two different configurations, from browser.ini and
 // browser-remote.ini. When running from browser-remote.ini, the tests are
 // copied to the sub-directory "test-oop-extensions", which we detect here, and
 // use to select our configuration.
 let remote = gTestPath.includes("test-oop-extensions");
 SpecialPowers.pushPrefEnv({set: [
   ["extensions.webextensions.remote", remote],
 ]});
@@ -455,8 +461,23 @@ function closePageAction(extension, win 
 
 function promisePrefChangeObserved(pref) {
   return new Promise((resolve, reject) =>
     Preferences.observe(pref, function prefObserver() {
       Preferences.ignore(pref, prefObserver);
       resolve();
     }));
 }
+
+function awaitEvent(eventName, id) {
+  return new Promise(resolve => {
+    let listener = (_eventName, ...args) => {
+      let extension = args[0];
+      if (_eventName === eventName &&
+          extension.id == id) {
+        Management.off(eventName, listener);
+        resolve();
+      }
+    };
+
+    Management.on(eventName, listener);
+  });
+}
--- a/toolkit/components/contextualidentity/ContextualIdentityService.jsm
+++ b/toolkit/components/contextualidentity/ContextualIdentityService.jsm
@@ -112,39 +112,28 @@ function _ContextualIdentityService(path
 
   _path: null,
   _dataReady: false,
 
   _saver: null,
 
   init(path) {
     this._path = path;
-    this._webExtensionUpdating = false;
 
     Services.prefs.addObserver(CONTEXTUAL_IDENTITY_ENABLED_PREF, this);
-    Services.obs.addObserver(this, "web-extension-preferences-replacing");
-    Services.obs.addObserver(this, "web-extension-preferences-replaced");
   },
 
   async observe(aSubject, aTopic) {
-    switch (aTopic) {
-      case "web-extension-preferences-replacing":
-        this._webExtensionUpdating = true;
-        break;
-      case "web-extension-preferences-replaced":
-        this._webExtensionUpdating = false;
-        // We want to check the pref when the extension has been replaced too
-      case "nsPref:changed":
-        const contextualIdentitiesEnabled = Services.prefs.getBoolPref(CONTEXTUAL_IDENTITY_ENABLED_PREF);
-        if (!contextualIdentitiesEnabled && !this._webExtensionUpdating) {
-          await this.closeContainerTabs();
-          this.notifyAllContainersCleared();
-          this.resetDefault();
-        }
-        break;
+    if (aTopic === "nsPref:changed") {
+      const contextualIdentitiesEnabled = Services.prefs.getBoolPref(CONTEXTUAL_IDENTITY_ENABLED_PREF);
+      if (!contextualIdentitiesEnabled) {
+        await this.closeContainerTabs();
+        this.notifyAllContainersCleared();
+        this.resetDefault();
+      }
     }
   },
 
   load() {
     OS.File.read(this._path).then(bytes => {
       // If synchronous loading happened in the meantime, exit now.
       if (this._dataReady) {
         return;
--- a/toolkit/components/extensions/ExtensionPreferencesManager.jsm
+++ b/toolkit/components/extensions/ExtensionPreferencesManager.jsm
@@ -31,46 +31,30 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/ExtensionSettingsStore.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
                                   "resource://gre/modules/Preferences.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "defaultPreferences", function() {
   return new Preferences({defaultBranch: true});
 });
 
-const ADDON_REPLACE_REASONS = new Set([
-  "ADDON_DOWNGRADE",
-  "ADDON_UPGRADE",
-]);
+/* eslint-disable mozilla/balanced-listeners */
+Management.on("uninstall", (type, {id}) => {
+  ExtensionPreferencesManager.removeAll(id);
+});
 
-/* eslint-disable mozilla/balanced-listeners */
 Management.on("shutdown", (type, extension) => {
-  switch (extension.shutdownReason) {
-    case "ADDON_DISABLE":
-    case "ADDON_DOWNGRADE":
-    case "ADDON_UPGRADE":
-      if (ADDON_REPLACE_REASONS.has(extension.shutdownReason)) {
-        Services.obs.notifyObservers(null, "web-extension-preferences-replacing");
-      }
-      this.ExtensionPreferencesManager.disableAll(extension.id);
-      break;
-
-    case "ADDON_UNINSTALL":
-      this.ExtensionPreferencesManager.removeAll(extension.id);
-      break;
+  if (extension.shutdownReason == "ADDON_DISABLE") {
+    this.ExtensionPreferencesManager.disableAll(extension.id);
   }
 });
 
 Management.on("startup", async (type, extension) => {
-  if (["ADDON_ENABLE", "ADDON_UPGRADE", "ADDON_DOWNGRADE"].includes(extension.startupReason)) {
-    const enablePromise = this.ExtensionPreferencesManager.enableAll(extension.id);
-    if (ADDON_REPLACE_REASONS.has(extension.startupReason)) {
-      await enablePromise;
-      Services.obs.notifyObservers(null, "web-extension-preferences-replaced");
-    }
+  if (extension.startupReason == "ADDON_ENABLE") {
+    this.ExtensionPreferencesManager.enableAll(extension.id);
   }
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 const STORE_TYPE = "prefs";
 
 // Definitions of settings, each of which correspond to a different API.
 let settingsMap = new Map();