Bug 1564131 - re-enable syncing of builtin themes. r=rpl

Differential Revision: https://phabricator.services.mozilla.com/D37837

--HG--
extra : moz-landing-system : lando

browser/app/profile/firefox.js

@@ -1224,6 +1224,7 @@ pref("services.sync.prefs.sync.dom.disable_window_flip", true);
 pref("services.sync.prefs.sync.dom.disable_window_move_resize", true);
 pref("services.sync.prefs.sync.dom.event.contextmenu.enabled", true);
 pref("services.sync.prefs.sync.extensions.update.enabled", true);
+pref("services.sync.prefs.sync.extensions.activeThemeID", true);
 pref("services.sync.prefs.sync.intl.accept_languages", true);
 pref("services.sync.prefs.sync.layout.spellcheckDefault", true);
 pref("services.sync.prefs.sync.media.autoplay.default", true);

services/sync/modules/engines/prefs.js

@@ -31,6 +31,12 @@ XPCOMUtils.defineLazyGetter(this, "PREFS_GUID", () =>
   CommonUtils.encodeBase64URL(Services.appinfo.ID)
 );
 
+ChromeUtils.defineModuleGetter(
+  this,
+  "AddonManager",
+  "resource://gre/modules/AddonManager.jsm"
+);
+
 // In bug 1538015, we decided that it isn't always safe to allow all "incoming"
 // preferences to be applied locally. So we have introduced another preference,
 // which if false (the default) will ignore all incoming preferences which don't
@@ -200,6 +206,10 @@ PrefStore.prototype = {
   },
 
   _setAllPrefs(values) {
+    const selectedThemeIDPref = "extensions.activeThemeID";
+    let selectedThemeIDBefore = this._prefs.get(selectedThemeIDPref, null);
+    let selectedThemeIDAfter = selectedThemeIDBefore;
+
     // Update 'services.sync.prefs.sync.foo.pref' before 'foo.pref', otherwise
     // _isSynced returns false when 'foo.pref' doesn't exist (e.g., on a new device).
     let prefs = Object.keys(values).sort(
@@ -250,17 +260,55 @@ PrefStore.prototype = {
         continue;
       }
 
-      if (value == null) {
-        // Pref has gone missing. The best we can do is reset it.
-        this._prefs.reset(pref);
-      } else {
-        try {
-          this._prefs.set(pref, value);
-        } catch (ex) {
-          this._log.trace(`Failed to set pref: ${pref}`, ex);
-        }
+      switch (pref) {
+        // Some special prefs we don't want to set directly.
+        case selectedThemeIDPref:
+          selectedThemeIDAfter = value;
+          break;
+
+        // default is to just set the pref
+        default:
+          if (value == null) {
+            // Pref has gone missing. The best we can do is reset it.
+            this._prefs.reset(pref);
+          } else {
+            try {
+              this._prefs.set(pref, value);
+            } catch (ex) {
+              this._log.trace(`Failed to set pref: ${pref}`, ex);
+            }
+          }
       }
     }
+    // Themes are a little messy. Themes which have been installed are handled
+    // by the addons engine - but default themes aren't seen by that engine.
+    // So if there's a new default theme ID and that ID corresponds to a
+    // system addon, then we arrange to enable that addon here.
+    if (selectedThemeIDBefore != selectedThemeIDAfter) {
+      this._maybeEnableBuiltinTheme(selectedThemeIDAfter).catch(e => {
+        this._log.error("Failed to maybe update the default theme", e);
+      });
+    }
+  },
+
+  async _maybeEnableBuiltinTheme(themeId) {
+    let addon = null;
+    try {
+      addon = await AddonManager.getAddonByID(themeId);
+    } catch (ex) {
+      this._log.trace(
+        `There's no addon with ID '${themeId} - it can't be a builtin theme`
+      );
+      return;
+    }
+    if (addon && addon.isBuiltin && addon.type == "theme") {
+      this._log.trace(`Enabling builtin theme '${themeId}'`);
+      await addon.enable();
+    } else {
+      this._log.trace(
+        `Have incoming theme ID of '${themeId}' but it's not a builtin theme`
+      );
+    }
   },
 
   async getAllIDs() {

services/sync/tests/unit/test_prefs_store.js

@@ -11,6 +11,9 @@ const { Service } = ChromeUtils.import("resource://services-sync/service.js");
 
 const PREFS_GUID = CommonUtils.encodeBase64URL(Services.appinfo.ID);
 
+const DEFAULT_THEME_ID = "default-theme@mozilla.org";
+const COMPACT_THEME_ID = "firefox-compact-light@mozilla.org";
+
 AddonTestUtils.init(this);
 AddonTestUtils.createAppInfo(
   "xpcshell@tests.mozilla.org",
@@ -19,18 +22,29 @@ AddonTestUtils.createAppInfo(
   "1.9.2"
 );
 AddonTestUtils.overrideCertDB();
-AddonTestUtils.awaitPromise(AddonTestUtils.promiseStartupManager());
-
-function makePersona(id) {
-  return {
-    id: id || Math.random().toString(),
-    name: Math.random().toString(),
-    headerURL: "http://localhost:1234/a",
-  };
-}
 
 add_task(async function run_test() {
   _("Test fixtures.");
+  // Part of this test ensures the default theme, via the preference
+  // extensions.activeThemeID, is synced correctly - so we do a little
+  // addons initialization to allow this to work.
+
+  // Enable application scopes to ensure the builtin theme is going to
+  // be installed as part of the the addon manager startup.
+  Preferences.set("extensions.enabledScopes", AddonManager.SCOPE_APPLICATION);
+  await AddonTestUtils.promiseStartupManager();
+
+  // Install another built-in theme.
+  await AddonManager.installBuiltinAddon("resource:///modules/themes/light/");
+
+  const defaultThemeAddon = await AddonManager.getAddonByID(DEFAULT_THEME_ID);
+  ok(defaultThemeAddon, "Got an addon wrapper for the default theme");
+
+  const otherThemeAddon = await AddonManager.getAddonByID(COMPACT_THEME_ID);
+  ok(otherThemeAddon, "Got an addon wrapper for the compact theme");
+
+  await otherThemeAddon.enable();
+
   // read our custom prefs file before doing anything.
   Services.prefs.readDefaultPrefsFromFile(
     do_get_file("prefs_test_prefs_store.js")
@@ -40,6 +54,9 @@ add_task(async function run_test() {
   let store = engine._store;
   let prefs = new Preferences();
   try {
+    _("Expect the compact light theme to be active");
+    Assert.strictEqual(prefs.get("extensions.activeThemeID"), COMPACT_THEME_ID);
+
     _("The GUID corresponds to XUL App ID.");
     let allIDs = await store.getAllIDs();
     let ids = Object.keys(allIDs);
@@ -142,6 +159,7 @@ add_task(async function run_test() {
     );
     record = new PrefRec("prefs", PREFS_GUID);
     record.value = {
+      "extensions.activeThemeID": DEFAULT_THEME_ID,
       "testing.int": 42,
       "testing.string": "im in ur prefs",
       "testing.bool": false,
@@ -160,6 +178,9 @@ add_task(async function run_test() {
       "services.sync.prefs.dangerously_allow_arbitrary": true,
       "services.sync.prefs.sync.services.sync.prefs.dangerously_allow_arbitrary": true,
     };
+
+    const onceAddonEnabled = AddonTestUtils.promiseAddonEvent("onEnabled");
+
     await store.update(record);
     Assert.strictEqual(prefs.get("testing.int"), 42);
     Assert.strictEqual(prefs.get("testing.string"), "im in ur prefs");
@@ -201,6 +222,16 @@ add_task(async function run_test() {
       undefined
     );
 
+    await onceAddonEnabled;
+    ok(
+      !defaultThemeAddon.userDisabled,
+      "the default theme should have been enabled"
+    );
+    ok(
+      otherThemeAddon.userDisabled,
+      "the compact theme should have been disabled"
+    );
+
     _("Only the current app's preferences are applied.");
     record = new PrefRec("prefs", "some-fake-app");
     record.value = {