Bug 1353373 - Fix Edge readinglist imports. r=jaws, a=lizzard
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 04 Apr 2017 16:18:17 +0100
changeset 379449 db91f4e6e0a88c46f6dc02ffe31a933f5fe538bb
parent 379448 0274a55c5f302c23813c6731827c5e580cb03826
child 379450 27126f140a2cf381f070afe905cf03659be8cd98
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, lizzard
bugs1353373
milestone53.0
Bug 1353373 - Fix Edge readinglist imports. r=jaws, a=lizzard MozReview-Commit-ID: Fu9n3FIrxf6
browser/components/migration/EdgeProfileMigrator.js
browser/components/migration/tests/unit/test_Edge_db_migration.js
--- a/browser/components/migration/EdgeProfileMigrator.js
+++ b/browser/components/migration/EdgeProfileMigrator.js
@@ -146,38 +146,41 @@ EdgeTypedURLMigrator.prototype = {
       ignoreResults: true,
       handleCompletion(updatedCount) {
         aCallback(updatedCount > 0);
       }
     });
   },
 };
 
-function EdgeReadingListMigrator() {
+function EdgeReadingListMigrator(dbOverride) {
+  this.dbOverride = dbOverride;
 }
 
 EdgeReadingListMigrator.prototype = {
   type: MigrationUtils.resourceTypes.BOOKMARKS,
 
+  get db() { return this.dbOverride || gEdgeDatabase },
+
   get exists() {
-    return !!gEdgeDatabase;
+    return !!this.db;
   },
 
   migrate(callback) {
     this._migrateReadingList(PlacesUtils.bookmarks.menuGuid).then(
       () => callback(true),
       ex => {
         Cu.reportError(ex);
         callback(false);
       }
     );
   },
 
   _migrateReadingList: Task.async(function*(parentGuid) {
-    if (yield ESEDBReader.dbLocked(gEdgeDatabase)) {
+    if (yield ESEDBReader.dbLocked(this.db)) {
       throw new Error("Edge seems to be running - its database is locked.");
     }
     let columnFn = db => {
       let columns = [
         {name: "URL", type: "string"},
         {name: "Title", type: "string"},
         {name: "AddedDate", type: "date"}
       ];
@@ -189,34 +192,34 @@ EdgeReadingListMigrator.prototype = {
       }
       return columns;
     };
 
     let filterFn = row => {
       return !row.IsDeleted;
     };
 
-    let readingListItems = readTableFromEdgeDB("ReadingList", columnFn, filterFn);
+    let readingListItems = readTableFromEdgeDB("ReadingList", columnFn, filterFn, this.db);
     if (!readingListItems.length) {
       return;
     }
 
     let destFolderGuid = yield this._ensureReadingListFolder(parentGuid);
     let bookmarks = [];
     for (let item of readingListItems) {
       let dateAdded = item.AddedDate || new Date();
       // Avoid including broken URLs:
       try {
         new URL(item.URL);
       } catch (ex) {
         continue;
       }
       bookmarks.push({ url: item.URL, title: item.Title, dateAdded });
     }
-    yield MigrationUtils.insertManyBookmarksWrapper(readingListItems, destFolderGuid);
+    yield MigrationUtils.insertManyBookmarksWrapper(bookmarks, destFolderGuid);
   }),
 
   _ensureReadingListFolder: Task.async(function*(parentGuid) {
     if (!this.__readingListFolderGuid) {
       let folderTitle = MigrationUtils.getLocalizedString("importedEdgeReadingList");
       let folderSpec = {type: PlacesUtils.bookmarks.TYPE_FOLDER, parentGuid, title: folderTitle};
       this.__readingListFolderGuid = (yield MigrationUtils.insertBookmarkWrapper(folderSpec)).guid;
     }
@@ -345,20 +348,24 @@ EdgeBookmarksMigrator.prototype = {
 };
 
 function EdgeProfileMigrator() {
   this.wrappedJSObject = this;
 }
 
 EdgeProfileMigrator.prototype = Object.create(MigratorPrototype);
 
-EdgeProfileMigrator.prototype.getESEMigratorForTesting = function(dbOverride) {
+EdgeProfileMigrator.prototype.getBookmarksMigratorForTesting = function(dbOverride) {
   return new EdgeBookmarksMigrator(dbOverride);
 };
 
+EdgeProfileMigrator.prototype.getReadingListMigratorForTesting = function(dbOverride) {
+  return new EdgeReadingListMigrator(dbOverride);
+};
+
 EdgeProfileMigrator.prototype.getResources = function() {
   let resources = [
     new EdgeBookmarksMigrator(),
     MSMigrationUtils.getCookiesMigrator(MSMigrationUtils.MIGRATION_TYPE_EDGE),
     new EdgeTypedURLMigrator(),
     new EdgeReadingListMigrator(),
   ];
   let windowsVaultFormPasswordsMigrator =
--- a/browser/components/migration/tests/unit/test_Edge_db_migration.js
+++ b/browser/components/migration/tests/unit/test_Edge_db_migration.js
@@ -160,17 +160,17 @@ function convertValueForWriting(value, v
   }
 
   throw new Error("Unknown type " + valueType);
 }
 
 let initializedESE = false;
 
 let eseDBWritingHelpers = {
-  setupDB(dbFile, tableName, columns, rows) {
+  setupDB(dbFile, tables) {
     if (!initializedESE) {
       initializedESE = true;
       loadLibraries();
 
       KERNEL.SystemTimeToFileTime = gLibs.kernel.declare("SystemTimeToFileTime",
           ctypes.default_abi, ctypes.bool, KERNEL.SYSTEMTIME.ptr, KERNEL.FILETIME.ptr);
 
       declareESEFunction("CreateDatabaseW", ESE.JET_SESID, ESE.JET_PCWSTR,
@@ -216,48 +216,51 @@ let eseDBWritingHelpers = {
       this._sessionCreated = true;
 
       this._dbId = new ESE.JET_DBID();
       this._dbPath = rootPath + "spartan.edb";
       ESE.CreateDatabaseW(this._sessionId, this._dbPath, null,
                           this._dbId.address(), 0);
       this._opened = this._attached = true;
 
-      let tableCreationWrapper = createTableCreationWrapper(tableName, columns);
-      ESE.CreateTableColumnIndexW(this._sessionId, this._dbId,
-                                  tableCreationWrapper.table.address());
-      this._tableId = tableCreationWrapper.table.tableid;
+      for (let [tableName, data] of tables) {
+        let {rows, columns} = data;
+        let tableCreationWrapper = createTableCreationWrapper(tableName, columns);
+        ESE.CreateTableColumnIndexW(this._sessionId, this._dbId,
+                                    tableCreationWrapper.table.address());
+        this._tableId = tableCreationWrapper.table.tableid;
 
-      let columnIdMap = new Map();
-      if (rows.length) {
-        // Iterate over the struct we passed into ESENT because they have the
-        // created column ids.
-        let columnCount = ctypes.UInt64.lo(tableCreationWrapper.table.cColumns);
-        let columnsPassed = tableCreationWrapper.table.rgcolumncreate;
-        for (let i = 0; i < columnCount; i++) {
-          let column = columnsPassed.contents;
-          columnIdMap.set(column.szColumnName.readString(), column);
-          columnsPassed = columnsPassed.increment();
+        let columnIdMap = new Map();
+        if (rows.length) {
+          // Iterate over the struct we passed into ESENT because they have the
+          // created column ids.
+          let columnCount = ctypes.UInt64.lo(tableCreationWrapper.table.cColumns);
+          let columnsPassed = tableCreationWrapper.table.rgcolumncreate;
+          for (let i = 0; i < columnCount; i++) {
+            let column = columnsPassed.contents;
+            columnIdMap.set(column.szColumnName.readString(), column);
+            columnsPassed = columnsPassed.increment();
+          }
+          ESE.ManualMove(this._sessionId, this._tableId,
+                         -2147483648 /* JET_MoveFirst */, 0);
+          ESE.BeginTransaction(this._sessionId);
+          for (let row of rows) {
+            ESE.PrepareUpdate(this._sessionId, this._tableId, 0 /* JET_prepInsert */);
+            for (let columnName in row) {
+              let col = columnIdMap.get(columnName);
+              let colId = col.columnid;
+              let [val, valSize] = convertValueForWriting(row[columnName], col.coltyp);
+              /* JET_bitSetOverwriteLV */
+              ESE.SetColumn(this._sessionId, this._tableId, colId, val.address(), valSize, 4, null);
+            }
+            let actualBookmarkSize = new ctypes.unsigned_long();
+            ESE.Update(this._sessionId, this._tableId, null, 0, actualBookmarkSize.address());
+          }
+          ESE.CommitTransaction(this._sessionId, 0 /* JET_bitWaitLastLevel0Commit */);
         }
-        ESE.ManualMove(this._sessionId, this._tableId,
-                       -2147483648 /* JET_MoveFirst */, 0);
-        ESE.BeginTransaction(this._sessionId);
-        for (let row of rows) {
-          ESE.PrepareUpdate(this._sessionId, this._tableId, 0 /* JET_prepInsert */);
-          for (let columnName in row) {
-            let col = columnIdMap.get(columnName);
-            let colId = col.columnid;
-            let [val, valSize] = convertValueForWriting(row[columnName], col.coltyp);
-            /* JET_bitSetOverwriteLV */
-            ESE.SetColumn(this._sessionId, this._tableId, colId, val.address(), valSize, 4, null);
-          }
-          let actualBookmarkSize = new ctypes.unsigned_long();
-          ESE.Update(this._sessionId, this._tableId, null, 0, actualBookmarkSize.address());
-        }
-        ESE.CommitTransaction(this._sessionId, 0 /* JET_bitWaitLastLevel0Commit */);
       }
     } finally {
       try {
         this._close();
       } catch (ex) {
         Cu.reportError(ex);
       }
     }
@@ -296,17 +299,17 @@ add_task(function*() {
   db.append("spartan.edb");
 
   let logs = tempFile.clone();
   logs.append("LogFiles");
   logs.create(tempFile.DIRECTORY_TYPE, 0o600);
 
   let creationDate = new Date(Date.now() - 5000);
   const kEdgeMenuParent = "62d07e2b-5f0d-4e41-8426-5f5ec9717beb";
-  let itemsInDB = [
+  let bookmarkReferenceItems = [
     {
       URL: "http://www.mozilla.org/",
       Title: "Mozilla",
       DateUpdated: new Date(creationDate.valueOf() + 100),
       ItemId: "1c00c10a-15f6-4618-92dd-22575102a4da",
       ParentId: kEdgeMenuParent,
       IsFolder: false,
       IsDeleted: false,
@@ -367,30 +370,59 @@ add_task(function*() {
       URL: "http://www.iteminfavoritesbar.org/",
       DateUpdated: new Date(creationDate.valueOf() + 800),
       ItemId: "9f2b1ff8-b651-46cf-8f41-16da8bcb6791",
       ParentId: "921dc8a0-6c83-40ef-8df1-9bd1c5c56aaf",
       IsFolder: false,
       IsDeleted: false,
     },
   ];
-  eseDBWritingHelpers.setupDB(db, "Favorites", [
-    {type: COLUMN_TYPES.JET_coltypLongText, name: "URL", cbMax: 4096},
-    {type: COLUMN_TYPES.JET_coltypLongText, name: "Title", cbMax: 4096},
-    {type: COLUMN_TYPES.JET_coltypLongLong, name: "DateUpdated"},
-    {type: COLUMN_TYPES.JET_coltypGUID, name: "ItemId"},
-    {type: COLUMN_TYPES.JET_coltypBit, name: "IsDeleted"},
-    {type: COLUMN_TYPES.JET_coltypBit, name: "IsFolder"},
-    {type: COLUMN_TYPES.JET_coltypGUID, name: "ParentId"},
-  ], itemsInDB);
+
+  let readingListReferenceItems = [
+    {
+      Title: "Some mozilla page",
+      URL: "http://www.mozilla.org/somepage/",
+      AddedDate: new Date(creationDate.valueOf() + 900),
+      ItemId: "c88426fd-52a7-419d-acbc-d2310e8afebe",
+      IsDeleted: false,
+    },
+    {
+      Title: "Some other page",
+      URL: "https://www.example.org/somepage/",
+      AddedDate: new Date(creationDate.valueOf() + 1000),
+      ItemId: "a35fc843-5d5a-4d1e-9be8-45214be24b5c",
+      IsDeleted: false,
+    },
+  ];
+  eseDBWritingHelpers.setupDB(db, new Map([["Favorites", {
+    columns: [
+      {type: COLUMN_TYPES.JET_coltypLongText, name: "URL", cbMax: 4096},
+      {type: COLUMN_TYPES.JET_coltypLongText, name: "Title", cbMax: 4096},
+      {type: COLUMN_TYPES.JET_coltypLongLong, name: "DateUpdated"},
+      {type: COLUMN_TYPES.JET_coltypGUID, name: "ItemId"},
+      {type: COLUMN_TYPES.JET_coltypBit, name: "IsDeleted"},
+      {type: COLUMN_TYPES.JET_coltypBit, name: "IsFolder"},
+      {type: COLUMN_TYPES.JET_coltypGUID, name: "ParentId"},
+    ],
+    rows: bookmarkReferenceItems,
+  }], ["ReadingList", {
+    columns: [
+      {type: COLUMN_TYPES.JET_coltypLongText, name: "URL", cbMax: 4096},
+      {type: COLUMN_TYPES.JET_coltypLongText, name: "Title", cbMax: 4096},
+      {type: COLUMN_TYPES.JET_coltypLongLong, name: "AddedDate"},
+      {type: COLUMN_TYPES.JET_coltypGUID, name: "ItemId"},
+      {type: COLUMN_TYPES.JET_coltypBit, name: "IsDeleted"},
+    ],
+    rows: readingListReferenceItems,
+  }]]));
 
   let migrator = Cc["@mozilla.org/profile/migrator;1?app=browser&type=edge"]
                  .createInstance(Ci.nsIBrowserProfileMigrator);
-  let bookmarksMigrator = migrator.wrappedJSObject.getESEMigratorForTesting(db);
-  Assert.ok(bookmarksMigrator.exists, "Should recognize table we just created");
+  let bookmarksMigrator = migrator.wrappedJSObject.getBookmarksMigratorForTesting(db);
+  Assert.ok(bookmarksMigrator.exists, "Should recognize db we just created");
 
   let source = MigrationUtils.getLocalizedString("sourceNameEdge");
   let sourceLabel = MigrationUtils.getLocalizedString("importedBookmarksFolder", [source]);
 
   let seenBookmarks = [];
   let bookmarkObserver = {
     onItemAdded(itemId, parentId, index, itemType, url, title, dateAdded, itemGuid, parentGuid) {
       if (title.startsWith("Deleted")) {
@@ -421,20 +453,20 @@ add_task(function*() {
 
   let menuParents = seenBookmarks.filter(item => item.parentGuid == PlacesUtils.bookmarks.menuGuid);
   Assert.equal(menuParents.length, 1, "Should have a single folder added to the menu");
   let toolbarParents = seenBookmarks.filter(item => item.parentGuid == PlacesUtils.bookmarks.toolbarGuid);
   Assert.equal(toolbarParents.length, 1, "Should have a single item added to the toolbar");
   let menuParentGuid = menuParents[0].itemGuid;
   let toolbarParentGuid = toolbarParents[0].itemGuid;
 
-  let expectedTitlesInMenu = itemsInDB.filter(item => item.ParentId == kEdgeMenuParent).map(item => item.Title);
+  let expectedTitlesInMenu = bookmarkReferenceItems.filter(item => item.ParentId == kEdgeMenuParent).map(item => item.Title);
   // Hacky, but seems like much the simplest way:
   expectedTitlesInMenu.push("Item in deleted folder (should be in root)");
-  let expectedTitlesInToolbar = itemsInDB.filter(item => item.ParentId == "921dc8a0-6c83-40ef-8df1-9bd1c5c56aaf").map(item => item.Title);
+  let expectedTitlesInToolbar = bookmarkReferenceItems.filter(item => item.ParentId == "921dc8a0-6c83-40ef-8df1-9bd1c5c56aaf").map(item => item.Title);
 
   let edgeNameStr = MigrationUtils.getLocalizedString("sourceNameEdge");
   let importParentFolderName = MigrationUtils.getLocalizedString("importedBookmarksFolder", [edgeNameStr]);
 
   for (let bookmark of seenBookmarks) {
     let shouldBeInMenu = expectedTitlesInMenu.includes(bookmark.title);
     let shouldBeInToolbar = expectedTitlesInToolbar.includes(bookmark.title);
     if (bookmark.title == "Folder" || bookmark.title == importParentFolderName) {
@@ -453,18 +485,59 @@ add_task(function*() {
       Assert.ok(true, "Expect toolbar and menu folders to not be in menu or toolbar");
     } else {
       // Bit hacky, but we do need to check this.
       Assert.equal(bookmark.title, "Item in folder", "Subfoldered item shouldn't be in menu or toolbar");
       let parent = seenBookmarks.find(maybeParent => maybeParent.itemGuid == bookmark.parentGuid);
       Assert.equal(parent && parent.title, "Folder", "Subfoldered item should be in subfolder labeled 'Folder'");
     }
 
-    let dbItem = itemsInDB.find(someItem => bookmark.title == someItem.Title);
+    let dbItem = bookmarkReferenceItems.find(someItem => bookmark.title == someItem.Title);
     if (!dbItem) {
       Assert.equal(bookmark.title, importParentFolderName, "Only the extra layer of folders isn't in the input we stuck in the DB.");
       Assert.ok([menuParentGuid, toolbarParentGuid].includes(bookmark.itemGuid), "This item should be one of the containers");
     } else {
       Assert.equal(dbItem.URL || null, bookmark.url && bookmark.url.spec, "URL is correct");
       Assert.equal(dbItem.DateUpdated.valueOf(), (new Date(bookmark.dateAdded / 1000)).valueOf(), "Date added is correct");
     }
   }
+
+  MigrationUtils._importQuantities.bookmarks = 0;
+  seenBookmarks = [];
+  bookmarkObserver = {
+    onItemAdded(itemId, parentId, index, itemType, url, title, dateAdded, itemGuid, parentGuid) {
+      seenBookmarks.push({itemId, parentId, index, itemType, url, title, dateAdded, itemGuid, parentGuid});
+    },
+    onBeginUpdateBatch() {},
+    onEndUpdateBatch() {},
+    onItemRemoved() {},
+    onItemChanged() {},
+    onItemVisited() {},
+    onItemMoved() {},
+  };
+  PlacesUtils.bookmarks.addObserver(bookmarkObserver, false);
+
+  let readingListMigrator = migrator.wrappedJSObject.getReadingListMigratorForTesting(db);
+  Assert.ok(readingListMigrator.exists, "Should recognize db we just created");
+  migrateResult = yield new Promise(resolve => readingListMigrator.migrate(resolve)).catch(ex => {
+    Cu.reportError(ex);
+    Assert.ok(false, "Got an exception trying to migrate data! " + ex);
+    return false;
+  });
+  PlacesUtils.bookmarks.removeObserver(bookmarkObserver);
+  Assert.ok(migrateResult, "Migration should succeed");
+  Assert.equal(seenBookmarks.length, 3, "Should have seen 3 items being bookmarked (2 items + 1 folder).");
+  Assert.equal(seenBookmarks.filter(bm => bm.title != sourceLabel).length,
+               MigrationUtils._importQuantities.bookmarks,
+               "Telemetry should have items except for 'From Microsoft Edge' folders");
+  let readingListContainerLabel = MigrationUtils.getLocalizedString("importedEdgeReadingList");
+
+  for (let bookmark of seenBookmarks) {
+    if (readingListContainerLabel == bookmark.title) {
+      continue;
+    }
+    let referenceItem = readingListReferenceItems.find(item => item.Title == bookmark.title);
+    Assert.ok(referenceItem, "Should have imported what we expected");
+    Assert.equal(referenceItem.URL, bookmark.url.spec, "Should have the right URL");
+    readingListReferenceItems.splice(readingListReferenceItems.findIndex(item => item.Title == bookmark.title), 1);
+  }
+  Assert.ok(!readingListReferenceItems.length, "Should have seen all expected items.");
 });