Bug 1541317 support homepage setting on upgrade r=rpl,mkaply
☠☠ backed out by 5e69a320a5a1 ☠ ☠
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 26 Apr 2019 18:54:19 +0000
changeset 530418 f706ae6979228b41360809a5f5debb65a00b403f
parent 530417 1b7dc4afb065cf062dbf29500b05991319052132
child 530419 8292a59d195731409c327d64a5ca40cfc7777d03
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl, mkaply
bugs1541317
milestone68.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 1541317 support homepage setting on upgrade r=rpl,mkaply Differential Revision: https://phabricator.services.mozilla.com/D28265
browser/components/extensions/parent/ext-chrome-settings-overrides.js
browser/components/extensions/test/xpcshell/head.js
browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js
browser/components/extensions/test/xpcshell/test_ext_settings_overrides_search.js
toolkit/components/extensions/ExtensionPreferencesManager.jsm
--- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js
+++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js
@@ -176,24 +176,24 @@ this.chrome_settings_overrides = class e
     let {extension} = this;
     let {manifest} = extension;
 
     await ExtensionSettingsStore.initialize();
 
     let homepageUrl = manifest.chrome_settings_overrides.homepage;
 
     if (homepageUrl) {
-      let inControl;
-      if (extension.startupReason == "ADDON_INSTALL" ||
-          extension.startupReason == "ADDON_ENABLE") {
+      // Determine inControl before applying any update so that if a controlling extension
+      // updates its homepage url, the pref will get updated properly.
+      let item = await ExtensionPreferencesManager.getSetting("homepage_override");
+      let inControl = item && (item.id == extension.id);
+      if (["ADDON_INSTALL", "ADDON_ENABLE", "ADDON_UPGRADE"].includes(extension.startupReason)) {
+        const setAsCurrentHomepage = inControl || extension.startupReason === "ADDON_INSTALL";
         inControl = await ExtensionPreferencesManager.setSetting(
-          extension.id, "homepage_override", homepageUrl);
-      } else {
-        let item = await ExtensionPreferencesManager.getSetting("homepage_override");
-        inControl = item.id == extension.id;
+          extension.id, "homepage_override", homepageUrl, setAsCurrentHomepage);
       }
       // We need to add the listener here too since onPrefsChanged won't trigger on a
       // restart (the prefs are already set).
       if (inControl) {
         Services.prefs.setBoolPref(HOMEPAGE_PRIVATE_ALLOWED, extension.privateBrowsingAllowed);
         // Also set this now as an upgraded browser will need this.
         Services.prefs.setBoolPref(HOMEPAGE_EXTENSION_CONTROLLED, true);
         if (extension.startupReason == "APP_STARTUP") {
--- a/browser/components/extensions/test/xpcshell/head.js
+++ b/browser/components/extensions/test/xpcshell/head.js
@@ -1,30 +1,35 @@
 "use strict";
 
-/* exported createHttpServer, promiseConsoleOutput  */
+/* exported createHttpServer, promiseConsoleOutput, delay  */
 
 var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 var {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 // eslint-disable-next-line no-unused-vars
 XPCOMUtils.defineLazyModuleGetters(this, {
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   Extension: "resource://gre/modules/Extension.jsm",
   ExtensionData: "resource://gre/modules/Extension.jsm",
   ExtensionTestUtils: "resource://testing-common/ExtensionXPCShellUtils.jsm",
   FileUtils: "resource://gre/modules/FileUtils.jsm",
   HttpServer: "resource://testing-common/httpd.js",
   NetUtil: "resource://gre/modules/NetUtil.jsm",
   Schemas: "resource://gre/modules/Schemas.jsm",
   TestUtils: "resource://testing-common/TestUtils.jsm",
+  setTimeout: "resource://gre/modules/Timer.jsm",
 });
 
 Services.prefs.setBoolPref("extensions.webextensions.remote", false);
 
+function delay(ms = 0) {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
 ExtensionTestUtils.init(this);
 
 
 /**
  * Creates a new HttpServer for testing, and begins listening on the
  * specified port. Automatically shuts down the server when the test
  * unit ends.
  *
--- a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js
@@ -1,49 +1,46 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 const {AddonTestUtils} = ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm");
 const {HomePage} = ChromeUtils.import("resource:///modules/HomePage.jsm");
 
-const {
-  createAppInfo,
-  promiseShutdownManager,
-  promiseStartupManager,
-} = AddonTestUtils;
-
 AddonTestUtils.init(this);
 AddonTestUtils.overrideCertDB();
 
-createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "42");
+AddonTestUtils.createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "42");
+
+
+const HOMEPAGE_URL_PREF = "browser.startup.homepage";
+
+function promisePrefChanged(value) {
+  return new Promise((resolve, reject) => {
+    Services.prefs.addObserver(HOMEPAGE_URL_PREF, function observer() {
+      if (HomePage.get().endsWith(value)) {
+        Services.prefs.removeObserver(HOMEPAGE_URL_PREF, observer);
+        resolve();
+      }
+    });
+  });
+}
+
+add_task(async function startup() {
+  await AddonTestUtils.promiseStartupManager();
+});
 
 add_task(async function test_overrides_update_removal() {
   /* This tests the scenario where the manifest key for homepage and/or
    * search_provider are removed between updates and therefore the
    * settings are expected to revert. */
 
   const EXTENSION_ID = "test_overrides_update@tests.mozilla.org";
   const HOMEPAGE_URI = "webext-homepage-1.html";
 
-  const HOMEPAGE_URL_PREF = "browser.startup.homepage";
-
-  function promisePrefChanged(value) {
-    return new Promise((resolve, reject) => {
-      Services.prefs.addObserver(HOMEPAGE_URL_PREF, function observer() {
-        if (HomePage.get().endsWith(value)) {
-          Services.prefs.removeObserver(HOMEPAGE_URL_PREF, observer);
-          resolve();
-        }
-      });
-    });
-  }
-
-  await promiseStartupManager();
-
   let extensionInfo = {
     useAddonManager: "permanent",
     manifest: {
       "version": "1.0",
       "applications": {
         "gecko": {
           "id": EXTENSION_ID,
         },
@@ -66,37 +63,201 @@ add_task(async function test_overrides_u
   let prefPromise = promisePrefChanged(HOMEPAGE_URI);
   await extension.startup();
   await AddonTestUtils.waitForSearchProviderStartup(extension);
   await prefPromise;
 
   equal(extension.version, "1.0", "The installed addon has the expected version.");
   ok(HomePage.get().endsWith(HOMEPAGE_URI),
      "Home page url is overridden by the extension.");
-  equal((await Services.search.getDefault()).name,
-        "DuckDuckGo",
+  let engine = await Services.search.getDefault();
+  equal(engine.name, "DuckDuckGo",
+        "Default engine is overridden by the extension");
+  // Extensions cannot change a built-in search engine.
+  let url = engine._getURLOfType("text/html").template;
+  equal(url, "https://duckduckgo.com/",
+        "Extension cannot override default engine search template.");
+
+  // test changing the homepage
+  extensionInfo.manifest = {
+    "version": "2.0",
+    "applications": {
+      "gecko": {
+        "id": EXTENSION_ID,
+      },
+    },
+    "chrome_settings_overrides": {
+      "homepage": "webext-homepage-2.html",
+      "search_provider": {
+        "name": "DuckDuckGo",
+        "search_url": "https://example.org/?q={searchTerms}",
+        "is_default": true,
+      },
+    },
+  };
+
+  prefPromise = promisePrefChanged("webext-homepage-2.html");
+  await extension.upgrade(extensionInfo);
+  await AddonTestUtils.waitForSearchProviderStartup(extension);
+  await prefPromise;
+
+  equal(extension.version, "2.0", "The installed addon has the expected version.");
+  ok(HomePage.get().endsWith("webext-homepage-2.html"),
+     "Home page url is overridden by the extension.");
+  engine = await Services.search.getDefault();
+  equal(engine.name, "DuckDuckGo",
+        "Default engine is overridden by the extension");
+  url = engine._getURLOfType("text/html").template;
+  // Extensions cannot change a built-in search engine.
+  equal(url, "https://duckduckgo.com/",
         "Default engine is overridden by the extension");
 
   extensionInfo.manifest = {
-    "version": "2.0",
+    "version": "3.0",
     "applications": {
       "gecko": {
         "id": EXTENSION_ID,
       },
     },
   };
 
   prefPromise = promisePrefChanged(defaultHomepageURL);
   await extension.upgrade(extensionInfo);
   await prefPromise;
 
-  equal(extension.version, "2.0", "The updated addon has the expected version.");
+  equal(extension.version, "3.0", "The updated addon has the expected version.");
   equal(HomePage.get(),
         defaultHomepageURL,
         "Home page url reverted to the default after update.");
   equal((await Services.search.getDefault()).name,
         defaultEngineName,
         "Default engine reverted to the default after update.");
 
   await extension.unload();
+});
 
-  await promiseShutdownManager();
+add_task(async function test_overrides_update_adding() {
+  /* This tests the scenario where an addon adds support for
+   * a homepage or search service when upgrading. Neither
+   * should override existing entries for those when added
+   * in an upgrade. */
+
+  const EXTENSION_ID = "test_overrides_update@tests.mozilla.org";
+  const HOMEPAGE_URI = "webext-homepage-1.html";
+
+  let extensionInfo = {
+    useAddonManager: "permanent",
+    manifest: {
+      "version": "1.0",
+      "applications": {
+        "gecko": {
+          "id": EXTENSION_ID,
+        },
+      },
+    },
+  };
+  let extension = ExtensionTestUtils.loadExtension(extensionInfo);
+
+  let defaultHomepageURL = HomePage.get();
+  let defaultEngineName = (await Services.search.getDefault()).name;
+
+  await extension.startup();
+
+  equal(extension.version, "1.0", "The installed addon has the expected version.");
+  equal(HomePage.get(),
+        defaultHomepageURL,
+        "Home page url is the default after startup.");
+  equal((await Services.search.getDefault()).name,
+        defaultEngineName,
+        "Default engine is the default after startup.");
+
+  extensionInfo.manifest = {
+    "version": "2.0",
+    "applications": {
+      "gecko": {
+        "id": EXTENSION_ID,
+      },
+    },
+    "chrome_settings_overrides": {
+      "homepage": HOMEPAGE_URI,
+      "search_provider": {
+        "name": "MozSearch",
+        "search_url": "https://example.com/?q={searchTerms}",
+        "is_default": true,
+      },
+    },
+  };
+
+  await extension.upgrade(extensionInfo);
+  // The homepage pref shouldn't change here, we delay a tick to give
+  // a pref change a chance in case it were to happen.
+  await Promise.all([
+    AddonTestUtils.waitForSearchProviderStartup(extension),
+    delay(),
+  ]);
+
+  equal(extension.version, "2.0", "The updated addon has the expected version.");
+  ok(HomePage.get().endsWith(defaultHomepageURL),
+     "Home page url is not overridden by the extension during upgrade.");
+  ok(!!Services.search.getEngineByName("MozSearch"),
+     "Engine was installed by extension");
+  // An upgraded extension adding a search engine cannot override
+  // the default engine.
+  equal((await Services.search.getDefault()).name,
+        defaultEngineName,
+        "Default engine is the default after startup.");
+
+  await extension.unload();
 });
+
+add_task(async function test_overrides_update_changing() {
+  /* This tests the scenario where the homepage url changes
+   * due to the upgrade. */
+
+  const EXTENSION_ID = "test_overrides_changing@tests.mozilla.org";
+  const HOMEPAGE_URI = "webext-homepage-1.html";
+
+  let extensionInfo = {
+    useAddonManager: "temporary",
+    manifest: {
+      "version": "1.0",
+      "applications": {
+        "gecko": {
+          "id": EXTENSION_ID,
+        },
+      },
+      "chrome_settings_overrides": {
+        "homepage": HOMEPAGE_URI,
+      },
+    },
+  };
+  let extension = ExtensionTestUtils.loadExtension(extensionInfo);
+
+  let prefPromise = promisePrefChanged(HOMEPAGE_URI);
+  await extension.startup();
+  await prefPromise;
+
+  equal(extension.version, "1.0", "The installed addon has the expected version.");
+  ok(HomePage.get().endsWith(HOMEPAGE_URI),
+     "Home page url is the default after startup.");
+
+  extensionInfo.manifest = {
+    "version": "2.0",
+    "applications": {
+      "gecko": {
+        "id": EXTENSION_ID,
+      },
+    },
+    "chrome_settings_overrides": {
+      "homepage": HOMEPAGE_URI + ".2",
+    },
+  };
+
+  prefPromise = promisePrefChanged(HOMEPAGE_URI + ".2");
+  await extension.upgrade(extensionInfo);
+  await prefPromise;
+
+  equal(extension.version, "2.0", "The updated addon has the expected version.");
+  ok(HomePage.get().endsWith(HOMEPAGE_URI + ".2"),
+     "Home page url is not overridden by the extension during upgrade.");
+
+  await extension.unload();
+});
--- a/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_search.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_search.js
@@ -91,17 +91,16 @@ add_task(async function test_extension_a
 
   await ext1.unload();
   await delay();
 
   engine = Services.search.getEngineByName("MozSearch");
   ok(!engine, "Engine should not exist");
 });
 
-
 add_task(async function test_upgrade_default_position_engine() {
   let ext1 = ExtensionTestUtils.loadExtension({
     manifest: {
       "chrome_settings_overrides": {
         "search_provider": {
           "name": "MozSearch",
           "keyword": "MozSearch",
           "search_url": "https://example.com/?q={searchTerms}",
@@ -190,8 +189,66 @@ add_task(async function test_extension_p
 
   let expectedSuggestURL = kSearchSuggestURL.replace("{searchTerms}", kSearchTerm);
   let submissionSuggest = engine.getSubmission(kSearchTerm, URLTYPE_SUGGEST_JSON);
   equal(submissionSuggest.uri.spec, expectedSuggestURL, "Suggest URLs should match");
   equal(submissionSuggest.postData.data.data, "foo=bar&bar=foo", "Suggest postData should match");
 
   await ext1.unload();
 });
+
+// Test that an upgrade changing the search engine url will work.
+add_task(async function test_upgrade_searchengine_url() {
+  let ext1 = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "chrome_settings_overrides": {
+        "search_provider": {
+          "name": "MozSearch",
+          "keyword": "MozSearch",
+          "search_url": "https://example.com/?q={searchTerms}",
+        },
+      },
+      "applications": {
+        "gecko": {
+          "id": "testengine@mozilla.com",
+        },
+      },
+      "version": "0.1",
+    },
+    useAddonManager: "temporary",
+  });
+
+  await ext1.startup();
+  await AddonTestUtils.waitForSearchProviderStartup(ext1);
+  let engine = await Services.search.getDefault();
+  equal(engine.name, "MozSearch", "engine is default");
+  let url = engine._getURLOfType("text/html").template;
+  equal(url, "https://example.com/?q={searchTerms}",
+        "engine url is correct");
+
+  await ext1.upgrade({
+    manifest: {
+      "chrome_settings_overrides": {
+        "search_provider": {
+          "name": "MozSearch",
+          "keyword": "MozSearch",
+          "search_url": "https://example.org/?q={searchTerms}",
+        },
+      },
+      "applications": {
+        "gecko": {
+          "id": "testengine@mozilla.com",
+        },
+      },
+      "version": "0.2",
+    },
+    useAddonManager: "temporary",
+  });
+  await AddonTestUtils.waitForSearchProviderStartup(ext1);
+
+  engine = await Services.search.getDefault();
+  equal(engine.name, "MozSearch", "engine is default");
+  url = engine._getURLOfType("text/html").template;
+  equal(url, "https://example.org/?q={searchTerms}",
+        "engine url is correct");
+
+  await ext1.unload();
+});
--- a/toolkit/components/extensions/ExtensionPreferencesManager.jsm
+++ b/toolkit/components/extensions/ExtensionPreferencesManager.jsm
@@ -180,27 +180,29 @@ this.ExtensionPreferencesManager = {
    *
    * @param {string} id
    *        The id of the extension for which a setting is being set.
    * @param {string} name
    *        The unique id of the setting.
    * @param {any} value
    *        The value to be stored in the settings store for this
    *        group of preferences.
+   * @param {boolean} setPref
+   *        Update the prefs after adding the setting. Defaults to true.
    *
    * @returns {Promise}
    *          Resolves to true if the preferences were changed and to false if
    *          the preferences were not changed.
    */
-  async setSetting(id, name, value) {
+  async setSetting(id, name, value, setPref = true) {
     let setting = settingsMap.get(name);
     await ExtensionSettingsStore.initialize();
     let item = await ExtensionSettingsStore.addSetting(
       id, STORE_TYPE, name, value, initialValueCallback.bind(setting));
-    if (item) {
+    if (item && setPref) {
       setPrefs(setting, item);
       return true;
     }
     return false;
   },
 
   /**
    * Indicates that this extension wants to temporarily cede control over the