Title: [294695] trunk
Revision
294695
Author
pan...@apple.com
Date
2022-05-23 16:42:31 -0700 (Mon, 23 May 2022)

Log Message

Web Inspector: Layout panel doesn't always match visible nodes/order of nodes in document
https://bugs.webkit.org/show_bug.cgi?id=240775
rdar://93727833

Reviewed by Devin Rousso.

There were a couple issues preventing the layout panel from showing nodes that could actually have an overlay shown in
a predictable order.

First, DOMManager.prototype.nodesWithLayoutContextType provided an array of nodes sorted by nodeId, not their order in
the document. This meant that nodes created later and inserted into the page did not get placed in their logical order
relative to other containers, but at the end of the list. This is fixed with a new iterator that iterated through the
document and its tree in a way the preserves the order of elements as they would appear in a Tree view.

Second, LayoutDetailsSidebarPanel was not instrumenting the insertion/deletion of nodes, which meant that a node with a
layout context could be removed from the DOM tree, but still exist. These nodes weren't actually useful in the list
because you can't turn on the overlay for them. A similar issue existed for inserting a known node back into the DOM
tree.

Also fix an assertion reached from NodeOverlayListSection when toggling all overlays off.

* Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js:
(WI.DOMManager.prototype.attachedNodes):
(WI.DOMManager.prototype.nodesWithLayoutContextType):

* Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js:
(WI.LayoutDetailsSidebarPanel.prototype.attached):
(WI.LayoutDetailsSidebarPanel.prototype.detached):
(WI.LayoutDetailsSidebarPanel.prototype._handleNodeInserted):
(WI.LayoutDetailsSidebarPanel.prototype._handleNodeRemoved):
(WI.LayoutDetailsSidebarPanel.prototype._handleLayoutContextTypeChanged):
(WI.LayoutDetailsSidebarPanel.prototype._removeNodeFromNodeSets):
(WI.LayoutDetailsSidebarPanel.prototype._invalidateNodeSets):
(WI.LayoutDetailsSidebarPanel.prototype._refreshNodeSets):
- Instead of iterating all the nodes twice every time something changes, iterate all the nodes once, an only do so when
doing layout to prevent multiple iterations for an eventual single layout.

* Source/WebInspectorUI/UserInterface/Views/NodeOverlayListSection.js:
(WI.NodeOverlayListSection.prototype._handleToggleAllCheckboxChanged):
- Fix for an assertion reachable while manually testing this patch. Toggling the overlay off when it is already off is
not allowed, and turning an overlay on that is already on is needlessly chatty over the protocol (unless changing the
settings/color, which is left untouched).

* LayoutTests/inspector/dom/attachedNodes-expected.txt: Added.
* LayoutTests/inspector/dom/attachedNodes.html: Added.

* LayoutTests/inspector/css/setLayoutContextTypeChangedMode.html:
- Update to use WI.domManager.attachedNodes() iterator.

Canonical link: https://commits.webkit.org/250897@main

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/inspector/css/setLayoutContextTypeChangedMode.html (294694 => 294695)


--- trunk/LayoutTests/inspector/css/setLayoutContextTypeChangedMode.html	2022-05-23 23:28:36 UTC (rev 294694)
+++ trunk/LayoutTests/inspector/css/setLayoutContextTypeChangedMode.html	2022-05-23 23:42:31 UTC (rev 294695)
@@ -22,17 +22,22 @@
         WI.cssManager.layoutContextTypeChangedMode = layoutContextTypeChangedMode;
     }
 
+    function nodesWithLayoutContextType(type)
+    {
+        return Array.from(WI.domManager.attachedNodes({filter: (node) => node.layoutContextType === type}));
+    }
+
     suite.addTestCase({
         name: "CSS.setLayoutContextTypeChangedMode.queryGrid",
         description: "Test that the expected number of grids are instrumented without chagning the LayoutContextTypeChangedMode.",
         async test() {
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 0, "0 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 0, "0 grid nodes should be instrumented.");
 
             // Query for the node, sending it to the frontend.
             InspectorTest.log("Querying document for selector `#queryGrid`...");
             let documentNode = await WI.domManager.requestDocument();
             let queryNode = WI.domManager.nodeForId(await documentNode.querySelector("#queryGrid"));
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
 
             InspectorTest.log("Changing `#queryGrid` to `display: block;`...");
             await Promise.all([
@@ -39,7 +44,7 @@
                 queryNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("queryGrid", "block"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 0, "0 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 0, "0 grid nodes should be instrumented.");
 
             InspectorTest.log("Changing `#queryGrid` to `display: grid;`...");
             await Promise.all([
@@ -46,7 +51,7 @@
                 queryNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("queryGrid", "grid"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
 
             InspectorTest.log("Changing `#queryGrid` to `display: block;`...");
             await Promise.all([
@@ -53,7 +58,7 @@
                 queryNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("queryGrid", "block"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 0, "0 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 0, "0 grid nodes should be instrumented.");
 
             InspectorTest.log("Changing `#queryGrid` to `display: inline-grid;`...");
             await Promise.all([
@@ -60,7 +65,7 @@
                 queryNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("queryGrid", "inline-grid"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
         }
     });
 
@@ -70,7 +75,7 @@
         async test() {
             await WI.domManager.requestDocument();
 
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
 
             // Grid layout contexts are sent to the frontend when the mode is changed to all.
             InspectorTest.log("Changing `layoutContextTypeChangedMode` to `All`...");
@@ -78,12 +83,12 @@
                 WI.domManager.awaitEvent(WI.DOMManager.Event.NodeInserted),
                 setLayoutContextTypeChangeMode(WI.CSSManager.LayoutContextTypeChangedMode.All),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
 
             // Once a node has been observed, it will always be kept up-to-date.
             InspectorTest.log("Changing `layoutContextTypeChangedMode` to `Observed`...");
             await setLayoutContextTypeChangeMode(WI.CSSManager.LayoutContextTypeChangedMode.Observed),
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
 
             InspectorTest.log("Changing `#normalGrid` to `display: block;`...");
             await Promise.all([
@@ -90,7 +95,7 @@
                 WI.DOMNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("normalGrid", "block"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 1, "1 grid node should be instrumented.");
 
             InspectorTest.log("Changing `#normalGrid` to `display: grid;`...");
             await Promise.all([
@@ -97,7 +102,7 @@
                 WI.DOMNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("normalGrid", "grid"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
         }
     });
 
@@ -107,21 +112,21 @@
         async test() {
             await WI.domManager.requestDocument();
 
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
 
             // Changes to unobserved nodes should not change the grid count.
             InspectorTest.log("Changing `#normalNonGrid` to `display: grid;`...");
             await changeElementDisplayValue("normalNonGrid", "grid");
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
 
             InspectorTest.log("Changing `#normalNonGrid` to `display: block;`...");
             await changeElementDisplayValue("normalNonGrid", "block");
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
 
             // Enabling `All` mode should not change the grid count.
             InspectorTest.log("Changing `layoutContextTypeChangedMode` to `All`...");
             await setLayoutContextTypeChangeMode(WI.CSSManager.LayoutContextTypeChangedMode.All),
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
 
             // Changing a node to a grid after enabling `All` mode will increase the count.
             InspectorTest.log("Changing `#normalNonGrid` to `display: grid;`...");
@@ -129,7 +134,7 @@
                 WI.DOMNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("normalNonGrid", "grid"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 3, "3 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 3, "3 grid nodes should be instrumented.");
 
             InspectorTest.log("Changing `#normalNonGrid` to `display: block;`...");
             await Promise.all([
@@ -136,7 +141,7 @@
                 WI.DOMNode.awaitEvent(WI.DOMNode.Event.LayoutContextTypeChanged),
                 changeElementDisplayValue("normalNonGrid", "block"),
             ]);
-            InspectorTest.expectEqual(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
+            InspectorTest.expectEqual(nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid).length, 2, "2 grid nodes should be instrumented.");
         }
     });
 

Added: trunk/LayoutTests/inspector/dom/attachedNodes-expected.txt (0 => 294695)


--- trunk/LayoutTests/inspector/dom/attachedNodes-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/dom/attachedNodes-expected.txt	2022-05-23 23:42:31 UTC (rev 294695)
@@ -0,0 +1,118 @@
+Tests for the DOMManager.attachedNodes.
+
+
+== Running test suite: DOM.attachedNodes
+-- Running test case: DOM.attachedNodes.Unfiltered
+Dumping nodes:
+  #document
+  html
+  html
+  head
+  script
+  script
+  [text node]
+  style
+  [text node]
+  body
+  p#test-description
+  [text node]
+  div#a
+  ::before
+  div#a1
+  div#a2
+  div#a3
+  ::after
+  div#b
+  div#b1
+  div#b2
+  div#b3
+
+-- Running test case: DOM.attachedNodes.Filtered
+Dumping nodes:
+  div#a
+  div#a1
+  div#a2
+  div#a3
+  div#b
+  div#b1
+  div#b2
+  div#b3
+
+-- Running test case: DOM.attachedNodes.DetachedNode
+Creating detached node...
+Created detached node: div#script-created-node
+Dumping nodes:
+  #document
+  html
+  html
+  head
+  script
+  script
+  [text node]
+  style
+  [text node]
+  body
+  p#test-description
+  [text node]
+  div#a
+  ::before
+  div#a1
+  div#a2
+  div#a3
+  ::after
+  div#b
+  div#b1
+  div#b2
+  div#b3
+Attaching node to DOM tree...
+DOM node attached to tree.
+Dumping nodes:
+  #document
+  html
+  html
+  head
+  script
+  script
+  [text node]
+  style
+  [text node]
+  body
+  p#test-description
+  [text node]
+  div#a
+  ::before
+  div#script-created-node
+  div#a1
+  div#a2
+  div#a3
+  ::after
+  div#b
+  div#b1
+  div#b2
+  div#b3
+Detaching node from DOM tree...
+DOM node detached from tree.
+Dumping nodes:
+  #document
+  html
+  html
+  head
+  script
+  script
+  [text node]
+  style
+  [text node]
+  body
+  p#test-description
+  [text node]
+  div#a
+  ::before
+  div#a1
+  div#a2
+  div#a3
+  ::after
+  div#b
+  div#b1
+  div#b2
+  div#b3
+

Added: trunk/LayoutTests/inspector/dom/attachedNodes.html (0 => 294695)


--- trunk/LayoutTests/inspector/dom/attachedNodes.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/dom/attachedNodes.html	2022-05-23 23:42:31 UTC (rev 294695)
@@ -0,0 +1,132 @@
+<!doctype html>
+<html>
+<head>
+<script src=""
+<script>
+function createDetachedNode()
+{
+    window.__testDetachedNode = document.createElement("div");
+    window.__testDetachedNode.id = "script-created-node";
+    return window.__testDetachedNode;
+}
+
+function insertDetachedNode()
+{
+    let parentElement = document.getElementById("a");
+    parentElement.insertBefore(window.__testDetachedNode, parentElement.firstChild);
+}
+
+function removePreviouslyDetachedNode()
+{
+    window.__testDetachedNode.remove();
+}
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("DOM.attachedNodes");
+
+    async function ensureDocumentSubtree() {
+        let documentNode = await WI.domManager.requestDocument();
+
+        const entireSubtreeDepth = -1;
+        await WI.assumingMainTarget().DOMAgent.requestChildNodes(documentNode.id, entireSubtreeDepth);
+    }
+
+    function logNodes(nodesIterator) {
+        InspectorTest.log("Dumping nodes:")
+        for (let node of nodesIterator) {
+            // Each test adds more content to the DOM tree, but we don't want to clutter test output with those.
+            if (node.escapedIdSelector === "#end-of-prewritten-test-content")
+                break;
+            
+            logNode(node);
+        }
+    }
+
+    function logNode(node) {
+        if (node.nodeType() === Node.TEXT_NODE)
+            InspectorTest.log("  [text node]");
+        else
+            InspectorTest.log("  " + node.unescapedSelector);
+    }
+
+    suite.addTestCase({
+        name: "DOM.attachedNodes.Unfiltered",
+        description: "Ensure that `attachedNodes` provides a correctly ordered list of all attached DOM nodes.",
+        async test() {
+            await ensureDocumentSubtree();
+            logNodes(WI.domManager.attachedNodes());
+        }
+    });
+
+    suite.addTestCase({
+        name: "DOM.attachedNodes.Filtered",
+        description: "Ensure that `attachedNodes` correctly filters nodes.",
+        async test() {
+            await ensureDocumentSubtree();
+            logNodes(WI.domManager.attachedNodes({filter: (node) => node.nodeNameInCorrectCase() === "div"}));
+        }
+    });
+
+    suite.addTestCase({
+        name: "DOM.attachedNodes.DetachedNode",
+        description: "Ensure that `attachedNodes` provides a correctly ordered list of all attached DOM nodes, and ignored detached nodes that are known to the backend.",
+        async test() {
+            await ensureDocumentSubtree();
+
+            InspectorTest.log("Creating detached node...");
+            let detachedNodeRemoteObject = await InspectorTest.evaluateInPage(`createDetachedNode()`);
+            let detachedNodeId = (await WI.assumingMainTarget().DOMAgent.requestNode(detachedNodeRemoteObject.objectId)).nodeId;
+            let detachedNode = WI.domManager.nodeForId(detachedNodeId);
+            InspectorTest.log("Created detached node: " + detachedNode.unescapedSelector);
+
+            logNodes(WI.domManager.attachedNodes());
+
+            InspectorTest.log("Attaching node to DOM tree...");
+            await Promise.all([
+                InspectorTest.evaluateInPage(`insertDetachedNode()`),
+                WI.domManager.awaitEvent(WI.DOMManager.Event.NodeInserted),
+            ]);
+            InspectorTest.log("DOM node attached to tree.");
+
+            logNodes(WI.domManager.attachedNodes());
+
+            InspectorTest.log("Detaching node from DOM tree...");
+            await Promise.all([
+                InspectorTest.evaluateInPage(`removePreviouslyDetachedNode()`),
+                WI.domManager.awaitEvent(WI.DOMManager.Event.NodeRemoved),
+            ]);
+            InspectorTest.log("DOM node detached from tree.");
+
+            logNodes(WI.domManager.attachedNodes());
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+<style>
+#a::before {
+    content: "before";
+}
+
+#a::after {
+    content: "after";
+}
+</style>
+</head>
+<body _onload_="runTest()">
+<p id="test-description">Tests for the DOMManager.attachedNodes.</p>
+<div id="a">
+    <div id="a1"></div>
+    <div id="a2"></div>
+    <div id="a3"></div>
+</div>
+<div id="b">
+    <div id="b1"></div>
+    <div id="b2"></div>
+    <div id="b3"></div>
+</div>
+<div id="end-of-prewritten-test-content"></div>
+</body>
+</html>

Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js (294694 => 294695)


--- trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js	2022-05-23 23:28:36 UTC (rev 294694)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js	2022-05-23 23:42:31 UTC (rev 294695)
@@ -154,6 +154,49 @@
         return Array.from(this._breakpointsForEventListeners.values());
     }
 
+    *attachedNodes({filter} = {})
+    {
+        if (!this._document)
+            return;
+
+        filter ??= (node) => true;
+
+        // Traverse the node tree in the same order items would appear if the entire tree were expanded in order to
+        // provide a predictable order for the results.
+        let currentBranch = [this._document];
+        while (currentBranch.length) {
+            let currentNode = currentBranch.at(-1);
+
+            if (filter(currentNode))
+                yield currentNode;
+
+            // The `::before` pseudo element is the first child of any node.
+            let beforePseudoElement = currentNode.beforePseudoElement();
+            if (beforePseudoElement && filter(beforePseudoElement))
+                yield beforePseudoElement;
+
+            let firstChild = currentNode.children?.[0];
+            if (firstChild) {
+                currentBranch.push(firstChild);
+                continue;
+            }
+
+            while (currentBranch.length) {
+                let parent = currentBranch.pop();
+
+                // The `::after` pseudo element is the last child of any node.
+                let parentAfterPseudoElement = parent.afterPseudoElement();
+                if (parentAfterPseudoElement && filter(parentAfterPseudoElement))
+                    yield parentAfterPseudoElement;
+
+                if (parent.nextSibling) {
+                    currentBranch.push(parent.nextSibling);
+                    break;
+                }
+            }
+        }
+    }
+
     requestDocument(callback)
     {
         if (typeof callback !== "function")
@@ -240,11 +283,6 @@
         domNode.layoutContextType = layoutContextType;
     }
 
-    nodesWithLayoutContextType(layoutContextType)
-    {
-        return Object.values(this._idToDOMNode).filter((node) => node.layoutContextType === layoutContextType);
-    }
-
     // Private
 
     _dispatchWhenDocumentAvailable(func, callback)

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js (294694 => 294695)


--- trunk/Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js	2022-05-23 23:28:36 UTC (rev 294694)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js	2022-05-23 23:42:31 UTC (rev 294695)
@@ -29,8 +29,8 @@
     {
         super("layout-details", WI.UIString("Layout", "Layout @ Styles Sidebar", "Title of the CSS style panel."));
 
-        this._flexNodeSet = new Set;
-        this._gridNodeSet = new Set;
+        this._flexNodeSet = null;
+        this._gridNodeSet = null;
         this._nodeStyles = null;
         this.element.classList.add("layout-panel");
     }
@@ -78,12 +78,15 @@
     {
         super.attached();
 
+        WI.domManager.addEventListener(WI.DOMManager.Event.NodeInserted, this._handleNodeInserted, this);
+        WI.domManager.addEventListener(WI.DOMManager.Event.NodeRemoved, this._handleNodeRemoved, this);
+
         WI.DOMNode.addEventListener(WI.DOMNode.Event.LayoutContextTypeChanged, this._handleLayoutContextTypeChanged, this);
         WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
 
         WI.cssManager.layoutContextTypeChangedMode = WI.CSSManager.LayoutContextTypeChangedMode.All;
 
-        this._refreshNodeSets();
+        this._invalidateNodeSets();
     }
 
     detached()
@@ -90,6 +93,9 @@
     {
         WI.cssManager.layoutContextTypeChangedMode = WI.CSSManager.LayoutContextTypeChangedMode.Observed;
 
+        WI.domManager.removeEventListener(WI.DOMManager.Event.NodeInserted, this._handleNodeInserted, this);
+        WI.domManager.removeEventListener(WI.DOMManager.Event.NodeRemoved, this._handleNodeRemoved, this);
+
         WI.DOMNode.removeEventListener(WI.DOMNode.Event.LayoutContextTypeChanged, this._handleLayoutContextTypeChanged, this);
         WI.Frame.removeEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
 
@@ -116,6 +122,9 @@
     {
         super.layout();
 
+        if (!this._gridNodeSet || !this._flexNodeSet)
+            this._refreshNodeSets();
+
         if (this._gridNodeSet.size) {
             this._gridDetailsSectionRow.hideEmptyMessage();
             this._gridDetailsSectionRow.element.appendChild(this._gridSection.element);
@@ -149,26 +158,27 @@
 
     // Private
 
+    _handleNodeInserted(event)
+    {
+        this._invalidateNodeSets();
+        this.needsLayout();
+    }
+
+    _handleNodeRemoved(event)
+    {
+        let domNode = event.target.node;
+        this._removeNodeFromNodeSets(domNode);
+        this.needsLayout();
+    }
+
     _handleLayoutContextTypeChanged(event)
     {
         let domNode = event.target;
+        if (domNode.layoutContextType)
+            this._invalidateNodeSets();
+        else
+            this._removeNodeFromNodeSets(domNode);
 
-        // A node may switch layout context type between grid and flex.
-        // Remove it from both node sets in case it was previously added.
-        // It is also the default case when the layout context type switches to something unknown.
-        this._flexNodeSet.delete(domNode);
-        this._gridNodeSet.delete(domNode);
-
-        switch (domNode.layoutContextType) {
-        case WI.DOMNode.LayoutContextType.Grid:
-            this._gridNodeSet.add(domNode);
-            break;
-
-        case WI.DOMNode.LayoutContextType.Flex:
-            this._flexNodeSet.add(domNode);
-            break;
-        }
-
         this.needsLayout();
     }
 
@@ -192,9 +202,37 @@
             this._nodeStyles?.refresh();
     }
 
+    _removeNodeFromNodeSets(domNode)
+    {
+        this._flexNodeSet?.delete(domNode);
+        this._gridNodeSet?.delete(domNode);
+    }
+
+    _invalidateNodeSets()
+    {
+        this._flexNodeSet = null;
+        this._gridNodeSet = null;
+    }
+
     _refreshNodeSets()
     {
-        this._gridNodeSet = new Set(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Grid));
-        this._flexNodeSet = new Set(WI.domManager.nodesWithLayoutContextType(WI.DOMNode.LayoutContextType.Flex));
+        this._gridNodeSet = new Set;
+        this._flexNodeSet = new Set;
+
+        for (let node of WI.domManager.attachedNodes({filter: (node) => node.layoutContextType})) {
+            switch (node.layoutContextType) {
+            case WI.DOMNode.LayoutContextType.Grid:
+                this._gridNodeSet.add(node);
+                break;
+
+            case WI.DOMNode.LayoutContextType.Flex:
+                this._flexNodeSet.add(node);
+                break;
+
+            default:
+                console.assert(false, "Unknown layout context type.", node, node.type);
+                break;
+            }
+        }
     }
 };

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NodeOverlayListSection.js (294694 => 294695)


--- trunk/Source/WebInspectorUI/UserInterface/Views/NodeOverlayListSection.js	2022-05-23 23:28:36 UTC (rev 294694)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NodeOverlayListSection.js	2022-05-23 23:42:31 UTC (rev 294695)
@@ -160,9 +160,9 @@
         this._suppressUpdateToggleAllCheckbox = true;
 
         for (let domNode of this._nodeSet) {
-            if (isChecked)
+            if (isChecked && !domNode.layoutOverlayShowing)
                 domNode.showLayoutOverlay();
-            else
+            else if (!isChecked && domNode.layoutOverlayShowing)
                 domNode.hideLayoutOverlay();
         }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to