Bug 1640528 - Allow extension storage to be enabled and disabled independently of add-ons. r=markh
authorLina Cambridge <lina@yakshaving.ninja>
Mon, 25 May 2020 01:12:16 +0000
changeset 531821 442f8bd2e723e114e5b507ab4496dd6c2a32b233
parent 531820 4b153b20e8d1822f8b103c537d346d541cee7052
child 531822 1745a602e6eb385ac9714f4b85f120b07661eeda
push id37446
push userabutkovits@mozilla.com
push dateMon, 25 May 2020 09:34:40 +0000
treeherdermozilla-central@9155018d4978 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1640528
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1640528 - Allow extension storage to be enabled and disabled independently of add-ons. r=markh By default, toggling the add-ons engine also toggles extension storage. However, it's also possible to enable extension storage without add-ons by setting an override pref. If the override pref is set, we'll treat extension storage as an engine that can be toggled independently. If it's not set, we'll ignore attempts to toggle the engine separately. Differential Revision: https://phabricator.services.mozilla.com/D76635
services/sync/modules/engines/extension-storage.js
services/sync/tests/unit/test_extension_storage_engine.js
--- a/services/sync/modules/engines/extension-storage.js
+++ b/services/sync/modules/engines/extension-storage.js
@@ -25,46 +25,64 @@ XPCOMUtils.defineLazyModuleGetters(this,
 
 XPCOMUtils.defineLazyServiceGetter(
   this,
   "StorageSyncService",
   "@mozilla.org/extensions/storage/sync;1",
   "nsIInterfaceRequestor"
 );
 
+const PREF_FORCE_ENABLE = "engine.extension-storage.force";
+
 // A helper to indicate whether extension-storage is enabled - it's based on
 // the "addons" pref. The same logic is shared between both engine impls.
-function isEngineEnabled() {
+function getEngineEnabled() {
   // By default, we sync extension storage if we sync addons. This
   // lets us simplify the UX since users probably don't consider
   // "extension preferences" a separate category of syncing.
   // However, we also respect engine.extension-storage.force, which
   // can be set to true or false, if a power user wants to customize
   // the behavior despite the lack of UI.
-  const forced = Svc.Prefs.get("engine.extension-storage.force", undefined);
+  const forced = Svc.Prefs.get(PREF_FORCE_ENABLE, undefined);
   if (forced !== undefined) {
     return forced;
   }
   return Svc.Prefs.get("engine.addons", false);
 }
 
+function setEngineEnabled(enabled) {
+  // This will be called by the engine manager when declined on another device.
+  // Things will go a bit pear-shaped if the engine manager tries to end up
+  // with 'addons' and 'extension-storage' in different states - however, this
+  // *can* happen given we support the `engine.extension-storage.force`
+  // preference. So if that pref exists, we set it to this value. If that pref
+  // doesn't exist, we just ignore it and hope that the 'addons' engine is also
+  // going to be set to the same state.
+  if (Svc.Prefs.has(PREF_FORCE_ENABLE)) {
+    Svc.Prefs.set(PREF_FORCE_ENABLE, enabled);
+  }
+}
+
 // A "bridged engine" to our webext-storage component.
 function ExtensionStorageEngineBridge(service) {
   let bridge = StorageSyncService.getInterface(Ci.mozIBridgedSyncEngine);
   BridgedEngine.call(this, bridge, "Extension-Storage", service);
 }
 
 ExtensionStorageEngineBridge.prototype = {
   __proto__: BridgedEngine.prototype,
   syncPriority: 10,
   // we don't support repair at all!
   _skipPercentageChance: 100,
 
   get enabled() {
-    return isEngineEnabled();
+    return getEngineEnabled();
+  },
+  set enabled(enabled) {
+    setEngineEnabled(enabled);
   },
 };
 
 /**
  *****************************************************************************
  *
  * Deprecated support for Kinto
  *
@@ -98,17 +116,23 @@ ExtensionStorageEngineKinto.prototype = 
   syncPriority: 10,
   allowSkippedRecord: false,
 
   async _sync() {
     return extensionStorageSync.syncAll();
   },
 
   get enabled() {
-    return isEngineEnabled();
+    return getEngineEnabled();
+  },
+  // We only need the enabled setter for the edge-case where info/collections
+  // has `extension-storage` - which could happen if the pref to flip the new
+  // engine on was once set but no longer is.
+  set enabled(enabled) {
+    setEngineEnabled(enabled);
   },
 
   _wipeClient() {
     return extensionStorageSync.clearAll();
   },
 
   shouldSkipSync(syncReason) {
     if (syncReason == "user" || syncReason == "startup") {
--- a/services/sync/tests/unit/test_extension_storage_engine.js
+++ b/services/sync/tests/unit/test_extension_storage_engine.js
@@ -46,16 +46,67 @@ add_task(async function test_switching_b
     assertUsingKinto("Should use Kinto engine after flipping pref");
   }
 
   _("Clean up");
   Services.prefs.clearUserPref("webextensions.storage.sync.kinto");
   await Service.engineManager.switchAlternatives();
 });
 
+add_task(async function test_enable() {
+  const PREF = "services.sync.engine.extension-storage.force";
+
+  let addonsEngine = Service.engineManager.get("addons");
+  let extensionStorageEngine = Service.engineManager.get("extension-storage");
+
+  try {
+    Assert.ok(
+      addonsEngine.enabled,
+      "Add-ons engine should be enabled by default"
+    );
+    Assert.ok(
+      extensionStorageEngine.enabled,
+      "Extension storage engine should be enabled by default"
+    );
+
+    addonsEngine.enabled = false;
+    Assert.ok(
+      !extensionStorageEngine.enabled,
+      "Disabling add-ons should disable extension storage"
+    );
+
+    extensionStorageEngine.enabled = true;
+    Assert.ok(
+      !extensionStorageEngine.enabled,
+      "Enabling extension storage without override pref shouldn't work"
+    );
+
+    Services.prefs.setBoolPref(PREF, true);
+    Assert.ok(
+      extensionStorageEngine.enabled,
+      "Setting override pref should enable extension storage"
+    );
+
+    extensionStorageEngine.enabled = false;
+    Assert.ok(
+      !extensionStorageEngine.enabled,
+      "Disabling extension storage engine with override pref should work"
+    );
+
+    extensionStorageEngine.enabled = true;
+    Assert.ok(
+      extensionStorageEngine.enabled,
+      "Enabling extension storage with override pref should work"
+    );
+  } finally {
+    addonsEngine.enabled = true;
+    Services.prefs.clearUserPref(PREF);
+  }
+});
+
 // It's difficult to know what to test - there's already tests for the bridged
 // engine etc - so we just try and check that this engine conforms to the
 // mozIBridgedSyncEngine interface guarantees.
 add_task(async function test_engine() {
   let engine = new ExtensionStorageEngineBridge(Service);
   Assert.equal(engine.version, 1);
 
   Assert.deepEqual(await engine.getSyncID(), null);