Backed out changeset 8d5cc47b248f (bug 1401700) for failing xpcshell services/sync/tests/unit/test_bookmark_repair.js on OS X 10.10 opt. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Wed, 11 Oct 2017 23:44:26 +0200
changeset 679146 b9dd71f3f20f077171eddb8f20d1683701ef24e5
parent 679145 5b8fb23f16afc92457ce0703eb0d4367279727b9
child 679147 21a76cf94e1fa8321bc3075514bd25e4c7969ec0
push id84141
push userbmo:schien@mozilla.com
push dateThu, 12 Oct 2017 11:13:04 +0000
reviewersbackout
bugs1401700
milestone58.0a1
backs out8d5cc47b248f5b6c995fc836b076a3c0d1655313
Backed out changeset 8d5cc47b248f (bug 1401700) for failing xpcshell services/sync/tests/unit/test_bookmark_repair.js on OS X 10.10 opt. r=backout
services/sync/modules/engines/clients.js
services/sync/tests/unit/test_clients_engine.js
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -45,17 +45,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "getRepairResponder",
   "resource://services-sync/collection_repair.js");
 
 const CLIENTS_TTL = 1814400; // 21 days
 const CLIENTS_TTL_REFRESH = 604800; // 7 days
 const STALE_CLIENT_REMOTE_AGE = 604800; // 7 days
 
 const SUPPORTED_PROTOCOL_VERSIONS = [SYNC_API_VERSION];
-const LAST_MODIFIED_ON_PROCESS_COMMAND_PREF = "services.sync.clients.lastModifiedOnProcessCommands";
 
 function hasDupeCommand(commands, action) {
   if (!commands) {
     return false;
   }
   return commands.some(other => other.command == action.command &&
     Utils.deepEquals(other.args, action.args));
 }
@@ -88,27 +87,16 @@ this.ClientEngine = function ClientEngin
 ClientEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: ClientStore,
   _recordObj: ClientsRec,
   _trackerObj: ClientsTracker,
   allowSkippedRecord: false,
   _knownStaleFxADeviceIds: null,
 
-  // These two properties allow us to avoid replaying the same commands
-  // continuously if we cannot manage to upload our own record.
-  _localClientLastModified: 0,
-  get _lastModifiedOnProcessCommands() {
-    return Services.prefs.getIntPref(LAST_MODIFIED_ON_PROCESS_COMMAND_PREF, -1);
-  },
-
-  set _lastModifiedOnProcessCommands(value) {
-    Services.prefs.setIntPref(LAST_MODIFIED_ON_PROCESS_COMMAND_PREF, value);
-  },
-
   // Always sync client data as it controls other sync behavior
   get enabled() {
     return true;
   },
 
   get lastRecordUpload() {
     return Svc.Prefs.get(this.name + ".lastRecordUpload", 0);
   },
@@ -399,17 +387,16 @@ ClientEngine.prototype = {
         }
       }
       let localFxADeviceId = await fxAccounts.getDeviceId();
       // Bug 1264498: Mobile clients don't remove themselves from the clients
       // collection when the user disconnects Sync, so we mark as stale clients
       // with the same name that haven't synced in over a week.
       // (Note we can't simply delete them, or we re-apply them next sync - see
       // bug 1287687)
-      this._localClientLastModified = Math.round(this._incomingClients[this.localID]);
       delete this._incomingClients[this.localID];
       let names = new Set([this.localName]);
       let seenDeviceIds = new Set([localFxADeviceId]);
       let idToLastModifiedList = Object.entries(this._incomingClients)
                                  .sort((a, b) => b[1] - a[1]);
       for (let [id, serverLastModified] of idToLastModifiedList) {
         let record = this._store._remoteClients[id];
         // stash the server last-modified time on the record.
@@ -678,21 +665,19 @@ ClientEngine.prototype = {
 
   /**
    * Check if the local client has any remote commands and perform them.
    *
    * @return false to abort sync
    */
   async processIncomingCommands() {
     return this._notify("clients:process-commands", "", async function() {
-      if (!this.localCommands ||
-          this._lastModifiedOnProcessCommands == this._localClientLastModified) {
+      if (!this.localCommands) {
         return true;
       }
-      this._lastModifiedOnProcessCommands = this._localClientLastModified;
 
       const clearedCommands = await this._readCommands()[this.localID];
       const commands = this.localCommands.filter(command => !hasDupeCommand(clearedCommands, command));
       let didRemoveCommand = false;
       let URIsToDisplay = [];
       // Process each command in order.
       for (let rawCommand of commands) {
         let shouldRemoveCommand = true; // most commands are auto-removed.
--- a/services/sync/tests/unit/test_clients_engine.js
+++ b/services/sync/tests/unit/test_clients_engine.js
@@ -1347,17 +1347,17 @@ add_task(async function test_keep_cleare
       },
       {
         command: "displayURI",
         args: ["https://deviceclink2.com", deviceCID, "Device C link 2"],
         flowID: Utils.makeGUID(),
       }],
       version: "48",
       protocols: ["1.5"],
-    }), now - 5));
+    }), now - 10));
 
     // Simulate reboot
     SyncEngine.prototype._uploadOutgoing = oldUploadOutgoing;
     engine = Service.clientsEngine = new ClientEngine(Service);
     await engine.initialize();
 
     commandsProcessed = 0;
     engine._handleDisplayURIs = (uris) => { commandsProcessed = uris.length };
@@ -1789,31 +1789,13 @@ add_task(async function process_incoming
 
   await engine._processIncoming();
   ok(stubRefresh.notCalled, "Should not refresh the known stale clients since it's already populated");
 
   stubProcessIncoming.restore();
   stubRefresh.restore();
 });
 
-add_task(async function process_incoming_refreshes_known_stale_clients() {
-  Services.prefs.clearUserPref("services.sync.clients.lastModifiedOnProcessCommands");
-  engine._localClientLastModified = Math.round(Date.now() / 1000);
-
-  const stubRemoveLocalCommand = sinon.stub(engine, "removeLocalCommand");
-  const tabProcessedSpy = sinon.spy(engine, "_handleDisplayURIs");
-  engine.localCommands = [{ command: "displayURI", args: ["https://foo.bar", "fxaid1", "foo"] }];
-
-  await engine.processIncomingCommands();
-  ok(tabProcessedSpy.calledOnce);
-  // Let's say we failed to upload and we end up calling processIncomingCommands again
-  await engine.processIncomingCommands();
-  ok(tabProcessedSpy.calledOnce);
-
-  tabProcessedSpy.restore();
-  stubRemoveLocalCommand.restore();
-});
-
 function run_test() {
   initTestLogging("Trace");
   Log.repository.getLogger("Sync.Engine.Clients").level = Log.Level.Trace;
   run_next_test();
 }