Bug 1297945 - have PlacesSyncUtils use a more efficient technique for checking whether a GUID exists. r=kitcambridge
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 22 Aug 2016 14:36:51 +1000
changeset 311746 ee96068b9523274d9354b0b530f97d2b15ce9a08
parent 311745 29f27e5ab2177d8f4ee0c3c9eb06d8705985b14a
child 311747 02a345df379c3d36556166722ca712a24fb29070
push id30621
push userryanvm@gmail.com
push dateTue, 30 Aug 2016 13:51:36 +0000
treeherdermozilla-central@987bda408af3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs1297945
milestone51.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 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