Title: [295100] trunk
Revision
295100
Author
pan...@apple.com
Date
2022-06-01 11:52:36 -0700 (Wed, 01 Jun 2022)

Log Message

Web Inspector: Even after r293565, button/select elements created after Web Inspector is open are considered Flexbox containers
https://bugs.webkit.org/show_bug.cgi?id=241054
rdar://94063718

Reviewed by Devin Rousso.

r293565 updated the logic for determining the layout type for RenderObjects, but that fix did not account for the fact
that `InspectorCSSAgent::nodeLayoutContextTypeChanged` is called during the creation of `RenderFlexibleBox`, at which
point the creation of subclass-specific bits, including overrides will not have occurred, including `isFlexibleBoxImpl`
which we use to determine if the flexbox container is a "real" flexbox container, or an internal implementation detail.
We should instead determine the layout context type later just before we send the event to the frontend (it is already
delayed specifically because `nodeLayoutContextTypeChanged` can be called in destructors, which can be the result of
garbage collection). This doesn't change when the frontend receives any information, only adjust when we resolve the
layout context type.

* LayoutTests/inspector/css/nodeLayoutContextTypeChanged-expected.txt:
* LayoutTests/inspector/css/nodeLayoutContextTypeChanged.html:
* Source/WebCore/inspector/agents/InspectorCSSAgent.cpp:
* Source/WebCore/inspector/agents/InspectorCSSAgent.h:

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

Modified Paths

Diff

Modified: trunk/LayoutTests/inspector/css/nodeLayoutContextTypeChanged-expected.txt (295099 => 295100)


--- trunk/LayoutTests/inspector/css/nodeLayoutContextTypeChanged-expected.txt	2022-06-01 18:51:03 UTC (rev 295099)
+++ trunk/LayoutTests/inspector/css/nodeLayoutContextTypeChanged-expected.txt	2022-06-01 18:52:36 UTC (rev 295100)
@@ -32,3 +32,7 @@
 -- Running test case: CSS.nodeLayoutContextTypeChanged.NotFlex.Button
 PASS: Layout context should be `null`.
 
+-- Running test setup.
+-- Running test case: CSS.nodeLayoutContextTypeChanged.NotFlex.DynamicallyAddedButton
+PASS: Layout context should be `null`.
+

Modified: trunk/LayoutTests/inspector/css/nodeLayoutContextTypeChanged.html (295099 => 295100)


--- trunk/LayoutTests/inspector/css/nodeLayoutContextTypeChanged.html	2022-06-01 18:51:03 UTC (rev 295099)
+++ trunk/LayoutTests/inspector/css/nodeLayoutContextTypeChanged.html	2022-06-01 18:52:36 UTC (rev 295100)
@@ -8,6 +8,13 @@
     document.getElementById(id).style.display = value;
 }
 
+function appendElementToBody(tag, id)
+{
+    let element = document.createElement(tag);
+    element.id = id;
+    document.body.appendChild(element);
+}
+
 function test()
 {
     let documentNode;
@@ -14,11 +21,12 @@
 
     let suite = InspectorTest.createAsyncSuite("CSS.nodeLayoutContextTypeChanged");
 
-    function addTestCase({name, description, selector, domNodeHandler})
+    function addTestCase({name, description, selector, setup, domNodeHandler})
     {
         suite.addTestCase({
             name,
             description,
+            setup,
             async test() {
                 let nodeId = await documentNode.querySelector(selector);
                 let domNode = WI.domManager.nodeForId(nodeId);
@@ -33,6 +41,11 @@
         await InspectorTest.evaluateInPage(`changeElementDisplayValue("${id}", "${value}")`);
     }
 
+    async function appendElementToBody(tag, id)
+    {
+        await InspectorTest.evaluateInPage(`appendElementToBody("${tag}", "${id}")`);
+    }
+
     addTestCase({
         name: "CSS.nodeLayoutContextTypeChanged.GridToNonGrid",
         description: "Change a `grid` to a non-grid.",
@@ -113,9 +126,9 @@
         }
     });
 
-    function addEnsureLayoutContextTypeTestCase({name, description, selector, expectedLayoutContextType})
+    function addEnsureLayoutContextTypeTestCase({name, description, selector, expectedLayoutContextType, setup})
     {
-        addTestCase({name, description, selector, async domNodeHandler(domNode) {
+        addTestCase({name, description, selector, setup, async domNodeHandler(domNode) {
                 InspectorTest.expectEqual(domNode.layoutContextType, expectedLayoutContextType, `Layout context should be \`${expectedLayoutContextType}\`.`);
             }
         });
@@ -142,6 +155,16 @@
         expectedLayoutContextType: null,
     });
 
+    addEnsureLayoutContextTypeTestCase({
+        name: "CSS.nodeLayoutContextTypeChanged.NotFlex.DynamicallyAddedButton",
+        description: "Make sure a `button` that is added dynamically is not considered a flex container.",
+        selector: "#dynamicallyAddedFlexButton",
+        expectedLayoutContextType: null,
+        async setup() {
+            await appendElementToBody("button", "dynamicallyAddedFlexButton");
+        }
+    });
+
     WI.domManager.requestDocument().then((doc) => {
         documentNode = doc;
         suite.runTestCasesAndFinish();

Modified: trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.cpp (295099 => 295100)


--- trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.cpp	2022-06-01 18:51:03 UTC (rev 295099)
+++ trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.cpp	2022-06-01 18:52:36 UTC (rev 295100)
@@ -1000,7 +1000,7 @@
     if (!nodeId)
         return;
 
-    m_nodesWithPendingLayoutContextTypeChanges.set(nodeId, layoutContextTypeForRenderer(newRenderer));
+    m_nodesWithPendingLayoutContextTypeChanges.set(nodeId, newRenderer);
     if (!m_layoutContextTypeChangedTimer.isActive())
         m_layoutContextTypeChangedTimer.startOneShot(0_s);
 }
@@ -1007,8 +1007,8 @@
 
 void InspectorCSSAgent::layoutContextTypeChangedTimerFired()
 {
-    for (auto&& [nodeId, layoutContextType] : std::exchange(m_nodesWithPendingLayoutContextTypeChanges, { }))
-        m_frontendDispatcher->nodeLayoutContextTypeChanged(nodeId, WTFMove(layoutContextType));
+    for (auto&& [nodeId, renderer] : std::exchange(m_nodesWithPendingLayoutContextTypeChanges, { }))
+        m_frontendDispatcher->nodeLayoutContextTypeChanged(nodeId, layoutContextTypeForRenderer(renderer.get()));
 }
 
 InspectorStyleSheetForInlineStyle& InspectorCSSAgent::asInspectorStyleSheet(StyledElement& element)

Modified: trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.h (295099 => 295100)


--- trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.h	2022-06-01 18:51:03 UTC (rev 295099)
+++ trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.h	2022-06-01 18:52:36 UTC (rev 295100)
@@ -177,7 +177,7 @@
     int m_lastStyleSheetId { 1 };
     bool m_creatingViaInspectorStyleSheet { false };
 
-    HashMap<Inspector::Protocol::DOM::NodeId, std::optional<Inspector::Protocol::CSS::LayoutContextType>> m_nodesWithPendingLayoutContextTypeChanges;
+    HashMap<Inspector::Protocol::DOM::NodeId, WeakPtr<RenderObject>> m_nodesWithPendingLayoutContextTypeChanges;
     Timer m_layoutContextTypeChangedTimer;
     Inspector::Protocol::CSS::LayoutContextTypeChangedMode m_layoutContextTypeChangedMode { Inspector::Protocol::CSS::LayoutContextTypeChangedMode::Observed };
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to