Bug 1448929 - Fix first sync check in `gSync.syncConfiguredAndLoading`. r=eoger

MozReview-Commit-ID: 8Xk6DMHroTm

--HG--
extra : rebase_source : 4dc5bbf2a2c70fb96872d6f9c68570241e6e2438

browser/base/content/browser-sync.js

@@ -53,9 +53,7 @@ var gSync = {
   // if any remote clients exist.
   get syncConfiguredAndLoading() {
     return UIState.get().status == UIState.STATUS_SIGNED_IN &&
-           (!this.syncReady ||
-           // lastSync will be non-zero after the first sync
-           Weave.Service.clientsEngine.lastSync == 0);
+           (!this.syncReady || Weave.Service.clientsEngine.isFirstSync);
   },
 
   get isSignedIn() {

services/sync/modules/engines/clients.js

@@ -114,6 +114,10 @@ ClientEngine.prototype = {
     Services.prefs.setIntPref(LAST_MODIFIED_ON_PROCESS_COMMAND_PREF, value);
   },
 
+  get isFirstSync() {
+    return !this.lastRecordUpload;
+  },
+
   // Always sync client data as it controls other sync behavior
   get enabled() {
     return true;
@@ -363,18 +367,16 @@ ClientEngine.prototype = {
   },
 
   async _syncStartup() {
-    this.isFirstSync = !this.lastRecordUpload;
     // Reupload new client record periodically.
     if (Date.now() / 1000 - this.lastRecordUpload > CLIENTS_TTL_REFRESH) {
       await this._tracker.addChangedID(this.localID);
-      this.lastRecordUpload = Date.now() / 1000;
     }
     return SyncEngine.prototype._syncStartup.call(this);
   },
 
   async _processIncoming() {
     // Fetch all records from the server.
-    await this.setLastSync(0);
+    await this.resetLastSync();
     this._incomingClients = {};
     try {
       await SyncEngine.prototype._processIncoming.call(this);
@@ -453,7 +455,9 @@ ClientEngine.prototype = {
     // Record the response time as the server time for each item we uploaded.
     let lastSync = await this.getLastSync();
     for (let id of updatedIDs) {
-      if (id != this.localID) {
+      if (id == this.localID) {
+        this.lastRecordUpload = lastSync;
+      } else {
         this._store._remoteClients[id].serverLastModified = lastSync;
       }
     }

services/sync/tests/unit/test_clients_engine.js

@@ -112,10 +112,12 @@ add_task(async function test_bad_hmac() {
 
     _("First sync, client record is uploaded");
     equal(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     check_clients_count(0);
     await syncClientsEngine(server);
     check_clients_count(1);
     ok(engine.lastRecordUpload > 0);
+    ok(!engine.isFirstSync);
 
     // Our uploaded record has a version.
     await check_record_version(user, engine.localID);
@@ -248,8 +250,10 @@ add_task(async function test_full_sync() {
 
     _("First sync. 2 records downloaded; our record uploaded.");
     strictEqual(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     await syncClientsEngine(server);
     ok(engine.lastRecordUpload > 0);
+    ok(!engine.isFirstSync);
     deepEqual(user.collection("clients").keys().sort(),
               [activeID, deletedID, engine.localID].sort(),
               "Our record should be uploaded on first sync");
@@ -298,9 +302,11 @@ add_task(async function test_sync() {
     _("First sync. Client record is uploaded.");
     equal(clientWBO(), undefined);
     equal(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     await syncClientsEngine(server);
     ok(!!clientWBO().payload);
     ok(engine.lastRecordUpload > 0);
+    ok(!engine.isFirstSync);
 
     _("Let's time travel more than a week back, new record should've been uploaded.");
     engine.lastRecordUpload -= MORE_THAN_CLIENTS_TTL_REFRESH;
@@ -309,6 +315,7 @@ add_task(async function test_sync() {
     await syncClientsEngine(server);
     ok(!!clientWBO().payload);
     ok(engine.lastRecordUpload > lastweek);
+    ok(!engine.isFirstSync);
 
     _("Remove client record.");
     await engine.removeClientData();
@@ -320,6 +327,7 @@ add_task(async function test_sync() {
     await syncClientsEngine(server);
     equal(clientWBO().payload, undefined);
     equal(engine.lastRecordUpload, yesterday);
+    ok(!engine.isFirstSync);
 
   } finally {
     await cleanup();
@@ -645,8 +653,10 @@ add_task(async function test_filter_duplicate_names() {
 
     _("First sync");
     strictEqual(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     await syncClientsEngine(server);
     ok(engine.lastRecordUpload > 0);
+    ok(!engine.isFirstSync);
     deepEqual(user.collection("clients").keys().sort(),
               [recentID, dupeID, oldID, engine.localID].sort(),
               "Our record should be uploaded on first sync");
@@ -792,6 +802,7 @@ add_task(async function test_command_sync() {
     _("Checking record was uploaded.");
     notEqual(clientWBO(engine.localID).payload, undefined);
     ok(engine.lastRecordUpload > 0);
+    ok(!engine.isFirstSync);
 
     notEqual(clientWBO(remoteId).payload, undefined);
 
@@ -1102,6 +1113,7 @@ add_task(async function test_merge_commands() {
   try {
     _("First sync. 2 records downloaded.");
     strictEqual(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     await syncClientsEngine(server);
 
     _("Broadcast logout to all clients");
@@ -1155,6 +1167,7 @@ add_task(async function test_duplicate_remote_commands() {
   try {
     _("First sync. 1 record downloaded.");
     strictEqual(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     await syncClientsEngine(server);
 
     _("Send tab to client");
@@ -1228,6 +1241,7 @@ add_task(async function test_upload_after_reboot() {
   try {
     _("First sync. 2 records downloaded.");
     strictEqual(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
     await syncClientsEngine(server);
 
     _("Send tab to client");
@@ -1325,6 +1339,7 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
   try {
     _("First sync. Download remote and our record.");
     strictEqual(engine.lastRecordUpload, 0);
+    ok(engine.isFirstSync);
 
     const oldUploadOutgoing = SyncEngine.prototype._uploadOutgoing;
     SyncEngine.prototype._uploadOutgoing = async () => engine._onRecordsWritten([], [deviceBID]);