Bug 1413843 - Improve speed of operations when pasting to the left-hand library pane by getting the right-hand pane to use for batching. r?mak draft
authorMark Banner <standard8@mozilla.com>
Thu, 02 Nov 2017 10:37:14 +0000
changeset 694131 a568a952f4b78eb8624c9e2e1bf17b543850c3a5
parent 694130 923836aebbc328d1a971f6ce32a99d9aa4d1345a
child 739261 bdc1b1beb9351c332984356850d395efd68d6b05
push id88050
push userbmo:standard8@mozilla.com
push dateTue, 07 Nov 2017 12:24:02 +0000
reviewersmak
bugs1413843
milestone58.0a1
Bug 1413843 - Improve speed of operations when pasting to the left-hand library pane by getting the right-hand pane to use for batching. r?mak MozReview-Commit-ID: 2Q1rIpIhmmM
browser/components/places/content/controller.js
browser/components/places/content/treeView.js
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -1323,24 +1323,28 @@ PlacesController.prototype = {
         let insertionIndex = await ip.getIndex();
         let doCopy = action == "copy";
         let newTransactions = await getTransactionsForTransferItems(type,
           items, insertionIndex, ip.guid, doCopy);
         if (newTransactions.length) {
           transactions = [...transactions, ...newTransactions];
         }
 
-        await PlacesUIUtils.batchUpdatesForNode(this._view.result, transactions.length, async () => {
-          await PlacesTransactions.batch(async () => {
-            for (let transaction of transactions) {
-              let guid = await transaction.transact();
-              itemsToSelect.push(await PlacesUtils.promiseItemId(guid));
-            }
+        // Note: this._view may be a view or the tree element.
+        let resultForBatching = getResultForBatching(this._view);
+
+        await PlacesUIUtils.batchUpdatesForNode(resultForBatching,
+          transactions.length, async () => {
+            await PlacesTransactions.batch(async () => {
+              for (let transaction of transactions) {
+                let guid = await transaction.transact();
+                itemsToSelect.push(await PlacesUtils.promiseItemId(guid));
+              }
+            });
           });
-        });
       }
     } else {
       let transactions = [];
       let insertionIndex = await ip.getIndex();
       for (let index = insertionIndex, i = 0; i < items.length; ++i) {
         if (ip.isTag) {
           // Pasting into a tag container means tagging the item, regardless of
           // the requested action.
@@ -1604,19 +1608,18 @@ var PlacesControllerDragHelper = {
   },
 
   /**
    * Handles the drop of one or more items onto a view.
    *
    * @param {Object} insertionPoint The insertion point where the items should
    *                                be dropped.
    * @param {Object} dt             The dataTransfer information for the drop.
-   * @param {Object} view           The tree view where this object is being
-   *                                dropped to. This allows batching to take
-   *                                place.
+   * @param {Object} view           The view or the tree element. This allows
+   *                                batching to take place.
    */
   async onDrop(insertionPoint, dt, view) {
     let doCopy = ["copy", "link"].includes(dt.dropEffect);
 
     let transactions = [];
     let dropCount = dt.mozItemCount;
     let parentGuid = insertionPoint.guid;
     let tagName = insertionPoint.tagName;
@@ -1718,19 +1721,21 @@ var PlacesControllerDragHelper = {
       }
     }
     // Check if we actually have something to add, if we don't it probably wasn't
     // valid, or it was moving to the same location, so just ignore it.
     if (!transactions.length) {
       return;
     }
     if (PlacesUIUtils.useAsyncTransactions) {
-      await PlacesUIUtils.batchUpdatesForNode(view && view.result, transactions.length, async () => {
-        await PlacesTransactions.batch(transactions);
-      });
+      let resultForBatching = getResultForBatching(view);
+      await PlacesUIUtils.batchUpdatesForNode(resultForBatching,
+        transactions.length, async () => {
+          await PlacesTransactions.batch(transactions);
+        });
     } else {
       let txn = new PlacesAggregatedTransaction("DropItems", transactions);
       PlacesUtils.transactionManager.doTransaction(txn);
     }
   },
 
   /**
    * Checks if we can insert into a container.
@@ -1806,16 +1811,40 @@ function doGetPlacesControllerForCommand
 
 function goDoPlacesCommand(aCommand) {
   let controller = doGetPlacesControllerForCommand(aCommand);
   if (controller && controller.isCommandEnabled(aCommand))
     controller.doCommand(aCommand);
 }
 
 /**
+ * This gets the most appropriate item for using for batching. In the case of multiple
+ * views being related, the method returns the most expensive result to batch.
+ * For example, if it detects the left-hand library pane, then it will look for
+ * and return the reference to the right-hand pane.
+ *
+ * @param {Object} viewOrElement The item to check.
+ * @return {Object} Will return the best result node to batch, or null
+ *                  if one could not be found.
+ */
+function getResultForBatching(viewOrElement) {
+  if (viewOrElement && viewOrElement instanceof Ci.nsIDOMElement &&
+      viewOrElement.id === "placesList") {
+    // Note: fall back to the existing item if we can't find the right-hane pane.
+    viewOrElement = document.getElementById("placeContent") || viewOrElement;
+  }
+
+  if (viewOrElement && viewOrElement.result) {
+    return viewOrElement.result;
+  }
+
+  return null;
+}
+
+/**
  * Processes a set of transfer items and returns transactions to insert or
  * move them.
  *
  * @param {String} dataFlavor The transfer flavor for the items.
  * @param {Array} items A list of unwrapped nodes to get transactions for.
  * @param {Integer} insertionIndex The requested index for insertion.
  * @param {String} insertionParentGuid The guid of the parent folder to insert
  *                                     or move the items to.
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1448,17 +1448,17 @@ PlacesTreeView.prototype = {
   },
 
   drop: function PTV_drop(aRow, aOrientation, aDataTransfer) {
     // We are responsible for translating the |index| and |orientation|
     // parameters into a container id and index within the container,
     // since this information is specific to the tree view.
     let ip = this._getInsertionPoint(aRow, aOrientation);
     if (ip) {
-      PlacesControllerDragHelper.onDrop(ip, aDataTransfer, this)
+      PlacesControllerDragHelper.onDrop(ip, aDataTransfer, this._tree.element)
                                 .catch(Components.utils.reportError)
                                 .then(() => {
                                   // We should only clear the drop target once
                                   // the onDrop is complete, as it is an async function.
                                   PlacesControllerDragHelper.currentDropTarget = null;
                                 });
     }
   },