Bug 1446508 - Track prefs that had their default values changed, and restore them automatically when restarting the policy engine. r=MattN draft
authorFelipe Gomes <felipc@gmail.com>
Wed, 21 Mar 2018 01:10:17 -0300
changeset 770371 35417378df1b4c349463e5afacbf14b12176ac7f
parent 770368 055ee0486fb58c99b49d30a6776195607206c745
child 770372 08ad5a6dd1cad4f8475bcc5f5301808ae2b65bb9
push id103392
push userfelipc@gmail.com
push dateWed, 21 Mar 2018 04:12:56 +0000
reviewersMattN
bugs1446508
milestone61.0a1
Bug 1446508 - Track prefs that had their default values changed, and restore them automatically when restarting the policy engine. r=MattN MozReview-Commit-ID: 1pASgMovedA
browser/components/enterprisepolicies/tests/EnterprisePolicyTesting.jsm
browser/components/enterprisepolicies/tests/browser/browser_policies_setAndLockPref_API.js
browser/components/enterprisepolicies/tests/browser/browser_policy_default_browser_check.js
browser/components/enterprisepolicies/tests/browser/browser_policy_disable_fxaccounts.js
browser/components/enterprisepolicies/tests/browser/browser_policy_disable_pocket.js
browser/components/enterprisepolicies/tests/browser/browser_policy_enable_tracking_protection.js
browser/components/enterprisepolicies/tests/browser/browser_policy_set_homepage.js
browser/components/enterprisepolicies/tests/browser/head.js
--- a/browser/components/enterprisepolicies/tests/EnterprisePolicyTesting.jsm
+++ b/browser/components/enterprisepolicies/tests/EnterprisePolicyTesting.jsm
@@ -6,19 +6,19 @@
 
 ChromeUtils.import("resource://gre/modules/Preferences.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/osfile.jsm");
 ChromeUtils.import("resource://testing-common/Assert.jsm");
 ChromeUtils.defineModuleGetter(this, "FileTestUtils",
                                "resource://testing-common/FileTestUtils.jsm");
 
-this.EXPORTED_SYMBOLS = ["EnterprisePolicyTesting"];
+var EXPORTED_SYMBOLS = ["EnterprisePolicyTesting", "PoliciesPrefTracker"];
 
-this.EnterprisePolicyTesting = {
+var EnterprisePolicyTesting = {
   // |json| must be an object representing the desired policy configuration, OR a
   // path to the JSON file containing the policy configuration.
   setupPolicyEngineWithJson: async function setupPolicyEngineWithJson(json, customSchema) {
     let filePath;
     if (typeof(json) == "object") {
       filePath = FileTestUtils.getTempFile("policies.json").path;
 
       // This file gets automatically deleted by FileTestUtils
@@ -68,8 +68,83 @@ this.EnterprisePolicyTesting = {
     for (let base of runOnceBaseKeys) {
       for (let key of Services.prefs.getChildList(base, {})) {
         if (Services.prefs.prefHasUserValue(key))
           Services.prefs.clearUserPref(key);
       }
     }
   },
 };
+
+/**
+ * This helper will track prefs that have been changed
+ * by the policy engine through the setAndLockPref and
+ * setDefaultPref APIs (from Policies.jsm) and make sure
+ * that they are restored to their original values when
+ * the test ends or another test case restarts the engine.
+ */
+var PoliciesPrefTracker = {
+  _originalFunc: null,
+  _originalValues: new Map(),
+
+  start() {
+    let PoliciesBackstage = ChromeUtils.import("resource:///modules/policies/Policies.jsm", {});
+    this._originalFunc = PoliciesBackstage.setDefaultPref;
+    PoliciesBackstage.setDefaultPref = this.hoistedSetDefaultPref.bind(this);
+  },
+
+  stop() {
+    this.restoreDefaultValues();
+
+    let PoliciesBackstage = ChromeUtils.import("resource:///modules/policies/Policies.jsm", {});
+    PoliciesBackstage.setDefaultPref = this._originalFunc;
+    this._originalFunc = null;
+  },
+
+  hoistedSetDefaultPref(prefName, prefValue) {
+    // If this pref is seen multiple times, the very first
+    // value seen is the one that is actually the default.
+    if (!this._originalValues.has(prefName)) {
+      let defaults = new Preferences({defaultBranch: true});
+      let stored = {};
+
+      if (defaults.has(prefName)) {
+        stored.originalDefaultValue = defaults.get(prefName);
+      }
+
+      if (Preferences.isSet(prefName) &&
+          Preferences.get(prefName) == prefValue) {
+        // If a user value exists, and we're changing the default
+        // value to be th same as the user value, that will cause
+        // the user value to be dropped. In that case, let's also
+        // store it to ensure that we restore everything correctly.
+        stored.originalUserValue = Preferences.get(prefName);
+      }
+
+      this._originalValues.set(prefName, stored);
+    }
+
+    // Now that we've stored the original values, call the
+    // original setDefaultPref function.
+    this._originalFunc(prefName, prefValue);
+  },
+
+  restoreDefaultValues() {
+    let defaults = new Preferences({defaultBranch: true});
+
+    for (let [prefName, stored] of this._originalValues) {
+      // If a pref was used through setDefaultPref instead
+      // of setAndLockPref, it wasn't locked, but calling
+      // unlockPref is harmless
+      Preferences.unlock(prefName);
+
+      if (stored.originalDefaultValue) {
+        defaults.set(prefName, stored.originalDefaultValue);
+      }
+
+      if (stored.originalUserValue) {
+        Preferences.set(prefName, stored.originalUserValue);
+      }
+    }
+
+    this._originalValues.clear();
+  },
+};
--- a/browser/components/enterprisepolicies/tests/browser/browser_policies_setAndLockPref_API.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policies_setAndLockPref_API.js
@@ -1,16 +1,21 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-let { Policies, setAndLockPref } = ChromeUtils.import("resource:///modules/policies/Policies.jsm", {});
+let {
+  Policies,
+  setAndLockPref,
+  setDefaultPref,
+} = ChromeUtils.import("resource:///modules/policies/Policies.jsm", {});
 
 add_task(async function test_API_directly() {
+  await setupPolicyEngineWithJson("");
   setAndLockPref("policies.test.boolPref", true);
   checkLockedPref("policies.test.boolPref", true);
 
   // Check that a previously-locked pref can be changed
   // (it will be unlocked first).
   setAndLockPref("policies.test.boolPref", false);
   checkLockedPref("policies.test.boolPref", false);
 
@@ -99,8 +104,40 @@ add_task(async function test_API_through
   checkLockedPref("policies.test2.boolPref", true);
   checkLockedPref("policies.test2.intPref", 42);
   checkLockedPref("policies.test2.stringPref", "policies test 2");
 
   delete Policies.bool_policy;
   delete Policies.int_policy;
   delete Policies.string_policy;
 });
+
+add_task(async function test_pref_tracker() {
+  // Tests the test harness functionality that tracks usage of
+  // the setAndLockPref and setDefualtPref APIs.
+
+  let defaults = Services.prefs.getDefaultBranch("");
+
+  // Test prefs that had a default value and got changed to another
+  defaults.setIntPref("test1.pref1", 10);
+  defaults.setStringPref("test1.pref2", "test");
+
+  setAndLockPref("test1.pref1", 20);
+  setDefaultPref("test1.pref2", "NEW VALUE");
+
+  PoliciesPrefTracker.restoreDefaultValues();
+
+  is(Services.prefs.getIntPref("test1.pref1"), 10, "Expected value for test1.pref1");
+  is(Services.prefs.getStringPref("test1.pref2"), "test", "Expected value for test1.pref2");
+  is(Services.prefs.prefIsLocked("test1.pref1"), false, "test1.pref1 got unlocked");
+
+  // Test a pref that had a default value and a user value
+  defaults.setIntPref("test2.pref1", 10);
+  Services.prefs.setIntPref("test2.pref1", 20);
+
+  setAndLockPref("test2.pref1", 20);
+
+  PoliciesPrefTracker.restoreDefaultValues();
+
+  is(Services.prefs.getIntPref("test2.pref1"), 20, "Correct user value");
+  is(defaults.getIntPref("test2.pref1"), 10, "Correct default value");
+  is(Services.prefs.prefIsLocked("test2.pref1"), false, "felipe pref is not locked");
+});
--- a/browser/components/enterprisepolicies/tests/browser/browser_policy_default_browser_check.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policy_default_browser_check.js
@@ -19,13 +19,9 @@ add_task(async function test_default_bro
   });
 
   is(ShellService.shouldCheckDefaultBrowser, false, "Policy changed it to not check");
 
   // Try to change it to true and check that it doesn't take effect
   ShellService.shouldCheckDefaultBrowser = true;
 
   is(ShellService.shouldCheckDefaultBrowser, false, "Policy is enforced");
-
-  // Unlock the pref because if it stays locked, and this test runs twice in a row,
-  // the first sanity check will fail.
-  Services.prefs.unlockPref("browser.shell.checkDefaultBrowser");
 });
--- a/browser/components/enterprisepolicies/tests/browser/browser_policy_disable_fxaccounts.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policy_disable_fxaccounts.js
@@ -8,16 +8,9 @@ add_task(async function test_policy_disa
 
   await setupPolicyEngineWithJson({
     "policies": {
       "DisableFirefoxAccounts": true
     }
   });
 
   is(gSync.SYNC_ENABLED, false, "Sync is disabled after setting the policy.");
-
-  // Manually clean-up the change made by the policy engine.
-  // This is needed in case this test runs twice in a row
-  // (such as in test-verify), in order for the first check
-  // to pass again.
-  Services.prefs.unlockPref("identity.fxaccounts.enabled");
-  Services.prefs.getDefaultBranch("").setBoolPref("identity.fxaccounts.enabled", true);
 });
--- a/browser/components/enterprisepolicies/tests/browser/browser_policy_disable_pocket.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policy_disable_pocket.js
@@ -19,18 +19,10 @@ add_task(async function test_disable_fir
 
     await setupPolicyEngineWithJson({
       "policies": {
         "DisablePocket": true
       }
     });
 
     await checkPocket(false);
-
-    // Clear the change made by the engine to restore Pocket.
-    // This is needed in case this test runs twice in a row
-    // (such as in test-verify), in order for the first sanity check
-    // to pass again.
-    await setupPolicyEngineWithJson("");
-    Services.prefs.unlockPref(PREF_POCKET);
-    Services.prefs.getDefaultBranch("").setBoolPref(PREF_POCKET, true);
   });
 });
--- a/browser/components/enterprisepolicies/tests/browser/browser_policy_enable_tracking_protection.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policy_enable_tracking_protection.js
@@ -25,12 +25,9 @@ add_task(async function test_policy_enab
         "Locked": true
       }
     }
   });
 
   is(Services.prefs.getBoolPref("privacy.trackingprotection.enabled"), false, "Tracking protection has been disabled by default.");
   is(Services.prefs.getBoolPref("privacy.trackingprotection.pbmode.enabled"), false, "Tracking protection has been disabled by default in private browsing mode.");
   is(Services.prefs.prefIsLocked("privacy.trackingprotection.enabled"), true, "Tracking protection pref is locked.");
-
-  Services.prefs.unlockPref("privacy.trackingprotection.enabled");
-  Services.prefs.unlockPref("privacy.trackingprotection.pbmode.enabled");
 });
--- a/browser/components/enterprisepolicies/tests/browser/browser_policy_set_homepage.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policy_set_homepage.js
@@ -1,41 +1,18 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
-// Prefs that will be touched by the test and need to be reset when the test
-// cleans up.
-const TOUCHED_PREFS = {
-  "browser.startup.homepage": "String",
-  "browser.startup.page": "Int",
-  "pref.browser.homepage.disable_button.current_page": "Bool",
-  "pref.browser.homepage.disable_button.bookmark_page": "Bool",
-  "pref.browser.homepage.disable_button.restore_default": "Bool",
-};
-let ORIGINAL_PREF_VALUE = {};
-
-add_task(function read_original_pref_values() {
-  for (let pref in TOUCHED_PREFS) {
-    let prefType = TOUCHED_PREFS[pref];
-    ORIGINAL_PREF_VALUE[pref] =
-      Services.prefs[`get${prefType}Pref`](pref, undefined);
-  }
-});
 registerCleanupFunction(function restore_pref_values() {
-  let defaults = Services.prefs.getDefaultBranch("");
-  for (let pref in TOUCHED_PREFS) {
-    Services.prefs.unlockPref(pref);
-    Services.prefs.clearUserPref(pref);
-    let originalValue = ORIGINAL_PREF_VALUE[pref];
-    let prefType = TOUCHED_PREFS[pref];
-    if (originalValue !== undefined) {
-      defaults[`set${prefType}Pref`](pref, originalValue);
-    }
-  }
+  // These two prefs are set as user prefs in case the "Locked"
+  // option from this policy was not used. In this case, it won't
+  // be tracked nor restored by the PoliciesPrefTracker.
+  Services.prefs.clearUserPref("browser.startup.homepage");
+  Services.prefs.clearUserPref("browser.startup.page");
 });
 
 add_task(async function homepage_test_simple() {
   await setupPolicyEngineWithJson({
     "policies": {
       "Homepage": {
         "URL": "http://example1.com/"
       }
--- a/browser/components/enterprisepolicies/tests/browser/head.js
+++ b/browser/components/enterprisepolicies/tests/browser/head.js
@@ -1,17 +1,23 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const {EnterprisePolicyTesting} = ChromeUtils.import("resource://testing-common/EnterprisePolicyTesting.jsm", {});
+const {
+  EnterprisePolicyTesting,
+  PoliciesPrefTracker,
+} = ChromeUtils.import("resource://testing-common/EnterprisePolicyTesting.jsm", {});
+
+PoliciesPrefTracker.start();
 
 async function setupPolicyEngineWithJson(json, customSchema) {
+  PoliciesPrefTracker.restoreDefaultValues();
   if (typeof(json) != "object") {
     let filePath = getTestFilePath(json ? json : "non-existing-file.json");
     return EnterprisePolicyTesting.setupPolicyEngineWithJson(filePath, customSchema);
   }
   return EnterprisePolicyTesting.setupPolicyEngineWithJson(json, customSchema);
 }
 
 function checkLockedPref(prefName, prefValue) {
@@ -31,9 +37,10 @@ add_task(async function policies_headjs_
 
 registerCleanupFunction(async function policies_headjs_finishWithCleanSlate() {
   if (Services.policies.status != Ci.nsIEnterprisePolicies.INACTIVE) {
     await setupPolicyEngineWithJson("");
   }
   is(Services.policies.status, Ci.nsIEnterprisePolicies.INACTIVE, "Engine is inactive at the end of the test");
 
   EnterprisePolicyTesting.resetRunOnceState();
+  PoliciesPrefTracker.stop();
 });