Bug 1410940 - Avoid a bookmark fetch when dropping bookmarks by supplying the necessary information in the transfer data. r?mak draft
authorMark Banner <standard8@mozilla.com>
Mon, 23 Oct 2017 15:51:05 +0100
changeset 686022 f008f13765206a02bafc013660be6f985662230b
parent 685608 a124f4901430f6db74cfc7fe3b07957a1c691b40
child 686023 c3d829f1cfa27838a64a2a0c995e2f390b007652
push id86074
push userbmo:standard8@mozilla.com
push dateWed, 25 Oct 2017 09:56:20 +0000
reviewersmak
bugs1410940
milestone58.0a1
Bug 1410940 - Avoid a bookmark fetch when dropping bookmarks by supplying the necessary information in the transfer data. r?mak We can do this, since we only need the parent information when dragging bookmarks within the same instance of Firefox. MozReview-Commit-ID: AhL0QJzp6ey
browser/components/places/content/controller.js
browser/components/places/tests/browser/browser_controller_onDrop.js
toolkit/components/places/PlacesUtils.jsm
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -1668,42 +1668,49 @@ var PlacesControllerDragHelper = {
 
       for (let unwrapped of nodes) {
         let index = await insertionPoint.getIndex();
 
         if (index != -1 && unwrapped.itemGuid) {
           // Note: we use the parent from the existing bookmark as the sidebar
           // gives us an unwrapped.parent that is actually a query and not the real
           // parent.
-          let existingBookmark = await PlacesUtils.bookmarks.fetch(unwrapped.itemGuid);
+          let existingParentGuid = unwrapped.parentGuid;
+          if (!existingParentGuid) {
+            let bookmark = await PlacesUtils.bookmarks.fetch(unwrapped.itemGuid);
+            if (bookmark) {
+              existingParentGuid = bookmark.parentGuid;
+              unwrapped.currentIndex = bookmark.index;
+            }
+          }
 
           // If we're dropping on the same folder, then we may need to adjust
           // the index to insert at the correct place.
-          if (existingBookmark && parentGuid == existingBookmark.parentGuid) {
+          if (parentGuid == existingParentGuid) {
             if (PlacesUIUtils.useAsyncTransactions) {
-              if (index < existingBookmark.index) {
+              if (index < unwrapped.currentIndex) {
                 // When you drag multiple elts upward: need to increment index or
                 // each successive elt will be inserted at the same index, each
                 // above the previous.
                 index += movedCount++;
-              } else if (index > existingBookmark.index) {
+              } else if (index > unwrapped.currentIndex) {
                 // If we're dragging down, we need to go one lower to insert at
                 // the real point as moving the element changes the index of
                 // everything below by 1.
                 index--;
               } else {
                 // This isn't moving so we skip it.
                 continue;
               }
             } else {
               // Sync Transactions. Adjust insertion index to prevent reversal
               // of dragged items. When you drag multiple elts upward: need to
               // increment index or each successive elt will be inserted at the
               // same index, each above the previous.
-              if (index < existingBookmark.index) { // eslint-disable-line no-lonely-if
+              if (index < unwrapped.currentIndex) { // eslint-disable-line no-lonely-if
                 index += movedCount++;
               }
             }
           }
         }
 
         // If dragging over a tag container we should tag the item.
         if (insertionPoint.isTag) {
--- a/browser/components/places/tests/browser/browser_controller_onDrop.js
+++ b/browser/components/places/tests/browser/browser_controller_onDrop.js
@@ -87,16 +87,18 @@ async function run_drag_test(startBookma
       index: insertionIndex,
       orientation: Ci.nsITreeView.DROP_ON
     });
 
     let bookmarkWithId = JSON.stringify(Object.assign({
       id: bookmarkIds.get(dragBookmark.guid),
       itemGuid: dragBookmark.guid,
       parent: PlacesUtils.unfiledBookmarksFolderId,
+      parentGuid: dragBookmark.parentGuid,
+      currentIndex: dragBookmark.index
     }, dragBookmark));
 
     let dt = {
       dropEffect: "move",
       mozCursor: "auto",
       mozItemCount: 1,
       types: [ PlacesUtils.TYPE_X_MOZ_PLACE ],
       mozTypesAt(i) {
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -133,22 +133,26 @@ function serializeNode(aNode, aIsLivemar
   let data = {};
 
   data.title = aNode.title;
   data.id = aNode.itemId;
   data.livemark = aIsLivemark;
   // Add an instanceId so we can tell which instance of an FF session the data
   // is coming from.
   data.instanceId = PlacesUtils.instanceId;
+  // Add a currentIndex reference to speed up drops & pastes.
+  data.currentIndex = aNode.bookmarkIndex;
 
   let guid = aNode.bookmarkGuid;
   if (guid) {
     data.itemGuid = guid;
-    if (aNode.parent)
+    if (aNode.parent) {
       data.parent = aNode.parent.itemId;
+      data.parentGuid = PlacesUtils.getConcreteItemGuid(aNode.parent);
+    }
     let grandParent = aNode.parent && aNode.parent.parent;
     if (grandParent)
       data.grandParentId = grandParent.itemId;
 
     data.dateAdded = aNode.dateAdded;
     data.lastModified = aNode.lastModified;
 
     let annos = PlacesUtils.getAnnotationsForItem(data.id);