Bug 1344759 - batch-insert bookmarks when importing from Edge, IE, Safari or Chrome, r=dao a=gchang
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 13 Mar 2017 23:46:16 +0000
changeset 395313 03017680b934a017d651d609b942d263db7fc143
parent 395312 3d9b17867a0e678181e1ea5005d90dac00cfca6b
child 395314 eb8eade361cb7a9b41dde9c2c93fb851186e695d
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao, gchang
bugs1344759
milestone54.0a2
Bug 1344759 - batch-insert bookmarks when importing from Edge, IE, Safari or Chrome, r=dao a=gchang MozReview-Commit-ID: Dhvw0Y0fm5x
browser/components/migration/ChromeProfileMigrator.js
browser/components/migration/EdgeProfileMigrator.js
browser/components/migration/MSMigrationUtils.jsm
browser/components/migration/MigrationUtils.jsm
browser/components/migration/SafariProfileMigrator.js
--- a/browser/components/migration/ChromeProfileMigrator.js
+++ b/browser/components/migration/ChromeProfileMigrator.js
@@ -76,49 +76,46 @@ function chromeTimeToDate(aTime) {
  * @return  Chrome time
  * @note    For details on Chrome time, see chromeTimeToDate.
  */
 function dateToChromeTime(aDate) {
   return (aDate * 10000 + S100NS_FROM1601TO1970) / S100NS_PER_MS;
 }
 
 /**
- * Insert bookmark items into specific folder.
+ * Converts an array of chrome bookmark objects into one our own places code
+ * understands.
  *
- * @param   parentGuid
- *          GUID of the folder where items will be inserted
  * @param   items
- *          bookmark items to be inserted
+ *          bookmark items to be inserted on this parent
  * @param   errorAccumulator
  *          function that gets called with any errors thrown so we don't drop them on the floor.
  */
-function* insertBookmarkItems(parentGuid, items, errorAccumulator) {
+function convertBookmarks(items, errorAccumulator) {
+  let itemsToInsert = [];
   for (let item of items) {
     try {
       if (item.type == "url") {
         if (item.url.trim().startsWith("chrome:")) {
           // Skip invalid chrome URIs. Creating an actual URI always reports
           // messages to the console, so we avoid doing that.
           continue;
         }
-        yield MigrationUtils.insertBookmarkWrapper({
-          parentGuid, url: item.url, title: item.name
-        });
+        itemsToInsert.push({url: item.url, title: item.name});
       } else if (item.type == "folder") {
-        let newFolderGuid = (yield MigrationUtils.insertBookmarkWrapper({
-          parentGuid, type: PlacesUtils.bookmarks.TYPE_FOLDER, title: item.name
-        })).guid;
-
-        yield insertBookmarkItems(newFolderGuid, item.children, errorAccumulator);
+        let folderItem = {type: PlacesUtils.bookmarks.TYPE_FOLDER, title: item.name};
+        folderItem.children = convertBookmarks(item.children, errorAccumulator);
+        itemsToInsert.push(folderItem);
       }
-    } catch (e) {
-      Cu.reportError(e);
-      errorAccumulator(e);
+    } catch (ex) {
+      Cu.reportError(ex);
+      errorAccumulator(ex);
     }
   }
+  return itemsToInsert;
 }
 
 function ChromeProfileMigrator() {
   let chromeUserDataFolder =
     getDataFolder(["Google", "Chrome"], ["Google", "Chrome"], ["google-chrome"]);
   this._chromeUserDataFolder = chromeUserDataFolder.exists() ?
     chromeUserDataFolder : null;
 }
@@ -255,57 +252,44 @@ function GetBookmarksResource(aProfileFo
 
   return {
     type: MigrationUtils.resourceTypes.BOOKMARKS,
 
     migrate(aCallback) {
       return Task.spawn(function* () {
         let gotErrors = false;
         let errorGatherer = function() { gotErrors = true };
-        let jsonStream = yield new Promise((resolve, reject) => {
-          let options = {
-            uri: NetUtil.newURI(bookmarksFile),
-            loadUsingSystemPrincipal: true
-          };
-          NetUtil.asyncFetch(options, (inputStream, resultCode) => {
-            if (Components.isSuccessCode(resultCode)) {
-              resolve(inputStream);
-            } else {
-              reject(new Error("Could not read Bookmarks file"));
-            }
-          });
-        });
-
         // Parse Chrome bookmark file that is JSON format
-        let bookmarkJSON = NetUtil.readInputStreamToString(
-          jsonStream, jsonStream.available(), { charset : "UTF-8" });
+        let bookmarkJSON = yield OS.File.read(bookmarksFile.path, {encoding: "UTF-8"});
         let roots = JSON.parse(bookmarkJSON).roots;
 
         // Importing bookmark bar items
         if (roots.bookmark_bar.children &&
             roots.bookmark_bar.children.length > 0) {
           // Toolbar
           let parentGuid = PlacesUtils.bookmarks.toolbarGuid;
+          let bookmarks = convertBookmarks(roots.bookmark_bar.children, errorGatherer);
           if (!MigrationUtils.isStartupMigration) {
             parentGuid =
               yield MigrationUtils.createImportedBookmarksFolder("Chrome", parentGuid);
           }
-          yield insertBookmarkItems(parentGuid, roots.bookmark_bar.children, errorGatherer);
+          yield MigrationUtils.insertManyBookmarksWrapper(bookmarks, parentGuid);
         }
 
         // Importing bookmark menu items
         if (roots.other.children &&
             roots.other.children.length > 0) {
           // Bookmark menu
           let parentGuid = PlacesUtils.bookmarks.menuGuid;
+          let bookmarks = convertBookmarks(roots.other.children, errorGatherer);
           if (!MigrationUtils.isStartupMigration) {
-            parentGuid =
-              yield MigrationUtils.createImportedBookmarksFolder("Chrome", parentGuid);
+            parentGuid
+              = yield MigrationUtils.createImportedBookmarksFolder("Chrome", parentGuid);
           }
-          yield insertBookmarkItems(parentGuid, roots.other.children, errorGatherer);
+          yield MigrationUtils.insertManyBookmarksWrapper(bookmarks, parentGuid);
         }
         if (gotErrors) {
           throw new Error("The migration included errors.");
         }
       }).then(() => aCallback(true),
               () => aCallback(false));
     }
   };
--- a/browser/components/migration/EdgeProfileMigrator.js
+++ b/browser/components/migration/EdgeProfileMigrator.js
@@ -13,16 +13,18 @@ Cu.import("resource://gre/modules/Task.j
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource:///modules/MigrationUtils.jsm"); /* globals MigratorPrototype */
 Cu.import("resource:///modules/MSMigrationUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ESEDBReader",
                                   "resource:///modules/ESEDBReader.jsm");
 
+Cu.importGlobalProperties(["URL"]);
+
 const kEdgeRegistryRoot = "SOFTWARE\\Classes\\Local Settings\\Software\\" +
   "Microsoft\\Windows\\CurrentVersion\\AppContainer\\Storage\\" +
   "microsoft.microsoftedge_8wekyb3d8bbwe\\MicrosoftEdge";
 const kEdgeDatabasePath = "AC\\MicrosoftEdge\\User\\Default\\DataStore\\Data\\";
 
 XPCOMUtils.defineLazyGetter(this, "gEdgeDatabase", function() {
   let edgeDir = MSMigrationUtils.getEdgeLocalDataFolder();
   if (!edgeDir) {
@@ -189,31 +191,28 @@ EdgeReadingListMigrator.prototype = {
     };
 
     let readingListItems = readTableFromEdgeDB("ReadingList", columnFn, filterFn);
     if (!readingListItems.length) {
       return;
     }
 
     let destFolderGuid = yield this._ensureReadingListFolder(parentGuid);
-    let exceptionThrown;
+    let bookmarks = [];
     for (let item of readingListItems) {
       let dateAdded = item.AddedDate || new Date();
-      yield MigrationUtils.insertBookmarkWrapper({
-        parentGuid: destFolderGuid, url: item.URL, title: item.Title, dateAdded
-      }).catch(ex => {
-        if (!exceptionThrown) {
-          exceptionThrown = ex;
-        }
-        Cu.reportError(ex);
-      });
+      // Avoid including broken URLs:
+      try {
+        new URL(item.URL);
+      } catch (ex) {
+        continue;
+      }
+      bookmarks.push({ url: item.URL, title: item.Title, dateAdded });
     }
-    if (exceptionThrown) {
-      throw exceptionThrown;
-    }
+    yield MigrationUtils.insertManyBookmarksWrapper(readingListItems, 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;
     }
@@ -235,83 +234,40 @@ EdgeBookmarksMigrator.prototype = {
   get exists() {
     if (!("_exists" in this)) {
       this._exists = !!this.db;
     }
     return this._exists;
   },
 
   migrate(callback) {
-    this._migrateBookmarks(PlacesUtils.bookmarks.menuGuid).then(
+    this._migrateBookmarks().then(
       () => callback(true),
       ex => {
         Cu.reportError(ex);
         callback(false);
       }
     );
   },
 
-  _migrateBookmarks: Task.async(function*(rootGuid) {
-    let {bookmarks, folderMap} = this._fetchBookmarksFromDB();
-    if (!bookmarks.length) {
-      return;
-    }
-    yield this._importBookmarks(bookmarks, folderMap, rootGuid);
-  }),
-
-  _importBookmarks: Task.async(function*(bookmarks, folderMap, rootGuid) {
-    if (!MigrationUtils.isStartupMigration) {
-      rootGuid =
-        yield MigrationUtils.createImportedBookmarksFolder("Edge", rootGuid);
-    }
-
-    let exceptionThrown;
-    for (let bookmark of bookmarks) {
-      // If this is a folder, we might have created it already to put other bookmarks in.
-      if (bookmark.IsFolder && bookmark._guid) {
-        continue;
+  _migrateBookmarks: Task.async(function*() {
+    let {toplevelBMs, toolbarBMs} = this._fetchBookmarksFromDB();
+    if (toplevelBMs.length) {
+      let parentGuid = PlacesUtils.bookmarks.menuGuid;
+      if (!MigrationUtils.isStartupMigration) {
+        parentGuid = yield MigrationUtils.createImportedBookmarksFolder("Edge", parentGuid);
       }
-
-      // If this is a folder, just create folders up to and including that folder.
-      // Otherwise, create folders until we have a parent for this bookmark.
-      // This avoids duplicating logic for the bookmarks bar.
-      let folderId = bookmark.IsFolder ? bookmark.ItemId : bookmark.ParentId;
-      let parentGuid = yield this._getGuidForFolder(folderId, folderMap, rootGuid).catch(ex => {
-        if (!exceptionThrown) {
-          exceptionThrown = ex;
-        }
-        Cu.reportError(ex);
-      });
-
-      // If this was a folder, we're done with this item
-      if (bookmark.IsFolder) {
-        continue;
+      yield MigrationUtils.insertManyBookmarksWrapper(toplevelBMs, parentGuid);
+    }
+    if (toolbarBMs.length) {
+      let parentGuid = PlacesUtils.bookmarks.toolbarGuid;
+      if (!MigrationUtils.isStartupMigration) {
+        parentGuid = yield MigrationUtils.createImportedBookmarksFolder("Edge", parentGuid);
       }
-
-      if (!parentGuid) {
-        // If we couldn't sort out a parent, fall back to importing on the root:
-        parentGuid = rootGuid;
-      }
-      let placesInfo = {
-        parentGuid,
-        url: bookmark.URL,
-        dateAdded: bookmark.DateUpdated || new Date(),
-        title: bookmark.Title,
-      };
-
-      yield MigrationUtils.insertBookmarkWrapper(placesInfo).catch(ex => {
-        if (!exceptionThrown) {
-          exceptionThrown = ex;
-        }
-        Cu.reportError(ex);
-      });
-    }
-
-    if (exceptionThrown) {
-      throw exceptionThrown;
+      yield MigrationUtils.insertManyBookmarksWrapper(toolbarBMs, parentGuid);
     }
   }),
 
   _fetchBookmarksFromDB() {
     let folderMap = new Map();
     let columns = [
       {name: "URL", type: "string"},
       {name: "Title", type: "string"},
@@ -326,54 +282,64 @@ EdgeBookmarksMigrator.prototype = {
         return false;
       }
       if (row.IsFolder) {
         folderMap.set(row.ItemId, row);
       }
       return true;
     };
     let bookmarks = readTableFromEdgeDB(this.TABLE_NAME, columns, filterFn, this.db);
-    return {bookmarks, folderMap};
-  },
-
-  _getGuidForFolder: Task.async(function*(folderId, folderMap, rootGuid) {
-    // If the folderId is not known as a folder in the folder map, we assume
-    // we just need the root
-    if (!folderMap.has(folderId)) {
-      return rootGuid;
-    }
-    let folder = folderMap.get(folderId);
-    // If the folder already has a places guid, just return that.
-    if (folder._guid) {
-      return folder._guid;
-    }
+    let toplevelBMs = [], toolbarBMs = [];
+    for (let bookmark of bookmarks) {
+      let bmToInsert;
+      // Ignore invalid URLs:
+      if (!bookmark.IsFolder) {
+        try {
+          new URL(bookmark.URL);
+        } catch (ex) {
+          Cu.reportError(`Ignoring ${bookmark.URL} when importing from Edge because of exception: ${ex}`);
+          continue;
+        }
+        bmToInsert = {
+          dateAdded: bookmark.DateUpdated || new Date(),
+          title: bookmark.Title,
+          url: bookmark.URL,
+        };
+      } else /* bookmark.IsFolder */ {
+        // Ignore the favorites bar bookmark itself.
+        if (bookmark.Title == "_Favorites_Bar_") {
+          continue;
+        }
+        if (!bookmark._childrenRef) {
+          bookmark._childrenRef = [];
+        }
+        bmToInsert = {
+          title: bookmark.Title,
+          type: PlacesUtils.bookmarks.TYPE_FOLDER,
+          dateAdded: bookmark.DateUpdated || new Date(),
+          children: bookmark._childrenRef,
+        };
+      }
 
-    // Hacks! The bookmarks bar is special:
-    if (folder.Title == "_Favorites_Bar_") {
-      let toolbarGuid = PlacesUtils.bookmarks.toolbarGuid;
-      if (!MigrationUtils.isStartupMigration) {
-        toolbarGuid =
-          yield MigrationUtils.createImportedBookmarksFolder("Edge", toolbarGuid);
+      if (!folderMap.has(bookmark.ParentId)) {
+        toplevelBMs.push(bmToInsert);
+      } else {
+        let parent = folderMap.get(bookmark.ParentId);
+        if (parent.Title == "_Favorites_Bar_") {
+          toolbarBMs.push(bmToInsert);
+          continue;
+        }
+        if (!parent._childrenRef) {
+          parent._childrenRef = [];
+        }
+        parent._childrenRef.push(bmToInsert);
       }
-      folder._guid = toolbarGuid;
-      return folder._guid;
     }
-    // Otherwise, get the right parent guid recursively:
-    let parentGuid = yield this._getGuidForFolder(folder.ParentId, folderMap, rootGuid);
-    let folderInfo = {
-      title: folder.Title,
-      type: PlacesUtils.bookmarks.TYPE_FOLDER,
-      dateAdded: folder.DateUpdated || new Date(),
-      parentGuid,
-    };
-    // and add ourselves as a kid, and return the guid we got.
-    let parentBM = yield MigrationUtils.insertBookmarkWrapper(folderInfo);
-    folder._guid = parentBM.guid;
-    return folder._guid;
-  }),
+    return {toplevelBMs, toolbarBMs};
+  },
 };
 
 function EdgeProfileMigrator() {
   this.wrappedJSObject = this;
 }
 
 EdgeProfileMigrator.prototype = Object.create(MigratorPrototype);
 
--- a/browser/components/migration/MSMigrationUtils.jsm
+++ b/browser/components/migration/MSMigrationUtils.jsm
@@ -372,75 +372,71 @@ Bookmarks.prototype = {
           yield MigrationUtils.createImportedBookmarksFolder(this.importedAppLabel, folderGuid);
       }
       yield this._migrateFolder(this._favoritesFolder, folderGuid);
     }.bind(this)).then(() => aCallback(true),
                        e => { Cu.reportError(e); aCallback(false) });
   },
 
   _migrateFolder: Task.async(function* (aSourceFolder, aDestFolderGuid) {
+    let bookmarks = yield this._getBookmarksInFolder(aSourceFolder);
+    if (bookmarks.length) {
+      yield MigrationUtils.insertManyBookmarksWrapper(bookmarks, aDestFolderGuid);
+    }
+  }),
+
+  _getBookmarksInFolder: Task.async(function* (aSourceFolder) {
     // TODO (bug 741993): the favorites order is stored in the Registry, at
     // HCU\Software\Microsoft\Windows\CurrentVersion\Explorer\MenuOrder\Favorites
     // for IE, and in a similar location for Edge.
     // Until we support it, bookmarks are imported in alphabetical order.
     let entries = aSourceFolder.directoryEntries;
-    let succeeded = true;
+    let rv = [];
     while (entries.hasMoreElements()) {
       let entry = entries.getNext().QueryInterface(Ci.nsIFile);
       try {
         // Make sure that entry.path == entry.target to not follow .lnk folder
         // shortcuts which could lead to infinite cycles.
         // Don't use isSymlink(), since it would throw for invalid
         // lnk files pointing to URLs or to unresolvable paths.
         if (entry.path == entry.target && entry.isDirectory()) {
-          let folderGuid;
-          if (entry.leafName == this._toolbarFolderName &&
-              entry.parent.equals(this._favoritesFolder)) {
+          let isBookmarksFolder = entry.leafName == this._toolbarFolderName &&
+                                  entry.parent.equals(this._favoritesFolder);
+          if (isBookmarksFolder && entry.isReadable()) {
             // Import to the bookmarks toolbar.
-            folderGuid = PlacesUtils.bookmarks.toolbarGuid;
+            let folderGuid = PlacesUtils.bookmarks.toolbarGuid;
             if (!MigrationUtils.isStartupMigration) {
               folderGuid =
                 yield MigrationUtils.createImportedBookmarksFolder(this.importedAppLabel, folderGuid);
             }
-          } else {
-            // Import to a new folder.
-            folderGuid = (yield MigrationUtils.insertBookmarkWrapper({
+            yield this._migrateFolder(entry, folderGuid);
+          } else if (entry.isReadable()) {
+            let childBookmarks = yield this._getBookmarksInFolder(entry);
+            rv.push({
               type: PlacesUtils.bookmarks.TYPE_FOLDER,
-              parentGuid: aDestFolderGuid,
-              title: entry.leafName
-            })).guid;
-          }
-
-          if (entry.isReadable()) {
-            // Recursively import the folder.
-            yield this._migrateFolder(entry, folderGuid);
+              title: entry.leafName,
+              children: childBookmarks,
+            });
           }
         } else {
           // Strip the .url extension, to both check this is a valid link file,
           // and get the associated title.
           let matches = entry.leafName.match(/(.+)\.url$/i);
           if (matches) {
             let fileHandler = Cc["@mozilla.org/network/protocol;1?name=file"].
                               getService(Ci.nsIFileProtocolHandler);
             let uri = fileHandler.readURLFile(entry);
-            let title = matches[1];
-
-            yield MigrationUtils.insertBookmarkWrapper({
-              parentGuid: aDestFolderGuid, url: uri, title
-            });
+            rv.push({url: uri, title: matches[1]});
           }
         }
       } catch (ex) {
         Components.utils.reportError("Unable to import " + this.importedAppLabel + " favorite (" + entry.leafName + "): " + ex);
-        succeeded = false;
       }
     }
-    if (!succeeded) {
-      throw new Error("Failed to import all bookmarks correctly.");
-    }
+    return rv;
   }),
 
 };
 
 function Cookies(migrationType) {
   this._migrationType = migrationType;
 }
 
--- a/browser/components/migration/MigrationUtils.jsm
+++ b/browser/components/migration/MigrationUtils.jsm
@@ -986,16 +986,30 @@ this.MigrationUtils = Object.freeze({
       let {guid, lastModified, type} = bm;
       gUndoData.get("bookmarks").push({
         parentGuid, guid, lastModified, type
       });
       return bm;
     });
   },
 
+  insertManyBookmarksWrapper(bookmarks, parent) {
+    let insertionPromise = PlacesUtils.bookmarks.insertTree({guid: parent, children: bookmarks});
+    return insertionPromise.then(insertedItems => {
+      this._importQuantities.bookmarks += insertedItems.length;
+      if (gKeepUndoData) {
+        let bmData = gUndoData.get("bookmarks");
+        for (let bm of insertedItems) {
+          let {parentGuid, guid, lastModified, type} = bm;
+          bmData.push({parentGuid, guid, lastModified, type});
+        }
+      }
+    }, ex => Cu.reportError(ex));
+  },
+
   insertVisitsWrapper(places, options) {
     this._importQuantities.history += places.length;
     if (gKeepUndoData) {
       this._updateHistoryUndo(places);
     }
     return PlacesUtils.asyncHistory.updatePlaces(places, options, true);
   },
 
--- a/browser/components/migration/SafariProfileMigrator.js
+++ b/browser/components/migration/SafariProfileMigrator.js
@@ -22,16 +22,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/PropertyListUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormHistory",
                                   "resource://gre/modules/FormHistory.jsm");
 
+Cu.importGlobalProperties(["URL"]);
+
 function Bookmarks(aBookmarksFile) {
   this._file = aBookmarksFile;
 }
 Bookmarks.prototype = {
   type: MigrationUtils.resourceTypes.BOOKMARKS,
 
   migrate: function B_migrate(aCallback) {
     return Task.spawn(function* () {
@@ -142,43 +144,48 @@ Bookmarks.prototype = {
     if (folderGuid == -1)
       throw new Error("Invalid folder GUID");
 
     yield this._migrateEntries(entriesFiltered, folderGuid);
   }),
 
   // migrate the given array of safari bookmarks to the given places
   // folder.
-  _migrateEntries: Task.async(function* (entries, parentGuid) {
-    for (let entry of entries) {
+  _migrateEntries(entries, parentGuid) {
+    let convertedEntries = this._convertEntries(entries);
+    return MigrationUtils.insertManyBookmarksWrapper(convertedEntries, parentGuid);
+  },
+
+  _convertEntries(entries) {
+    return entries.map(function(entry) {
       let type = entry.get("WebBookmarkType");
       if (type == "WebBookmarkTypeList" && entry.has("Children")) {
-        let title = entry.get("Title");
-        let newFolderGuid = (yield MigrationUtils.insertBookmarkWrapper({
-          parentGuid, type: PlacesUtils.bookmarks.TYPE_FOLDER, title
-        })).guid;
-
-        // Empty folders may not have a children array.
-        if (entry.has("Children"))
-          yield this._migrateEntries(entry.get("Children"), newFolderGuid, false);
-      } else if (type == "WebBookmarkTypeLeaf" && entry.has("URLString")) {
+        return {
+          title: entry.get("Title"),
+          type: PlacesUtils.bookmarks.TYPE_FOLDER,
+          children: this._convertEntries(entry.get("Children")),
+        };
+      }
+      if (type == "WebBookmarkTypeLeaf" && entry.has("URLString")) {
+        // Check we understand this URL before adding it:
+        let url = entry.get("URLString");
+        try {
+          new URL(url);
+        } catch (ex) {
+          Cu.reportError(`Ignoring ${url} when importing from Safari because of exception: ${ex}`);
+          return null;
+        }
         let title;
         if (entry.has("URIDictionary"))
           title = entry.get("URIDictionary").get("title");
-
-        try {
-          yield MigrationUtils.insertBookmarkWrapper({
-            parentGuid, url: entry.get("URLString"), title
-          });
-        } catch (ex) {
-          Cu.reportError("Invalid Safari bookmark: " + ex);
-        }
+        return { url, title };
       }
-    }
-  })
+      return null;
+    }).filter(e => !!e);
+  },
 };
 
 function History(aHistoryFile) {
   this._file = aHistoryFile;
 }
 History.prototype = {
   type: MigrationUtils.resourceTypes.HISTORY,