Backed out 2 changesets (bug 1635235, bug 1635239) for causing Bug 1640583.

CLOSED TREE Backed out changeset 97ecda13df18 (bug 1635239) Backed out changeset c9f80397bbec (bug 1635235)
2020-05-25 16:43:03 +03:00 · 2020-05-25 16:43:03 +03:00 · 33899422e7
--- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js
+++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js
@ -267,7 +267,7 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
  static async onUninstall(id) {
    let searchStartupPromise = pendingSearchSetupTasks.get(id);
    if (searchStartupPromise) {
-      await searchStartupPromise.catch(Cu.reportError);
+      await searchStartupPromise;
    }
    // Note: We do not have to deal with homepage here as it is managed by
    // the ExtensionPreferencesManager.
@ -423,7 +423,7 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
      } else {
        // Needs to be called every time to handle reenabling, but
        // only sets default for install or enable.
-        await this.setDefault(engineName);
+        this.setDefault(engineName);
      }
    }
  }
--- a/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_defaults.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_defaults.js
@ -11,16 +11,6 @@ const { SearchTestUtils } = ChromeUtils.import(
  "resource://testing-common/SearchTestUtils.jsm"
 );

-const { SearchUtils } = ChromeUtils.import(
-  "resource://gre/modules/SearchUtils.jsm"
-);
-
-const { RemoteSettings } = ChromeUtils.import(
-  "resource://services-settings/remote-settings.js"
-);
-
-const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
-
 const URLTYPE_SUGGEST_JSON = "application/x-suggestions+json";

 AddonTestUtils.init(this);
@ -128,8 +118,7 @@ add_task(async function test_extension_changing_to_app_provided_default() {
 });

 add_task(async function test_extension_overriding_app_provided_default() {
-  const settings = await RemoteSettings(SearchUtils.SETTINGS_ALLOWLIST_KEY);
-  sinon.stub(settings, "get").returns([
+  Services.search.wrappedJSObject._getSearchDefaultOverrideAllowlist = () => [
    {
      thirdPartyId: "test@thirdparty.example.com",
      overridesId: "test2@search.mozilla.org",
@ -139,7 +128,7 @@ add_task(async function test_extension_overriding_app_provided_default() {
        },
      ],
    },
-  ]);
+  ];

  let ext1 = ExtensionTestUtils.loadExtension({
    manifest: {
--- a/browser/installer/allowed-dupes.mn
+++ b/browser/installer/allowed-dupes.mn
@ -62,7 +62,6 @@ browser/chrome/browser/res/payments/formautofill/autofillEditForms.js
 # Bug 1451050 - Remote settings empty dumps (will be populated with data eventually)
 browser/defaults/settings/pinning/pins.json
 browser/defaults/settings/main/example.json
-browser/defaults/settings/main/search-default-override-allowlist.json

 #ifdef MOZ_EME_WIN32_ARTIFACT
 gmp-clearkey/0.1/manifest.json
--- a/services/settings/dumps/main/moz.build
+++ b/services/settings/dumps/main/moz.build
@ -9,7 +9,6 @@ FINAL_TARGET_FILES.defaults.settings.main += [
    'language-dictionaries.json',
    'onboarding.json',
    'search-config.json',
-    'search-default-override-allowlist.json',
    'sites-classification.json',
    'url-classifier-skip-urls.json',
 ]
--- a/services/settings/dumps/main/search-default-override-allowlist.json
+++ b/services/settings/dumps/main/search-default-override-allowlist.json
@ -1 +0,0 @@
-{"data":[]}
--- a/toolkit/components/search/SearchEngine.jsm
+++ b/toolkit/components/search/SearchEngine.jsm
@ -1919,12 +1919,7 @@ SearchEngine.prototype = {
   * @returns {string}
   */
  get telemetryId() {
-    let telemetryId =
-      this._telemetryId || this.identifier || `other-${this.name}`;
-    if (this.getAttr("overriddenBy")) {
-      return telemetryId + "-addon";
-    }
-    return telemetryId;
+    return this._telemetryId || this.identifier || `other-${this.name}`;
  },

  /**
--- a/toolkit/components/search/SearchService.jsm
+++ b/toolkit/components/search/SearchService.jsm
@ -21,7 +21,6 @@ XPCOMUtils.defineLazyModuleGetters(this, {
  IgnoreLists: "resource://gre/modules/IgnoreLists.jsm",
  OS: "resource://gre/modules/osfile.jsm",
  Region: "resource://gre/modules/Region.jsm",
-  RemoteSettings: "resource://services-settings/remote-settings.js",
  SearchEngine: "resource://gre/modules/SearchEngine.jsm",
  SearchEngineSelector: "resource://gre/modules/SearchEngineSelector.jsm",
  SearchStaticData: "resource://gre/modules/SearchStaticData.jsm",
@ -52,13 +51,6 @@ XPCOMUtils.defineLazyPreferenceGetter(
  }
 );

-XPCOMUtils.defineLazyGetter(this, "logConsole", () => {
-  return console.createInstance({
-    prefix: "SearchService",
-    maxLogLevel: SearchUtils.loggingEnabled ? "Debug" : "Warn",
-  });
-});
-
 // A text encoder to UTF8, used whenever we commit the cache to disk.
 XPCOMUtils.defineLazyGetter(this, "gEncoder", function() {
  return new TextEncoder();
@ -606,9 +598,6 @@ SearchService.prototype = {
   */
  _metaData: {},

-  // A reference to the handler for the default override allow list.
-  _defaultOverrideAllowlist: null,
-
  // This reflects the combined values of the prefs for enabling the separate
  // private default UI, and for the user choosing a separate private engine.
  // If either one is disabled, then we don't enable the separate private default.
@ -853,6 +842,32 @@ SearchService.prototype = {
    return false;
  },

+  async _getSearchDefaultOverrideAllowlist() {
+    return [];
+  },
+
+  async _canOverrideDefault(extension, appProvidedExtensionId) {
+    const overrideTable = await this._getSearchDefaultOverrideAllowlist();
+
+    let entry = overrideTable.find(e => e.thirdPartyId == extension.id);
+    if (!entry) {
+      return false;
+    }
+
+    if (appProvidedExtensionId != entry.overridesId) {
+      return false;
+    }
+    let searchProvider =
+      extension.manifest.chrome_settings_overrides.search_provider;
+    return entry.urls.some(
+      e =>
+        searchProvider.search_url == e.search_url &&
+        searchProvider.search_form == e.search_form &&
+        searchProvider.search_url_get_params == e.search_url_get_params &&
+        searchProvider.search_url_post_params == e.search_url_post_params
+    );
+  },
+
  async maybeSetAndOverrideDefault(extension) {
    let searchProvider =
      extension.manifest.chrome_settings_overrides.search_provider;
@ -866,10 +881,6 @@ SearchService.prototype = {
      SearchUtils.DEFAULT_TAG
    );

-    if (!this._defaultOverrideAllowlist) {
-      this._defaultOverrideAllowlist = new SearchDefaultOverrideAllowlistHandler();
-    }
-
    if (
      extension.startupReason === "ADDON_INSTALL" ||
      extension.startupReason === "ADDON_ENABLE"
@ -878,16 +889,7 @@ SearchService.prototype = {
      if (this.defaultEngine.name == searchProvider.name) {
        return false;
      }
-      if (
-        !(await this._defaultOverrideAllowlist.canOverride(
-          extension,
-          engine._extensionID
-        ))
-      ) {
-        logConsole.debug(
-          "Allowing default engine to be set to app-provided.",
-          extension.id
-        );
+      if (!(await this._canOverrideDefault(extension, engine._extensionID))) {
        // We don't allow overriding the engine in this case, but we can allow
        // the extension to change the default engine.
        return true;
@ -895,26 +897,15 @@ SearchService.prototype = {
      if (extension.startupReason === "ADDON_INSTALL") {
        // We're ok to override.
        engine.overrideWithExtension(params);
-        logConsole.debug(
-          "Allowing default engine to be set to app-provided and overridden.",
-          extension.id
-        );
        return true;
      }
    }

    if (
      engine.getAttr("overriddenBy") == extension.id &&
-      (await this._defaultOverrideAllowlist.canOverride(
-        extension,
-        engine._extensionID
-      ))
+      (await this._canOverrideDefault(extension, engine._extensionID))
    ) {
      engine.overrideWithExtension(params);
-      logConsole.debug(
-        "Re-enabling overriding of core extension by",
-        extension.id
-      );
      return true;
    }

@ -3892,81 +3883,4 @@ XPCOMUtils.defineLazyServiceGetter(
  "nsIIdleService"
 );

-/**
- * Handles getting and checking extensions against the allow list.
- */
-class SearchDefaultOverrideAllowlistHandler {
-  /**
-   * @param {function} listener
-   *   A listener for configuration update changes.
-   */
-  constructor(listener) {
-    this._remoteConfig = RemoteSettings(SearchUtils.SETTINGS_ALLOWLIST_KEY);
-  }
-
-  /**
-   * Determines if a search engine extension can override a default one
-   * according to the allow list.
-   *
-   * @param {object} extension
-   *   The extension object (from add-on manager) that will override the
-   *   app provided search engine.
-   * @param {string} appProvidedExtensionId
-   *   The id of the search engine that will be overriden.
-   * @returns {boolean}
-   *   Returns true if the search engine extension may override the app provided
-   *   instance.
-   */
-  async canOverride(extension, appProvidedExtensionId) {
-    const overrideTable = await this._getAllowlist();
-
-    let entry = overrideTable.find(e => e.thirdPartyId == extension.id);
-    if (!entry) {
-      return false;
-    }
-
-    if (appProvidedExtensionId != entry.overridesId) {
-      return false;
-    }
-
-    let searchProvider =
-      extension.manifest.chrome_settings_overrides.search_provider;
-
-    return entry.urls.some(
-      e =>
-        searchProvider.search_url == e.search_url &&
-        searchProvider.search_form == e.search_form &&
-        searchProvider.search_url_get_params == e.search_url_get_params &&
-        searchProvider.search_url_post_params == e.search_url_post_params
-    );
-  }
-
-  /**
-   * Obtains the configuration from remote settings. This includes
-   * verifying the signature of the record within the database.
-   *
-   * If the signature in the database is invalid, the database will be wiped
-   * and the stored dump will be used, until the settings next update.
-   *
-   * Note that this may cause a network check of the certificate, but that
-   * should generally be quick.
-   *
-   * @returns {array}
-   *   An array of objects in the database, or an empty array if none
-   *   could be obtained.
-   */
-  async _getAllowlist() {
-    let result = [];
-    try {
-      result = await this._remoteConfig.get();
-    } catch (ex) {
-      // Don't throw an error just log it, just continue with no data, and hopefully
-      // a sync will fix things later on.
-      Cu.reportError(ex);
-    }
-    logConsole.debug("Allow list is:", result);
-    return result;
-  }
-}
-
 var EXPORTED_SYMBOLS = ["SearchService"];
--- a/toolkit/components/search/SearchUtils.jsm
+++ b/toolkit/components/search/SearchUtils.jsm
@ -23,18 +23,6 @@ var SearchUtils = {

  SETTINGS_KEY: "search-config",

-  /**
-   * This is the Remote Settings key that we use to get the ignore lists for
-   * engines.
-   */
-  SETTINGS_IGNORELIST_KEY: "hijack-blocklists",
-
-  /**
-   * This is the Remote Settings key that we use to get the allow lists for
-   * overriding the default engines.
-   */
-  SETTINGS_ALLOWLIST_KEY: "search-default-override-allowlist",
-
  /**
   * Topic used for events involving the service itself.
   */
@ -80,6 +68,12 @@ var SearchUtils = {
  // resolution causes windows-1252 to be actually used.
  DEFAULT_QUERY_CHARSET: "ISO-8859-1",

+  /**
+   * This is the Remote Settings key that we use to get the ignore lists for
+   * engines.
+   */
+  SETTINGS_IGNORELIST_KEY: "hijack-blocklists",
+
  // A tag to denote when we are using the "default_locale" of an engine.
  DEFAULT_TAG: "default",

--- a/toolkit/components/search/tests/xpcshell/test_override_allowlist.js
+++ b/toolkit/components/search/tests/xpcshell/test_override_allowlist.js
@ -1,341 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const kBaseURL = "https://example.com/";
-const kSearchEngineURL = `${kBaseURL}?q={searchTerms}&foo=myparams`;
-const kOverriddenEngineName = "Simple Engine";
-
-SearchTestUtils.initXPCShellAddonManager(this);
-
-const whitelist = [
-  {
-    thirdPartyId: "test@thirdparty.example.com",
-    overridesId: "simple@search.mozilla.org",
-    urls: [],
-  },
-];
-
-const tests = [
-  {
-    title: "test_not_changing_anything",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: "MozParamsTest2",
-      keyword: "MozSearch",
-      search_url: kSearchEngineURL,
-    },
-    expected: {
-      switchToDefaultAllowed: false,
-      overridesEngine: false,
-    },
-  },
-  {
-    title: "test_changing_default_engine",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kSearchEngineURL,
-    },
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: false,
-    },
-  },
-  {
-    title: "test_overriding_default_engine",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kSearchEngineURL,
-    },
-    whitelistUrls: [
-      {
-        search_url: kSearchEngineURL,
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: true,
-      searchUrl: kSearchEngineURL,
-    },
-  },
-  {
-    title: "test_overriding_default_engine_different_url",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kSearchEngineURL + "a",
-    },
-    whitelistUrls: [
-      {
-        search_url: kSearchEngineURL,
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: false,
-    },
-  },
-  {
-    title: "test_overriding_default_engine_get_params",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kBaseURL,
-      search_url_get_params: "q={searchTerms}&enc=UTF-8",
-    },
-    whitelistUrls: [
-      {
-        search_url: kBaseURL,
-        search_url_get_params: "q={searchTerms}&enc=UTF-8",
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: true,
-      searchUrl: `${kBaseURL}?q={searchTerms}&enc=UTF-8`,
-    },
-  },
-  {
-    title: "test_overriding_default_engine_different_get_params",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kBaseURL,
-      search_url_get_params: "q={searchTerms}&enc=UTF-8a",
-    },
-    whitelistUrls: [
-      {
-        search_url: kBaseURL,
-        search_url_get_params: "q={searchTerms}&enc=UTF-8",
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: false,
-    },
-  },
-  {
-    title: "test_overriding_default_engine_post_params",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kBaseURL,
-      search_url_post_params: "q={searchTerms}&enc=UTF-8",
-    },
-    whitelistUrls: [
-      {
-        search_url: kBaseURL,
-        search_url_post_params: "q={searchTerms}&enc=UTF-8",
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: true,
-      searchUrl: `${kBaseURL}`,
-      postData: "q={searchTerms}&enc=UTF-8",
-    },
-  },
-  {
-    title: "test_overriding_default_engine_different_post_params",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kBaseURL,
-      search_url_post_params: "q={searchTerms}&enc=UTF-8a",
-    },
-    whitelistUrls: [
-      {
-        search_url: kBaseURL,
-        search_url_post_params: "q={searchTerms}&enc=UTF-8",
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: false,
-    },
-  },
-  {
-    title: "test_overriding_default_engine_search_form",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kBaseURL,
-      search_form: "https://example.com/form",
-    },
-    whitelistUrls: [
-      {
-        search_url: kBaseURL,
-        search_form: "https://example.com/form",
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: true,
-      searchUrl: `${kBaseURL}`,
-      searchForm: "https://example.com/form",
-    },
-  },
-  {
-    title: "test_overriding_default_engine_different_search_form",
-    startupReason: "ADDON_INSTALL",
-    search_provider: {
-      is_default: true,
-      name: kOverriddenEngineName,
-      keyword: "MozSearch",
-      search_url: kBaseURL,
-      search_form: "https://example.com/forma",
-    },
-    whitelistUrls: [
-      {
-        search_url: kBaseURL,
-        search_form: "https://example.com/form",
-      },
-    ],
-    expected: {
-      switchToDefaultAllowed: true,
-      overridesEngine: false,
-    },
-  },
-];
-
-let baseExtension;
-let remoteSettingsStub;
-
-add_task(async function setup() {
-  await SearchTestUtils.useTestEngines("simple-engines");
-  await AddonTestUtils.promiseStartupManager();
-  await Services.search.init();
-
-  baseExtension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      applications: {
-        gecko: {
-          id: "test@thirdparty.example.com",
-        },
-      },
-    },
-    useAddonManager: "permanent",
-  });
-  await baseExtension.startup();
-
-  const settings = await RemoteSettings(SearchUtils.SETTINGS_ALLOWLIST_KEY);
-  remoteSettingsStub = sinon.stub(settings, "get").returns([]);
-
-  registerCleanupFunction(async () => {
-    await baseExtension.unload();
-  });
-});
-
-for (const test of tests) {
-  add_task(async () => {
-    info(test.title);
-
-    let extension = {
-      ...baseExtension,
-      startupReason: test.startupReason,
-      manifest: {
-        chrome_settings_overrides: {
-          search_provider: test.search_provider,
-        },
-      },
-    };
-
-    if (test.expected.overridesEngine) {
-      remoteSettingsStub.returns([
-        { ...whitelist[0], urls: test.whitelistUrls },
-      ]);
-    }
-
-    Assert.equal(
-      await Services.search.maybeSetAndOverrideDefault(extension),
-      test.expected.switchToDefaultAllowed,
-      "Should have returned the correct value for allowing switch to default or not."
-    );
-
-    let engine = await Services.search.getEngineByName(kOverriddenEngineName);
-    Assert.equal(
-      !!engine.wrappedJSObject.getAttr("overriddenBy"),
-      test.expected.overridesEngine,
-      "Should have correctly overridden or not."
-    );
-
-    Assert.equal(
-      engine.telemetryId,
-      "simple" + (test.expected.overridesEngine ? "-addon" : ""),
-      "Should set the correct telemetry Id"
-    );
-
-    if (test.expected.overridesEngine) {
-      let submission = engine.getSubmission("{searchTerms}");
-      Assert.equal(
-        decodeURI(submission.uri.spec),
-        test.expected.searchUrl,
-        "Should have set the correct url on an overriden engine"
-      );
-
-      if (test.expected.search_form) {
-        Assert.equal(
-          engine.wrappedJSObject._searchForm,
-          test.expected.searchForm,
-          "Should have overridden the search form."
-        );
-      }
-
-      if (test.expected.postData) {
-        let sis = Cc["@mozilla.org/scriptableinputstream;1"].createInstance(
-          Ci.nsIScriptableInputStream
-        );
-        sis.init(submission.postData);
-        let data = sis.read(submission.postData.available());
-        Assert.equal(
-          decodeURIComponent(data),
-          test.expected.postData,
-          "Should have overridden the postData"
-        );
-      }
-
-      // As we're not testing the WebExtension manager as well,
-      // set this engine as default so we can check the telemetry data.
-      let oldDefaultEngine = Services.search.defaultEngine;
-      Services.search.defaultEngine = engine;
-
-      let engineInfo = await Services.search.getDefaultEngineInfo();
-      Assert.deepEqual(
-        engineInfo,
-        {
-          defaultSearchEngine: "simple-addon",
-          defaultSearchEngineData: {
-            loadPath: "[other]addEngineWithDetails:simple@search.mozilla.org",
-            name: "Simple Engine",
-            origin: "default",
-            submissionURL: test.expected.searchUrl.replace("{searchTerms}", ""),
-          },
-        },
-        "Should return the extended identifier and alternate submission url to telemetry"
-      );
-      Services.search.defaultEngine = oldDefaultEngine;
-
-      engine.wrappedJSObject.removeExtensionOverride();
-    }
-  });
-}
--- a/toolkit/components/search/tests/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell-common.ini
@ -52,7 +52,6 @@ support-files =
 [test_nodb_pluschanges.js]
 [test_notifications.js]
 [test_originalDefaultEngine.js]
-[test_override_allowlist.js]
 [test_paramSubstitution.js]
 [test_parseSubmissionURL.js]
 [test_pref.js]