[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..b644e4702bb7d603eff7c6dd0c90b233e0c9edcb 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,74 @@ 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) {
+    switch (domNode.nodeType()) {
+      case Node.ELEMENT_NODE:
+        if (domNode.pseudoType()) {
+          return '::' + domNode.pseudoType();
+        } else {

[lushnikov, 2016/12/08 05:37:03]

style: no "else" after the "then" branch was terminated with the "return".

if (domNode.pseudoType())
  return;
Componens.DOMPresentation...

[phulce, 2016/12/08 19:03:27]

done, should I apply that to all the case statements as well? (i.e. remove
break?)

+          Components.DOMPresentationUtils.decorateNodeLabel(domNode, crumbElement);
+          return null;
+        }
+        break;
+
+      case Node.TEXT_NODE:
+        return Common.UIString('(text)');
+        break;
+
+      case Node.COMMENT_NODE:
+        return '<!-->';
+        break;
+
+      case Node.DOCUMENT_TYPE_NODE:
+        return '<!DOCTYPE>';
+        break;
+
+      case Node.DOCUMENT_FRAGMENT_NODE:
+        return domNode.shadowRootType() ? '#shadow-root' : domNode.nodeNameInCorrectCase();
+        break;
+
+      default:
+        return domNode.nodeNameInCorrectCase();
+    }
+  }
+
   /**
    * @param {boolean=} force
    */
@@ -91,77 +159,17 @@ 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);
+    var onClickCrumb = this._onClickCrumb.bind(this);

[lushnikov, 2016/12/08 05:37:03]

can we inline this?

[phulce, 2016/12/08 19:03:27]

sure, we'll bind inside a loop but I guess the breadcrumbs aren't supposed to be
*that* deep?

     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);
-
-      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;
+      crumb.addEventListener('mousedown', onClickCrumb, false);
 
-        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 +186,10 @@ Elements.ElementsBreadcrumbs = class extends UI.HBox {
 
   /**
    * @param {!Element=} focusedCrumb
+   * @return {!Object}
    */
-  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,35 +210,68 @@ 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;
-    var normalSizes = [];
-    for (var i = 0; i < crumbs.childNodes.length; ++i) {
+    return {selectedIndex, focusedIndex, selectedCrumb};
+  }
+
+  /**
+   * @return {!Object}
+   */
+  _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
+    var collapsedElem = createElementWithClass('span', 'crumb collapsed');
+    crumbs.insertBefore(collapsedElem, crumbs.firstChild);
+
+    var available = crumbs.offsetWidth;

[lushnikov, 2016/12/08 05:37:03]

can we use .measurePrefferedSize() method?

[phulce, 2016/12/08 19:03:27]

oh, jw why would that be better? looks like that will force an extra layout :/

+    var collapsed = crumbs.firstChild.offsetWidth;
+
+    var normal = [];
+    for (var i = 1; i < crumbs.childNodes.length; ++i) {
       var crumb = crumbs.childNodes[i];
-      normalSizes[i] = crumb.offsetWidth;
+      normal[i - 1] = crumb.offsetWidth;
     }
 
+    crumbs.removeChild(collapsedElem);
+
     // Layout 2: Measure collapsed crumb sizes
-    var compactSizes = [];
+    var compact = [];
     for (var i = 0; i < crumbs.childNodes.length; ++i) {
       var crumb = crumbs.childNodes[i];
       crumb.classList.add('compact');
     }
     for (var i = 0; i < crumbs.childNodes.length; ++i) {
       var crumb = crumbs.childNodes[i];
-      compactSizes[i] = crumb.offsetWidth;
+      compact[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, compact, collapsed, available};

[lushnikov, 2016/12/08 05:37:03]

so far we don't use shorthand notation yet. No one checked if it compiles
successfully with closure, and 
we haven't discussed if it's a good thing to have

[phulce, 2016/12/08 19:03:27]

apparently it does :) haha will remove for now

+  }
+
+  /**
+   * @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 +279,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())