[DevTools] ElementsBreadcrumbs Perf Improvements & Cleanup

third_party/WebKit/Source/devtools/front_end/elements/ElementsBreadcrumbs.js

Index: third_party/WebKit/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
diff --git a/third_party/WebKit/Source/devtools/front_end/elements/ElementsBreadcrumbs.js b/third_party/WebKit/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
index 5bc9f28084edfbc5fd483d949db4b352f1f5a0be..3fe088be3b7e81a4677391c91874ce77f96351cd 100644
--- a/third_party/WebKit/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
+++ b/third_party/WebKit/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
@@ -43,7 +43,7 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
    */
   setSelectedNode(node) {
     this._currentDOMNode = node;
-    this.update();
+    this.crumbsElement.window().requestAnimationFrame(() => this.update());
   }
 
   _mouseMovedInCrumbs(event) {
@@ -59,6 +59,63 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
       SDK.DOMModel.hideDOMNodeHighlight();
   }
 
+
+  /**
+   * @param {!Event} event
+   * @this {Elements.ElementsBreadcrumbs}
+   */
+  _onClickCrumb(event) {
+    event.preventDefault();
+    var crumb = /** @type {!Element} */ (event.currentTarget);
+    if (!crumb.classList.contains('collapsed')) {
+      this.dispatchEventToListeners(Elements.ElementsBreadcrumbs.Events.NodeSelected, crumb[this._nodeSymbol]);
+      return;
+    }
+
+    // Clicking a collapsed crumb will expose the hidden crumbs.
+    if (crumb === this.crumbsElement.firstChild) {
+      // If the clicked crumb is the first child, pick the farthest crumb
+      // that is still hidden. This allows the user to expose every crumb.
+      var currentCrumb = crumb;
+      while (currentCrumb) {
+        var hidden = currentCrumb.classList.contains('hidden');
+        var collapsed = currentCrumb.classList.contains('collapsed');
+        if (!hidden && !collapsed)
+          break;
+        crumb = currentCrumb;
+        currentCrumb = currentCrumb.nextSiblingElement;
+      }
+    }
+
+    this.updateSizes(crumb);
+  }
+
+  /**
+   * @param {!SDK.DOMNode} domNode
+   * @param {!Element} crumbElement
+   * @return {?string}
+   */
+  _determineElementTitle(domNode, crumbElement) {

[lushnikov, 2016/12/08 21:20:43]

I would expect this method to accept only a domNode and return title, without
causing any
side-effects. Having it mutating crumbElement doesn't feel clean.

Can we either keep it inline or avoid passing crumbElement in?

[phulce, 2016/12/09 00:46:30]

Done.

+    switch (domNode.nodeType()) {
+      case Node.ELEMENT_NODE:
+        if (domNode.pseudoType())
+          return '::' + domNode.pseudoType();
+
+        Components.DOMPresentationUtils.decorateNodeLabel(domNode, crumbElement);
+        return null;
+      case Node.TEXT_NODE:
+        return Common.UIString('(text)');
+      case Node.COMMENT_NODE:
+        return '<!-->';
+      case Node.DOCUMENT_TYPE_NODE:
+        return '<!DOCTYPE>';
+      case Node.DOCUMENT_FRAGMENT_NODE:
+        return domNode.shadowRootType() ? '#shadow-root' : domNode.nodeNameInCorrectCase();
+      default:
+        return domNode.nodeNameInCorrectCase();
+    }
+  }
+
   /**
    * @param {boolean=} force
    */
@@ -91,77 +148,16 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
 
     crumbs.removeChildren();
 
-    var panel = this;
-
-    /**
-     * @param {!Event} event
-     * @this {Elements.ElementsBreadcrumbs}
-     */
-    function selectCrumb(event) {
-      event.preventDefault();
-      var crumb = /** @type {!Element} */ (event.currentTarget);
-      if (!crumb.classList.contains('collapsed')) {
-        this.dispatchEventToListeners(Elements.ElementsBreadcrumbs.Events.NodeSelected, crumb[this._nodeSymbol]);
-        return;
-      }
-
-      // Clicking a collapsed crumb will expose the hidden crumbs.
-      if (crumb === panel.crumbsElement.firstChild) {
-        // If the focused crumb is the first child, pick the farthest crumb
-        // that is still hidden. This allows the user to expose every crumb.
-        var currentCrumb = crumb;
-        while (currentCrumb) {
-          var hidden = currentCrumb.classList.contains('hidden');
-          var collapsed = currentCrumb.classList.contains('collapsed');
-          if (!hidden && !collapsed)
-            break;
-          crumb = currentCrumb;
-          currentCrumb = currentCrumb.nextSiblingElement;
-        }
-      }
-
-      this.updateSizes(crumb);
-    }
-
-    var boundSelectCrumb = selectCrumb.bind(this);
     for (var current = currentDOMNode; current; current = current.parentNode) {
       if (current.nodeType() === Node.DOCUMENT_NODE)
         continue;
 
       crumb = createElementWithClass('span', 'crumb');
       crumb[this._nodeSymbol] = current;
-      crumb.addEventListener('mousedown', boundSelectCrumb, false);
+      crumb.addEventListener('mousedown', this._onClickCrumb.bind(this), false);
 
-      var crumbTitle = '';
-      switch (current.nodeType()) {
-        case Node.ELEMENT_NODE:
-          if (current.pseudoType())
-            crumbTitle = '::' + current.pseudoType();
-          else
-            Components.DOMPresentationUtils.decorateNodeLabel(current, crumb);
-          break;
-
-        case Node.TEXT_NODE:
-          crumbTitle = Common.UIString('(text)');
-          break;
-
-        case Node.COMMENT_NODE:
-          crumbTitle = '<!-->';
-          break;
-
-        case Node.DOCUMENT_TYPE_NODE:
-          crumbTitle = '<!DOCTYPE>';
-          break;
-
-        case Node.DOCUMENT_FRAGMENT_NODE:
-          crumbTitle = current.shadowRootType() ? '#shadow-root' : current.nodeNameInCorrectCase();
-          break;
-
-        default:
-          crumbTitle = current.nodeNameInCorrectCase();
-      }
-
-      if (!crumb.childNodes.length) {
+      var crumbTitle = this._determineElementTitle(current, crumb);
+      if (crumbTitle) {
         var nameElement = createElement('span');
         nameElement.textContent = crumbTitle;
         crumb.appendChild(nameElement);
@@ -178,15 +174,10 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
 
   /**
    * @param {!Element=} focusedCrumb
+   * @return {!Object}

[lushnikov, 2016/12/08 21:20:43]

let's annotate it with details: {{selectedIndex: number, focusedIndex: number,
selectedCrumb: Element}}

[phulce, 2016/12/09 00:46:30]

Done.

    */
-  updateSizes(focusedCrumb) {
-    if (!this.isShowing())
-      return;
-
+  _resetCrumbStylesAndFindSelections(focusedCrumb) {
     var crumbs = this.crumbsElement;
-    if (!crumbs.firstChild)
-      return;
-
     var selectedIndex = 0;
     var focusedIndex = 0;
     var selectedCrumb;
@@ -207,14 +198,31 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
       crumb.classList.remove('compact', 'collapsed', 'hidden');
     }
 
-    // Layout 1: Measure total and normal crumb sizes
-    var contentElementWidth = this.contentElement.offsetWidth;
+    return {selectedIndex, focusedIndex, selectedCrumb};

[lushnikov, 2016/12/08 21:20:43]

nit: let's avoid object shorthands here as well

[phulce, 2016/12/09 00:46:30]

Done.

+  }
+
+  /**
+   * @return {!Object}

[lushnikov, 2016/12/08 21:20:43]

let's annotate this with details

[phulce, 2016/12/09 00:46:30]

Done.

+   */
+  _measureElementSizes() {
+    var crumbs = this.crumbsElement;
+
+    // Layout 1: Measure total and normal crumb sizes at the same time as a
+    // dummy element for the collapsed size

[lushnikov, 2016/12/08 21:20:43]

style: "." in the end of the comment

[phulce, 2016/12/09 00:46:30]

Done.

+    var collapsedElem = createElementWithClass('span', 'crumb collapsed');

[lushnikov, 2016/12/08 21:20:43]

var collapsedElement = ...

style: we try to avoid as much abbreviations as possible

[phulce, 2016/12/09 00:46:30]

Done.

+    crumbs.insertBefore(collapsedElem, crumbs.firstChild);
+
+    var available = crumbs.offsetWidth;
+    var collapsed = crumbs.firstChild.offsetWidth;

[lushnikov, 2016/12/08 21:20:43]

var collapsed = collapsedElem.offsetWidth;

[phulce, 2016/12/09 00:46:30]

Done.

+
     var normalSizes = [];
-    for (var i = 0; i < crumbs.childNodes.length; ++i) {
+    for (var i = 1; i < crumbs.childNodes.length; ++i) {
       var crumb = crumbs.childNodes[i];
-      normalSizes[i] = crumb.offsetWidth;
+      normalSizes[i - 1] = crumb.offsetWidth;
     }
 
+    crumbs.removeChild(collapsedElem);
+
     // Layout 2: Measure collapsed crumb sizes
     var compactSizes = [];
     for (var i = 0; i < crumbs.childNodes.length; ++i) {
@@ -226,16 +234,32 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
       compactSizes[i] = crumb.offsetWidth;
     }
 
-    // Layout 3: Measure collapsed crumb size
-    crumbs.firstChild.classList.add('collapsed');
-    var collapsedSize = crumbs.firstChild.offsetWidth;
-
     // Clean up.
     for (var i = 0; i < crumbs.childNodes.length; ++i) {
       var crumb = crumbs.childNodes[i];
       crumb.classList.remove('compact', 'collapsed');
     }
 
+    return {normal: normalSizes, compact: compactSizes, collapsed: collapsed, available: available};
+  }
+
+  /**
+   * @param {!Element=} focusedCrumb
+   */
+  updateSizes(focusedCrumb) {
+    if (!this.isShowing())
+      return;
+
+    var crumbs = this.crumbsElement;
+    if (!crumbs.firstChild)
+      return;
+
+    var selections = this._resetCrumbStylesAndFindSelections(focusedCrumb);
+    var sizes = this._measureElementSizes();
+    var selectedIndex = selections.selectedIndex;
+    var focusedIndex = selections.focusedIndex;
+    var selectedCrumb = selections.selectedCrumb;
+
     function crumbsAreSmallerThanContainer() {
       var totalSize = 0;
       for (var i = 0; i < crumbs.childNodes.length; ++i) {
@@ -243,13 +267,13 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
         if (crumb.classList.contains('hidden'))
           continue;
         if (crumb.classList.contains('collapsed')) {
-          totalSize += collapsedSize;
+          totalSize += sizes.collapsed;
           continue;
         }
-        totalSize += crumb.classList.contains('compact') ? compactSizes[i] : normalSizes[i];
+        totalSize += crumb.classList.contains('compact') ? sizes.compact[i] : sizes.normal[i];
       }
       const rightPadding = 10;
-      return totalSize + rightPadding < contentElementWidth;
+      return totalSize + rightPadding < sizes.available;
     }
 
     if (crumbsAreSmallerThanContainer())