Bug 1638888 - Make disabling and enabling a WebExtension restore the default search engine correctly. r=mixedpuppy,extension-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D76503

browser/components/extensions/parent/ext-chrome-settings-overrides.js

@@ -185,31 +185,23 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
     await ExtensionSettingsStore.initialize();
     let item = ExtensionSettingsStore.getSetting(
       DEFAULT_SEARCH_STORE_TYPE,
-      DEFAULT_SEARCH_SETTING_NAME
+      DEFAULT_SEARCH_SETTING_NAME,
+      id
     );
     if (!item) {
       return;
     }
-    if (
-      Services.search.defaultEngine.name != item.value &&
-      Services.search.defaultEngine.name != item.initialValue
-    ) {
-      // The current engine is not the same as the value that the ExtensionSettingsStore has.
-      // This means that the user changed the engine, so we shouldn't control it anymore.
-      // Do nothing and remove our entry from the ExtensionSettingsStore.
-      ExtensionSettingsStore.removeSetting(
-        id,
-        DEFAULT_SEARCH_STORE_TYPE,
-        DEFAULT_SEARCH_SETTING_NAME
-      );
-      return;
-    }
+    let control = await ExtensionSettingsStore.getLevelOfControl(
+      id,
+      DEFAULT_SEARCH_STORE_TYPE,
+      DEFAULT_SEARCH_SETTING_NAME
+    );
     item = ExtensionSettingsStore[action](
       id,
       DEFAULT_SEARCH_STORE_TYPE,
       DEFAULT_SEARCH_SETTING_NAME
     );
-    if (item) {
+    if (item && control == "controlled_by_this_extension") {
       try {
         let engine = Services.search.getEngineByName(
           item.value || item.initialValue
@@ -261,7 +253,19 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
   }
 
   static async onEnabling(id) {
-    chrome_settings_overrides.processDefaultSearchSetting("enable", id);
+    await ExtensionSettingsStore.initialize();
+    let item = await ExtensionSettingsStore.getSetting(
+      DEFAULT_SEARCH_STORE_TYPE,
+      DEFAULT_SEARCH_SETTING_NAME,
+      id
+    );
+    if (item) {
+      ExtensionSettingsStore.enable(
+        id,
+        DEFAULT_SEARCH_STORE_TYPE,
+        DEFAULT_SEARCH_SETTING_NAME
+      );
+    }
   }
 
   static async onUninstall(id) {
@@ -311,11 +315,11 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
     }
   }
 
-  static onDisable(id) {
+  static async onDisable(id) {
     homepagePopup.clearConfirmation(id);
 
-    chrome_settings_overrides.processDefaultSearchSetting("disable", id);
-    chrome_settings_overrides.removeEngine(id);
+    await chrome_settings_overrides.processDefaultSearchSetting("disable", id);
+    await chrome_settings_overrides.removeEngine(id);
   }
 
   async onManifestEntry(entryName) {
@@ -391,6 +395,8 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
       await this.setDefault(engineName);
     }
     if (!result.canInstallEngine) {
+      // This extension is overriding an app-provided one, so we don't
+      // add its engine as well.
       return;
     }
     await this.addSearchEngine();
@@ -443,6 +449,9 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
     if (extension.startupReason === "ADDON_INSTALL") {
       let defaultEngine = await Services.search.getDefault();
       await ExtensionSettingsStore.initialize();
+      // We should only get here if an extension is setting an app-provided
+      // engine to default and we are ignoring the addons other engine settings.
+      // In this case we do not show the prompt to the user.
       let item = await ExtensionSettingsStore.addSetting(
         extension.id,
         DEFAULT_SEARCH_STORE_TYPE,
@@ -454,10 +463,18 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
         Services.search.getEngineByName(item.value)
       );
     } else if (extension.startupReason === "ADDON_ENABLE") {
-      chrome_settings_overrides.processDefaultSearchSetting(
-        "enable",
-        extension.id
+      // We would be called for every extension being enabled, we should verify
+      // that it has control and only then set it as default
+      let control = await ExtensionSettingsStore.getLevelOfControl(
+        extension.id,
+        DEFAULT_SEARCH_STORE_TYPE,
+        DEFAULT_SEARCH_SETTING_NAME
       );
+      if (control === "controlled_by_this_extension") {
+        await Services.search.setDefault(
+          Services.search.getEngineByName(engineName)
+        );
+      }
     }
   }
 

browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js

@@ -12,9 +12,14 @@ ChromeUtils.defineModuleGetter(
 const { AddonTestUtils } = ChromeUtils.import(
   "resource://testing-common/AddonTestUtils.jsm"
 );
+const { SearchTestUtils } = ChromeUtils.import(
+  "resource://testing-common/SearchTestUtils.jsm"
+);
 
 const EXTENSION1_ID = "extension1@mozilla.com";
 const EXTENSION2_ID = "extension2@mozilla.com";
+const DEFAULT_SEARCH_STORE_TYPE = "default_search";
+const DEFAULT_SEARCH_SETTING_NAME = "defaultSearch";
 
 AddonTestUtils.initMochitest(this);
 
@@ -115,6 +120,11 @@ add_task(async function test_extension_setting_default_engine_external() {
   async function startExtension(win = window) {
     let extension = ExtensionTestUtils.loadExtension({
       manifest: {
+        applications: {
+          gecko: {
+            id: EXTENSION1_ID,
+          },
+        },
         chrome_settings_overrides: {
           search_provider: {
             name: NAME,
@@ -170,6 +180,29 @@ add_task(async function test_extension_setting_default_engine_external() {
     "Default engine was changed after accepting prompt"
   );
 
+  // Do this twice to make sure we're definitely handling disable/enable
+  // correctly.
+  let disabledPromise = awaitEvent("shutdown", EXTENSION1_ID);
+  let addon = await AddonManager.getAddonByID(EXTENSION1_ID);
+  await addon.disable();
+  await disabledPromise;
+
+  is(
+    (await Services.search.getDefault()).name,
+    "Google",
+    "Default engine is Google after disabling"
+  );
+
+  let processedPromise = awaitEvent("searchEngineProcessed", EXTENSION1_ID);
+  await addon.enable();
+  await processedPromise;
+
+  is(
+    (await Services.search.getDefault()).name,
+    NAME,
+    `Default engine is ${NAME} after enabling`
+  );
+
   await extension.unload();
 
   is(
@@ -353,6 +386,12 @@ add_task(async function test_user_changing_default_engine() {
 
   let engine = Services.search.getEngineByName("Bing");
   await Services.search.setDefault(engine);
+  // This simulates the preferences UI when the setting is changed.
+  ExtensionSettingsStore.select(
+    ExtensionSettingsStore.SETTING_USER_SET,
+    DEFAULT_SEARCH_STORE_TYPE,
+    DEFAULT_SEARCH_SETTING_NAME
+  );
 
   await ext1.unload();
 
@@ -396,6 +435,12 @@ add_task(async function test_user_change_with_disabling() {
 
   let engine = Services.search.getEngineByName("Bing");
   await Services.search.setDefault(engine);
+  // This simulates the preferences UI when the setting is changed.
+  ExtensionSettingsStore.select(
+    ExtensionSettingsStore.SETTING_USER_SET,
+    DEFAULT_SEARCH_STORE_TYPE,
+    DEFAULT_SEARCH_SETTING_NAME
+  );
 
   is(
     (await Services.search.getDefault()).name,
@@ -684,9 +729,14 @@ add_task(async function test_two_addons_with_second_disabled() {
     "Default engine is DuckDuckGo"
   );
 
+  let defaultPromise = SearchTestUtils.promiseSearchNotification(
+    "engine-default",
+    "browser-search-engine-modified"
+  );
   let enabledPromise = awaitEvent("ready", EXTENSION2_ID);
   await addon2.enable();
   await enabledPromise;
+  await defaultPromise;
 
   is(
     (await Services.search.getDefault()).name,

toolkit/components/extensions/ExtensionSettingsStore.jsm

@@ -588,7 +588,7 @@ var ExtensionSettingsStore = {
       // When user set, the setting is never "controllable" unless the installDate
       // is later than the user date.
       let addon = await AddonManager.getAddonByID(id);
-      return keyInfo.selectedDate > addon.installDate.valueOf()
+      return !addon || keyInfo.selectedDate > addon.installDate.valueOf()
         ? "not_controllable"
         : "controllable_by_this_extension";
     }
@@ -604,7 +604,7 @@ var ExtensionSettingsStore = {
     }
 
     let addon = await AddonManager.getAddonByID(id);
-    return topItem.installDate > addon.installDate.valueOf()
+    return !addon || topItem.installDate > addon.installDate.valueOf()
       ? "controlled_by_other_extensions"
       : "controllable_by_this_extension";
   },