Bug 1095427 - Convert HTML bookmarks import code to the new async Bookmarks.jsm API. r=mak
authorJosh Aas <jaas@kflag.net>
Mon, 02 Oct 2017 16:53:00 +0100
changeset 385871 6925cf5fb0c2fca5dc22ff55392ab5e3897c92f7
parent 385870 a1b4287a3427641de921fd60fbb35b81448b79d8
child 385872 c5695ec7ac409ad20c0076197cdeb3e792f26ef1
push id32672
push userarchaeopteryx@coole-files.de
push dateFri, 13 Oct 2017 09:00:05 +0000
treeherdermozilla-central@3efcb26e5f37 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1095427
milestone58.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 1095427 - Convert HTML bookmarks import code to the new async Bookmarks.jsm API. r=mak Original patch by Josh, completed by Standard8. MozReview-Commit-ID: Baun3n69n1b
toolkit/components/places/BookmarkHTMLUtils.jsm
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/unit/bookmarks.preplaces.html
toolkit/components/places/tests/unit/test_384370.js
toolkit/components/places/tests/unit/test_bookmarks_html.js
--- a/toolkit/components/places/BookmarkHTMLUtils.jsm
+++ b/toolkit/components/places/BookmarkHTMLUtils.jsm
@@ -133,69 +133,65 @@ this.BookmarkHTMLUtils = Object.freeze({
    *        file to be loaded.
    * @param aInitialImport
    *        Whether this is the initial import executed on a new profile.
    *
    * @return {Promise}
    * @resolves When the new bookmarks have been created.
    * @rejects JavaScript exception.
    */
-  importFromURL: function BHU_importFromURL(aSpec, aInitialImport) {
-    return (async function() {
-      notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN, aInitialImport);
-      try {
-        let importer = new BookmarkImporter(aInitialImport);
-        await importer.importFromURL(aSpec);
+  async importFromURL(aSpec, aInitialImport) {
+    notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN, aInitialImport);
+    try {
+      let importer = new BookmarkImporter(aInitialImport);
+      await importer.importFromURL(aSpec);
 
-        notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aInitialImport);
-      } catch (ex) {
-        Cu.reportError("Failed to import bookmarks from " + aSpec + ": " + ex);
-        notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aInitialImport);
-        throw ex;
-      }
-    })();
+      notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aInitialImport);
+    } catch (ex) {
+      Cu.reportError("Failed to import bookmarks from " + aSpec + ": " + ex);
+      notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aInitialImport);
+      throw ex;
+    }
   },
 
   /**
    * Loads the current bookmarks hierarchy from a "bookmarks.html" file.
    *
    * @param aFilePath
    *        OS.File path string of the "bookmarks.html" file to be loaded.
    * @param aInitialImport
    *        Whether this is the initial import executed on a new profile.
    *
    * @return {Promise}
    * @resolves When the new bookmarks have been created.
    * @rejects JavaScript exception.
    * @deprecated passing an nsIFile is deprecated
    */
-  importFromFile: function BHU_importFromFile(aFilePath, aInitialImport) {
+  async importFromFile(aFilePath, aInitialImport) {
     if (aFilePath instanceof Ci.nsIFile) {
       Deprecated.warning("Passing an nsIFile to BookmarksJSONUtils.importFromFile " +
                          "is deprecated. Please use an OS.File path string instead.",
                          "https://developer.mozilla.org/docs/JavaScript_OS.File");
       aFilePath = aFilePath.path;
     }
 
-    return (async function() {
-      notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN, aInitialImport);
-      try {
-        if (!(await OS.File.exists(aFilePath))) {
-          throw new Error("Cannot import from nonexisting html file: " + aFilePath);
-        }
-        let importer = new BookmarkImporter(aInitialImport);
-        await importer.importFromURL(OS.Path.toFileURI(aFilePath));
+    notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN, aInitialImport);
+    try {
+      if (!(await OS.File.exists(aFilePath))) {
+        throw new Error("Cannot import from nonexisting html file: " + aFilePath);
+      }
+      let importer = new BookmarkImporter(aInitialImport);
+      await importer.importFromURL(OS.Path.toFileURI(aFilePath));
 
-        notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aInitialImport);
-      } catch (ex) {
-        Cu.reportError("Failed to import bookmarks from " + aFilePath + ": " + ex);
-        notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aInitialImport);
-        throw ex;
-      }
-    })();
+      notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aInitialImport);
+    } catch (ex) {
+      Cu.reportError("Failed to import bookmarks from " + aFilePath + ": " + ex);
+      notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aInitialImport);
+      throw ex;
+    }
   },
 
   /**
    * Saves the current bookmarks hierarchy to a "bookmarks.html" file.
    *
    * @param aFilePath
    *        OS.File path string for the "bookmarks.html" file to be created.
    *
@@ -234,18 +230,18 @@ this.BookmarkHTMLUtils = Object.freeze({
   get defaultPath() {
     try {
       return Services.prefs.getCharPref("browser.bookmarks.file");
     } catch (ex) {}
     return OS.Path.join(OS.Constants.Path.profileDir, "bookmarks.html")
   }
 });
 
-function Frame(aFrameId) {
-  this.containerId = aFrameId;
+function Frame(aFolder) {
+  this.folder = aFolder;
 
   /**
    * How many <dl>s have been nested. Each frame/container should start
    * with a heading, and is then followed by a <dl>, <ul>, or <menu>. When
    * that list is complete, then it is the end of this container and we need
    * to pop back up one level for new items. If we never get an open tag for
    * one of these things, we should assume that the container is empty and
    * that things we find should be siblings of it. Normally, these <dl>s won't
@@ -285,49 +281,62 @@ function Frame(aFrameId) {
   this.inDescription = false;
 
   /**
    * contains the URL of the previous bookmark created. This is used so that
    * when we encounter a <dd>, we know what bookmark to associate the text with.
    * This is cleared whenever we hit a <h3>, so that we know NOT to save this
    * with a bookmark, but to keep it until
    */
-  this.previousLink = null; // nsIURI
+  this.previousLink = null;
 
   /**
    * contains the URL of the previous livemark, so that when the link ends,
    * and the livemark title is known, we can create it.
    */
-  this.previousFeed = null; // nsIURI
+  this.previousFeed = null;
 
   /**
-   * Contains the id of an imported, or newly created bookmark.
+   * Contains a reference to the last created bookmark or folder object.
    */
-  this.previousId = 0;
+  this.previousItem = null;
 
   /**
    * Contains the date-added and last-modified-date of an imported item.
    * Used to override the values set by insertBookmark, createFolder, etc.
    */
-  this.previousDateAdded = 0;
-  this.previousLastModifiedDate = 0;
+  this.previousDateAdded = null;
+  this.previousLastModifiedDate = null;
 }
 
 function BookmarkImporter(aInitialImport) {
   this._isImportDefaults = aInitialImport;
   // The bookmark change source, used to determine the sync status and change
   // counter.
   this._source = aInitialImport ? PlacesUtils.bookmarks.SOURCE_IMPORT_REPLACE :
                                   PlacesUtils.bookmarks.SOURCE_IMPORT;
+
+  // This root is where we construct the bookmarks tree into, following the format
+  // of the imported file.
+  // If we're doing an initial import, the non-menu roots will be created as
+  // children of this root, so in _getBookmarkTrees we'll split them out.
+  // If we're not doing an initial import, everything gets imported under the
+  // bookmark menu folder, so there won't be any need for _getBookmarkTrees to
+  // do separation.
+  this._bookmarkTree = {
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    guid: PlacesUtils.bookmarks.menuGuid,
+    children: []
+  };
+
   this._frames = [];
-  this._frames.push(new Frame(PlacesUtils.bookmarksMenuFolderId));
+  this._frames.push(new Frame(this._bookmarkTree));
 }
 
 BookmarkImporter.prototype = {
-
   _safeTrim: function safeTrim(aStr) {
     return aStr ? aStr.trim() : aStr;
   },
 
   get _curFrame() {
     return this._frames[this._frames.length - 1];
   },
 
@@ -335,99 +344,86 @@ BookmarkImporter.prototype = {
     return this._frames[this._frames.length - 2];
   },
 
   /**
    * This is called when there is a new folder found. The folder takes the
    * name from the previous frame's heading.
    */
   _newFrame: function newFrame() {
-    let containerId = -1;
     let frame = this._curFrame;
     let containerTitle = frame.previousText;
     frame.previousText = "";
     let containerType = frame.lastContainerType;
 
+    let folder = {
+      children: [],
+      type: PlacesUtils.bookmarks.TYPE_FOLDER
+    };
+
     switch (containerType) {
       case Container_Normal:
-        // append a new folder
-        containerId =
-          PlacesUtils.bookmarks.createFolder(frame.containerId,
-                                             containerTitle,
-                                             PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                             /* aGuid */ null, this._source);
+        // This can only be a sub-folder so no need to set a guid here.
+        folder.title = containerTitle;
         break;
       case Container_Places:
-        containerId = PlacesUtils.placesRootId;
+        folder.guid = PlacesUtils.bookmarks.rootGuid;
         break;
       case Container_Menu:
-        containerId = PlacesUtils.bookmarksMenuFolderId;
+        folder.guid = PlacesUtils.bookmarks.menuGuid;
         break;
       case Container_Unfiled:
-        containerId = PlacesUtils.unfiledBookmarksFolderId;
+        folder.guid = PlacesUtils.bookmarks.unfiledGuid;
         break;
       case Container_Toolbar:
-        containerId = PlacesUtils.toolbarFolderId;
+        folder.guid = PlacesUtils.bookmarks.toolbarGuid;
         break;
       default:
         // NOT REACHED
-        throw new Error("Unreached");
+        throw new Error("Unknown bookmark container type!");
+    }
+
+    frame.folder.children.push(folder);
+
+    if (frame.previousDateAdded != null) {
+      folder.dateAdded = frame.previousDateAdded;
+      frame.previousDateAdded = null;
     }
 
-    if (frame.previousDateAdded > 0) {
-      try {
-        PlacesUtils.bookmarks.setItemDateAdded(containerId, frame.previousDateAdded, this._source);
-      } catch (e) {
-      }
-      frame.previousDateAdded = 0;
-    }
-    if (frame.previousLastModifiedDate > 0) {
-      try {
-        PlacesUtils.bookmarks.setItemLastModified(containerId, frame.previousLastModifiedDate, this._source);
-      } catch (e) {
-      }
-      // don't clear last-modified, in case there's a description
+    if (frame.previousLastModifiedDate != null) {
+      folder.lastModified = frame.previousLastModifiedDate;
+      frame.previousLastModifiedDate = null;
     }
 
-    frame.previousId = containerId;
+    if (!folder.hasOwnProperty("dateAdded") &&
+         folder.hasOwnProperty("lastModified")) {
+      folder.dateAdded = folder.lastModified;
+    }
 
-    this._frames.push(new Frame(containerId));
+    frame.previousItem = folder;
+
+    this._frames.push(new Frame(folder));
   },
 
   /**
    * Handles <hr> as a separator.
    *
    * @note Separators may have a title in old html files, though Places dropped
    *       support for them.
    *       We also don't import ADD_DATE or LAST_MODIFIED for separators because
    *       pre-Places bookmarks did not support them.
    */
   _handleSeparator: function handleSeparator(aElt) {
     let frame = this._curFrame;
-    try {
-      frame.previousId =
-        PlacesUtils.bookmarks.insertSeparator(frame.containerId,
-                                              PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                              /* aGuid */ null,
-                                              this._source);
-    } catch (e) {}
-  },
 
-  /**
-   * Handles <H1>. We check for the attribute PLACES_ROOT and reset the
-   * container id if it's found. Otherwise, the default bookmark menu
-   * root is assumed and imported things will go into the bookmarks menu.
-   */
-  _handleHead1Begin: function handleHead1Begin(aElt) {
-    if (this._frames.length > 1) {
-      return;
-    }
-    if (aElt.hasAttribute("places_root")) {
-      this._curFrame.containerId = PlacesUtils.placesRootId;
-    }
+    let separator = {
+      type: PlacesUtils.bookmarks.TYPE_SEPARATOR
+    };
+    frame.folder.children.push(separator);
+    frame.previousItem = separator;
   },
 
   /**
    * Called for h2,h3,h4,h5,h6. This just stores the correct information in
    * the current frame; the actual new frame corresponding to the container
    * associated with the heading will be created when the tag has been closed
    * and we know the title (we don't know to create a new folder or to merge
    * with an existing one until we have the title).
@@ -498,22 +494,19 @@ BookmarkImporter.prototype = {
   /*
    * Handles "<a" tags by creating a new bookmark. The title of the bookmark
    * will be the text content, which will be stuffed in previousText for us
    * and which will be saved by handleLinkEnd
    */
   _handleLinkBegin: function handleLinkBegin(aElt) {
     let frame = this._curFrame;
 
-    // Make sure that the feed URIs from previous frames are emptied.
     frame.previousFeed = null;
-    // Make sure that the bookmark id from previous frames are emptied.
-    frame.previousId = 0;
-    // mPreviousText will hold link text, clear it.
-    frame.previousText = "";
+    frame.previousItem = null;
+    frame.previousText = "";  // Will hold link text, clear it.
 
     // Get the attributes we care about.
     let href = this._safeTrim(aElt.getAttribute("href"));
     let feedUrl = this._safeTrim(aElt.getAttribute("feedurl"));
     let icon = this._safeTrim(aElt.getAttribute("icon"));
     let iconUri = this._safeTrim(aElt.getAttribute("icon_uri"));
     let lastCharset = this._safeTrim(aElt.getAttribute("last_charset"));
     let keyword = this._safeTrim(aElt.getAttribute("shortcuturl"));
@@ -521,149 +514,126 @@ BookmarkImporter.prototype = {
     let webPanel = this._safeTrim(aElt.getAttribute("web_panel"));
     let dateAdded = this._safeTrim(aElt.getAttribute("add_date"));
     let lastModified = this._safeTrim(aElt.getAttribute("last_modified"));
     let tags = this._safeTrim(aElt.getAttribute("tags"));
 
     // For feeds, get the feed URL.  If it is invalid, mPreviousFeed will be
     // NULL and we'll create it as a normal bookmark.
     if (feedUrl) {
-      frame.previousFeed = NetUtil.newURI(feedUrl);
+      frame.previousFeed = feedUrl;
     }
 
     // Ignore <a> tags that have no href.
     if (href) {
       // Save the address if it's valid.  Note that we ignore errors if this is a
       // feed since href is optional for them.
       try {
-        frame.previousLink = NetUtil.newURI(href);
+        frame.previousLink = Services.io.newURI(href).spec;
       } catch (e) {
         if (!frame.previousFeed) {
           frame.previousLink = null;
           return;
         }
       }
     } else {
       frame.previousLink = null;
       // The exception is for feeds, where the href is an optional component
       // indicating the source web site.
       if (!frame.previousFeed) {
         return;
       }
     }
 
+    let bookmark = {};
+
+    // Only set the url for bookmarks, not for livemarks.
+    if (frame.previousLink && !frame.previousFeed) {
+      bookmark.url = frame.previousLink;
+    }
+
+    if (dateAdded) {
+      bookmark.dateAdded = this._convertImportedDateToInternalDate(dateAdded);
+    }
     // Save bookmark's last modified date.
     if (lastModified) {
-      frame.previousLastModifiedDate =
-        this._convertImportedDateToInternalDate(lastModified);
+      bookmark.lastModified = this._convertImportedDateToInternalDate(lastModified);
     }
 
-    // If this is a live bookmark, we will handle it in HandleLinkEnd(), so we
-    // can skip bookmark creation.
+    if (!dateAdded && lastModified) {
+      bookmark.dateAdded = bookmark.lastModified;
+    }
+
     if (frame.previousFeed) {
+      // This is a livemark, we've done all we need to do here, so finish early.
+      frame.folder.children.push(bookmark);
+      frame.previousItem = bookmark;
       return;
     }
 
-    // Create the bookmark.  The title is unknown for now, we will set it later.
-    try {
-      frame.previousId =
-        PlacesUtils.bookmarks.insertBookmark(frame.containerId,
-                                             frame.previousLink,
-                                             PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                             /* aTitle */ "",
-                                             /* aGuid */ null,
-                                             this._source);
-    } catch (e) {
-      return;
-    }
+    if (tags) {
+      bookmark.tags = tags.split(",").filter(aTag => aTag.length > 0 &&
+        aTag.length <= Ci.nsITaggingService.MAX_TAG_LENGTH);
 
-    // Set the date added value, if we have it.
-    if (dateAdded) {
-      try {
-        PlacesUtils.bookmarks.setItemDateAdded(frame.previousId,
-          this._convertImportedDateToInternalDate(dateAdded), this._source);
-      } catch (e) {
-      }
-    }
-
-    // Adds tags to the URI, if there are any.
-    if (tags) {
-      try {
-        let tagsArray = tags.split(",");
-        PlacesUtils.tagging.tagURI(frame.previousLink, tagsArray, this._source);
-      } catch (e) {
+      // If we end up with none, then delete the property completely.
+      if (!bookmark.tags.length) {
+        delete bookmark.tags;
       }
     }
 
-    // Save the favicon.
-    if (icon || iconUri) {
-      let iconUriObject;
-      try {
-        iconUriObject = NetUtil.newURI(iconUri);
-      } catch (e) {
+    if (webPanel && webPanel.toLowerCase() == "true") {
+      if (!bookmark.hasOwnProperty("annos")) {
+        bookmark.annos = [];
       }
-      if (icon || iconUriObject) {
-        try {
-          this._setFaviconForURI(frame.previousLink, iconUriObject, icon);
-        } catch (e) {
-        }
-      }
+      bookmark.annos.push({ "name": LOAD_IN_SIDEBAR_ANNO,
+                            "flags": 0,
+                            "expires": 4,
+                            "value": 1
+                          });
+    }
+
+    if (lastCharset) {
+      bookmark.charset = lastCharset;
     }
 
-    // Save the keyword.
     if (keyword) {
-      let kwPromise = PlacesUtils.keywords.insert({ keyword,
-                                                    url: frame.previousLink.spec,
-                                                    postData,
-                                                    source: this._source });
-      this._importPromises.push(kwPromise);
+      bookmark.keyword = keyword;
+    }
+
+    if (postData) {
+      bookmark.postData = postData;
     }
 
-    // Set load-in-sidebar annotation for the bookmark.
-    if (webPanel && webPanel.toLowerCase() == "true") {
-      try {
-        PlacesUtils.annotations.setItemAnnotation(frame.previousId,
-                                                  LOAD_IN_SIDEBAR_ANNO,
-                                                  1,
-                                                  0,
-                                                  PlacesUtils.annotations.EXPIRE_NEVER,
-                                                  this._source);
-      } catch (e) {
-      }
+    if (icon) {
+      bookmark.icon = icon;
     }
 
-    // Import last charset.
-    if (lastCharset) {
-      let chPromise = PlacesUtils.setCharsetForURI(frame.previousLink, lastCharset, this._source);
-      this._importPromises.push(chPromise);
+    if (iconUri) {
+      bookmark.iconUri = iconUri;
     }
+
+    // Add bookmark to the tree.
+    frame.folder.children.push(bookmark);
+    frame.previousItem = bookmark;
   },
 
   _handleContainerBegin: function handleContainerBegin() {
     this._curFrame.containerNesting++;
   },
 
   /**
    * Our "indent" count has decreased, and when we hit 0 that means that this
    * container is complete and we need to pop back to the outer frame. Never
    * pop the toplevel frame
    */
   _handleContainerEnd: function handleContainerEnd() {
     let frame = this._curFrame;
     if (frame.containerNesting > 0)
       frame.containerNesting --;
     if (this._frames.length > 1 && frame.containerNesting == 0) {
-      // we also need to re-set the imported last-modified date here. Otherwise
-      // the addition of items will override the imported field.
-      let prevFrame = this._previousFrame;
-      if (prevFrame.previousLastModifiedDate > 0) {
-        PlacesUtils.bookmarks.setItemLastModified(frame.containerId,
-                                                  prevFrame.previousLastModifiedDate,
-                                                  this._source);
-      }
       this._frames.pop();
     }
   },
 
   /**
    * Creates the new frame for this heading now that we know the name of the
    * container (tokens since the heading open tag will have been placed in
    * previousText).
@@ -674,63 +644,48 @@ BookmarkImporter.prototype = {
 
   /**
    * Saves the title for the given bookmark.
    */
   _handleLinkEnd: function handleLinkEnd() {
     let frame = this._curFrame;
     frame.previousText = frame.previousText.trim();
 
-    try {
+    if (frame.previousItem != null) {
       if (frame.previousFeed) {
-        // The is a live bookmark.  We create it here since in HandleLinkBegin we
-        // don't know the title.
-        let lmPromise = PlacesUtils.livemarks.addLivemark({
-          "title": frame.previousText,
-          "parentId": frame.containerId,
-          "index": PlacesUtils.bookmarks.DEFAULT_INDEX,
-          "feedURI": frame.previousFeed,
-          "siteURI": frame.previousLink,
-          "source": this._source,
+        if (!frame.previousItem.hasOwnProperty("annos")) {
+          frame.previousItem.annos = [];
+        }
+        frame.previousItem.type = PlacesUtils.bookmarks.TYPE_FOLDER;
+        frame.previousItem.annos.push({
+          "name": PlacesUtils.LMANNO_FEEDURI,
+          "flags": 0,
+          "expires": 4,
+          "value": frame.previousFeed
         });
-        this._importPromises.push(lmPromise);
-      } else if (frame.previousLink) {
-        // This is a common bookmark.
-        PlacesUtils.bookmarks.setItemTitle(frame.previousId,
-                                           frame.previousText,
-                                           this._source);
+        if (frame.previousLink) {
+          frame.previousItem.annos.push({
+            "name": PlacesUtils.LMANNO_SITEURI,
+            "flags": 0,
+            "expires": 4,
+            "value": frame.previousLink
+          });
+        }
       }
-    } catch (e) {
-    }
-
-
-    // Set last modified date as the last change.
-    if (frame.previousId > 0 && frame.previousLastModifiedDate > 0) {
-      try {
-        PlacesUtils.bookmarks.setItemLastModified(frame.previousId,
-                                                  frame.previousLastModifiedDate,
-                                                  this._source);
-      } catch (e) {
-      }
-      // Note: don't clear previousLastModifiedDate, because if this item has a
-      // description, we'll need to set it again.
+      frame.previousItem.title = frame.previousText;
     }
 
     frame.previousText = "";
-
   },
 
   _openContainer: function openContainer(aElt) {
     if (aElt.namespaceURI != "http://www.w3.org/1999/xhtml") {
       return;
     }
     switch (aElt.localName) {
-      case "h1":
-        this._handleHead1Begin(aElt);
-        break;
       case "h2":
       case "h3":
       case "h4":
       case "h5":
       case "h6":
         this._handleHeadBegin(aElt);
         break;
       case "a":
@@ -755,55 +710,27 @@ BookmarkImporter.prototype = {
 
     // see the comment for the definition of inDescription. Basically, we commit
     // any text in previousText to the description of the node/folder if there
     // is any.
     if (frame.inDescription) {
       // NOTE ES5 trim trims more than the previous C++ trim.
       frame.previousText = frame.previousText.trim(); // important
       if (frame.previousText) {
-
-        let itemId = !frame.previousLink ? frame.containerId
-                                         : frame.previousId;
-
-        try {
-          if (!PlacesUtils.annotations.itemHasAnnotation(itemId, DESCRIPTION_ANNO)) {
-            PlacesUtils.annotations.setItemAnnotation(itemId,
-                                                      DESCRIPTION_ANNO,
-                                                      frame.previousText,
-                                                      0,
-                                                      PlacesUtils.annotations.EXPIRE_NEVER,
-                                                      this._source);
-          }
-        } catch (e) {
+        let item = frame.previousLink ? frame.previousItem : frame.folder;
+        if (!item.hasOwnProperty("annos")) {
+          item.annos = [];
         }
+        item.annos.push({
+          "name": DESCRIPTION_ANNO,
+          "flags": 0,
+          "expires": 4,
+          "value": frame.previousText
+        });
         frame.previousText = "";
-
-        // Set last-modified a 2nd time for all items with descriptions
-        // we need to set last-modified as the *last* step in processing
-        // any item type in the bookmarks.html file, so that we do
-        // not overwrite the imported value. for items without descriptions,
-        // setting this value after setting the item title is that
-        // last point at which we can save this value before it gets reset.
-        // for items with descriptions, it must set after that point.
-        // however, at the point at which we set the title, there's no way
-        // to determine if there will be a description following,
-        // so we need to set the last-modified-date at both places.
-
-        let lastModified;
-        if (!frame.previousLink) {
-          lastModified = this._previousFrame.previousLastModifiedDate;
-        } else {
-          lastModified = frame.previousLastModifiedDate;
-        }
-
-        if (itemId > 0 && lastModified > 0) {
-          PlacesUtils.bookmarks.setItemLastModified(itemId, lastModified,
-                                                    this._source);
-        }
       }
       frame.inDescription = false;
     }
 
     if (aElt.namespaceURI != "http://www.w3.org/1999/xhtml") {
       return;
     }
     switch (aElt.localName) {
@@ -888,36 +815,34 @@ BookmarkImporter.prototype = {
     PlacesUtils.favicons.replaceFaviconDataFromDataURL(faviconURI, aData, 0,
                                                        Services.scriptSecurityManager.getSystemPrincipal());
     PlacesUtils.favicons.setAndFetchFaviconForPage(aPageURI, faviconURI, false,
                                                    PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, null,
                                                    Services.scriptSecurityManager.getSystemPrincipal());
   },
 
   /**
-   * Converts a string date in seconds to an int date in microseconds
+   * Converts a string date in seconds to a date object
    */
   _convertImportedDateToInternalDate: function convertImportedDateToInternalDate(aDate) {
-    if (aDate && !isNaN(aDate)) {
-      return parseInt(aDate) * 1000000; // in bookmarks.html this value is in seconds, not microseconds
+    try {
+      if (aDate && !isNaN(aDate)) {
+        return new Date(parseInt(aDate) * 1000); // in bookmarks.html this value is in seconds
+      }
+    } catch (ex) {
+      // Do nothing.
     }
-    return Date.now();
+    return new Date();
   },
 
-  runBatched: function runBatched(aDoc) {
+  _walkTreeForImport(aDoc) {
     if (!aDoc) {
       return;
     }
 
-    if (this._isImportDefaults) {
-      PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.bookmarksMenuFolderId, this._source);
-      PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.toolbarFolderId, this._source);
-      PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId, this._source);
-    }
-
     let current = aDoc;
     let next;
     for (;;) {
       switch (current.nodeType) {
         case Ci.nsIDOMNode.ELEMENT_NODE:
           this._openContainer(current);
           break;
         case Ci.nsIDOMNode.TEXT_NODE:
@@ -939,48 +864,81 @@ BookmarkImporter.prototype = {
           current = next;
           break;
         }
         current = current.parentNode;
       }
     }
   },
 
-  _walkTreeForImport: function walkTreeForImport(aDoc) {
-    PlacesUtils.bookmarks.runInBatchMode(this, aDoc);
+  /**
+   * Returns the bookmark tree(s) from the importer. These are suitable for
+   * passing to PlacesUtils.bookmarks.insertTree().
+   *
+   * @returns {Array} An array of bookmark trees.
+   */
+  _getBookmarkTrees() {
+    // If we're not importing defaults, then everything gets imported under the
+    // Bookmarks menu.
+    if (!this._isImportDefaults) {
+      return [this._bookmarkTree];
+    }
+
+    // If we are importing defaults, we need to separate out the top-level
+    // default folders into separate items, for the caller to pass into insertTree.
+    let bookmarkTrees = [this._bookmarkTree];
+
+    // The children of this "root" element will contain normal children of the
+    // bookmark menu as well as the places roots. Hence, we need to filter out
+    // the separate roots, but keep the children that are relevant to the
+    // bookmark menu.
+    this._bookmarkTree.children = this._bookmarkTree.children.filter(child => {
+      if (child.guid && PlacesUtils.bookmarks.userContentRoots.includes(child.guid)) {
+        bookmarkTrees.push(child);
+        return false;
+      }
+      return true;
+    });
+
+    return bookmarkTrees;
   },
 
+  /**
+   * Imports the bookmarks from the importer into the places database.
+   *
+   * @param {BookmarkImporter} importer The importer from which to get the
+   *                                    bookmark information.
+   */
+  async _importBookmarks() {
+    if (this._isImportDefaults) {
+      await PlacesUtils.bookmarks.eraseEverything();
+    }
+
+    let bookmarksTrees = this._getBookmarkTrees();
+    for (let tree of bookmarksTrees) {
+      if (!tree.children.length) {
+        continue;
+      }
+
+      // Give the tree the source.
+      tree.source = this._source;
+      await PlacesUtils.bookmarks.insertTree(tree);
+      insertFaviconsForTree(tree);
+    }
+  },
+
+  /**
+   * Imports data into the places database from the supplied url.
+   *
+   * @param {String} href The url to import data from.
+   */
   async importFromURL(href) {
-    this._importPromises = [];
-    await new Promise((resolve, reject) => {
-      let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
-                  .createInstance(Ci.nsIXMLHttpRequest);
-      xhr.onload = () => {
-        try {
-          this._walkTreeForImport(xhr.responseXML);
-          resolve();
-        } catch (e) {
-          reject(e);
-        }
-      };
-      xhr.onabort = xhr.onerror = xhr.ontimeout = () => {
-        reject(new Error("xmlhttprequest failed"));
-      };
-      xhr.open("GET", href);
-      xhr.responseType = "document";
-      xhr.overrideMimeType("text/html");
-      xhr.send();
-    });
-    // TODO (bug 1095427) once converted to the new bookmarks API, methods will
-    // yield, so this hack should not be needed anymore.
-    try {
-      await Promise.all(this._importPromises);
-    } finally {
-      delete this._importPromises;
-    }
+    let data = await fetchData(href);
+    this._walkTreeForImport(data);
+    await this._importBookmarks();
   },
 };
 
 function BookmarkExporter(aBookmarksTree) {
   // Create a map of the roots.
   let rootsMap = new Map();
   for (let child of aBookmarksTree.children) {
     if (child.root)
@@ -1179,8 +1137,89 @@ BookmarkExporter.prototype = {
 
   _writeDescription(aItem, aIndent) {
     let descriptionAnno = aItem.annos &&
                           aItem.annos.find(anno => anno.name == DESCRIPTION_ANNO);
     if (descriptionAnno)
       this._writeLine(aIndent + "<DD>" + escapeHtmlEntities(descriptionAnno.value));
   }
 };
+
+/**
+ * Handles inserting favicons into the database for a bookmark node.
+ * It is assumed the node has already been inserted into the bookmarks
+ * database.
+ *
+ * @param {Object} node The bookmark node for icons to be inserted.
+ */
+function insertFaviconForNode(node) {
+  if (node.icon) {
+    try {
+      // Create a fake faviconURI to use (FIXME: bug 523932)
+      let faviconURI = Services.io.newURI("fake-favicon-uri:" + node.url);
+      PlacesUtils.favicons.replaceFaviconDataFromDataURL(
+        faviconURI, node.icon, 0,
+        Services.scriptSecurityManager.getSystemPrincipal());
+      PlacesUtils.favicons.setAndFetchFaviconForPage(
+        Services.io.newURI(node.url), faviconURI, false,
+        PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, null,
+        Services.scriptSecurityManager.getSystemPrincipal());
+    } catch (ex) {
+      Components.utils.reportError("Failed to import favicon data:" + ex);
+    }
+  }
+
+  if (!node.iconUri) {
+    return;
+  }
+
+  try {
+    PlacesUtils.favicons.setAndFetchFaviconForPage(
+      Services.io.newURI(node.url), Services.io.newURI(node.iconUri), false,
+      PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, null,
+      Services.scriptSecurityManager.getSystemPrincipal());
+  } catch (ex) {
+    Components.utils.reportError("Failed to import favicon URI:" + ex);
+  }
+}
+
+/**
+ * Handles inserting favicons into the database for a bookmark tree - a node
+ * and its children.
+ *
+ * It is assumed the nodes have already been inserted into the bookmarks
+ * database.
+ *
+ * @param {Object} nodeTree The bookmark node tree for icons to be inserted.
+ */
+function insertFaviconsForTree(nodeTree) {
+  insertFaviconForNode(nodeTree);
+
+  if (nodeTree.children) {
+    for (let child of nodeTree.children) {
+      insertFaviconsForTree(child);
+    }
+  }
+}
+
+/**
+ * Handles fetching data from a URL.
+ *
+ * @param {String} href The url to fetch data from.
+ * @return {Promise} Returns a promise that is resolved with the data once
+ *                   the fetch is complete, or is rejected if it fails.
+ */
+function fetchData(href) {
+  return new Promise((resolve, reject) => {
+    let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
+                .createInstance(Ci.nsIXMLHttpRequest);
+    xhr.onload = () => {
+      resolve(xhr.responseXML);
+    };
+    xhr.onabort = xhr.onerror = xhr.ontimeout = () => {
+      reject(new Error("xmlhttprequest failed"));
+    };
+    xhr.open("GET", href);
+    xhr.responseType = "document";
+    xhr.overrideMimeType("text/html");
+    xhr.send();
+  });
+}
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -1402,17 +1402,18 @@ function insertBookmark(item, parent) {
 
 /**
  * Determines if a bookmark is a Livemark depending on how it is annotated.
  *
  * @param {Object} node The bookmark node to check.
  * @returns {Boolean} True if the node is a Livemark, false otherwise.
  */
 function isLivemark(node) {
-  return node.annos && node.annos.some(anno => anno.name == PlacesUtils.LMANNO_FEEDURI);
+  return node.type == Bookmarks.TYPE_FOLDER && node.annos &&
+         node.annos.some(anno => anno.name == PlacesUtils.LMANNO_FEEDURI);
 }
 
 function insertBookmarkTree(items, source, parent, urls, lastAddedForParent) {
   return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: insertBookmarkTree", async function(db) {
     await db.executeTransaction(async function transaction() {
       await maybeInsertManyPlaces(db, urls);
 
       let syncChangeDelta =
--- a/toolkit/components/places/tests/unit/bookmarks.preplaces.html
+++ b/toolkit/components/places/tests/unit/bookmarks.preplaces.html
@@ -25,11 +25,12 @@
     <DL><p>
         <DT><A HREF="http://example.tld">Example.tld</A>
     </DL><p>
     <DT><H3 LAST_MODIFIED="1177541040" PERSONAL_TOOLBAR_FOLDER="true" ID="rdf:#$FvPhC3">Bookmarks Toolbar Folder</H3>
 <DD>Add bookmarks to this folder to see them displayed on the Bookmarks Toolbar
     <DL><p>
         <DT><A HREF="http://en-US.www.mozilla.com/en-US/firefox/central/" ICON="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABGdBTUEAAK/INwWK6QAAABl0RVh0U29mdHdhcmUAQWRvYmUgSW1hZ2VSZWFkeXHJZTwAAAHWSURBVHjaYvz//z8DJQAggJiQOe/fv2fv7Oz8rays/N+VkfG/iYnJfyD/1+rVq7ffu3dPFpsBAAHEAHIBCJ85c8bN2Nj4vwsDw/8zQLwKiO8CcRoQu0DxqlWrdsHUwzBAAIGJmTNnPgYa9j8UqhFElwPxf2MIDeIrKSn9FwSJoRkAEEAM0DD4DzMAyPi/G+QKY4hh5WAXGf8PDQ0FGwJ22d27CjADAAIIrLmjo+MXA9R2kAHvGBA2wwx6B8W7od6CeQcggKCmCEL8bgwxYCbUIGTDVkHDBia+CuotgACCueD3TDQN75D4xmAvCoK9ARMHBzAw0AECiBHkAlC0Mdy7x9ABNA3obAZXIAa6iKEcGlMVQHwWyjYuL2d4v2cPg8vZswx7gHyAAAK7AOif7SAbOqCmn4Ha3AHFsIDtgPq/vLz8P4MSkJ2W9h8ggBjevXvHDo4FQUQg/kdypqCg4H8lUIACnQ/SOBMYI8bAsAJFPcj1AAEEjwVQqLpAbXmH5BJjqI0gi9DTAAgDBBCcAVLkgmQ7yKCZxpCQxqUZhAECCJ4XgMl493ug21ZD+aDAXH0WLM4A9MZPXJkJIIAwTAR5pQMalaCABQUULttBGCCAGCnNzgABBgAMJ5THwGvJLAAAAABJRU5ErkJggg==" ID="rdf:#$GvPhC3">Getting Started</A>
         <DT><A HREF="http://en-US.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/" LAST_MODIFIED="1177541035" FEEDURL="http://en-US.fxfeeds.mozilla.com/en-US/firefox/headlines.xml" ID="rdf:#$HvPhC3">Latest Headlines</A>
+        <DT><A LAST_MODIFIED="1177541035" FEEDURL="http://en-US.fxfeeds.mozilla.com/en-US/firefox/headlines.xml" ID="rdf:#$HvPhC3">Latest Headlines No Site</A>
 <DD>Livemark test comment
     </DL><p>
 </DL><p>
--- a/toolkit/components/places/tests/unit/test_384370.js
+++ b/toolkit/components/places/tests/unit/test_384370.js
@@ -121,33 +121,41 @@ async function testMenuBookmarks() {
 
   folderNode.containerOpen = false;
   root.containerOpen = false;
 }
 
 async function testToolbarBookmarks() {
   let root = PlacesUtils.getFolderContents(PlacesUtils.toolbarFolderId).root;
 
-  // child count (add 2 for pre-existing items)
-  Assert.equal(root.childCount, bookmarkData.length + 2);
+  // child count (add 3 for pre-existing items)
+  Assert.equal(root.childCount, bookmarkData.length + 3);
 
   let livemarkNode = root.getChild(1);
   Assert.equal("Latest Headlines", livemarkNode.title);
 
   let livemark = await PlacesUtils.livemarks.getLivemark({ id: livemarkNode.itemId });
   Assert.equal("http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/",
                livemark.siteURI.spec);
   Assert.equal("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml",
                livemark.feedURI.spec);
 
+  livemarkNode = root.getChild(2);
+  Assert.equal("Latest Headlines No Site", livemarkNode.title);
+
+  livemark = await PlacesUtils.livemarks.getLivemark({ id: livemarkNode.itemId });
+  Assert.equal(null, livemark.siteURI);
+  Assert.equal("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml",
+               livemark.feedURI.spec);
+
   // test added bookmark data
-  let bookmarkNode = root.getChild(2);
+  let bookmarkNode = root.getChild(3);
   Assert.equal(bookmarkNode.uri, bookmarkData[0].uri.spec);
   Assert.equal(bookmarkNode.title, bookmarkData[0].title);
-  bookmarkNode = root.getChild(3);
+  bookmarkNode = root.getChild(4);
   Assert.equal(bookmarkNode.uri, bookmarkData[1].uri.spec);
   Assert.equal(bookmarkNode.title, bookmarkData[1].title);
 
   root.containerOpen = false;
 }
 
 function testUnfiledBookmarks() {
   let root = PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId).root;
--- a/toolkit/components/places/tests/unit/test_bookmarks_html.js
+++ b/toolkit/components/places/tests/unit/test_bookmarks_html.js
@@ -54,16 +54,19 @@ var test_bookmarks = {
   toolbar: [
     { title: "Getting Started",
       url: "http://en-us.www.mozilla.com/en-US/firefox/central/",
       icon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABGdBTUEAAK/INwWK6QAAABl0RVh0U29mdHdhcmUAQWRvYmUgSW1hZ2VSZWFkeXHJZTwAAAHWSURBVHjaYvz//z8DJQAggJiQOe/fv2fv7Oz8rays/N+VkfG/iYnJfyD/1+rVq7ffu3dPFpsBAAHEAHIBCJ85c8bN2Nj4vwsDw/8zQLwKiO8CcRoQu0DxqlWrdsHUwzBAAIGJmTNnPgYa9j8UqhFElwPxf2MIDeIrKSn9FwSJoRkAEEAM0DD4DzMAyPi/G+QKY4hh5WAXGf8PDQ0FGwJ22d27CjADAAIIrLmjo+MXA9R2kAHvGBA2wwx6B8W7od6CeQcggKCmCEL8bgwxYCbUIGTDVkHDBia+CuotgACCueD3TDQN75D4xmAvCoK9ARMHBzAw0AECiBHkAlC0Mdy7x9ABNA3obAZXIAa6iKEcGlMVQHwWyjYuL2d4v2cPg8vZswx7gHyAAAK7AOif7SAbOqCmn4Ha3AHFsIDtgPq/vLz8P4MSkJ2W9h8ggBjevXvHDo4FQUQg/kdypqCg4H8lUIACnQ/SOBMYI8bAsAJFPcj1AAEEjwVQqLpAbXmH5BJjqI0gi9DTAAgDBBCcAVLkgmQ7yKCZxpCQxqUZhAECCJ4XgMl493ug21ZD+aDAXH0WLM4A9MZPXJkJIIAwTAR5pQMalaCABQUULttBGCCAGCnNzgABBgAMJ5THwGvJLAAAAABJRU5ErkJggg=="
     },
     { title: "Latest Headlines",
       url: "http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/",
       feedUrl: "http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml"
+    },
+    { title: "Latest Headlines No Site",
+      feedUrl: "http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml"
     }
   ],
   unfiled: [
     { title: "Example.tld",
       url: "http://example.tld/"
     }
   ]
 };
@@ -73,23 +76,22 @@ var gBookmarksFileOld;
 // Places bookmarks.html file pointer.
 var gBookmarksFileNew;
 
 add_task(async function setup() {
   // Avoid creating smart bookmarks during the test.
   Services.prefs.setIntPref("browser.places.smartBookmarksVersion", -1);
 
   // File pointer to legacy bookmarks file.
-  gBookmarksFileOld = do_get_file("bookmarks.preplaces.html");
+  gBookmarksFileOld = OS.Path.join(do_get_cwd().path, "bookmarks.preplaces.html");
 
   // File pointer to a new Places-exported bookmarks file.
-  gBookmarksFileNew = Services.dirsvc.get("ProfD", Ci.nsIFile);
-  gBookmarksFileNew.append("bookmarks.exported.html");
-  if (gBookmarksFileNew.exists()) {
-    gBookmarksFileNew.remove(false);
+  gBookmarksFileNew = OS.Path.join(OS.Constants.Path.profileDir, "bookmarks.exported.html");
+  if (await OS.File.exists(gBookmarksFileNew)) {
+    await OS.File.remove(gBookmarksFileNew);
   }
 
   // This test must be the first one, since it setups the new bookmarks.html.
   // Test importing a pre-Places canonical bookmarks file.
   // 1. import bookmarks.preplaces.html
   // 2. run the test-suite
   // Note: we do not empty the db before this import to catch bugs like 380999
   await BookmarkHTMLUtils.importFromFile(gBookmarksFileOld, true);
@@ -342,17 +344,19 @@ function checkItem(aExpected, aNode) {
           break;
         }
         case "charset":
           let testURI = NetUtil.newURI(aNode.uri);
           do_check_eq((await PlacesUtils.getCharsetForURI(testURI)), aExpected.charset);
           break;
         case "feedUrl":
           let livemark = await PlacesUtils.livemarks.getLivemark({ id });
-          do_check_eq(livemark.siteURI.spec, aExpected.url);
+          if (aExpected.url) {
+            do_check_eq(livemark.siteURI.spec, aExpected.url);
+          }
           do_check_eq(livemark.feedURI.spec, aExpected.feedUrl);
           break;
         case "children":
           let folder = aNode.QueryInterface(Ci.nsINavHistoryContainerResultNode);
           do_check_eq(folder.hasChildren, aExpected.children.length > 0);
           folder.containerOpen = true;
           do_check_eq(folder.childCount, aExpected.children.length);