Bug 1523665 - Bookmarks.insertTree should take account of invalid bookmark info causing an insertion of zero items. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D17954

--HG--
extra : moz-landing-system : lando

toolkit/components/places/Bookmarks.jsm

@@ -350,7 +350,7 @@ var Bookmarks = Object.freeze({
    *           bugs in the calling code.
    *
    * @return {Promise} resolved when the creation is complete.
-   * @resolves to an object representing the created bookmark.
+   * @resolves to an array of objects representing the created bookmark(s).
    * @rejects if it's not possible to create the requested bookmark.
    * @throws if the arguments are invalid.
    */
@@ -496,6 +496,12 @@ var Bookmarks = Object.freeze({
     // and the SQL query with which we insert will update it as necessary.
     let lastAddedForParent = appendInsertionInfoForInfoArray(tree.children, null, tree.guid);
 
+    // appendInsertionInfoForInfoArray will remove invalid items and may leave
+    // us with nothing to insert, if so, just return early.
+    if (!insertInfos.length) {
+      return [];
+    }
+
     return (async function() {
       let treeParent = await fetchBookmark({ guid: tree.guid });
       if (!treeParent) {

toolkit/components/places/tests/unit/bookmarks_corrupt.json

@@ -46,6 +46,27 @@
           "uri": "http:///"
         }
       ]
+    },
+    {
+      "guid": "toolbar_____",
+      "title": "Bookmarks Toolbar",
+      "id": 2,
+      "parent": 1,
+      "dateAdded": 1361551978957783,
+      "lastModified": 1361551979382837,
+      "type": "text/x-moz-place-container",
+      "root": "toolbarFolder",
+      "children": [
+        {
+          "guid": "OCyeUO5uu9FG",
+          "title": "Bad URL",
+          "id": 9,
+          "dateAdded": 1361551979356436,
+          "lastModified": 1361551979362718,
+          "type": "text/x-moz-place",
+          "uri": "http:///"
+        }
+      ]
     }
   ]
 }

toolkit/components/places/tests/unit/test_bookmarks_json_corrupt.js

@@ -2,6 +2,23 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/**
+ * Tests for importing a corrupt json file.
+ *
+ * The corrupt json file attempts to import into:
+ *   - the menu folder:
+ *     - A bookmark with an invalid type.
+ *     - A valid bookmark.
+ *     - A bookmark with an invalid url.
+ *   - the toolbar folder:
+ *     - A bookmark with an invalid url.
+ *
+ * The menu case ensure that we strip out invalid bookmarks, but retain valid
+ * ones.
+ * The toolbar case ensures that if no valid bookmarks remain, then we do not
+ * throw an error.
+ */
+
 const {BookmarkJSONUtils} = ChromeUtils.import("resource://gre/modules/BookmarkJSONUtils.jsm");
 
 // Exported bookmarks file pointer.
@@ -15,10 +32,14 @@ add_task(async function test_import_bookmarks() {
 
   let bookmarks = await PlacesUtils.promiseBookmarksTree(PlacesUtils.bookmarks.menuGuid);
 
-  Assert.equal(bookmarks.children.length, 1, "should only be one bookmark");
+  Assert.equal(bookmarks.children.length, 1, "should only be one bookmark in the menu");
   let bookmark = bookmarks.children[0];
   Assert.equal(bookmark.guid, "OCyeUO5uu9FH", "should have correct guid");
   Assert.equal(bookmark.title, "Customize Firefox", "should have correct title");
   Assert.equal(bookmark.uri, "http://en-us.www.mozilla.com/en-US/firefox/customize/",
     "should have correct uri");
+
+  bookmarks = await PlacesUtils.promiseBookmarksTree(PlacesUtils.bookmarks.toolbarGuid);
+
+  Assert.ok(!bookmarks.children, "should not have any bookmarks in the toolbar");
 });