Bug 802808 - Tabs engine should explicitly skip its own record. r=rnewman

services/sync/modules/engines/tabs.js

@@ -83,6 +83,17 @@ TabEngine.prototype = {
       urls.add(entry.urlHistory[0]);
     }
     return urls;
+  },
+
+  _reconcile: function (item) {
+    // Skip our own record.
+    // TabStore.itemExists tests only against our local client ID.
+    if (this._store.itemExists(item.id)) {
+      this._log.trace("Ignoring incoming tab item because of its id: " + item.id);
+      return false;
+    }
+
+    return SyncEngine.prototype._reconcile.call(this, item);
   }
 };
 

services/sync/tests/unit/test_tab_engine.js

@@ -2,8 +2,10 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-sync/engines/tabs.js");
+Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
 
 function getMocks() {
   let engine = new TabEngine(Service);
@@ -14,6 +16,10 @@ function getMocks() {
 }
 
 function run_test() {
+  run_next_test();
+}
+
+add_test(function test_getOpenURLs() {
   _("Test getOpenURLs.");
   let [engine, store] = getMocks();
 
@@ -28,9 +34,102 @@ function run_test() {
   _("  test matching works (true)");
   let openurlsset = engine.getOpenURLs();
   matches = openurlsset.has("http://foo.com");
-  do_check_true(matches);
+  ok(matches);
 
   _("  test matching works (false)");
   matches = openurlsset.has("http://barfoo.com");
-  do_check_false(matches);
-}
+  ok(!matches);
+
+  run_next_test();
+});
+
+add_test(function test_tab_engine_skips_incoming_local_record() {
+  _("Ensure incoming records that match local client ID are never applied.");
+  let [engine, store] = getMocks();
+  let localID = engine.service.clientsEngine.localID;
+  let apply = store.applyIncoming;
+  let applied = [];
+
+  store.applyIncoming = function (record) {
+    notEqual(record.id, localID, "Only apply tab records from remote clients");
+    applied.push(record);
+    apply.call(store, record);
+  }
+
+  let collection = new ServerCollection();
+
+  _("Creating remote tab record with local client ID");
+  let localRecord = encryptPayload({id: localID, clientName: "local"});
+  collection.insert(localID, localRecord);
+
+  _("Creating remote tab record with a different client ID");
+  let remoteID = "different";
+  let remoteRecord = encryptPayload({id: remoteID, clientName: "not local"});
+  collection.insert(remoteID, remoteRecord);
+
+  _("Setting up Sync server");
+  let server = sync_httpd_setup({
+      "/1.1/foo/storage/tabs": collection.handler()
+  });
+
+  let syncTesting = new SyncTestingInfrastructure(server);
+  Service.identity.username = "foo";
+
+  let meta_global = Service.recordManager.set(engine.metaURL,
+                                              new WBORecord(engine.metaURL));
+  meta_global.payload.engines = {tabs: {version: engine.version,
+                                        syncID: engine.syncID}};
+
+  generateNewKeys(Service.collectionKeys);
+
+  let syncFinish = engine._syncFinish;
+  engine._syncFinish = function () {
+    equal(applied.length, 1, "Remote client record was applied");
+    equal(applied[0].id, remoteID, "Remote client ID matches");
+
+    syncFinish.call(engine);
+    run_next_test();
+  }
+
+  _("Start sync");
+  engine._sync();
+});
+
+add_test(function test_reconcile() {
+  let [engine, store] = getMocks();
+
+  _("Setup engine for reconciling");
+  engine._syncStartup();
+
+  _("Create an incoming remote record");
+  let remoteRecord = {id: "remote id",
+                      cleartext: "stuff and things!",
+                      modified: 1000};
+
+  ok(engine._reconcile(remoteRecord), "Apply a recently modified remote record");
+
+  remoteRecord.modified = 0;
+  ok(engine._reconcile(remoteRecord), "Apply a remote record modified long ago");
+
+  // Remote tab records are never tracked locally, so the only
+  // time they're skipped is when they're marked as deleted.
+  remoteRecord.deleted = true;
+  ok(!engine._reconcile(remoteRecord), "Skip a deleted remote record");
+
+  _("Create an incoming local record");
+  // The locally tracked tab record always takes precedence over its
+  // remote counterparts.
+  let localRecord = {id: engine.service.clientsEngine.localID,
+                     cleartext: "this should always be skipped",
+                     modified: 2000};
+
+  ok(!engine._reconcile(localRecord), "Skip incoming local if recently modified");
+
+  localRecord.modified = 0;
+  ok(!engine._reconcile(localRecord), "Skip incoming local if modified long ago");
+
+  localRecord.deleted = true;
+  ok(!engine._reconcile(localRecord), "Skip incoming local if deleted");
+
+  run_next_test();
+});