Bug 1584249 - Update enabled sync engines before opening CWTS dialog. r=markh
authorEdouard Oger <eoger@fastmail.com>
Mon, 07 Oct 2019 15:25:38 +0000
changeset 496540 0ff644ae9d66c24d49861f78a0be3061c87160f9
parent 496539 59e1ccf2ff04a68d0718099ae3e5e13b5c0fe4c7
child 496541 c13a124f1515a6f13ef3dd2599b0d1d3623027f5
push id97316
push usereoger@mozilla.com
push dateMon, 07 Oct 2019 15:26:23 +0000
treeherderautoland@0ff644ae9d66 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1584249
milestone71.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 1584249 - Update enabled sync engines before opening CWTS dialog. r=markh Differential Revision: https://phabricator.services.mozilla.com/D48272
browser/components/preferences/in-content/sync.js
services/sync/modules/service.js
services/sync/tests/unit/test_service_sync_updateEnabledEngines.js
--- a/browser/components/preferences/in-content/sync.js
+++ b/browser/components/preferences/in-content/sync.js
@@ -278,17 +278,27 @@ var gSyncPane = {
             state.lastSync
           );
       document
         .getElementById("syncNow")
         .setAttribute("tooltiptext", tooltiptext);
     });
   },
 
-  _chooseWhatToSync(isAlreadySyncing) {
+  async _chooseWhatToSync(isAlreadySyncing) {
+    // Assuming another device is syncing and we're not,
+    // we update the engines selection so the correct
+    // checkboxes are pre-filed.
+    if (!isAlreadySyncing) {
+      try {
+        await Weave.Service.updateLocalEnginesState();
+      } catch (err) {
+        console.error("Error updating the local engines state", err);
+      }
+    }
     let params = {};
     if (isAlreadySyncing) {
       // If we are already syncing then we also offer to disconnect.
       params.disconnectFun = () => this.disconnectSync();
     }
     gSubDialog.open(
       "chrome://browser/content/preferences/in-content/syncChooseWhatToSync.xul",
       "" /* aFeatures */,
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -495,16 +495,53 @@ Sync11Service.prototype = {
       } catch (ex) {
         this._log.warn("Could not register engine " + name, ex);
       }
     }
 
     this.engineManager.setDeclined(declined);
   },
 
+  /**
+   * This method updates the local engines state from an existing meta/global
+   * when Sync is disabled.
+   * Running this code if sync is enabled would end up in very weird results
+   * (but we're nice and we check before doing anything!).
+   */
+  async updateLocalEnginesState() {
+    await this.promiseInitialized;
+
+    // Sanity check, this method is not meant to be run if Sync is enabled!
+    if (Svc.Prefs.get("username", "")) {
+      throw new Error("Sync is enabled!");
+    }
+
+    // For historical reasons the behaviour of setCluster() is bizarre,
+    // so just check what we care about - the meta URL.
+    if (!this.metaURL) {
+      await this.identity.setCluster();
+      if (!this.metaURL) {
+        this._log.warn("Could not find a cluster.");
+        return;
+      }
+    }
+    // Clear the cache so we always fetch the latest meta/global.
+    this.recordManager.clearCache();
+    let meta = await this.recordManager.get(this.metaURL);
+    if (!meta) {
+      this._log.info("Meta record is null, aborting engine state update.");
+      return;
+    }
+    const declinedEngines = meta.payload.declined;
+    const allEngines = this.engineManager.getAll().map(e => e.name);
+    for (const engine of allEngines) {
+      Svc.Prefs.set(`engine.${engine}`, !declinedEngines.includes(engine));
+    }
+  },
+
   QueryInterface: ChromeUtils.generateQI([
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference,
   ]),
 
   observe(subject, topic, data) {
     switch (topic) {
       // Ideally this observer should be in the SyncScheduler, but it would require
--- a/services/sync/tests/unit/test_service_sync_updateEnabledEngines.js
+++ b/services/sync/tests/unit/test_service_sync_updateEnabledEngines.js
@@ -453,8 +453,57 @@ add_task(async function test_dependentEn
     _("Engines continue to be disabled.");
     Assert.ok(!steamEngine.enabled);
     Assert.ok(!stirlingEngine.enabled);
   } finally {
     await Service.startOver();
     await promiseStopServer(server);
   }
 });
+
+add_task(async function test_service_updateLocalEnginesState() {
+  Service.syncID = "abcdefghij";
+  const engine = Service.engineManager.get("steam");
+  const metaWBO = new ServerWBO("global", {
+    syncID: Service.syncID,
+    storageVersion: STORAGE_VERSION,
+    declined: ["steam"],
+    engines: {},
+  });
+  const server = httpd_setup({
+    "/1.1/johndoe/storage/meta/global": metaWBO.handler(),
+  });
+  await SyncTestingInfrastructure(server, "johndoe");
+
+  // Disconnect sync.
+  await Service.startOver();
+  Service._ignorePrefObserver = true;
+  // Steam engine is enabled on our machine.
+  engine.enabled = true;
+  Service._ignorePrefObserver = false;
+  Service.identity._findCluster = () => server.baseURI + "/1.1/johndoe/";
+
+  // Update engine state from the server.
+  await Service.updateLocalEnginesState();
+  // Now disabled.
+  Assert.ok(!engine.enabled);
+});
+
+add_task(async function test_service_updateLocalEnginesState_no_meta_global() {
+  Service.syncID = "abcdefghij";
+  const engine = Service.engineManager.get("steam");
+  // The server doesn't contain /meta/global (sync was never enabled).
+  const server = httpd_setup({});
+  await SyncTestingInfrastructure(server, "johndoe");
+
+  // Disconnect sync.
+  await Service.startOver();
+  Service._ignorePrefObserver = true;
+  // Steam engine is enabled on our machine.
+  engine.enabled = true;
+  Service._ignorePrefObserver = false;
+  Service.identity._findCluster = () => server.baseURI + "/1.1/johndoe/";
+
+  // Update engine state from the server.
+  await Service.updateLocalEnginesState();
+  // Still enabled.
+  Assert.ok(engine.enabled);
+});