Bug 1337993 - Ensure we mark all current bookmarks for reupload when bookmark engine enabled state changes. r?kitcambridge draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Mon, 13 Feb 2017 14:28:06 -0500
changeset 484824 975730351f11079749d2ab0891747ced8806a68a
parent 484823 9228f08cacdee37c5e741ff1abc844714a55257b
child 545861 16ac12df2d2712152fe8730c770835e4461676d1
push id45558
push userbmo:tchiovoloni@mozilla.com
push dateWed, 15 Feb 2017 21:00:33 +0000
reviewerskitcambridge
bugs1337993
milestone54.0a1
Bug 1337993 - Ensure we mark all current bookmarks for reupload when bookmark engine enabled state changes. r?kitcambridge MozReview-Commit-ID: 4gOmqrzUr77
services/sync/modules/engines/bookmarks.js
services/sync/tests/unit/test_bookmark_decline_undecline.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -562,18 +562,18 @@ BookmarksEngine.prototype = {
     let changes = this._modified.changes;
     Async.promiseSpinningly(PlacesSyncUtils.bookmarks.pushChanges(changes));
   },
 
   _deleteId(id) {
     this._noteDeletedId(id);
   },
 
-  resetClient() {
-    SyncEngine.prototype.resetClient.call(this);
+  _resetClient() {
+    SyncEngine.prototype._resetClient.call(this);
     Async.promiseSpinningly(PlacesSyncUtils.bookmarks.reset());
   },
 
   // Called when _findDupe returns a dupe item and the engine has decided to
   // switch the existing item to the new incoming item.
   _switchItemToDupe(localDupeGUID, incomingItem) {
     let newChanges = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.dedupe(
       localDupeGUID, incomingItem.id, incomingItem.parentid));
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_bookmark_decline_undecline.js
@@ -0,0 +1,104 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
+Cu.import("resource://gre/modules/BookmarkJSONUtils.jsm");
+Cu.import("resource://gre/modules/Log.jsm");
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/engines.js");
+Cu.import("resource://services-sync/engines/bookmarks.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/util.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
+
+initTestLogging("Trace");
+
+Service.engineManager.register(BookmarksEngine);
+
+function serverForFoo(engine) {
+  // The bookmarks engine *always* tracks changes, meaning we might try
+  // and sync due to the bookmarks we ourselves create! Worse, because we
+  // do an engine sync only, there's no locking - so we end up with multiple
+  // syncs running. Neuter that by making the threshold very large.
+  Service.scheduler.syncThreshold = 10000000;
+  let clientsEngine = Service.clientsEngine;
+  return serverForUsers({"foo": "password"}, {
+    meta: {
+      global: {
+        syncID: Service.syncID,
+        storageVersion: STORAGE_VERSION,
+        engines: {
+          clients: {
+            version: clientsEngine.version,
+            syncID: clientsEngine.syncID,
+          },
+          bookmarks: {
+            version: engine.version,
+            syncID: engine.syncID,
+          },
+        },
+      },
+    },
+    crypto: {
+      keys: encryptPayload({
+        id: "keys",
+        // Generate a fake default key bundle to avoid resetting the client
+        // before the first sync.
+        default: [
+          Svc.Crypto.generateRandomKey(),
+          Svc.Crypto.generateRandomKey(),
+        ],
+      }),
+    },
+    bookmarks: {}
+  });
+}
+
+// A stored reference to the collection won't be valid after disabling.
+function getBookmarkWBO(server, guid) {
+  let coll = server.user("foo").collection("bookmarks");
+  if (!coll) {
+    return null;
+  }
+  return coll.wbo(guid);
+}
+
+add_task(async function test_decline_undecline() {
+  let engine = Service.engineManager.get("bookmarks");
+  let server = serverForFoo(engine);
+  await SyncTestingInfrastructure(server);
+
+  try {
+    let bzGuid = "999999999999";
+    await PlacesSyncUtils.bookmarks.insert({
+      kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
+      syncId: bzGuid,
+      parentSyncId: "menu",
+      url: "https://bugzilla.mozilla.org",
+
+    });
+
+    ok(!getBookmarkWBO(server, bzGuid), "Shouldn't have been uploaded yet");
+    Service.sync();
+    ok(getBookmarkWBO(server, bzGuid), "Should be present on server");
+
+    engine.enabled = false;
+    Service.sync();
+    ok(!getBookmarkWBO(server, bzGuid), "Shouldn't be present on server anymore");
+
+    engine.enabled = true;
+    Service.sync();
+    ok(getBookmarkWBO(server, bzGuid), "Should be present on server again");
+
+  } finally {
+    await PlacesSyncUtils.bookmarks.reset();
+    await promiseStopServer(server);
+  }
+});
+
+function run_test() {
+  initTestLogging("Trace");
+  generateNewKeys(Service.collectionKeys);
+  run_next_test();
+}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -133,16 +133,17 @@ tags = addons
 [test_bookmark_invalid.js]
 [test_bookmark_legacy_microsummaries_support.js]
 [test_bookmark_livemarks.js]
 [test_bookmark_order.js]
 [test_bookmark_places_query_rewriting.js]
 [test_bookmark_record.js]
 [test_bookmark_smart_bookmarks.js]
 [test_bookmark_store.js]
+[test_bookmark_decline_undecline.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_bookmark_tracker.js]
 requesttimeoutfactor = 4
 [test_bookmark_validator.js]
 [test_clients_engine.js]
 [test_clients_escape.js]
 [test_extension_storage_crypto.js]