Bug 1576266 fix handling of non-default prefs used with ExtensionPreferencesManager r=robwu a=lizzard
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 11 Sep 2019 13:59:53 +0000
changeset 555234 1ee62510b8ebfce59f516dcf56e0e2e52206fafd
parent 555233 fd3a74b0bfaca4b93bb4fa90036387bad305d07d
child 555235 eaf5ba87c7fcafc0ace60fc7a69f67452032c274
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobwu, lizzard
bugs1576266
milestone70.0
Bug 1576266 fix handling of non-default prefs used with ExtensionPreferencesManager r=robwu a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D44077
toolkit/components/extensions/ExtensionPreferencesManager.jsm
toolkit/components/extensions/parent/ext-contextualIdentities.js
toolkit/components/extensions/parent/ext-privacy.js
toolkit/components/extensions/parent/ext-proxy.js
toolkit/components/extensions/test/xpcshell/test_ext_extensionPreferencesManager.js
--- a/toolkit/components/extensions/ExtensionPreferencesManager.jsm
+++ b/toolkit/components/extensions/ExtensionPreferencesManager.jsm
@@ -93,17 +93,17 @@ function initialValueCallback() {
  *        with item when the preferences change.
  * @param {Object} item
  *        An object that represents an item handed back from the setting store
  *        from which the new pref values can be calculated.
  */
 function setPrefs(setting, item) {
   let prefs = item.initialValue || setting.setCallback(item.value);
   let changed = false;
-  for (let pref in prefs) {
+  for (let pref of setting.prefNames) {
     if (prefs[pref] === undefined) {
       if (Preferences.isSet(pref)) {
         changed = true;
         Preferences.reset(pref);
       }
     } else if (Preferences.get(pref) != prefs[pref]) {
       Preferences.set(pref, prefs[pref]);
       changed = true;
--- a/toolkit/components/extensions/parent/ext-contextualIdentities.js
+++ b/toolkit/components/extensions/parent/ext-contextualIdentities.js
@@ -119,22 +119,17 @@ ExtensionPreferencesManager.addSetting(C
 
   setCallback(value) {
     if (value !== true) {
       return {
         ...CONTAINER_PREF_INSTALL_DEFAULTS,
         "privacy.userContext.extension": value,
       };
     }
-
-    let prefs = {};
-    for (let pref of this.prefNames) {
-      prefs[pref] = undefined;
-    }
-    return prefs;
+    return {};
   },
 });
 
 this.contextualIdentities = class extends ExtensionAPI {
   onStartup() {
     let { extension } = this;
 
     if (extension.hasPermission("contextualIdentities")) {
--- a/toolkit/components/extensions/parent/ext-privacy.js
+++ b/toolkit/components/extensions/parent/ext-privacy.js
@@ -96,20 +96,16 @@ ExtensionPreferencesManager.addSetting("
     "media.peerconnection.ice.default_address_only",
     "media.peerconnection.ice.no_host",
     "media.peerconnection.ice.proxy_only_if_behind_proxy",
     "media.peerconnection.ice.proxy_only",
   ],
 
   setCallback(value) {
     let prefs = {};
-    // Start with all prefs being reset.
-    for (let pref of this.prefNames) {
-      prefs[pref] = undefined;
-    }
     switch (value) {
       case "default":
         // All prefs are already set to be reset.
         break;
 
       case "default_public_and_private_interfaces":
         prefs["media.peerconnection.ice.default_address_only"] = true;
         break;
--- a/toolkit/components/extensions/parent/ext-proxy.js
+++ b/toolkit/components/extensions/parent/ext-proxy.js
@@ -76,19 +76,16 @@ ExtensionPreferencesManager.addSetting("
     for (let prop of ["http", "ftp", "ssl", "socks"]) {
       if (value[prop]) {
         let url = new URL(`http://${value[prop]}`);
         prefs[`network.proxy.${prop}`] = url.hostname;
         // Only fall back to defaults if no port provided.
         let [, rawPort] = value[prop].split(":");
         let port = parseInt(rawPort, 10) || DEFAULT_PORTS.get(prop);
         prefs[`network.proxy.${prop}_port`] = port;
-      } else {
-        prefs[`network.proxy.${prop}`] = undefined;
-        prefs[`network.proxy.${prop}_port`] = undefined;
       }
     }
 
     return prefs;
   },
 });
 
 function registerProxyFilterEvent(
--- a/toolkit/components/extensions/test/xpcshell/test_ext_extensionPreferencesManager.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_extensionPreferencesManager.js
@@ -542,44 +542,96 @@ add_task(async function test_preference_
     useAddonManager: "temporary",
     manifest: {
       applications: { gecko: { id } },
     },
   });
 
   await extension.startup();
 
+  // We test both a default pref and a user-set pref.  Get the default
+  // value off the pref we'll use.  We fake the default pref by setting
+  // a value on it before creating the setting.
+  Services.prefs.setBoolPref("bar", true);
+
+  function isUndefinedPref(pref) {
+    try {
+      Services.prefs.getStringPref(pref);
+      return false;
+    } catch (e) {
+      return true;
+    }
+  }
+  ok(isUndefinedPref("foo"), "test pref is not set");
+
   await ExtensionSettingsStore.initialize();
   ExtensionPreferencesManager.addSetting("some-pref", {
-    pref_names: ["foo"],
+    prefNames: ["foo", "bar"],
     setCallback(value) {
-      return { foo: value };
+      return { [this.prefNames[0]]: value, [this.prefNames[1]]: false };
     },
   });
 
-  // We want to test that ExtensionPreferencesManager.setSetting() will enable a
-  // disabled setting so we will manually add and disable it in
-  // ExtensionSettingsStore.
-  await ExtensionSettingsStore.addSetting(
-    id,
-    "prefs",
-    "some-pref",
+  await ExtensionPreferencesManager.setSetting(id, "some-pref", "my value");
+
+  let item = ExtensionSettingsStore.getSetting("prefs", "some-pref");
+  equal(item.value, "my value", "The value has been set");
+  equal(
+    Services.prefs.getStringPref("foo"),
     "my value",
-    () => "default"
+    "The user pref has been set"
+  );
+  equal(
+    Services.prefs.getBoolPref("bar"),
+    false,
+    "The default pref has been set"
   );
 
-  let item = ExtensionSettingsStore.getSetting("prefs", "some-pref");
-  equal(item.value, "my value", "The value is set");
+  await ExtensionPreferencesManager.disableSetting(id, "some-pref");
 
-  ExtensionSettingsStore.disable(id, "prefs", "some-pref");
-
+  // test that a disabled setting has been returned to the default value.  In this
+  // case the pref is not a default pref, so it will be undefined.
   item = ExtensionSettingsStore.getSetting("prefs", "some-pref");
-  equal(item.initialValue, "default", "The value is back to default");
+  equal(item.value, undefined, "The value is back to default");
+  equal(item.initialValue.foo, undefined, "The initialValue is correct");
+  ok(isUndefinedPref("foo"), "user pref is not set");
+  equal(
+    Services.prefs.getBoolPref("bar"),
+    true,
+    "The default pref has been restored to the default"
+  );
 
+  // test that setSetting() will enable a disabled setting
   await ExtensionPreferencesManager.setSetting(id, "some-pref", "new value");
 
   item = ExtensionSettingsStore.getSetting("prefs", "some-pref");
   equal(item.value, "new value", "The value is set again");
+  equal(
+    Services.prefs.getStringPref("foo"),
+    "new value",
+    "The user pref is set again"
+  );
+  equal(
+    Services.prefs.getBoolPref("bar"),
+    false,
+    "The default pref has been set again"
+  );
 
+  // Force settings to be serialized and reloaded to mimick what happens
+  // with settings through a restart of Firefox.  Bug 1576266.
+  await ExtensionSettingsStore._reloadFile(true);
+
+  // Now unload the extension to test prefs are reset properly.
   await extension.unload();
 
+  // Test that the pref is unset when an extension is uninstalled.
+  item = await ExtensionPreferencesManager.getSetting("prefs", "some-pref");
+  equal(item, null, "The value has been reset");
+  ok(isUndefinedPref("foo"), "user pref is not set");
+  equal(
+    Services.prefs.getBoolPref("bar"),
+    true,
+    "The default pref has been restored to the default"
+  );
+  Services.prefs.clearUserPref("bar");
+
   await promiseShutdownManager();
 });