Bug 978876 - Part 1: handle declined engines in desktop Sync meta/global. r=gps, a=sledru
authorRichard Newman <rnewman@mozilla.com>
Thu, 13 Mar 2014 16:37:25 -0700
changeset 184411 079b6fdf34c67fc896861b8f4fa9c2278b9a8399
parent 184410 4a52af0e75838d87f14acb6c6edc517db5179120
child 184412 20d7784290825d90cdd1d5d80ac21461a9be6b5e
push id462
push userraliiev@mozilla.com
push dateTue, 22 Apr 2014 00:22:30 +0000
treeherdermozilla-release@ac5db8c74ac0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps, sledru
bugs978876
milestone29.0a2
Bug 978876 - Part 1: handle declined engines in desktop Sync meta/global. r=gps, a=sledru * * * Bug 978876 - Part 2: refactor out meta/global upload.
services/sync/Makefile.in
services/sync/modules/engines.js
services/sync/modules/identity.js
services/sync/modules/service.js
services/sync/modules/stages/cluster.js
services/sync/modules/stages/declined.js
services/sync/modules/stages/enginesync.js
services/sync/services-sync.js
services/sync/tests/unit/test_declined.js
services/sync/tests/unit/test_enginemanager.js
services/sync/tests/unit/test_load_modules.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/Makefile.in
+++ b/services/sync/Makefile.in
@@ -46,16 +46,17 @@ sync_engine_modules := \
   history.js \
   passwords.js \
   prefs.js \
   tabs.js \
   $(NULL)
 
 sync_stage_modules := \
   cluster.js \
+  declined.js \
   enginesync.js \
   $(NULL)
 
 sync_testing_modules := \
   fakeservices.js \
   rotaryengine.js \
   utils.js \
   $(NULL)
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -440,19 +440,21 @@ Store.prototype = {
     throw "override wipe in a subclass";
   }
 };
 
 this.EngineManager = function EngineManager(service) {
   this.service = service;
 
   this._engines = {};
+
+  // This will be populated by Service on startup.
+  this._declined = new Set();
   this._log = Log.repository.getLogger("Sync.EngineManager");
-  this._log.level = Log.Level[Svc.Prefs.get(
-    "log.logger.service.engines", "Debug")];
+  this._log.level = Log.Level[Svc.Prefs.get("log.logger.service.engines", "Debug")];
 }
 EngineManager.prototype = {
   get: function (name) {
     // Return an array of engines if we have an array of names
     if (Array.isArray(name)) {
       let engines = [];
       name.forEach(function(name) {
         let engine = this.get(name);
@@ -472,20 +474,79 @@ EngineManager.prototype = {
     }
     return engine;
   },
 
   getAll: function () {
     return [engine for ([name, engine] in Iterator(this._engines))];
   },
 
+  /**
+   * N.B., does not pay attention to the declined list.
+   */
   getEnabled: function () {
     return this.getAll().filter(function(engine) engine.enabled);
   },
 
+  get enabledEngineNames() {
+    return [e.name for each (e in this.getEnabled())];
+  },
+
+  persistDeclined: function () {
+    Svc.Prefs.set("declinedEngines", [...this._declined].join(","));
+  },
+
+  /**
+   * Returns an array.
+   */
+  getDeclined: function () {
+    return [...this._declined];
+  },
+
+  setDeclined: function (engines) {
+    this._declined = new Set(engines);
+    this.persistDeclined();
+  },
+
+  isDeclined: function (engineName) {
+    return this._declined.has(engineName);
+  },
+
+  /**
+   * Accepts a Set or an array.
+   */
+  decline: function (engines) {
+    for (let e of engines) {
+      this._declined.add(e);
+    }
+    this.persistDeclined();
+  },
+
+  undecline: function (engines) {
+    for (let e of engines) {
+      this._declined.delete(e);
+    }
+    this.persistDeclined();
+  },
+
+  /**
+   * Mark any non-enabled engines as declined.
+   *
+   * This is useful after initial customization during setup.
+   */
+  declineDisabled: function () {
+    for (let e of this.getAll()) {
+      if (!e.enabled) {
+        this._log.debug("Declining disabled engine " + e.name);
+        this._declined.add(e.name);
+      }
+    }
+    this.persistDeclined();
+  },
+
   /**
    * Register an Engine to the service. Alternatively, give an array of engine
    * objects to register.
    *
    * @param engineObject
    *        Engine object used to get an instance of the engine
    * @return The engine object if anything failed
    */
@@ -633,26 +694,26 @@ SyncEngine.kRecoveryStrategy = {
   retry:  "retry",
   error:  "error"
 };
 
 SyncEngine.prototype = {
   __proto__: Engine.prototype,
   _recordObj: CryptoWrapper,
   version: 1,
-  
+
   // How many records to pull in a single sync. This is primarily to avoid very
   // long first syncs against profiles with many history records.
   downloadLimit: null,
-  
+
   // How many records to pull at one time when specifying IDs. This is to avoid
   // URI length limitations.
   guidFetchBatchSize: DEFAULT_GUID_FETCH_BATCH_SIZE,
   mobileGUIDFetchBatchSize: DEFAULT_MOBILE_GUID_FETCH_BATCH_SIZE,
-  
+
   // How many records to process in a single batch.
   applyIncomingBatchSize: DEFAULT_STORE_BATCH_SIZE,
 
   get storageURL() this.service.storageURL,
 
   get engineURL() this.storageURL + this.name,
 
   get cryptoKeysURL() this.storageURL + "crypto/keys",
@@ -858,17 +919,17 @@ SyncEngine.prototype = {
     }
 
     if (isMobile) {
       batchSize = MOBILE_BATCH_SIZE;
     }
     newitems.newer = this.lastSync;
     newitems.full  = true;
     newitems.limit = batchSize;
-    
+
     // applied    => number of items that should be applied.
     // failed     => number of items that failed in this sync.
     // newFailed  => number of items that failed for the first time in this sync.
     // reconciled => number of items that were reconciled.
     let count = {applied: 0, failed: 0, newFailed: 0, reconciled: 0};
     let handled = [];
     let applyBatch = [];
     let failed = [];
--- a/services/sync/modules/identity.js
+++ b/services/sync/modules/identity.js
@@ -548,10 +548,14 @@ IdentityManager.prototype = {
   onRESTRequestBasic: function onRESTRequestBasic(request) {
     let up = this.username + ":" + this.basicPassword;
     request.setHeader("authorization", "Basic " + btoa(up));
   },
 
   createClusterManager: function(service) {
     Cu.import("resource://services-sync/stages/cluster.js");
     return new ClusterManager(service);
-  }
+  },
+
+  offerSyncOptions: function () {
+    // TODO
+  },
 };
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -26,16 +26,17 @@ Cu.import("resource://services-sync/cons
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/engines/clients.js");
 Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-sync/policies.js");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/resource.js");
 Cu.import("resource://services-sync/rest.js");
 Cu.import("resource://services-sync/stages/enginesync.js");
+Cu.import("resource://services-sync/stages/declined.js");
 Cu.import("resource://services-sync/status.js");
 Cu.import("resource://services-sync/userapi.js");
 Cu.import("resource://services-sync/util.js");
 
 const ENGINE_MODULES = {
   Addons: "addons.js",
   Bookmarks: "bookmarks.js",
   Form: "forms.js",
@@ -60,20 +61,16 @@ Sync11Service.prototype = {
   _locked: false,
   _loggedIn: false,
 
   infoURL: null,
   storageURL: null,
   metaURL: null,
   cryptoKeyURL: null,
 
-  get enabledEngineNames() {
-    return [e.name for each (e in this.engineManager.getEnabled())];
-  },
-
   get serverURL() Svc.Prefs.get("serverURL"),
   set serverURL(value) {
     if (!value.endsWith("/")) {
       value += "/";
     }
 
     // Only do work if it's actually changing
     if (value == this.serverURL)
@@ -425,16 +422,22 @@ Sync11Service.prototype = {
     let engines = [];
     // Applications can provide this preference (comma-separated list)
     // to specify which engines should be registered on startup.
     let pref = Svc.Prefs.get("registerEngines");
     if (pref) {
       engines = pref.split(",");
     }
 
+    let declined = [];
+    pref = Svc.Prefs.get("declinedEngines");
+    if (pref) {
+      declined = pref.split(",");
+    }
+
     this.clientsEngine = new ClientEngine(this);
 
     for (let name of engines) {
       if (!name in ENGINE_MODULES) {
         this._log.info("Do not know about engine: " + name);
         continue;
       }
 
@@ -443,22 +446,24 @@ Sync11Service.prototype = {
         Cu.import("resource://services-sync/engines/" + ENGINE_MODULES[name], ns);
 
         let engineName = name + "Engine";
         if (!(engineName in ns)) {
           this._log.warn("Could not find exported engine instance: " + engineName);
           continue;
         }
 
-        this.engineManager.register(ns[engineName], this);
+        this.engineManager.register(ns[engineName]);
       } catch (ex) {
         this._log.warn("Could not register engine " + name + ": " +
                        CommonUtils.exceptionStr(ex));
       }
     }
+
+    this.engineManager.setDeclined(declined);
   },
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
                                          Ci.nsISupportsWeakReference]),
 
   // nsIObserver
 
   observe: function observe(subject, topic, data) {
@@ -1057,16 +1062,17 @@ Sync11Service.prototype = {
       // ... fetch the current record from the server, and COPY THE FLAGS.
       let newMeta = this.recordManager.get(this.metaURL);
 
       if (!this.recordManager.response.success || !newMeta) {
         this._log.debug("No meta/global record on the server. Creating one.");
         newMeta = new WBORecord("meta", "global");
         newMeta.payload.syncID = this.syncID;
         newMeta.payload.storageVersion = STORAGE_VERSION;
+        newMeta.payload.declined = this.engineManager.getDeclined();
 
         newMeta.isNew = true;
 
         this.recordManager.set(this.metaURL, newMeta);
         if (!newMeta.upload(this.resource(this.metaURL)).success) {
           this._log.warn("Unable to upload new meta/global. Failing remote setup.");
           return false;
         }
@@ -1240,20 +1246,54 @@ Sync11Service.prototype = {
       let synchronizer = new EngineSynchronizer(this);
       let cb = Async.makeSpinningCallback();
       synchronizer.onComplete = cb;
 
       synchronizer.sync();
       // wait() throws if the first argument is truthy, which is exactly what
       // we want.
       let result = cb.wait();
+
+      // We successfully synchronized. Now let's update our declined engines.
+      let meta = this.recordManager.get(this.metaURL);
+      if (!meta) {
+        this._log.warn("No meta/global; can't update declined state.");
+        return;
+      }
+
+      let declinedEngines = new DeclinedEngines(this);
+      let didChange = declinedEngines.updateDeclined(meta, this.engineManager);
+      if (!didChange) {
+        this._log.info("No change to declined engines. Not reuploading meta/global.");
+        return;
+      }
+
+      this.uploadMetaGlobal(meta);
     }))();
   },
 
   /**
+   * Upload meta/global, throwing the response on failure.
+   */
+  uploadMetaGlobal: function (meta) {
+    this._log.debug("Uploading meta/global: " + JSON.stringify(meta));
+
+    // It would be good to set the X-If-Unmodified-Since header to `timestamp`
+    // for this PUT to ensure at least some level of transactionality.
+    // Unfortunately, the servers don't support it after a wipe right now
+    // (bug 693893), so we're going to defer this until bug 692700.
+    let res = this.resource(this.metaURL);
+    let response = res.put(meta);
+    if (!response.success) {
+      throw response;
+    }
+    this.recordManager.set(this.metaURL, meta);
+  },
+
+  /**
    * If we have a passphrase, rather than a 25-alphadigit sync key,
    * use the provided sync ID to bootstrap it using PBKDF2.
    *
    * Store the new 'passphrase' back into the identity manager.
    *
    * We can check this as often as we want, because once it's done the
    * check will no longer succeed. It only matters that it happens after
    * we decide to bump the server storage version.
@@ -1298,36 +1338,29 @@ Sync11Service.prototype = {
 
     // Wipe the server.
     let wipeTimestamp = this.wipeServer();
 
     // Upload a new meta/global record.
     let meta = new WBORecord("meta", "global");
     meta.payload.syncID = this.syncID;
     meta.payload.storageVersion = STORAGE_VERSION;
+    meta.payload.declined = this.engineManager.getDeclined();
     meta.isNew = true;
 
-    this._log.debug("New metadata record: " + JSON.stringify(meta.payload));
-    let res = this.resource(this.metaURL);
-    // It would be good to set the X-If-Unmodified-Since header to `timestamp`
-    // for this PUT to ensure at least some level of transactionality.
-    // Unfortunately, the servers don't support it after a wipe right now
-    // (bug 693893), so we're going to defer this until bug 692700.
-    let resp = res.put(meta);
-    if (!resp.success) {
-      // If we got into a race condition, we'll abort the sync this way, too.
-      // That's fine. We'll just wait till the next sync. The client that we're
-      // racing is probably busy uploading stuff right now anyway.
-      throw resp;
-    }
-    this.recordManager.set(this.metaURL, meta);
+    // uploadMetaGlobal throws on failure -- including race conditions.
+    // If we got into a race condition, we'll abort the sync this way, too.
+    // That's fine. We'll just wait till the next sync. The client that we're
+    // racing is probably busy uploading stuff right now anyway.
+    this.uploadMetaGlobal(meta);
 
     // Wipe everything we know about except meta because we just uploaded it
     let engines = [this.clientsEngine].concat(this.engineManager.getAll());
     let collections = [engine.name for each (engine in engines)];
+    // TODO: there's a bug here. We should be calling resetClient, no?
 
     // Generate, upload, and download new keys. Do this last so we don't wipe
     // them...
     this.generateNewSymmetricKeys();
   },
 
   /**
    * Wipe user data from the server.
--- a/services/sync/modules/stages/cluster.js
+++ b/services/sync/modules/stages/cluster.js
@@ -1,11 +1,11 @@
 /* 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/. */
+ * 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/. */
 
 this.EXPORTED_SYMBOLS = ["ClusterManager"];
 
 const {utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/policies.js");
new file mode 100644
--- /dev/null
+++ b/services/sync/modules/stages/declined.js
@@ -0,0 +1,76 @@
+/* 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/. */
+
+/**
+ * This file contains code for maintaining the set of declined engines,
+ * in conjunction with EngineManager.
+ */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = ["DeclinedEngines"];
+
+const {utils: Cu} = Components;
+
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://gre/modules/Log.jsm");
+Cu.import("resource://services-common/utils.js");
+Cu.import("resource://services-common/observers.js");
+Cu.import("resource://gre/modules/Preferences.jsm");
+
+
+
+this.DeclinedEngines = function (service) {
+  this._log = Log.repository.getLogger("Sync.Declined");
+  this._log.level = Log.Level[new Preferences(PREFS_BRANCH).get("log.logger.declined")];
+
+  this.service = service;
+}
+this.DeclinedEngines.prototype = {
+  updateDeclined: function (meta, engineManager=this.service.engineManager) {
+    let enabled = new Set([e.name for each (e in engineManager.getEnabled())]);
+    let known = new Set([e.name for each (e in engineManager.getAll())]);
+    let remoteDeclined = new Set(meta.payload.declined || []);
+    let localDeclined = new Set(engineManager.getDeclined());
+
+    this._log.debug("Handling remote declined: " + JSON.stringify([...remoteDeclined]));
+    this._log.debug("Handling local declined: " + JSON.stringify([...localDeclined]));
+
+    // Any engines that are locally enabled should be removed from the remote
+    // declined list.
+    //
+    // Any engines that are locally declined should be added to the remote
+    // declined list.
+    let newDeclined = CommonUtils.union(localDeclined, CommonUtils.difference(remoteDeclined, enabled));
+
+    // If our declined set has changed, put it into the meta object and mark
+    // it as changed.
+    let declinedChanged = !CommonUtils.setEqual(newDeclined, remoteDeclined);
+    this._log.debug("Declined changed? " + declinedChanged);
+    if (declinedChanged) {
+      meta.changed = true;
+      meta.payload.declined = [...newDeclined];
+    }
+
+    // Update the engine manager regardless.
+    engineManager.setDeclined(newDeclined);
+
+    // Any engines that are locally known, locally disabled, and not remotely
+    // or locally declined, are candidates for enablement.
+    let undecided = CommonUtils.difference(CommonUtils.difference(known, enabled), newDeclined);
+    if (undecided.size) {
+      let subject = {
+        declined: newDeclined,
+        enabled: enabled,
+        known: known,
+        undecided: undecided,
+      };
+      CommonUtils.nextTick(() => {
+        Observers.notify("weave:engines:notdeclined", subject);
+      });
+    }
+
+    return declinedChanged;
+  },
+};
--- a/services/sync/modules/stages/enginesync.js
+++ b/services/sync/modules/stages/enginesync.js
@@ -1,11 +1,11 @@
 /* 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/. */
+ * 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/. */
 
 /**
  * This file contains code for synchronizing engines.
  */
 
 this.EXPORTED_SYMBOLS = ["EngineSynchronizer"];
 
 const {utils: Cu} = Components;
@@ -66,21 +66,23 @@ EngineSynchronizer.prototype = {
     let infoURL = this.service.infoURL;
     let now = Math.floor(Date.now() / 1000);
     let lastPing = Svc.Prefs.get("lastPing", 0);
     if (now - lastPing > 86400) { // 60 * 60 * 24
       infoURL += "?v=" + WEAVE_VERSION;
       Svc.Prefs.set("lastPing", now);
     }
 
+    let engineManager = this.service.engineManager;
+
     // Figure out what the last modified time is for each collection
     let info = this.service._fetchInfo(infoURL);
 
     // Convert the response to an object and read out the modified times
-    for (let engine of [this.service.clientsEngine].concat(this.service.engineManager.getAll())) {
+    for (let engine of [this.service.clientsEngine].concat(engineManager.getAll())) {
       engine.lastModified = info.obj[engine.name] || 0;
     }
 
     if (!(this.service._remoteSetup(info))) {
       this.onComplete(new Error("Aborting sync, remote setup failed"));
       return;
     }
 
@@ -92,23 +94,23 @@ EngineSynchronizer.prototype = {
       this._log.warn("Client engine sync failed. Aborting.");
       this.onComplete(null);
       return;
     }
 
     // Wipe data in the desired direction if necessary
     switch (Svc.Prefs.get("firstSync")) {
       case "resetClient":
-        this.service.resetClient(this.service.enabledEngineNames);
+        this.service.resetClient(engineManager.enabledEngineNames);
         break;
       case "wipeClient":
-        this.service.wipeClient(this.service.enabledEngineNames);
+        this.service.wipeClient(engineManager.enabledEngineNames);
         break;
       case "wipeRemote":
-        this.service.wipeRemote(this.service.enabledEngineNames);
+        this.service.wipeRemote(engineManager.enabledEngineNames);
         break;
     }
 
     if (this.service.clientsEngine.localCommands) {
       try {
         if (!(this.service.clientsEngine.processIncomingCommands())) {
           this.service.status.sync = ABORT_SYNC_COMMAND;
           this.onComplete(new Error("Processed command aborted sync."));
@@ -137,17 +139,17 @@ EngineSynchronizer.prototype = {
       this._log.debug("Updating enabled engines failed: " +
                       Utils.exceptionStr(ex));
       this.service.errorHandler.checkServerError(ex);
       this.onComplete(ex);
       return;
     }
 
     try {
-      for each (let engine in this.service.engineManager.getEnabled()) {
+      for (let engine of engineManager.getEnabled()) {
         // If there's any problems with syncing the engine, report the failure
         if (!(this._syncEngine(engine)) || this.service.status.enforceBackoff) {
           this._log.info("Aborting sync for failure in " + engine.name);
           break;
         }
       }
 
       // If _syncEngine fails for a 401, we might not have a cluster URL here.
@@ -155,22 +157,27 @@ EngineSynchronizer.prototype = {
       // throwing an exception when trying to fetch metaURL.
       if (!this.service.clusterURL) {
         this._log.debug("Aborting sync, no cluster URL: " +
                         "not uploading new meta/global.");
         this.onComplete(null);
         return;
       }
 
-      // Upload meta/global if any engines changed anything
+      // Upload meta/global if any engines changed anything.
       let meta = this.service.recordManager.get(this.service.metaURL);
       if (meta.isNew || meta.changed) {
-        this.service.resource(this.service.metaURL).put(meta);
-        delete meta.isNew;
-        delete meta.changed;
+        this._log.info("meta/global changed locally: reuploading.");
+        try {
+          this.service.uploadMetaGlobal(meta);
+          delete meta.isNew;
+          delete meta.changed;
+        } catch (error) {
+          this._log.error("Unable to upload meta/global. Leaving marked as new.");
+        }
       }
 
       // If there were no sync engine failures
       if (this.service.status.service != SYNC_FAILED_PARTIAL) {
         Svc.Prefs.set("lastSync", new Date().toString());
         this.service.status.sync = SYNC_SUCCEEDED;
       }
     } finally {
@@ -200,75 +207,102 @@ EngineSynchronizer.prototype = {
         // appropriate value.
         return false;
       }
     }
 
     return true;
   },
 
-  _updateEnabledEngines: function _updateEnabledEngines() {
+  _updateEnabledFromMeta: function (meta, numClients, engineManager=this.service.engineManager) {
     this._log.info("Updating enabled engines: " +
-                   this.service.scheduler.numClients + " clients.");
-    let meta = this.service.recordManager.get(this.service.metaURL);
-    if (meta.isNew || !meta.payload.engines)
+                    numClients + " clients.");
+
+    if (meta.isNew || !meta.payload.engines) {
+      this._log.debug("meta/global isn't new, or is missing engines. Not updating enabled state.");
       return;
+    }
 
     // If we're the only client, and no engines are marked as enabled,
     // thumb our noses at the server data: it can't be right.
     // Belt-and-suspenders approach to Bug 615926.
-    if ((this.service.scheduler.numClients <= 1) &&
+    if ((numClients <= 1) &&
         ([e for (e in meta.payload.engines) if (e != "clients")].length == 0)) {
       this._log.info("One client and no enabled engines: not touching local engine status.");
       return;
     }
 
     this.service._ignorePrefObserver = true;
 
-    let enabled = this.service.enabledEngineNames;
+    let enabled = engineManager.enabledEngineNames;
+
+    let toDecline = new Set();
+    let toUndecline = new Set();
+
     for (let engineName in meta.payload.engines) {
       if (engineName == "clients") {
         // Clients is special.
         continue;
       }
       let index = enabled.indexOf(engineName);
       if (index != -1) {
         // The engine is enabled locally. Nothing to do.
         enabled.splice(index, 1);
         continue;
       }
-      let engine = this.service.engineManager.get(engineName);
+      let engine = engineManager.get(engineName);
       if (!engine) {
         // The engine doesn't exist locally. Nothing to do.
         continue;
       }
 
       if (Svc.Prefs.get("engineStatusChanged." + engine.prefName, false)) {
         // The engine was disabled locally. Wipe server data and
         // disable it everywhere.
         this._log.trace("Wiping data for " + engineName + " engine.");
         engine.wipeServer();
         delete meta.payload.engines[engineName];
-        meta.changed = true;
+        meta.changed = true;             // TODO: Should we still do this?
+
+        // We also here mark the engine as declined, because the pref
+        // was explicitly changed to false.
+        // This will be reflected in meta/global in the next stage.
+        this._log.trace("Engine " + engineName + " was disabled locally. Marking as declined.");
+        toDecline.add(engineName);
       } else {
         // The engine was enabled remotely. Enable it locally.
+        this._log.trace("Engine " + engineName + " was enabled. Marking as non-declined.");
+        toUndecline.add(engineName);
         this._log.trace(engineName + " engine was enabled remotely.");
         engine.enabled = true;
       }
     }
 
     // Any remaining engines were either enabled locally or disabled remotely.
     for each (let engineName in enabled) {
-      let engine = this.service.engineManager.get(engineName);
+      let engine = engineManager.get(engineName);
       if (Svc.Prefs.get("engineStatusChanged." + engine.prefName, false)) {
         this._log.trace("The " + engineName + " engine was enabled locally.");
+        toUndecline.add(engineName);
       } else {
         this._log.trace("The " + engineName + " engine was disabled remotely.");
+
+        // Don't automatically mark it as declined!
         engine.enabled = false;
       }
     }
 
+    this.service.engineManager.decline(toDecline);
+    this.service.engineManager.undecline(toUndecline);
+
     Svc.Prefs.resetBranch("engineStatusChanged.");
     this.service._ignorePrefObserver = false;
   },
 
+  _updateEnabledEngines: function () {
+    let meta = this.service.recordManager.get(this.service.metaURL);
+    let numClients = this.service.scheduler.numClients;
+    let engineManager = this.service.engineManager;
+
+    this._updateEnabledFromMeta(meta, numClients, engineManager);
+  },
 };
 Object.freeze(EngineSynchronizer.prototype);
--- a/services/sync/services-sync.js
+++ b/services/sync/services-sync.js
@@ -52,16 +52,17 @@ pref("services.sync.addons.trustedSource
 pref("services.sync.log.appender.console", "Warn");
 pref("services.sync.log.appender.dump", "Error");
 pref("services.sync.log.appender.file.level", "Trace");
 pref("services.sync.log.appender.file.logOnError", true);
 pref("services.sync.log.appender.file.logOnSuccess", false);
 pref("services.sync.log.appender.file.maxErrorAge", 864000); // 10 days
 pref("services.sync.log.rootLogger", "Debug");
 pref("services.sync.log.logger.addonutils", "Debug");
+pref("services.sync.log.logger.declined", "Debug");
 pref("services.sync.log.logger.service.main", "Debug");
 pref("services.sync.log.logger.status", "Debug");
 pref("services.sync.log.logger.authenticator", "Debug");
 pref("services.sync.log.logger.network.resources", "Debug");
 pref("services.sync.log.logger.service.jpakeclient", "Debug");
 pref("services.sync.log.logger.engine.bookmarks", "Debug");
 pref("services.sync.log.logger.engine.clients", "Debug");
 pref("services.sync.log.logger.engine.forms", "Debug");
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_declined.js
@@ -0,0 +1,153 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://services-sync/stages/declined.js");
+Cu.import("resource://services-sync/stages/enginesync.js");
+Cu.import("resource://services-sync/engines.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-common/observers.js");
+
+function run_test() {
+  run_next_test();
+}
+
+function PetrolEngine() {}
+PetrolEngine.prototype.name = "petrol";
+
+function DieselEngine() {}
+DieselEngine.prototype.name = "diesel";
+
+function DummyEngine() {}
+DummyEngine.prototype.name = "dummy";
+
+function ActualEngine() {}
+ActualEngine.prototype = {__proto__: Engine.prototype,
+                          name: 'actual'};
+
+function getEngineManager() {
+  let manager = new EngineManager(Service);
+  Service.engineManager = manager;
+  manager._engines = {
+    "petrol": new PetrolEngine(),
+    "diesel": new DieselEngine(),
+    "dummy": new DummyEngine(),
+    "actual": new ActualEngine(),
+  };
+  return manager;
+}
+
+/**
+ * 'Fetch' a meta/global record that doesn't mention declined.
+ *
+ * Push it into the EngineSynchronizer to set enabled; verify that those are
+ * correct.
+ *
+ * Then push it into DeclinedEngines to set declined; verify that none are
+ * declined, and a notification is sent for our locally disabled-but-not-
+ * declined engines.
+ */
+add_test(function testOldMeta() {
+  let meta = {
+    payload: {
+      engines: {
+        "petrol": 1,
+        "diesel": 2,
+        "nonlocal": 3,             // Enabled but not supported.
+      },
+    },
+  };
+
+  _("Record: " + JSON.stringify(meta));
+
+  let manager = getEngineManager();
+
+  // Update enabled from meta/global.
+  let engineSync = new EngineSynchronizer(Service);
+  engineSync._updateEnabledFromMeta(meta, 3, manager);
+
+  Assert.ok(manager._engines["petrol"].enabled, "'petrol' locally enabled.");
+  Assert.ok(manager._engines["diesel"].enabled, "'diesel' locally enabled.");
+  Assert.ok(!("nonlocal" in manager._engines), "We don't know anything about the 'nonlocal' engine.");
+  Assert.ok(!manager._engines["actual"].enabled, "'actual' not locally enabled.");
+  Assert.ok(!manager.isDeclined("actual"), "'actual' not declined, though.");
+
+  let declinedEngines = new DeclinedEngines(Service);
+
+  function onNotDeclined(subject, topic, data) {
+    Observers.remove("weave:engines:notdeclined", onNotDeclined);
+    Assert.ok(subject.undecided.has("actual"), "EngineManager observed that 'actual' was undecided.");
+
+    let declined = manager.getDeclined();
+    _("Declined: " + JSON.stringify(declined));
+
+    Assert.ok(!meta.changed, "No need to upload a new meta/global.");
+    run_next_test();
+  }
+
+  Observers.add("weave:engines:notdeclined", onNotDeclined);
+
+  declinedEngines.updateDeclined(meta, manager);
+});
+
+/**
+ * 'Fetch' a meta/global that declines an engine we don't
+ * recognize. Ensure that we track that declined engine along
+ * with any we locally declined, and that the meta/global
+ * record is marked as changed and includes all declined
+ * engines.
+ */
+add_test(function testDeclinedMeta() {
+  let meta = {
+    payload: {
+      engines: {
+        "petrol": 1,
+        "diesel": 2,
+        "nonlocal": 3,             // Enabled but not supported.
+      },
+      declined: ["nonexistent"],   // Declined and not supported.
+    },
+  };
+
+  _("Record: " + JSON.stringify(meta));
+
+  let manager = getEngineManager();
+  manager._engines["petrol"].enabled = true;
+  manager._engines["diesel"].enabled = true;
+  manager._engines["dummy"].enabled = true;
+  manager._engines["actual"].enabled = false;   // Disabled but not declined.
+
+  manager.decline(["localdecline"]);            // Declined and not supported.
+
+  let declinedEngines = new DeclinedEngines(Service);
+
+  function onNotDeclined(subject, topic, data) {
+    Observers.remove("weave:engines:notdeclined", onNotDeclined);
+    Assert.ok(subject.undecided.has("actual"), "EngineManager observed that 'actual' was undecided.");
+
+    let declined = manager.getDeclined();
+    _("Declined: " + JSON.stringify(declined));
+
+    Assert.equal(declined.indexOf("actual"), -1, "'actual' is locally disabled, but not marked as declined.");
+
+    Assert.equal(declined.indexOf("clients"), -1, "'clients' is enabled and not remotely declined.");
+    Assert.equal(declined.indexOf("petrol"), -1, "'petrol' is enabled and not remotely declined.");
+    Assert.equal(declined.indexOf("diesel"), -1, "'diesel' is enabled and not remotely declined.");
+    Assert.equal(declined.indexOf("dummy"), -1, "'dummy' is enabled and not remotely declined.");
+
+    Assert.ok(0 <= declined.indexOf("nonexistent"), "'nonexistent' was declined on the server.");
+
+    Assert.ok(0 <= declined.indexOf("localdecline"), "'localdecline' was declined locally.");
+
+    // The meta/global is modified, too.
+    Assert.ok(0 <= meta.payload.declined.indexOf("nonexistent"), "meta/global's declined contains 'nonexistent'.");
+    Assert.ok(0 <= meta.payload.declined.indexOf("localdecline"), "meta/global's declined contains 'localdecline'.");
+    Assert.strictEqual(true, meta.changed, "meta/global was changed.");
+
+    run_next_test();
+  }
+
+  Observers.add("weave:engines:notdeclined", onNotDeclined);
+
+  declinedEngines.updateDeclined(meta, manager);
+});
+
--- a/services/sync/tests/unit/test_enginemanager.js
+++ b/services/sync/tests/unit/test_enginemanager.js
@@ -1,44 +1,54 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/service.js");
 
 function run_test() {
+  run_next_test();
+}
+
+function PetrolEngine() {}
+PetrolEngine.prototype.name = "petrol";
+
+function DieselEngine() {}
+DieselEngine.prototype.name = "diesel";
+
+function DummyEngine() {}
+DummyEngine.prototype.name = "dummy";
+
+function ActualEngine() {}
+ActualEngine.prototype = {__proto__: Engine.prototype,
+                          name: 'actual'};
+
+add_test(function test_basics() {
   _("We start out with a clean slate");
 
   let manager = new EngineManager(Service);
 
   let engines = manager.getAll();
   do_check_eq(engines.length, 0);
   do_check_eq(manager.get('dummy'), undefined);
 
   _("Register an engine");
-  function DummyEngine() {}
-  DummyEngine.prototype.name = "dummy";
   manager.register(DummyEngine);
   let dummy = manager.get('dummy');
   do_check_true(dummy instanceof DummyEngine);
 
   engines = manager.getAll();
   do_check_eq(engines.length, 1);
   do_check_eq(engines[0], dummy);
 
   _("Register an already registered engine is ignored");
   manager.register(DummyEngine);
   do_check_eq(manager.get('dummy'), dummy);
 
   _("Register multiple engines in one go");
-  function PetrolEngine() {}
-  PetrolEngine.prototype.name = "petrol";
-  function DieselEngine() {}
-  DieselEngine.prototype.name = "diesel";
-
   manager.register([PetrolEngine, DieselEngine]);
   let petrol = manager.get('petrol');
   let diesel = manager.get('diesel');
   do_check_true(petrol instanceof PetrolEngine);
   do_check_true(diesel instanceof DieselEngine);
 
   engines = manager.getAll();
   do_check_eq(engines.length, 3);
@@ -69,19 +79,19 @@ function run_test() {
   manager.unregister('dummy');
   do_check_eq(manager.get('dummy'), undefined);
   engines = manager.getAll();
   do_check_eq(engines.length, 2);
   do_check_eq(engines.indexOf(dummy), -1);
 
   _("Unregister an engine by value");
   // manager.unregister() checks for instanceof Engine, so let's make one:
-  function ActualEngine() {}
-  ActualEngine.prototype = {__proto__: Engine.prototype,
-                            name: 'actual'};
   manager.register(ActualEngine);
   let actual = manager.get('actual');
   do_check_true(actual instanceof ActualEngine);
   do_check_true(actual instanceof Engine);
 
   manager.unregister(actual);
   do_check_eq(manager.get('actual'), undefined);
-}
+
+  run_next_test();
+});
+
--- a/services/sync/tests/unit/test_load_modules.js
+++ b/services/sync/tests/unit/test_load_modules.js
@@ -21,16 +21,17 @@ const modules = [
   "main.js",
   "notifications.js",
   "policies.js",
   "record.js",
   "resource.js",
   "rest.js",
   "service.js",
   "stages/cluster.js",
+  "stages/declined.js",
   "stages/enginesync.js",
   "status.js",
   "userapi.js",
   "util.js",
 ];
 
 const testingModules = [
   "fakeservices.js",
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -97,16 +97,17 @@ skip-if = os == "android"
 skip-if = os == "android"
 [test_service_verifyLogin.js]
 [test_service_wipeClient.js]
 [test_service_wipeServer.js]
 # Bug 752243: Profile cleanup frequently fails
 skip-if = os == "mac" || os == "linux"
 
 [test_corrupt_keys.js]
+[test_declined.js]
 [test_errorhandler.js]
 [test_errorhandler_filelog.js]
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = os == "android"
 [test_errorhandler_sync_checkServerError.js]
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = os == "android"
 [test_errorhandler_eol.js]