From e017fd7a57c6093bbe07f103c319db9baec383c7 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 8 Jul 2020 17:58:09 +0000 Subject: [PATCH] Bug 1651285 - Remove bookmarks and history sync ID and last sync duplication logic. r=lina Differential Revision: https://phabricator.services.mozilla.com/D82645 --- services/sync/modules/engines/bookmarks.js | 7 - services/sync/modules/engines/history.js | 3 - .../sync/tests/unit/test_bookmark_engine.js | 172 ------------------ 3 files changed, 182 deletions(-) diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 6f82a954f696..03efdf9e4658 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -379,10 +379,6 @@ BaseBookmarksEngine.prototype = { { newSyncID } ); await this._ensureCurrentSyncID(newSyncID); - // Update the sync ID in prefs to allow downgrading to older Firefox - // releases that don't store Sync metadata in Places. This can be removed - // in bug 1443021. - await super.ensureCurrentSyncID(newSyncID); return newSyncID; } // We didn't take the new sync ID because we need to wipe the server @@ -411,7 +407,6 @@ BaseBookmarksEngine.prototype = { async resetLocalSyncID() { let newSyncID = await PlacesSyncUtils.bookmarks.resetSyncId(); this._log.debug("Assigned new sync ID ${newSyncID}", { newSyncID }); - await super.ensureCurrentSyncID(newSyncID); // Remove in bug 1443021. return newSyncID; }, @@ -563,7 +558,6 @@ BookmarksEngine.prototype = { async setLastSync(lastSync) { await PlacesSyncUtils.bookmarks.setLastSync(lastSync); - await super.setLastSync(lastSync); // Remove in bug 1443021. }, emptyChangeset() { @@ -873,7 +867,6 @@ BufferedBookmarksEngine.prototype = { // Update the last sync time in Places so that reverting to the original // bookmarks engine doesn't download records we've already applied. await PlacesSyncUtils.bookmarks.setLastSync(lastSync); - await super.setLastSync(lastSync); // Remove in bug 1443021. }, emptyChangeset() { diff --git a/services/sync/modules/engines/history.js b/services/sync/modules/engines/history.js index 1d64acee1454..7e97fa2b70da 100644 --- a/services/sync/modules/engines/history.js +++ b/services/sync/modules/engines/history.js @@ -70,7 +70,6 @@ HistoryEngine.prototype = { { newSyncID } ); await PlacesSyncUtils.history.ensureCurrentSyncId(newSyncID); - await super.ensureCurrentSyncID(newSyncID); // Remove in bug 1443021. return newSyncID; }, @@ -86,7 +85,6 @@ HistoryEngine.prototype = { async resetLocalSyncID() { let newSyncID = await PlacesSyncUtils.history.resetSyncId(); this._log.debug("Assigned new sync ID ${newSyncID}", { newSyncID }); - await super.ensureCurrentSyncID(newSyncID); // Remove in bug 1443021. return newSyncID; }, @@ -97,7 +95,6 @@ HistoryEngine.prototype = { async setLastSync(lastSync) { await PlacesSyncUtils.history.setLastSync(lastSync); - await super.setLastSync(lastSync); // Remove in bug 1443021. }, shouldSyncURL(url) { diff --git a/services/sync/tests/unit/test_bookmark_engine.js b/services/sync/tests/unit/test_bookmark_engine.js index ad92aca7e7b3..d7ec18fbe6ff 100644 --- a/services/sync/tests/unit/test_bookmark_engine.js +++ b/services/sync/tests/unit/test_bookmark_engine.js @@ -1411,178 +1411,6 @@ add_task(async function test_resume_buffer() { } }); -// The buffered engine stores the sync ID and last sync time in three places: -// prefs, Places, and the mirror. We can remove the prefs entirely in bug -// 1443021, and drop the last sync time from Places once we remove the legacy -// engine. This test ensures we keep them in sync (^_^), and handle mismatches -// in case the user copies Places or the mirror between accounts. See -// bug 1199077, comment 84 for the gory details. -add_task(async function test_mirror_syncID() { - let bufferedEngine = new BufferedBookmarksEngine(Service); - await bufferedEngine.initialize(); - let buf = await bufferedEngine._store.ensureOpenMirror(); - - info("Places and mirror don't have sync IDs"); - - let syncID = await bufferedEngine.resetLocalSyncID(); - - equal( - Svc.Prefs.get(`${bufferedEngine.name}.syncID`), - syncID, - "Should reset sync ID in prefs" - ); - strictEqual( - Svc.Prefs.get(`${bufferedEngine.name}.lastSync`), - "0", - "Should reset last sync in prefs" - ); - - equal( - await PlacesSyncUtils.bookmarks.getSyncId(), - syncID, - "Should reset sync ID in Places" - ); - strictEqual( - await PlacesSyncUtils.bookmarks.getLastSync(), - 0, - "Should reset last sync in Places" - ); - - equal(await buf.getSyncId(), syncID, "Should reset sync ID in mirror"); - strictEqual( - await buf.getCollectionHighWaterMark(), - 0, - "Should reset high water mark in mirror" - ); - - info("Places and mirror have matching sync ID"); - - await bufferedEngine.setLastSync(123.45); - await bufferedEngine.ensureCurrentSyncID(syncID); - - equal( - Svc.Prefs.get(`${bufferedEngine.name}.syncID`), - syncID, - "Should keep sync ID in prefs if Places and mirror match" - ); - strictEqual( - Svc.Prefs.get(`${bufferedEngine.name}.lastSync`), - "123.45", - "Should keep last sync in prefs if Places and mirror match" - ); - - equal( - await PlacesSyncUtils.bookmarks.getSyncId(), - syncID, - "Should keep sync ID in Places if Places and mirror match" - ); - strictEqual( - await PlacesSyncUtils.bookmarks.getLastSync(), - 123.45, - "Should keep last sync in Places if Places and mirror match" - ); - - equal(await buf.getSyncId(), syncID, "Should keep sync ID in mirror"); - equal( - await buf.getCollectionHighWaterMark(), - 123.45, - "Should keep high water mark in mirror" - ); - - info("Places and mirror have different sync IDs"); - - // Directly update the sync ID in the mirror, without resetting. - await buf.db.execute(`UPDATE meta SET value = :value WHERE key = :key`, { - key: SyncedBookmarksMirror.META_KEY.SYNC_ID, - value: "syncIdAAAAAA", - }); - await bufferedEngine.ensureCurrentSyncID(syncID); - - equal( - Svc.Prefs.get(`${bufferedEngine.name}.syncID`), - syncID, - "Should keep sync ID in prefs if Places and mirror don't match" - ); - strictEqual( - Svc.Prefs.get(`${bufferedEngine.name}.lastSync`), - "123.45", - "Should keep last sync in prefs if Places and mirror don't match" - ); - - equal( - await PlacesSyncUtils.bookmarks.getSyncId(), - syncID, - "Should keep existing sync ID in Places on mirror sync ID mismatch" - ); - strictEqual( - await PlacesSyncUtils.bookmarks.getLastSync(), - 123.45, - "Should keep existing last sync in Places on mirror sync ID mismatch" - ); - - equal( - await buf.getSyncId(), - syncID, - "Should reset mismatched sync ID in mirror" - ); - strictEqual( - await buf.getCollectionHighWaterMark(), - 0, - "Should reset high water mark on mirror sync ID mismatch" - ); - - info("Places has sync ID; mirror missing sync ID"); - await buf.reset(); - - equal( - await bufferedEngine.ensureCurrentSyncID(syncID), - syncID, - "Should not assign new sync ID if Places has sync ID; mirror missing" - ); - equal( - await buf.getSyncId(), - syncID, - "Should set sync ID in mirror to match Places" - ); - - info("Places missing sync ID; mirror has sync ID"); - - await buf.setCollectionLastModified(123.45); - await PlacesSyncUtils.bookmarks.reset(); - let newSyncID = await bufferedEngine.ensureCurrentSyncID("syncIdBBBBBB"); - - equal( - Svc.Prefs.get(`${bufferedEngine.name}.syncID`), - newSyncID, - "Should set new sync ID in prefs" - ); - strictEqual( - Svc.Prefs.get(`${bufferedEngine.name}.lastSync`), - "0", - "Should reset last sync in prefs on sync ID change" - ); - - equal( - await PlacesSyncUtils.bookmarks.getSyncId(), - newSyncID, - "Should set new sync ID in Places" - ); - equal( - await buf.getSyncId(), - newSyncID, - "Should update new sync ID in mirror" - ); - - strictEqual( - await buf.getCollectionHighWaterMark(), - 0, - "Should reset high water mark on sync ID change in Places" - ); - - await cleanupEngine(bufferedEngine); - await bufferedEngine.finalize(); -}); - add_bookmark_test(async function test_livemarks(engine) { _("Ensure we replace new and existing livemarks with tombstones");