Bug 1443021 - Remove sync ID pref migration for bookmarks and history. r=markh
authorLina Cambridge <lina@yakshaving.ninja>
Thu, 03 Jan 2019 22:30:38 +0000
changeset 509585 be38e913927a24349e504a5d254b6092c31e9184
parent 509584 0097778657282369dc3e4201818aff8c7ebfa9c3
child 509586 c11865063e01c0f64acbcd935c82e4aeb0b3b092
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1443021
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1443021 - Remove sync ID pref migration for bookmarks and history. r=markh Differential Revision: https://phabricator.services.mozilla.com/D15660
services/sync/modules/engines/bookmarks.js
services/sync/modules/engines/history.js
services/sync/tests/unit/test_bookmark_engine.js
services/sync/tests/unit/test_history_engine.js
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -317,44 +317,16 @@ BaseBookmarksEngine.prototype = {
   _recordObj: PlacesItem,
   _trackerObj: BookmarksTracker,
   version: 2,
   _defaultSort: "index",
 
   syncPriority: 4,
   allowSkippedRecord: false,
 
-  _migratedSyncMetadata: false,
-  async _migrateSyncMetadata({ migrateLastSync = true } = {}) {
-    if (this._migratedSyncMetadata) {
-      return;
-    }
-    let shouldWipeRemote = await PlacesSyncUtils.bookmarks.shouldWipeRemote();
-    if (!shouldWipeRemote) {
-      // Migrate the bookmarks sync ID and last sync time from prefs, to avoid
-      // triggering a full sync on upgrade. This can be removed in bug 1443021.
-      let existingSyncID = await super.getSyncID();
-      if (existingSyncID) {
-        this._log.debug("Migrating existing sync ID ${existingSyncID} from " +
-                        "prefs", { existingSyncID });
-        await this._ensureCurrentSyncID(existingSyncID);
-      }
-      if (migrateLastSync) {
-        let existingLastSync = await super.getLastSync();
-        if (existingLastSync) {
-          this._log.debug("Migrating existing last sync time " +
-                          "${existingLastSync} from prefs",
-                          { existingLastSync });
-          await PlacesSyncUtils.bookmarks.setLastSync(existingLastSync);
-        }
-      }
-    }
-    this._migratedSyncMetadata = true;
-  },
-
   // Exposed so that the buffered engine can override to store the sync ID in
   // the mirror.
   _ensureCurrentSyncID(newSyncID) {
     return PlacesSyncUtils.bookmarks.ensureCurrentSyncId(newSyncID);
   },
 
   async ensureCurrentSyncID(newSyncID) {
     let shouldWipeRemote = await PlacesSyncUtils.bookmarks.shouldWipeRemote();
@@ -612,17 +584,16 @@ BookmarksEngine.prototype = {
       return dupe;
     }
 
     this._log.trace("No dupe found for key " + key + ".");
     return undefined;
   },
 
   async _syncStartup() {
-    await this._migrateSyncMetadata();
     await SyncEngine.prototype._syncStartup.call(this);
 
     try {
       // For first-syncs, make a backup for the user to restore
       let lastSync = await this.getLastSync();
       if (!lastSync) {
         this._log.debug("Bookmarks backup starting.");
         await PlacesBackups.create(null, true);
@@ -790,23 +761,16 @@ BufferedBookmarksEngine.prototype = {
   // errors that happen when the buffered engine is enabled vs when the
   // non-buffered engine is enabled.
   overrideTelemetryName: "bookmarks-buffered",
 
   // Needed to ensure we don't miss items when resuming a sync that failed or
   // aborted early.
   _defaultSort: "oldest",
 
-  async _syncStartup() {
-    await this._migrateSyncMetadata({
-      migrateLastSync: false,
-    });
-    await super._syncStartup();
-  },
-
   async _ensureCurrentSyncID(newSyncID) {
     await super._ensureCurrentSyncID(newSyncID);
     let buf = await this._store.ensureOpenMirror();
     await buf.ensureCurrentSyncId(newSyncID);
   },
 
   async getSyncID() {
     return PlacesSyncUtils.bookmarks.getSyncId();
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -40,38 +40,16 @@ HistoryEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _recordObj: HistoryRec,
   _storeObj: HistoryStore,
   _trackerObj: HistoryTracker,
   downloadLimit: MAX_HISTORY_DOWNLOAD,
 
   syncPriority: 7,
 
-  _migratedSyncMetadata: false,
-  async _migrateSyncMetadata() {
-    if (this._migratedSyncMetadata) {
-      return;
-    }
-    // Migrate the history sync ID and last sync time from prefs, to avoid
-    // triggering a full sync on upgrade. This can be removed in bug 1443021.
-    let existingSyncID = await super.getSyncID();
-    if (existingSyncID) {
-      this._log.debug("Migrating existing sync ID ${existingSyncID} from prefs",
-                      { existingSyncID });
-      await PlacesSyncUtils.history.ensureCurrentSyncId(existingSyncID);
-    }
-    let existingLastSync = await super.getLastSync();
-    if (existingLastSync) {
-      this._log.debug("Migrating existing last sync time ${existingLastSync} " +
-                      "from prefs", { existingLastSync });
-      await PlacesSyncUtils.history.setLastSync(existingLastSync);
-    }
-    this._migratedSyncMetadata = true;
-  },
-
   async getSyncID() {
     return PlacesSyncUtils.history.getSyncId();
   },
 
   async ensureCurrentSyncID(newSyncID) {
     this._log.debug("Checking if server sync ID ${newSyncID} matches existing",
                     { newSyncID });
     await PlacesSyncUtils.history.ensureCurrentSyncId(newSyncID);
@@ -100,21 +78,16 @@ HistoryEngine.prototype = {
     return lastSync;
   },
 
   async setLastSync(lastSync) {
     await PlacesSyncUtils.history.setLastSync(lastSync);
     await super.setLastSync(lastSync); // Remove in bug 1443021.
   },
 
-  async _syncStartup() {
-    await this._migrateSyncMetadata();
-    await super._syncStartup();
-  },
-
   shouldSyncURL(url) {
     return !url.startsWith("file:");
   },
 
   async pullNewChanges() {
     const changedIDs = await this._tracker.getChangedIDs();
     let modifiedGUIDs = Object.keys(changedIDs);
     if (!modifiedGUIDs.length) {
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -1099,98 +1099,16 @@ add_task(async function test_resume_buff
     Assert.deepEqual(toolbarRecord.children.sort(), children.sort());
 
   } finally {
     await cleanup(engine, server);
     await engine.finalize();
   }
 });
 
-add_task(async function test_legacy_migrate_sync_metadata() {
-  let legacyEngine = new BookmarksEngine(Service);
-  await legacyEngine.initialize();
-  await legacyEngine.resetClient();
-
-  let syncID = Utils.makeGUID();
-  let lastSync = Date.now() / 1000;
-
-  Svc.Prefs.set(`${legacyEngine.name}.syncID`, syncID);
-  Svc.Prefs.set(`${legacyEngine.name}.lastSync`, lastSync.toString());
-
-  strictEqual(await legacyEngine.getSyncID(), "",
-    "Legacy engine should start with empty sync ID");
-  strictEqual(await legacyEngine.getLastSync(), 0,
-    "Legacy engine should start with empty last sync");
-
-  info("Migrate Sync metadata prefs");
-  await legacyEngine._migrateSyncMetadata();
-
-  equal(await legacyEngine.getSyncID(), syncID,
-    "Initializing legacy engine should migrate sync ID");
-  equal(await legacyEngine.getLastSync(), lastSync,
-    "Initializing legacy engine should migrate last sync time");
-
-  let newSyncID = Utils.makeGUID();
-  await legacyEngine.ensureCurrentSyncID(newSyncID);
-
-  equal(await legacyEngine.getSyncID(), newSyncID,
-    "Changing legacy engine sync ID should update Places");
-  strictEqual(await legacyEngine.getLastSync(), 0,
-    "Changing legacy engine sync ID should clear last sync in Places");
-
-  equal(Svc.Prefs.get(`${legacyEngine.name}.syncID`), newSyncID,
-    "Changing legacy engine sync ID should update prefs");
-  strictEqual(Svc.Prefs.get(`${legacyEngine.name}.lastSync`), "0",
-    "Changing legacy engine sync ID should clear last sync pref");
-
-  await cleanupEngine(legacyEngine);
-  await legacyEngine.finalize();
-});
-
-add_task(async function test_buffered_migate_sync_metadata() {
-  let bufferedEngine = new BufferedBookmarksEngine(Service);
-  await bufferedEngine.initialize();
-  await bufferedEngine.resetClient();
-
-  let syncID = Utils.makeGUID();
-  let lastSync = Date.now() / 1000;
-
-  Svc.Prefs.set(`${bufferedEngine.name}.syncID`, syncID);
-  Svc.Prefs.set(`${bufferedEngine.name}.lastSync`, lastSync.toString());
-
-  strictEqual(await bufferedEngine.getSyncID(), "",
-    "Buffered engine should start with empty sync ID");
-  strictEqual(await bufferedEngine.getLastSync(), 0,
-    "Buffered engine should start with empty last sync");
-
-  info("Migrate Sync metadata prefs");
-  await bufferedEngine._migrateSyncMetadata({
-    migrateLastSync: false,
-  });
-
-  equal(await bufferedEngine.getSyncID(), syncID,
-    "Initializing buffered engine should migrate sync ID");
-  strictEqual(await bufferedEngine.getLastSync(), 0,
-    "Initializing buffered engine should not migrate last sync time");
-
-  let newSyncID = Utils.makeGUID();
-  await bufferedEngine.ensureCurrentSyncID(newSyncID);
-
-  equal(await bufferedEngine.getSyncID(), newSyncID,
-    "Changing buffered engine sync ID should update Places");
-
-  equal(Svc.Prefs.get(`${bufferedEngine.name}.syncID`), newSyncID,
-    "Changing buffered engine sync ID should update prefs");
-  strictEqual(Svc.Prefs.get(`${bufferedEngine.name}.lastSync`), "0",
-    "Changing buffered engine sync ID should clear last sync pref");
-
-  await cleanupEngine(bufferedEngine);
-  await bufferedEngine.finalize();
-});
-
 // 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);
--- a/services/sync/tests/unit/test_history_engine.js
+++ b/services/sync/tests/unit/test_history_engine.js
@@ -255,48 +255,8 @@ add_task(async function test_history_vis
   ok(allVisits.find(x => x.date.getTime() === Date.UTC(2017, 11, 4)),
      "Should contain the Dec. 4th visit");
   ok(allVisits.find(x => x.date.getTime() === Date.UTC(2017, 11, 5)),
      "Should contain the Dec. 5th visit");
 
   await engine.wipeClient();
   await engine.finalize();
 });
-
-add_task(async function test_migrate_sync_metadata() {
-  let engine = new HistoryEngine(Service);
-  await engine.initialize();
-  await engine.resetClient();
-
-  let syncID = Utils.makeGUID();
-  let lastSync = Date.now() / 1000;
-
-  Svc.Prefs.set(`${engine.name}.syncID`, syncID);
-  Svc.Prefs.set(`${engine.name}.lastSync`, lastSync.toString());
-
-  strictEqual(await engine.getSyncID(), "",
-    "Engine should start with empty sync ID");
-  strictEqual(await engine.getLastSync(), 0,
-    "Engine should start with empty last sync");
-
-  info("Migrate Sync metadata prefs");
-  await engine._migrateSyncMetadata();
-
-  equal(await engine.getSyncID(), syncID,
-    "Initializing engine should migrate sync ID");
-  equal(await engine.getLastSync(), lastSync,
-    "Initializing engine should migrate last sync time");
-
-  let newSyncID = Utils.makeGUID();
-  await engine.ensureCurrentSyncID(newSyncID);
-
-  equal(await engine.getSyncID(), newSyncID,
-    "Changing engine sync ID should update Places");
-  strictEqual(await engine.getLastSync(), 0,
-    "Changing engine sync ID should clear last sync in Places");
-
-  equal(Svc.Prefs.get(`${engine.name}.syncID`), newSyncID,
-    "Changing engine sync ID should update prefs");
-  strictEqual(Svc.Prefs.get(`${engine.name}.lastSync`), "0",
-    "Changing engine sync ID should clear last sync pref");
-
-  await engine.wipeClient();
-});