Bug 623418 - Bookmark sync: use a SQL query to fetch child GUIDs. r=mconnor,sdwilsh

services/sync/modules/engines/bookmarks.js

@@ -660,18 +660,44 @@ BookmarksStore.prototype = {
     }
   },
 
-  _getChildGUIDsForId: function _getChildGUIDsForId(itemid) {
-    let node = node = this._getNode(itemid);
-    let childids = [];
-
-    if (node.type == node.RESULT_TYPE_FOLDER &&
-        !this._ls.isLivemark(node.itemId)) {
-      node.QueryInterface(Ci.nsINavHistoryQueryResultNode);
-      node.containerOpen = true;
-      for (var i = 0; i < node.childCount; i++)
-        childids.push(node.getChild(i).itemId);
+  __childGUIDsStm: null,
+  get _childGUIDsStm() {
+    if (this.__childGUIDsStm) {
+      return this.__childGUIDsStm;
     }
-    return childids.map(this.GUIDForId, this);
+
+    let stmt;
+    if (this._haveGUIDColumn) {
+      stmt = this._getStmt(
+        "SELECT id AS item_id, guid " +
+        "FROM moz_bookmarks " +
+        "WHERE parent = :parent " +
+        "ORDER BY position");
+    } else {
+      stmt = this._getStmt(
+        "SELECT b.id AS item_id, " +
+          "(SELECT id FROM moz_anno_attributes WHERE name = '" + GUID_ANNO + "') AS name_id," +
+          "a.content AS guid " +
+        "FROM moz_bookmarks b " +
+        "LEFT JOIN moz_items_annos a ON a.item_id = b.id " +
+                                   "AND a.anno_attribute_id = name_id " +
+        "WHERE b.parent = :parent " +
+        "ORDER BY b.position");
+    }
+    return this.__childGUIDsStm = stmt;
+  },
+
+  _getChildGUIDsForId: function _getChildGUIDsForId(itemid) {
+    let stmt = this._childGUIDsStm;
+    stmt.params.parent = itemid;
+    let rows = Utils.queryAsync(stmt, ["item_id", "guid"]);
+    return rows.map(function (row) {
+      if (row.guid) {
+        return row.guid;
+      }
+      // A GUID hasn't been assigned to this item yet, do this now.
+      return this.GUIDForId(row.item_id);
+    }, this);
   },
 
   // Create a record starting from the weave id (places guid)

services/sync/tests/unit/test_bookmark_store.js

@@ -72,7 +72,7 @@ function test_folder_create() {
     _("Have the store create a new record object. Verify that it has the same data.");
     let newrecord = store.createRecord(folder.id);
     do_check_true(newrecord instanceof BookmarkFolder);
-    for each (let property in ["title","title", "parentName", "parentid"])
+    for each (let property in ["title", "parentName", "parentid"])
       do_check_eq(newrecord[property], folder[property]);      
 
     _("Folders have high sort index to ensure they're synced first.");
@@ -83,6 +83,39 @@ function test_folder_create() {
   }
 }
 
+function test_folder_createRecord() {
+  try {
+    _("Create a folder.");
+    let folder1_id = Svc.Bookmark.createFolder(
+      Svc.Bookmark.toolbarFolder, "Folder1", 0);
+    let folder1_guid = store.GUIDForId(folder1_id);
+
+    _("Create two bookmarks in that folder without assigning them GUIDs.");
+    let bmk1_id = Svc.Bookmark.insertBookmark(
+      folder1_id, fxuri, Svc.Bookmark.DEFAULT_INDEX, "Get Firefox!");
+    let bmk2_id = Svc.Bookmark.insertBookmark(
+      folder1_id, tburi, Svc.Bookmark.DEFAULT_INDEX, "Get Thunderbird!");
+
+    _("Create a record for the folder and verify basic properties.");
+    let record = store.createRecord(folder1_guid);
+    do_check_true(record instanceof BookmarkFolder);
+    do_check_eq(record.title, "Folder1");
+    do_check_eq(record.parentid, "toolbar");
+    do_check_eq(record.parentName, "Bookmarks Toolbar");
+
+    _("Verify the folder's children. Ensures that the bookmarks were given GUIDs.");
+    let bmk1_guid = store.GUIDForId(bmk1_id);
+    let bmk2_guid = store.GUIDForId(bmk2_id);
+    do_check_eq(record.children.length, 2);
+    do_check_eq(record.children[0], bmk1_guid);
+    do_check_eq(record.children[1], bmk2_guid);
+
+  } finally {
+    _("Clean up.");
+    store.wipe();
+  }
+}
+
 function test_move_folder() {
   try {
     _("Create two folders and a bookmark in one of them.");
@@ -130,7 +163,6 @@ function test_move_order() {
     do_check_eq(Svc.Bookmark.getItemIndex(bmk1_id), 0);
     do_check_eq(Svc.Bookmark.getItemIndex(bmk2_id), 1);
     let toolbar = store.createRecord("toolbar");
-    dump(JSON.stringify(toolbar.cleartext));
     do_check_eq(toolbar.children.length, 2);
     do_check_eq(toolbar.children[0], bmk1_guid);
     do_check_eq(toolbar.children[1], bmk2_guid);
@@ -159,6 +191,7 @@ function test_move_order() {
 function run_test() {
   test_bookmark_create();
   test_folder_create();
+  test_folder_createRecord();
   test_move_folder();
   test_move_order();
 }