Bug 1297945 - have PlacesSyncUtils use a more efficient technique for checking whether a GUID exists. r=kitcambridge draft
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 22 Aug 2016 14:36:51 +1000
changeset 405941 d3d3b7dc933e119648b70a56959be1c4918d7cbb
parent 405272 a5571e6af4e9655ba6af3cb972286697f24813d1
child 529545 856ba648b0813857bfa34df5770478910ccb5a18
push id27606
push userbmo:markh@mozilla.com
push dateFri, 26 Aug 2016 06:19:21 +0000
reviewerskitcambridge
bugs1297945
milestone51.0a1
Bug 1297945 - have PlacesSyncUtils use a more efficient technique for checking whether a GUID exists. r=kitcambridge MozReview-Commit-ID: 3DJMtMblBOF
toolkit/components/places/PlacesSyncUtils.jsm
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -381,16 +381,30 @@ function notify(observers, notification,
   }
 }
 
 function isInvalidCachedGuidError(error) {
   return error && error.message ==
     "Trying to update the GUIDs cache with an invalid GUID";
 }
 
+// A helper for whenever we want to know if a GUID doesn't exist in the places
+// database. Primarily used to detect orphans on incoming records.
+var GUIDMissing = Task.async(function* (guid) {
+  try {
+    yield PlacesUtils.promiseItemId(guid);
+    return false;
+  } catch (ex) {
+    if (ex.message == "no item found for the given GUID") {
+      return true;
+    }
+    throw ex;
+  }
+});
+
 // Tag queries use a `place:` URL that refers to the tag folder ID. When we
 // apply a synced tag query from a remote client, we need to update the URL to
 // point to the local tag folder.
 var updateTagQueryFolder = Task.async(function* (item) {
   if (item.kind != BookmarkSyncUtils.KINDS.QUERY || !item.folder || !item.url ||
       item.url.protocol != "place:") {
     return item;
   }
@@ -457,59 +471,59 @@ var reparentOrphans = Task.async(functio
         PARENT_ANNO, SOURCE_SYNC);
     }
   }
 });
 
 // Inserts a synced bookmark into the database.
 var insertSyncBookmark = Task.async(function* (insertInfo) {
   let requestedParentGuid = insertInfo.parentGuid;
-  let parent = yield PlacesUtils.bookmarks.fetch(requestedParentGuid);
+  let isOrphan = yield GUIDMissing(insertInfo.parentGuid);
 
   // Default to "unfiled" for new bookmarks if the parent doesn't exist.
-  if (parent) {
+  if (!isOrphan) {
     BookmarkSyncLog.debug(`insertSyncBookmark: Item ${
       insertInfo.guid} is not an orphan`);
   } else {
     BookmarkSyncLog.debug(`insertSyncBookmark: Item ${
       insertInfo.guid} is an orphan: parent ${
       requestedParentGuid} doesn't exist; reparenting to unfiled`);
     insertInfo.parentGuid = PlacesUtils.bookmarks.unfiledGuid;
   }
 
   // If we're inserting a tag query, make sure the tag exists and fix the
   // folder ID to refer to the local tag folder.
   insertInfo = yield updateTagQueryFolder(insertInfo);
 
   let newItem;
   if (insertInfo.kind == BookmarkSyncUtils.KINDS.LIVEMARK) {
-    newItem = yield insertSyncLivemark(parent, insertInfo);
+    newItem = yield insertSyncLivemark(insertInfo);
   } else {
     let item = yield PlacesUtils.bookmarks.insert(insertInfo);
     let newId = yield PlacesUtils.promiseItemId(item.guid);
     newItem = yield insertBookmarkMetadata(newId, item, insertInfo);
   }
 
   if (!newItem) {
     return null;
   }
 
   // If the item is an orphan, annotate it with its real parent ID.
-  if (!parent) {
+  if (isOrphan) {
     yield annotateOrphan(newItem, requestedParentGuid);
   }
 
   // Reparent all orphans that expect this folder as the parent.
   yield reparentOrphans(newItem);
 
   return newItem;
 });
 
 // Inserts a synced livemark.
-var insertSyncLivemark = Task.async(function* (requestedParent, insertInfo) {
+var insertSyncLivemark = Task.async(function* (insertInfo) {
   let parentId = yield PlacesUtils.promiseItemId(insertInfo.parentGuid);
   let parentIsLivemark = PlacesUtils.annotations.itemHasAnnotation(parentId,
     PlacesUtils.LMANNO_FEEDURI);
   if (parentIsLivemark) {
     // A livemark can't be a descendant of another livemark.
     BookmarkSyncLog.debug(`insertSyncLivemark: Invalid parent ${
       insertInfo.parentGuid}; skipping livemark record ${insertInfo.guid}`);
     return null;
@@ -688,28 +702,27 @@ var updateSyncBookmark = Task.async(func
   let isOrphan = false, requestedParentGuid;
   if (updateInfo.hasOwnProperty("parentGuid")) {
     requestedParentGuid = updateInfo.parentGuid;
     if (requestedParentGuid != oldItem.parentGuid) {
       let oldId = yield PlacesUtils.promiseItemId(oldItem.guid);
       if (PlacesUtils.isRootItem(oldId)) {
         throw new Error(`Cannot move Places root ${oldId}`);
       }
-      let parent = yield PlacesUtils.bookmarks.fetch(requestedParentGuid);
-      if (parent) {
+      isOrphan = yield GUIDMissing(requestedParentGuid);
+      if (!isOrphan) {
         BookmarkSyncLog.debug(`updateSyncBookmark: Item ${
           updateInfo.guid} is not an orphan`);
       } else {
         // Don't move the item if the new parent doesn't exist. Instead, mark
         // the item as an orphan. We'll annotate it with its real parent after
         // updating.
         BookmarkSyncLog.trace(`updateSyncBookmark: Item ${
           updateInfo.guid} is an orphan: could not find parent ${
           requestedParentGuid}`);
-        isOrphan = true;
         delete updateInfo.parentGuid;
       }
       // If we're reparenting the item, pass the default index so that
       // `PlacesUtils.bookmarks.update` doesn't throw. Sync will reorder
       // children at the end of the sync.
       updateInfo.index = PlacesUtils.bookmarks.DEFAULT_INDEX;
     } else {
       // `PlacesUtils.bookmarks.update` requires us to specify an index if we