Bug 1627361 - Stop examining the clipboard data when updating Places commands. r=Standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 15 May 2020 14:20:58 +0000
changeset 530302 8a0aa5ff4cf2b340d3a01d977e6b29a1206d006b
parent 530301 1acf50c375418804e9e646f8154ad3bd8d97ecce
child 530303 625655f148034d62258f041c90a79c30f1ff8e7b
push id37420
push usernerli@mozilla.com
push dateFri, 15 May 2020 21:52:36 +0000
treeherdermozilla-central@f340bbb582d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1627361
milestone78.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 1627361 - Stop examining the clipboard data when updating Places commands. r=Standard8 We used to read the contents of the clipboard to tell if paste was enabled, that unfortunately means updating commands was extremely slow for large clipboard data. After this change we only check the data flavors. This means paste will be enabled more often, even for unsupported strings, but commands updating will be much faster. Places updates commands often, so this is quite useful. Differential Revision: https://phabricator.services.mozilla.com/D75202
browser/components/places/content/controller.js
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -123,16 +123,20 @@ PlacesController.prototype = {
 
     // All other Places Commands are prefixed with "placesCmd_" ... this
     // filters out other commands that we do _not_ support (see 329587).
     const CMD_PREFIX = "placesCmd_";
     return aCommand.substr(0, CMD_PREFIX.length) == CMD_PREFIX;
   },
 
   isCommandEnabled: function PC_isCommandEnabled(aCommand) {
+    // Determine whether or not nodes can be inserted.
+    let ip = this._view.insertionPoint;
+    let canInsert = ip && (aCommand.endsWith("_paste") || !ip.isTag);
+
     switch (aCommand) {
       case "cmd_undo":
         return PlacesTransactions.topUndoEntry != null;
       case "cmd_redo":
         return PlacesTransactions.topRedoEntry != null;
       case "cmd_cut":
       case "placesCmd_cut":
         for (let node of this._view.selectedNodes) {
@@ -150,39 +154,54 @@ PlacesController.prototype = {
       case "placesCmd_delete":
       case "placesCmd_deleteDataHost":
         return this._hasRemovableSelection();
       case "cmd_copy":
       case "placesCmd_copy":
         return this._view.hasSelection;
       case "cmd_paste":
       case "placesCmd_paste":
-        return this._canInsert(true) && this._isClipboardDataPasteable();
+        // If the clipboard contains a Places flavor it is definitely pasteable,
+        // otherwise we also allow pasting "text/unicode" and "text/x-moz-url" data.
+        // We don't check if the data is valid here, because the clipboard may
+        // contain very large blobs that would largely slowdown commands updating.
+        // Of course later paste() should ignore any invalid data.
+        return (
+          canInsert &&
+          this.clipboard.hasDataMatchingFlavors(
+            [
+              ...PlacesUIUtils.PLACES_FLAVORS,
+              PlacesUtils.TYPE_X_MOZ_URL,
+              PlacesUtils.TYPE_UNICODE,
+            ],
+            Ci.nsIClipboard.kGlobalClipboard
+          )
+        );
       case "cmd_selectAll":
         if (this._view.selType != "single") {
           let rootNode = this._view.result.root;
           if (rootNode.containerOpen && rootNode.childCount > 0) {
             return true;
           }
         }
         return false;
       case "placesCmd_open":
       case "placesCmd_open:window":
       case "placesCmd_open:privatewindow":
       case "placesCmd_open:tab": {
         let selectedNode = this._view.selectedNode;
         return selectedNode && PlacesUtils.nodeIsURI(selectedNode);
       }
       case "placesCmd_new:folder":
-        return this._canInsert();
+        return canInsert;
       case "placesCmd_new:bookmark":
-        return this._canInsert();
+        return canInsert;
       case "placesCmd_new:separator":
         return (
-          this._canInsert() &&
+          canInsert &&
           !PlacesUtils.asQuery(this._view.result.root).queryOptions
             .excludeItems &&
           this._view.result.sortingMode ==
             Ci.nsINavHistoryQueryOptions.SORT_BY_NONE
         );
       case "placesCmd_show:info": {
         let selectedNode = this._view.selectedNode;
         return (
@@ -199,18 +218,18 @@ PlacesController.prototype = {
           selectedNode &&
           PlacesUtils.nodeIsFolder(selectedNode) &&
           !PlacesUIUtils.isFolderReadOnly(selectedNode) &&
           this._view.result.sortingMode ==
             Ci.nsINavHistoryQueryOptions.SORT_BY_NONE
         );
       }
       case "placesCmd_createBookmark": {
-        return this._view.selectedNodes.every(
-          node => PlacesUtils.nodeIsURI(node) && node.itemId == -1
+        return !this._view.selectedNodes.some(
+          node => !PlacesUtils.nodeIsURI(node) || node.itemId != -1
         );
       }
       default:
         return false;
     }
   },
 
   doCommand: function PC_doCommand(aCommand) {
@@ -338,79 +357,16 @@ PlacesController.prototype = {
         }
       }
     }
 
     return true;
   },
 
   /**
-   * Determines whether or not nodes can be inserted relative to the selection.
-   */
-  _canInsert: function PC__canInsert(isPaste) {
-    var ip = this._view.insertionPoint;
-    return ip != null && (isPaste || !ip.isTag);
-  },
-
-  /**
-   * Looks at the data on the clipboard to see if it is paste-able.
-   * Paste-able data is:
-   *   - in a format that the view can receive
-   * @return true if: - clipboard data is of a TYPE_X_MOZ_PLACE_* flavor,
-   *                  - clipboard data is of type TEXT_UNICODE and
-   *                    is a valid URI.
-   */
-  _isClipboardDataPasteable: function PC__isClipboardDataPasteable() {
-    // if the clipboard contains TYPE_X_MOZ_PLACE_* data, it is definitely
-    // pasteable, with no need to unwrap all the nodes.
-
-    var flavors = PlacesUIUtils.PLACES_FLAVORS;
-    var clipboard = this.clipboard;
-    var hasPlacesData = clipboard.hasDataMatchingFlavors(
-      flavors,
-      Ci.nsIClipboard.kGlobalClipboard
-    );
-    if (hasPlacesData) {
-      return this._view.insertionPoint != null;
-    }
-
-    // if the clipboard doesn't have TYPE_X_MOZ_PLACE_* data, we also allow
-    // pasting of valid "text/unicode" and "text/x-moz-url" data
-    var xferable = Cc["@mozilla.org/widget/transferable;1"].createInstance(
-      Ci.nsITransferable
-    );
-    xferable.init(null);
-
-    xferable.addDataFlavor(PlacesUtils.TYPE_X_MOZ_URL);
-    xferable.addDataFlavor(PlacesUtils.TYPE_UNICODE);
-    clipboard.getData(xferable, Ci.nsIClipboard.kGlobalClipboard);
-
-    try {
-      // getAnyTransferData will throw if no data is available.
-      var data = {},
-        type = {};
-      xferable.getAnyTransferData(type, data);
-      data = data.value.QueryInterface(Ci.nsISupportsString).data;
-      if (
-        type.value != PlacesUtils.TYPE_X_MOZ_URL &&
-        type.value != PlacesUtils.TYPE_UNICODE
-      ) {
-        return false;
-      }
-
-      // unwrapNodes() will throw if the data blob is malformed.
-      PlacesUtils.unwrapNodes(data, type.value);
-      return this._view.insertionPoint != null;
-    } catch (e) {
-      // getAnyTransferData or unwrapNodes failed
-      return false;
-    }
-  },
-
-  /**
    * Gathers information about the selected nodes according to the following
    * rules:
    *    "link"              node is a URI
    *    "bookmark"          node is a bookmark
    *    "tagChild"          node is a child of a tag
    *    "folder"            node is a folder
    *    "query"             node is a query
    *    "separator"         node is a separator line