Bug 1505551 - Use branch instead of single preference for ASR configuration r=ursula a=jcristau
authork88hudson <k88hudson@gmail.com>
Thu, 22 Nov 2018 18:28:57 +0200
changeset 501330 fe71d237ab9576996b1216f8525c7399c145bf17
parent 501329 ae6c0e763e30e415e3b11ea34ed05e790839a36d
child 501331 77ebe1a00a465761edd0eb995807785caa987227
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersursula, jcristau
bugs1505551
milestone64.0
Bug 1505551 - Use branch instead of single preference for ASR configuration r=ursula a=jcristau Reviewers: ursula Bug #: 1505551 Differential Revision: https://phabricator.services.mozilla.com/D12473
browser/components/newtab/content-src/asrouter/README.md
browser/components/newtab/lib/ASRouterPreferences.jsm
browser/components/newtab/lib/ActivityStream.jsm
browser/components/newtab/test/browser/browser_asrouter_targeting.js
browser/components/newtab/test/unit/asrouter/ASRouter.test.js
browser/components/newtab/test/unit/asrouter/ASRouterPreferences.test.js
browser/components/newtab/test/unit/asrouter/templates/OnboardingMessage.test.jsx
browser/components/newtab/test/unit/unit-entry.js
--- a/browser/components/newtab/content-src/asrouter/README.md
+++ b/browser/components/newtab/content-src/asrouter/README.md
@@ -1,33 +1,38 @@
 # Activity Stream Router
 
 ## Preferences `browser.newtab.activity-stream.asrouter.*`
 
 Name | Used for | Type | Example value
 ---  | ---      | ---  | ---
 `whitelistHosts` | Whitelist a host in order to fetch messages from its endpoint | `[String]` |  `["gist.github.com", "gist.githubusercontent.com", "localhost:8000"]`
-`messageProviders` | Message provider options | `Object` | [see below](#message-providers)
+`providers.snippets` | Message provider options for snippets | `Object` | [see below](#message-providers)
+`providers.cfr` | Message provider options for cfr | `Object` | [see below](#message-providers)
+`providers.onboarding` | Message provider options for onboarding | `Object` | [see below](#message-providers)
 
-### Message providers
+### Message providers examples
 
 ```json
-[
-   {
-      "id" : "onboarding",
-      "type" : "local",
-      "localProvider" : "OnboardingMessageProvider"
-   },
-   {
-      "type" : "remote",
-      "url" : "https://snippets.cdn.mozilla.net/us-west/bundles/bundle_d6d90fb9098ce8b45e60acf601bcb91b68322309.json",
-      "updateCycleInMs" : 14400000,
-      "id" : "snippets"
-   }
-]
+{
+  "id" : "snippets",
+  "type" : "remote",
+  "enabled": true,
+  "url" : "https://snippets.cdn.mozilla.net/us-west/bundles/bundle_d6d90fb9098ce8b45e60acf601bcb91b68322309.json",
+  "updateCycleInMs" : 14400000
+}
+```
+
+```json
+{
+  "id" : "onboarding",
+  "enabled": true,
+  "type" : "local",
+  "localProvider" : "OnboardingMessageProvider"
+}
 ```
 
 ## Admin Interface
 
 * Navigate to `about:newtab#asrouter`
   * See all available messages and message providers
   * Block, unblock or force show a specific message
 
--- a/browser/components/newtab/lib/ASRouterPreferences.jsm
+++ b/browser/components/newtab/lib/ASRouterPreferences.jsm
@@ -1,22 +1,22 @@
 /* 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";
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
-const PROVIDER_PREF = "browser.newtabpage.activity-stream.asrouter.messageProviders";
+const PROVIDER_PREF_BRANCH = "browser.newtabpage.activity-stream.asrouter.providers.";
 const DEVTOOLS_PREF = "browser.newtabpage.activity-stream.asrouter.devtoolsEnabled";
 
 const DEFAULT_STATE = {
   _initialized: false,
   _providers: null,
-  _providerPref: PROVIDER_PREF,
+  _providerPrefBranch: PROVIDER_PREF_BRANCH,
   _devtoolsEnabled: null,
   _devtoolsPref: DEVTOOLS_PREF,
 };
 
 const USER_PREFERENCES = {
   snippets: "browser.newtabpage.activity-stream.feeds.snippets",
   cfr: "browser.newtabpage.activity-stream.asrouter.userprefs.cfr",
 };
@@ -30,59 +30,59 @@ const TEST_PROVIDER = {
 
 class _ASRouterPreferences {
   constructor() {
     Object.assign(this, DEFAULT_STATE);
     this._callbacks = new Set();
   }
 
   _getProviderConfig() {
-    try {
-      return JSON.parse(Services.prefs.getStringPref(this._providerPref, ""));
-    } catch (e) {
-      Cu.reportError(`Could not parse ASRouter preference. Try resetting ${this._providerPref} in about:config.`);
-    }
-    return null;
+    const prefList = Services.prefs.getChildList(this._providerPrefBranch);
+    return prefList.reduce((filtered, pref) => {
+      let value;
+      try {
+        value = JSON.parse(Services.prefs.getStringPref(pref, ""));
+      } catch (e) {
+        Cu.reportError(`Could not parse ASRouter preference. Try resetting ${pref} in about:config.`);
+      }
+      if (value) {
+        filtered.push(value);
+      }
+      return filtered;
+    }, []);
   }
 
   get providers() {
     if (!this._initialized || this._providers === null) {
-      const config = this._getProviderConfig() || [];
+      const config = this._getProviderConfig();
       const providers = config.map(provider => Object.freeze(provider));
       if (this.devtoolsEnabled) {
         providers.unshift(TEST_PROVIDER);
       }
       this._providers = Object.freeze(providers);
     }
 
     return this._providers;
   }
 
   enableOrDisableProvider(id, value) {
     const providers = this._getProviderConfig();
-    if (!providers) {
-      Cu.reportError(`Cannot enable/disable providers if ${this._providerPref} is unparseable.`);
-      return;
-    }
-    if (!providers.find(p => p.id === id)) {
-      Cu.reportError(`Cannot set enabled state for '${id}' because it does not exist in ${this._providerPref}`);
+    const config = providers.find(p => p.id === id);
+    if (!config) {
+      Cu.reportError(`Cannot set enabled state for '${id}' because the pref ${this._providerPrefBranch}${id} does not exist or is not correctly formatted.`);
       return;
     }
 
-    const newConfig = providers.map(provider => {
-      if (provider.id === id) {
-        return {...provider, enabled: value};
-      }
-      return provider;
-    });
-    Services.prefs.setStringPref(this._providerPref, JSON.stringify(newConfig));
+    Services.prefs.setStringPref(this._providerPrefBranch + id, JSON.stringify({...config, enabled: value}));
   }
 
   resetProviderPref() {
-    Services.prefs.clearUserPref(this._providerPref);
+    for (const pref of Services.prefs.getChildList(this._providerPrefBranch)) {
+      Services.prefs.clearUserPref(pref);
+    }
     for (const id of Object.keys(USER_PREFERENCES)) {
       Services.prefs.clearUserPref(USER_PREFERENCES[id]);
     }
   }
 
   get devtoolsEnabled() {
     if (!this._initialized || this._devtoolsEnabled === null) {
       this._devtoolsEnabled = Services.prefs.getBoolPref(this._devtoolsPref, false);
@@ -96,24 +96,21 @@ class _ASRouterPreferences {
       if (provider.id === "snippets" && provider.enabled) {
         allowLegacySnippets = false;
       }
     }
     return {allowLegacySnippets};
   }
 
   observe(aSubject, aTopic, aPrefName) {
-    switch (aPrefName) {
-      case this._providerPref:
-        this._providers = null;
-        break;
-      case this._devtoolsPref:
-        this._providers = null;
-        this._devtoolsEnabled = null;
-        break;
+    if (aPrefName && aPrefName.startsWith(this._providerPrefBranch)) {
+      this._providers = null;
+    } else if (aPrefName === this._devtoolsPref) {
+      this._providers = null;
+      this._devtoolsEnabled = null;
     }
     this._callbacks.forEach(cb => cb(aPrefName));
   }
 
   getUserPreference(providerId) {
     if (!USER_PREFERENCES[providerId]) {
       return null;
     }
@@ -127,27 +124,27 @@ class _ASRouterPreferences {
   removeListener(callback) {
     this._callbacks.delete(callback);
   }
 
   init() {
     if (this._initialized) {
       return;
     }
-    Services.prefs.addObserver(this._providerPref, this);
+    Services.prefs.addObserver(this._providerPrefBranch, this);
     Services.prefs.addObserver(this._devtoolsPref, this);
     for (const id of Object.keys(USER_PREFERENCES)) {
       Services.prefs.addObserver(USER_PREFERENCES[id], this);
     }
     this._initialized = true;
   }
 
   uninit() {
     if (this._initialized) {
-      Services.prefs.removeObserver(this._providerPref, this);
+      Services.prefs.removeObserver(this._providerPrefBranch, this);
       Services.prefs.removeObserver(this._devtoolsPref, this);
       for (const id of Object.keys(USER_PREFERENCES)) {
         Services.prefs.removeObserver(USER_PREFERENCES[id], this);
       }
     }
     Object.assign(this, DEFAULT_STATE);
     this._callbacks.clear();
   }
--- a/browser/components/newtab/lib/ActivityStream.jsm
+++ b/browser/components/newtab/lib/ActivityStream.jsm
@@ -197,43 +197,44 @@ const PREFS_CONFIG = new Map([
   ["asrouter.devtoolsEnabled", {
     title: "Are the asrouter devtools enabled?",
     value: false,
   }],
   ["asrouter.userprefs.cfr", {
     title: "Does the user allow CFR recommendations?",
     value: true,
   }],
-  ["asrouter.messageProviders", {
-    title: "Configuration for ASRouter message providers",
-
-    /**
-     * Each provider must have a unique id and a type of "local" or "remote".
-     * Local providers must specify the name of an ASRouter message provider.
-     * Remote providers must specify a `url` and an `updateCycleInMs`.
-     * Each provider must also have an `enabled` boolean.
-     */
-    value: JSON.stringify([{
+  ["asrouter.providers.onboarding", {
+    title: "Configuration for onboarding provider",
+    value: JSON.stringify({
       id: "onboarding",
       type: "local",
       localProvider: "OnboardingMessageProvider",
       enabled: true,
-    }, {
+    }),
+  }],
+  ["asrouter.providers.snippets", {
+    title: "Configuration for snippets provider",
+    value: JSON.stringify({
       id: "snippets",
       type: "remote",
       url: "https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/",
       updateCycleInMs: ONE_HOUR_IN_MS * 4,
       enabled: false,
-    }, {
+    }),
+  }],
+  ["asrouter.providers.cfr", {
+    title: "Configuration for CFR provider",
+    value: JSON.stringify({
       id: "cfr",
       type: "local",
       localProvider: "CFRMessageProvider",
       enabled: IS_NIGHTLY_OR_UNBRANDED_BUILD,
       cohort: IS_NIGHTLY_OR_UNBRANDED_BUILD ? "nightly" : "",
-    }]),
+    }),
   }],
 ]);
 
 // Array of each feed's FEEDS_CONFIG factory and values to add to PREFS_CONFIG
 const FEEDS_DATA = [
   {
     name: "aboutpreferences",
     factory: () => new AboutPreferences(),
--- a/browser/components/newtab/test/browser/browser_asrouter_targeting.js
+++ b/browser/components/newtab/test/browser/browser_asrouter_targeting.js
@@ -397,19 +397,22 @@ add_task(async function check_sync() {
     "should return correct desktopDevices info");
   is(await ASRouterTargeting.Environment.sync.mobileDevices, Services.prefs.getIntPref("services.sync.clients.devices.mobile", 0),
     "should return correct mobileDevices info");
   is(await ASRouterTargeting.Environment.sync.totalDevices, Services.prefs.getIntPref("services.sync.numClients", 0),
     "should return correct mobileDevices info");
 });
 
 add_task(async function check_provider_cohorts() {
-  await pushPrefs(["browser.newtabpage.activity-stream.asrouter.messageProviders", JSON.stringify([{id: "onboarding", messages: [], enabled: true, cohort: "foo"}, {id: "cfr", messages: [], cohort: "bar"}])]);
-  is(await ASRouterTargeting.Environment.providerCohorts.onboarding, "foo");
-  is(await ASRouterTargeting.Environment.providerCohorts.cfr, "bar");
+  await pushPrefs(["browser.newtabpage.activity-stream.asrouter.providers.onboarding", JSON.stringify({id: "onboarding", messages: [], enabled: true, cohort: "foo"})]);
+  await pushPrefs(["browser.newtabpage.activity-stream.asrouter.providers.cfr", JSON.stringify({id: "cfr", enabled: true, cohort: "bar"})]);
+  is(await ASRouterTargeting.Environment.providerCohorts.onboarding, "foo",
+    "should have cohort foo for onboarding");
+  is(await ASRouterTargeting.Environment.providerCohorts.cfr, "bar",
+    "should have cohort bar for cfr");
 });
 
 add_task(async function check_xpinstall_enabled() {
   // should default to true if pref doesn't exist
   is(await ASRouterTargeting.Environment.xpinstallEnabled, true);
   // flip to false, check targeting reflects that
   await pushPrefs(["xpinstall.enabled", false]);
   is(await ASRouterTargeting.Environment.xpinstallEnabled, false);
--- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js
+++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js
@@ -13,17 +13,17 @@ import {
 import {actionCreators as ac} from "common/Actions.jsm";
 import {ASRouterPreferences} from "lib/ASRouterPreferences.jsm";
 import {ASRouterTriggerListeners} from "lib/ASRouterTriggerListeners.jsm";
 import {CFRPageActions} from "lib/CFRPageActions.jsm";
 import {GlobalOverrider} from "test/unit/utils";
 import ProviderResponseSchema from "content-src/asrouter/schemas/provider-response.schema.json";
 import {QueryCache} from "lib/ASRouterTargeting.jsm";
 
-const MESSAGE_PROVIDER_PREF_NAME = "browser.newtabpage.activity-stream.asrouter.messageProviders";
+const MESSAGE_PROVIDER_PREF_NAME = "browser.newtabpage.activity-stream.asrouter.providers.snippets";
 const FAKE_PROVIDERS = [FAKE_LOCAL_PROVIDER, FAKE_REMOTE_PROVIDER, FAKE_REMOTE_SETTINGS_PROVIDER];
 const ALL_MESSAGE_IDS = [...FAKE_LOCAL_MESSAGES, ...FAKE_REMOTE_MESSAGES].map(message => message.id);
 const FAKE_BUNDLE = [FAKE_LOCAL_MESSAGES[1], FAKE_LOCAL_MESSAGES[2]];
 const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000;
 
 // Creates a message object that looks like messages returned by
 // RemotePageManager listeners
 function fakeAsyncMessage(action) {
--- a/browser/components/newtab/test/unit/asrouter/ASRouterPreferences.test.js
+++ b/browser/components/newtab/test/unit/asrouter/ASRouterPreferences.test.js
@@ -1,40 +1,58 @@
 import {_ASRouterPreferences, ASRouterPreferences as ASRouterPreferencesSingleton, TEST_PROVIDER} from "lib/ASRouterPreferences.jsm";
 const FAKE_PROVIDERS = [{id: "foo"}, {id: "bar"}];
 
-const PROVIDER_PREF = "browser.newtabpage.activity-stream.asrouter.messageProviders";
+const PROVIDER_PREF_BRANCH = "browser.newtabpage.activity-stream.asrouter.providers.";
 const DEVTOOLS_PREF = "browser.newtabpage.activity-stream.asrouter.devtoolsEnabled";
 const SNIPPETS_USER_PREF = "browser.newtabpage.activity-stream.feeds.snippets";
+const CFR_USER_PREF = "browser.newtabpage.activity-stream.asrouter.userprefs.cfr";
 
 /** NUMBER_OF_PREFS_TO_OBSERVE includes:
- *  1. asrouter.messageProvider
+ *  1. asrouter.providers. pref branch
  *  2. asrouter.devtoolsEnabled
  *  3. browser.newtabpage.activity-stream.feeds.snippets (user preference - snippets)
  *  4. browser.newtabpage.activity-stream.asrouter.userprefs.cfr (user preference - cfr)
  */
 const NUMBER_OF_PREFS_TO_OBSERVE = 4;
 
 describe("ASRouterPreferences", () => {
   let ASRouterPreferences;
   let sandbox;
   let addObserverStub;
   let stringPrefStub;
   let boolPrefStub;
+
   beforeEach(() => {
     ASRouterPreferences = new _ASRouterPreferences();
 
     sandbox = sinon.sandbox.create();
     addObserverStub = sandbox.stub(global.Services.prefs, "addObserver");
-    stringPrefStub =  sandbox.stub(global.Services.prefs, "getStringPref").withArgs(PROVIDER_PREF).returns(JSON.stringify(FAKE_PROVIDERS));
+    stringPrefStub =  sandbox.stub(global.Services.prefs, "getStringPref");
+    FAKE_PROVIDERS.forEach(provider => {
+      stringPrefStub.withArgs(`${PROVIDER_PREF_BRANCH}${provider.id}`).returns(JSON.stringify(provider));
+    });
+    sandbox.stub(global.Services.prefs, "getChildList")
+      .withArgs(PROVIDER_PREF_BRANCH).returns(FAKE_PROVIDERS.map(provider => `${PROVIDER_PREF_BRANCH}${provider.id}`));
+
     boolPrefStub = sandbox.stub(global.Services.prefs, "getBoolPref").returns(false);
   });
+
   afterEach(() => {
     sandbox.restore();
   });
+
+  function getPrefNameForProvider(providerId) {
+    return `${PROVIDER_PREF_BRANCH}${providerId}`;
+  }
+
+  function setPrefForProvider(providerId, value) {
+    stringPrefStub.withArgs(getPrefNameForProvider(providerId)).returns(JSON.stringify(value));
+  }
+
   it("ASRouterPreferences should be an instance of _ASRouterPreferences", () => {
     assert.instanceOf(ASRouterPreferencesSingleton, _ASRouterPreferences);
   });
   describe("#init", () => {
     it("should set ._initialized to true", () => {
       ASRouterPreferences.init();
       assert.isTrue(ASRouterPreferences._initialized);
     });
@@ -69,49 +87,51 @@ describe("ASRouterPreferences", () => {
       ASRouterPreferences.addListener(() => {});
       ASRouterPreferences.addListener(() => {});
       assert.equal(ASRouterPreferences._callbacks.size, 2);
       ASRouterPreferences.uninit();
       // Tests to make sure we don't remove observers that weren't set
       ASRouterPreferences.uninit();
 
       assert.callCount(removeStub, NUMBER_OF_PREFS_TO_OBSERVE);
-      assert.calledWith(removeStub, PROVIDER_PREF);
+      assert.calledWith(removeStub, PROVIDER_PREF_BRANCH);
       assert.calledWith(removeStub, DEVTOOLS_PREF);
       assert.isEmpty(ASRouterPreferences._callbacks);
     });
   });
   describe(".providers", () => {
     it("should return the value the first time .providers is accessed", () => {
       ASRouterPreferences.init();
 
       const result = ASRouterPreferences.providers;
       assert.deepEqual(result, FAKE_PROVIDERS);
-      assert.calledOnce(stringPrefStub);
+      // once per pref
+      assert.calledTwice(stringPrefStub);
     });
     it("should return the cached value the second time .providers is accessed", () => {
       ASRouterPreferences.init();
       const [, secondCall] = [ASRouterPreferences.providers, ASRouterPreferences.providers];
 
       assert.deepEqual(secondCall, FAKE_PROVIDERS);
-      assert.calledOnce(stringPrefStub);
+      // once per pref
+      assert.calledTwice(stringPrefStub);
     });
     it("should just parse the pref each time if ASRouterPreferences hasn't been initialized yet", () => {
       // Intentionally not initialized
       const [firstCall, secondCall] = [ASRouterPreferences.providers, ASRouterPreferences.providers];
 
       assert.deepEqual(firstCall, FAKE_PROVIDERS);
       assert.deepEqual(secondCall, FAKE_PROVIDERS);
-      assert.calledTwice(stringPrefStub);
+      assert.callCount(stringPrefStub, 4);
     });
-    it("should return [] if the pref was not parsable", () => {
-      stringPrefStub.withArgs(PROVIDER_PREF).returns("not json");
+    it("should skip the pref without throwing if a pref is not parsable", () => {
+      stringPrefStub.withArgs(`${PROVIDER_PREF_BRANCH}foo`).returns("not json");
       ASRouterPreferences.init();
 
-      assert.deepEqual(ASRouterPreferences.providers, []);
+      assert.deepEqual(ASRouterPreferences.providers, [{id: "bar"}]);
     });
     it("should include TEST_PROVIDER if devtools is turned on", () => {
       boolPrefStub.withArgs(DEVTOOLS_PREF).returns(true);
       ASRouterPreferences.init();
 
       assert.deepEqual(ASRouterPreferences.providers, [TEST_PROVIDER, ...FAKE_PROVIDERS]);
     });
   });
@@ -162,98 +182,105 @@ describe("ASRouterPreferences", () => {
     it("should return the user preference for snippets", () => {
       boolPrefStub.withArgs(SNIPPETS_USER_PREF).returns(true);
       assert.isTrue(ASRouterPreferences.getUserPreference("snippets"));
     });
   });
   describe("#enableOrDisableProvider", () => {
     it("should enable an existing provider if second param is true", () => {
       const setStub = sandbox.stub(global.Services.prefs, "setStringPref");
-      stringPrefStub.withArgs(PROVIDER_PREF).returns(JSON.stringify([{id: "foo", enabled: false}, {id: "bar", enabled: false}]));
+      setPrefForProvider("foo", {id: "foo", enabled: false});
       assert.isFalse(ASRouterPreferences.providers[0].enabled);
 
       ASRouterPreferences.enableOrDisableProvider("foo", true);
 
-      assert.calledWith(setStub, PROVIDER_PREF, JSON.stringify([{id: "foo", enabled: true}, {id: "bar", enabled: false}]));
+      assert.calledWith(setStub, getPrefNameForProvider("foo"), JSON.stringify({id: "foo", enabled: true}));
     });
     it("should disable an existing provider if second param is false", () => {
       const setStub = sandbox.stub(global.Services.prefs, "setStringPref");
-      stringPrefStub.withArgs(PROVIDER_PREF).returns(JSON.stringify([{id: "foo", enabled: true}, {id: "bar", enabled: true}]));
+      setPrefForProvider("foo", {id: "foo", enabled: true});
       assert.isTrue(ASRouterPreferences.providers[0].enabled);
 
       ASRouterPreferences.enableOrDisableProvider("foo", false);
 
-      assert.calledWith(setStub, PROVIDER_PREF, JSON.stringify([{id: "foo", enabled: false}, {id: "bar", enabled: true}]));
+      assert.calledWith(setStub, getPrefNameForProvider("foo"), JSON.stringify({id: "foo", enabled: false}));
     });
     it("should not throw if the id does not exist", () => {
       assert.doesNotThrow(() => {
         ASRouterPreferences.enableOrDisableProvider("does_not_exist", true);
       });
     });
     it("should not throw if pref is not parseable", () => {
-      stringPrefStub.withArgs(PROVIDER_PREF).returns("not valid");
+      stringPrefStub.withArgs(getPrefNameForProvider("foo")).returns("not valid");
       assert.doesNotThrow(() => {
         ASRouterPreferences.enableOrDisableProvider("foo", true);
       });
     });
   });
   describe("#resetProviderPref", () => {
     it("should reset the pref and user prefs", () => {
       const resetStub = sandbox.stub(global.Services.prefs, "clearUserPref");
       ASRouterPreferences.resetProviderPref();
-      assert.calledWith(resetStub, PROVIDER_PREF);
+      FAKE_PROVIDERS.forEach(provider => {
+        assert.calledWith(resetStub, getPrefNameForProvider(provider.id));
+      });
       assert.calledWith(resetStub, SNIPPETS_USER_PREF);
+      assert.calledWith(resetStub, CFR_USER_PREF);
     });
   });
   describe("observer, listeners", () => {
     it("should invalidate .providers when the pref is changed", () => {
-      const testProviders = [{id: "newstuff"}];
+      const testProvider = {id: "newstuff"};
+      const newProviders = [...FAKE_PROVIDERS, testProvider];
 
       ASRouterPreferences.init();
 
       assert.deepEqual(ASRouterPreferences.providers, FAKE_PROVIDERS);
-      stringPrefStub.withArgs(PROVIDER_PREF).returns(JSON.stringify(testProviders));
-      ASRouterPreferences.observe(null, null, PROVIDER_PREF);
+      stringPrefStub.withArgs(getPrefNameForProvider(testProvider.id)).returns(JSON.stringify(testProvider));
+      global.Services.prefs.getChildList
+        .withArgs(PROVIDER_PREF_BRANCH).returns(newProviders.map(provider => getPrefNameForProvider(provider.id)));
+      ASRouterPreferences.observe(null, null, getPrefNameForProvider(testProvider.id));
 
       // Cache should be invalidated so we access the new value of the pref now
-      assert.deepEqual(ASRouterPreferences.providers, testProviders);
+      assert.deepEqual(ASRouterPreferences.providers, newProviders);
     });
     it("should invalidate .devtoolsEnabled and .providers when the pref is changed", () => {
       ASRouterPreferences.init();
 
       assert.isFalse(ASRouterPreferences.devtoolsEnabled);
       boolPrefStub.withArgs(DEVTOOLS_PREF).returns(true);
-      stringPrefStub.withArgs(PROVIDER_PREF).returns("[]");
+      global.Services.prefs.getChildList
+        .withArgs(PROVIDER_PREF_BRANCH).returns([]);
       ASRouterPreferences.observe(null, null, DEVTOOLS_PREF);
 
       // Cache should be invalidated so we access the new value of the pref now
       // Note that providers needs to be invalidated because devtools adds test content to it.
       assert.isTrue(ASRouterPreferences.devtoolsEnabled);
       assert.deepEqual(ASRouterPreferences.providers, [TEST_PROVIDER]);
     });
     it("should call listeners added with .addListener", () => {
       const callback1 = sinon.stub();
       const callback2 = sinon.stub();
       ASRouterPreferences.init();
       ASRouterPreferences.addListener(callback1);
       ASRouterPreferences.addListener(callback2);
 
-      ASRouterPreferences.observe(null, null, PROVIDER_PREF);
-      assert.calledWith(callback1, PROVIDER_PREF);
+      ASRouterPreferences.observe(null, null, getPrefNameForProvider("foo"));
+      assert.calledWith(callback1, getPrefNameForProvider("foo"));
 
       ASRouterPreferences.observe(null, null, DEVTOOLS_PREF);
       assert.calledWith(callback2, DEVTOOLS_PREF);
     });
     it("should not call listeners after they are removed with .removeListeners", () => {
       const callback = sinon.stub();
       ASRouterPreferences.init();
       ASRouterPreferences.addListener(callback);
 
-      ASRouterPreferences.observe(null, null, PROVIDER_PREF);
-      assert.calledWith(callback, PROVIDER_PREF);
+      ASRouterPreferences.observe(null, null, getPrefNameForProvider("foo"));
+      assert.calledWith(callback, getPrefNameForProvider("foo"));
 
       callback.reset();
       ASRouterPreferences.removeListener(callback);
 
       ASRouterPreferences.observe(null, null, DEVTOOLS_PREF);
       assert.notCalled(callback);
     });
   });
--- a/browser/components/newtab/test/unit/asrouter/templates/OnboardingMessage.test.jsx
+++ b/browser/components/newtab/test/unit/asrouter/templates/OnboardingMessage.test.jsx
@@ -27,9 +27,8 @@ describe("OnboardingMessage", () => {
   it("should validate L10N_CONTENT", () => {
     assert.jsonSchema(L10N_CONTENT, schema);
   });
   it("should validate all messages from OnboardingMessageProvider", () => {
     const messages = OnboardingMessageProvider.getUntranslatedMessages();
     messages.forEach(msg => assert.jsonSchema(msg.content, schema));
   });
 });
-
--- a/browser/components/newtab/test/unit/unit-entry.js
+++ b/browser/components/newtab/test/unit/unit-entry.js
@@ -135,16 +135,17 @@ const TEST_GLOBAL = {
     },
     console: {logStringMessage: () => {}},
     prefs: {
       addObserver() {},
       prefHasUserValue() {},
       removeObserver() {},
       getPrefType() {},
       clearUserPref() {},
+      getChildList() { return []; },
       getStringPref() {},
       setStringPref() {},
       getIntPref() {},
       getBoolPref() {},
       setBoolPref() {},
       setIntPref() {},
       getBranch() {},
       PREF_BOOL: "boolean",