Title: [294234] trunk
Revision
294234
Author
pan...@apple.com
Date
2022-05-16 08:52:10 -0700 (Mon, 16 May 2022)

Log Message

Web Inspector: Regression(r266885) Crash sometimes when rehydrating imported audit results
https://bugs.webkit.org/show_bug.cgi?id=240366

Reviewed by Devin Rousso.

Source/_javascript_Core:

* inspector/protocol/DOM.json:

Source/WebCore:

Added test cases to inspector/model/dom-node.html

After r266885, there is no path to handle the possibility that there may not be a resulting node for calls to
`DOM.querySelector`. To correct this, mark the return value as optional (Web Inspector frontend already treats
it as optional) and return early if there was no Element matching the selector.

* inspector/agents/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::querySelector):
* inspector/agents/InspectorDOMAgent.h:

LayoutTests:

* inspector/model/dom-node.html:
* inspector/model/dom-node-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (294233 => 294234)


--- trunk/LayoutTests/ChangeLog	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/LayoutTests/ChangeLog	2022-05-16 15:52:10 UTC (rev 294234)
@@ -1,3 +1,13 @@
+2022-05-16  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: Regression(r266885) Crash sometimes when rehydrating imported audit results
+        https://bugs.webkit.org/show_bug.cgi?id=240366
+
+        Reviewed by Devin Rousso.
+
+        * inspector/model/dom-node.html:
+        * inspector/model/dom-node-expected.txt:
+
 2022-05-16  Rob Buis  <rb...@igalia.com>
 
         Remove some custom-element tests

Modified: trunk/LayoutTests/inspector/model/dom-node-expected.txt (294233 => 294234)


--- trunk/LayoutTests/inspector/model/dom-node-expected.txt	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/LayoutTests/inspector/model/dom-node-expected.txt	2022-05-16 15:52:10 UTC (rev 294234)
@@ -7,3 +7,11 @@
 class="test-class"
 data-item="test-data"
 
+-- Running test case: WI.DOMNode.querySelector
+Calling querySelector("#test-id") on document node.
+PASS: `querySelector("#test-id")` should return a WI.DOMNode
+Calling querySelector("#non-existent-id") on document node.
+PASS: `querySelector("#non-existent-id")` should return null.
+Calling querySelector("^\_(invalid selector)_/^") on document node.
+PASS: `querySelector` with an invalid selector should throw a SyntaxError.
+

Modified: trunk/LayoutTests/inspector/model/dom-node.html (294233 => 294234)


--- trunk/LayoutTests/inspector/model/dom-node.html	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/LayoutTests/inspector/model/dom-node.html	2022-05-16 15:52:10 UTC (rev 294234)
@@ -25,6 +25,34 @@
         }
     });
 
+    suite.addTestCase({
+        name: "WI.DOMNode.querySelector",
+        description: "Test getting a child node via querySelector.",
+        async test() {
+            let documentNode = await WI.domManager.requestDocument();
+
+            function querySelector(selector) {
+                InspectorTest.log(`Calling querySelector("${selector}") on document node.`);
+                return documentNode.querySelector(selector).then((nodeId) => {
+                    if (!nodeId)
+                        return null;
+                    return WI.domManager.nodeForId(nodeId);
+                });
+            }
+
+            let nodeFromQueryingExistingId = await querySelector("#test-id");
+            InspectorTest.expectThat(nodeFromQueryingExistingId instanceof WI.DOMNode, "`querySelector(\"#test-id\")` should return a WI.DOMNode");
+
+            let nodeFromQueryingNonExistantId = await querySelector("#non-existent-id");
+            InspectorTest.expectNull(nodeFromQueryingNonExistantId, "`querySelector(\"#non-existent-id\")` should return null.");
+
+            await querySelector("^\\_(invalid selector)_/^").catch((error) => {
+                InspectorTest.expectEqual(error.message, "SyntaxError", "`querySelector` with an invalid selector should throw a SyntaxError.");
+            });
+
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>

Modified: trunk/Source/_javascript_Core/ChangeLog (294233 => 294234)


--- trunk/Source/_javascript_Core/ChangeLog	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/Source/_javascript_Core/ChangeLog	2022-05-16 15:52:10 UTC (rev 294234)
@@ -1,3 +1,12 @@
+2022-05-16  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: Regression(r266885) Crash sometimes when rehydrating imported audit results
+        https://bugs.webkit.org/show_bug.cgi?id=240366
+
+        Reviewed by Devin Rousso.
+
+        * inspector/protocol/DOM.json:
+
 2022-05-13  Mark Lam  <mark....@apple.com>
 
         Enhance the ARM64Disassembler to print pc indices and better branch target labels.

Modified: trunk/Source/_javascript_Core/inspector/protocol/DOM.json (294233 => 294234)


--- trunk/Source/_javascript_Core/inspector/protocol/DOM.json	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/Source/_javascript_Core/inspector/protocol/DOM.json	2022-05-16 15:52:10 UTC (rev 294234)
@@ -204,7 +204,7 @@
                 { "name": "selector", "type": "string", "description": "Selector string." }
             ],
             "returns": [
-                { "name": "nodeId", "$ref": "NodeId", "description": "Query selector result." }
+                { "name": "nodeId", "$ref": "NodeId", "optional": true, "description": "Query selector result." }
             ]
         },
         {

Modified: trunk/Source/WebCore/ChangeLog (294233 => 294234)


--- trunk/Source/WebCore/ChangeLog	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/Source/WebCore/ChangeLog	2022-05-16 15:52:10 UTC (rev 294234)
@@ -1,3 +1,20 @@
+2022-05-16  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: Regression(r266885) Crash sometimes when rehydrating imported audit results
+        https://bugs.webkit.org/show_bug.cgi?id=240366
+
+        Reviewed by Devin Rousso.
+
+        Added test cases to inspector/model/dom-node.html
+
+        After r266885, there is no path to handle the possibility that there may not be a resulting node for calls to
+        `DOM.querySelector`. To correct this, mark the return value as optional (Web Inspector frontend already treats
+        it as optional) and return early if there was no Element matching the selector.
+
+        * inspector/agents/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::querySelector):
+        * inspector/agents/InspectorDOMAgent.h:
+
 2022-05-16  Alan Bujtas  <za...@apple.com>
 
         [LFC][FFC] Add "flex-direction: row-reverse" basic visual/logical conversion

Modified: trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp (294233 => 294234)


--- trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp	2022-05-16 15:52:10 UTC (rev 294234)
@@ -592,7 +592,7 @@
     return { };
 }
 
-Protocol::ErrorStringOr<Protocol::DOM::NodeId> InspectorDOMAgent::querySelector(Protocol::DOM::NodeId nodeId, const String& selector)
+Protocol::ErrorStringOr<std::optional<Protocol::DOM::NodeId>> InspectorDOMAgent::querySelector(Protocol::DOM::NodeId nodeId, const String& selector)
 {
     Protocol::ErrorString errorString;
 
@@ -606,11 +606,15 @@
     if (queryResult.hasException())
         return makeUnexpected(InspectorDOMAgent::toErrorString(queryResult.releaseException()));
 
-    auto resultNodeId = pushNodePathToFrontend(errorString, queryResult.releaseReturnValue());
+    auto* queryResultNode = queryResult.releaseReturnValue();
+    if (!queryResultNode)
+        return { };
+
+    auto resultNodeId = pushNodePathToFrontend(errorString, queryResultNode);
     if (!resultNodeId)
         return makeUnexpected(errorString);
 
-    return resultNodeId;
+    return { resultNodeId };
 }
 
 Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Protocol::DOM::NodeId>>> InspectorDOMAgent::querySelectorAll(Protocol::DOM::NodeId nodeId, const String& selector)

Modified: trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.h (294233 => 294234)


--- trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.h	2022-05-16 15:08:11 UTC (rev 294233)
+++ trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.h	2022-05-16 15:52:10 UTC (rev 294234)
@@ -106,7 +106,7 @@
     void willDestroyFrontendAndBackend(Inspector::DisconnectReason);
 
     // DOMBackendDispatcherHandler
-    Inspector::Protocol::ErrorStringOr<Inspector::Protocol::DOM::NodeId> querySelector(Inspector::Protocol::DOM::NodeId, const String& selector);
+    Inspector::Protocol::ErrorStringOr<std::optional<Inspector::Protocol::DOM::NodeId>> querySelector(Inspector::Protocol::DOM::NodeId, const String& selector);
     Inspector::Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Inspector::Protocol::DOM::NodeId>>> querySelectorAll(Inspector::Protocol::DOM::NodeId, const String& selector);
     Inspector::Protocol::ErrorStringOr<Ref<Inspector::Protocol::DOM::Node>> getDocument();
     Inspector::Protocol::ErrorStringOr<void> requestChildNodes(Inspector::Protocol::DOM::NodeId, std::optional<int>&& depth);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to