Bug 1523335 - Make use of parentFlexElement in Flexbox inspector r=pbro
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Wed, 06 Feb 2019 17:35:20 +0000
changeset 457608 bdef8bbcf4932da65c95ac54a08ed20648a7fca0
parent 457607 f946a261f4fc355d5e3a2d25d5e7e5557bbd6dd5
child 457609 6e17a2d88817afc47d8967a7d765500b08a0dae0
push id35516
push userrmaries@mozilla.com
push dateFri, 08 Feb 2019 04:23:26 +0000
treeherdermozilla-central@d599d1a73a3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1523335, 1523336
milestone67.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 1523335 - Make use of parentFlexElement in Flexbox inspector r=pbro This gives a very noticable increase in speed. When Brad finishes https://bugzil.la/1523336 we can stop walking the DOM and simply use `parentFlexElement` and `parentGridElement`. #### Try https://treeherder.mozilla.org/#/jobs?repo=try&revision=47d38f2c7dca6ca764862c8b00921644a974a975&group_state=expanded Differential Revision: https://phabricator.services.mozilla.com/D18674
devtools/server/actors/highlighters/box-model.js
devtools/server/actors/inspector/walker.js
devtools/server/actors/layout.js
devtools/shared/layout/utils.js
--- a/devtools/server/actors/highlighters/box-model.js
+++ b/devtools/server/actors/highlighters/box-model.js
@@ -11,17 +11,16 @@ const {
   createNode,
   createSVGNode,
   getBindingElementAndPseudo,
   hasPseudoClassLock,
   isNodeValid,
   moveInfobar,
 } = require("./utils/markup");
 const {
-  findFlexOrGridParentContainerForNode,
   getCurrentZoom,
   setIgnoreLayoutChanges,
  } = require("devtools/shared/layout/utils");
 const { getNodeDisplayName } = require("devtools/server/actors/inspector/utils");
 const nodeConstants = require("devtools/shared/dom-node-constants");
 
 loader.lazyRequireGetter(this, "FlexboxHighlighter",
   "devtools/server/actors/highlighters/flexbox", true);
@@ -435,17 +434,17 @@ class BoxModelHighlighter extends AutoRe
       // great job of that themselves).
       options.noContainerOutline = true;
 
       // Toggle the flag to show the flexbox highlighter.
       showFlexboxHighlighter = true;
     } else {
       // The highlighted element is not a flexbox container so we need to check
       // if it is a flex item.
-      const container = findFlexOrGridParentContainerForNode(node, "flex");
+      const container = node.parentFlexElement;
 
       if (container) {
         for (const region of BOX_MODEL_REGIONS) {
           const box = this.getElement(region);
 
           // Ensure that the box model regions are not faded. The content region
           // will reappear because it is regenerated by the highlighter.
           box.setAttribute("half-faded", "");
--- a/devtools/server/actors/inspector/walker.js
+++ b/devtools/server/actors/inspector/walker.js
@@ -31,17 +31,16 @@ loader.lazyRequireGetter(this, "nodeDocu
 loader.lazyRequireGetter(this, "standardTreeWalkerFilter", "devtools/server/actors/inspector/utils", true);
 
 loader.lazyRequireGetter(this, "CustomElementWatcher", "devtools/server/actors/inspector/custom-element-watcher", true);
 loader.lazyRequireGetter(this, "DocumentWalker", "devtools/server/actors/inspector/document-walker", true);
 loader.lazyRequireGetter(this, "SKIP_TO_SIBLING", "devtools/server/actors/inspector/document-walker", true);
 loader.lazyRequireGetter(this, "NodeActor", "devtools/server/actors/inspector/node", true);
 loader.lazyRequireGetter(this, "NodeListActor", "devtools/server/actors/inspector/node", true);
 loader.lazyRequireGetter(this, "LayoutActor", "devtools/server/actors/layout", true);
-loader.lazyRequireGetter(this, "findFlexOrGridParentContainerForNode", "devtools/server/actors/layout", true);
 loader.lazyRequireGetter(this, "getLayoutChangesObserver", "devtools/server/actors/reflow", true);
 loader.lazyRequireGetter(this, "releaseLayoutChangesObserver", "devtools/server/actors/reflow", true);
 loader.lazyRequireGetter(this, "WalkerSearch", "devtools/server/actors/utils/walker-search", true);
 
 loader.lazyServiceGetter(this, "eventListenerService",
   "@mozilla.org/eventlistenerservice;1", "nsIEventListenerService");
 
 loader.lazyRequireGetter(this, "ChromeUtils");
@@ -555,19 +554,17 @@ var WalkerActor = protocol.ActorClassWit
     //                    a link to the original node.
     // - we are a flex item -> these are always shown on their own lines so they can be
     //                         selected by the flexbox inspector.
     const isAssignedSlot =
       !!firstChild &&
       rawNode.nodeName === "SLOT" &&
       isDirectShadowHostChild(firstChild);
 
-    const isFlexItem =
-      !!firstChild &&
-      findFlexOrGridParentContainerForNode(firstChild, "flex", this);
+    const isFlexItem = !!(firstChild && firstChild.parentFlexElement);
 
     if (!firstChild ||
         walker.nextSibling() ||
         firstChild.nodeType !== Node.TEXT_NODE ||
         firstChild.nodeValue.length > gValueSummaryLength ||
         isAssignedSlot ||
         isFlexItem) {
       return undefined;
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -350,37 +350,37 @@ const LayoutActor = ActorClassWithSpec(l
         return null;
       }
 
       if (flexType && displayType.includes("flex")) {
         if (!onlyLookAtContainer) {
           return new FlexboxActor(this, node);
         }
 
-        const container = findFlexOrGridParentContainerForNode(node, type, this.walker);
-
+        const container = node.parentFlexElement;
         if (container) {
           return new FlexboxActor(this, container);
         }
 
         return null;
       } else if (gridType && displayType.includes("grid")) {
         return new GridActor(this, node);
       }
     }
 
     // Otherwise, check if this is a flex/grid item or the parent node is a flex/grid
     // container.
     // Note that text nodes that are children of flex/grid containers are wrapped in
     // anonymous containers, so even if their displayType getter returns null we still
     // want to walk up the chain to find their container.
-    const container = findFlexOrGridParentContainerForNode(node, type, this.walker);
-    if (container && flexType) {
-      return new FlexboxActor(this, container);
+    const parentFlexElement = node.parentFlexElement;
+    if (parentFlexElement && flexType) {
+      return new FlexboxActor(this, parentFlexElement);
     }
+    const container = findGridParentContainerForNode(node, this.walker);
     if (container && gridType) {
       return new GridActor(this, container);
     }
 
     return null;
   },
 
   /**
@@ -452,88 +452,49 @@ const LayoutActor = ActorClassWithSpec(l
   },
 });
 
 function isNodeDead(node) {
   return !node || (node.rawNode && Cu.isDeadWrapper(node.rawNode));
 }
 
 /**
- * If the provided node is a grid of flex item, then return its parent grid or flex
- * container.
+ * If the provided node is a grid item, then return its parent grid.
  *
  * @param  {DOMNode} node
- *         The node that is supposedly a grid or flex item.
- * @param  {String} type
- *         The type of container/item to look for: "flex" or "grid".
+ *         The node that is supposedly a grid item.
  * @param  {WalkerActor} walkerActor
  *         The current instance of WalkerActor.
  * @return {DOMNode|null}
- *         The parent grid or flex container if found, null otherwise.
+ *         The parent grid if found, null otherwise.
  */
-function findFlexOrGridParentContainerForNode(node, type, walker) {
+function findGridParentContainerForNode(node, walker) {
   const treeWalker = walker.getDocumentWalker(node, SHOW_ELEMENT);
   let currentNode = treeWalker.currentNode;
 
-  const flexType = type === "flex";
-  const gridType = type === "grid";
-
   try {
     while ((currentNode = treeWalker.parentNode())) {
       const displayType = walker.getNode(currentNode).displayType;
       if (!displayType) {
         break;
       }
 
-      if (flexType && displayType.includes("flex")) {
-        if (isNodeAFlexItemInContainer(node, currentNode, walker)) {
-          return currentNode;
-        }
-      } else if (gridType && displayType.includes("grid")) {
+      if (displayType.includes("grid")) {
         return currentNode;
       } else if (displayType === "contents") {
         // Continue walking up the tree since the parent node is a content element.
         continue;
       }
 
       break;
     }
   } catch (e) {
     // Getting the parentNode can fail when the supplied node is in shadow DOM.
   }
 
   return null;
 }
 
-/**
- * Returns whether or not the given node is actually considered a flex item by its
- * container.
- *
- * @param  {Node|NodeActor} supposedItem
- *         The node that might be a flex item of its container.
- * @param  {Node} container
- *         The node's container.
- * @return {Boolean}
- *         Whether or not the node we are looking at is a flex item of its container.
- */
-function isNodeAFlexItemInContainer(supposedItem, container, walker) {
-  const containerDisplayType = walker.getNode(container).displayType;
-
-  if (containerDisplayType.includes("flex")) {
-    const containerFlex = container.getAsFlexContainer();
-
-    for (const line of containerFlex.getLines()) {
-      for (const item of line.getItems()) {
-        if (item.node === supposedItem) {
-          return true;
-        }
-      }
-    }
-  }
-
-  return false;
-}
-
-exports.findFlexOrGridParentContainerForNode = findFlexOrGridParentContainerForNode;
+exports.findGridParentContainerForNode = findGridParentContainerForNode;
 exports.FlexboxActor = FlexboxActor;
 exports.FlexItemActor = FlexItemActor;
 exports.GridActor = GridActor;
 exports.LayoutActor = LayoutActor;
--- a/devtools/shared/layout/utils.js
+++ b/devtools/shared/layout/utils.js
@@ -903,97 +903,16 @@ function getUntransformedQuad(node, regi
     node = node.offsetParent;
   }
 
   return quad;
 }
 exports.getUntransformedQuad = getUntransformedQuad;
 
 /**
- * If the provided node is a grid of flex item, then return its parent grid or flex
- * container.
- *
- * @param  {DOMNode} node
- *         The node that is supposedly a grid or flex item.
- * @param  {String} type
- *         The type of container/item to look for: "flex" or "grid".
- * @return {DOMNode|null}
- *         The parent grid or flex container if found, null otherwise.
- */
-function findFlexOrGridParentContainerForNode(node, type) {
-  const doc = node.ownerDocument;
-  const win = doc.defaultView;
-  const treeWalker = doc.createTreeWalker(doc.body, NodeFilter.SHOW_ELEMENT);
-  const flexType = type === "flex";
-  const gridType = type === "grid";
-  let currentNode = null;
-
-  treeWalker.currentNode = node;
-
-  try {
-    while ((currentNode = treeWalker.parentNode())) {
-      const displayType = win.getComputedStyle(currentNode).display;
-      if (!displayType) {
-        break;
-      }
-
-      if (flexType && displayType.includes("flex")) {
-        if (isNodeAFlexItemInContainer(node, currentNode)) {
-          return currentNode;
-        }
-      } else if (gridType && displayType.includes("grid")) {
-        return currentNode;
-      } else if (displayType === "contents") {
-        // Continue walking up the tree since the parent node is a content element.
-        continue;
-      }
-
-      break;
-    }
-  } catch (e) {
-    // Getting the parentNode can fail when the supplied node is in shadow DOM.
-  }
-
-  return null;
-}
-exports.findFlexOrGridParentContainerForNode = findFlexOrGridParentContainerForNode;
-
-/**
- * Returns whether or not the given node is actually considered a flex item by its
- * container.
- *
- * @param  {DOMNode} supposedItem
- *         The node that might be a flex item of its container.
- * @param  {DOMNode} container
- *         The node's container.
- * @return {Boolean}
- *         Whether or not the node we are looking at is a flex item of its container.
- */
-function isNodeAFlexItemInContainer(supposedItem, container) {
-  const doc = container.ownerDocument;
-  const win = doc.defaultView;
-  const containerDisplayType = win.getComputedStyle(container).display;
-
-  if (containerDisplayType.includes("flex")) {
-    const containerFlex = container.getAsFlexContainer();
-
-    for (const line of containerFlex.getLines()) {
-      for (const item of line.getItems()) {
-        if (item.node === supposedItem) {
-          return true;
-        }
-      }
-    }
-  }
-
-  return false;
-}
-exports.isNodeAFlexItemInContainer = isNodeAFlexItemInContainer;
-
-/**
  * Calculate the total of the node and all of its ancestor's scrollTop and
  * scrollLeft values.
  *
  * @param  {DOMNode} node
  *         The node for which the absolute scroll offsets should be calculated.
  * @return {Object} object
  *         An object containing scrollTop and scrollLeft values.
  * @return {Number} object.scrollLeft