Web Inspector: XHR request with same URL as main resource should have…

… type XHR https://bugs.webkit.org/show_bug.cgi?id=257407 Reviewed by Devin Rousso. Only use cached resource type when couldn't infer resource type based on the current ResourceRequest because CachedResource for the same URL may have different type. This fixes the bug where XHR is sent with the same URL as the main resource and was wrongly marked as another request with type Document rather than XHR. * LayoutTests/http/tests/inspector/network/xhr-request-type-expected.txt: Added. * LayoutTests/http/tests/inspector/network/xhr-request-type.html: Added. * Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp: (WebCore::InspectorNetworkAgent::willSendRequest): * LayoutTests/platform/mac-wk1/TestExpectations: ignore new test in WK1. Canonical link: https://commits.webkit.org/264686@main
WebKit · May 30, 2023 · 883556d · 883556d
1 parent 38a1b59
commit 883556d
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 19 deletions.
diff --git a/LayoutTests/http/tests/inspector/network/xhr-request-type-expected.txt b/LayoutTests/http/tests/inspector/network/xhr-request-type-expected.txt
@@ -0,0 +1,17 @@
+Tests that XHRs requests have type XHR, inlcuding those with the same URL as the main resource.
+
+Bug 68646 Bug 257407
+
+== Running test suite: Resource.Type.XHR
+-- Running test case: Resource.Type.XHR.Same.As.Main.Resource.Url
+PASS: Resource should be XHR type.
+PASS: Resource should be a GET request.
+PASS: Resource should have a 200 response.
+PASS: Resource should receive main resource in response.
+
+-- Running test case: Resource.Type.XHR.Another.Url
+PASS: Resource should be XHR type.
+PASS: Resource should be a GET request.
+PASS: Resource should have a 200 response.
+PASS: Resource should receive json in response.
+
diff --git a/LayoutTests/http/tests/inspector/network/xhr-request-type.html b/LayoutTests/http/tests/inspector/network/xhr-request-type.html
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../resources/inspector-test.js"></script>
+<script>
+function triggerXHR(path) {
+    const x = new XMLHttpRequest();
+    x.open("GET", path || location.href, false);
+    x.send();
+}
+
+// ----
+
+function test()
+{
+    const suite = InspectorTest.createAsyncSuite("Resource.Type.XHR");
+
+    function addTestCase({name, description, expression, resourceHandler}) {
+        suite.addTestCase({
+            name, description,
+            async test() {
+                const completeEvent = InspectorTest.awaitEvent("Completed");
+                InspectorTest.evaluateInPage(expression);
+                const event = await WI.Frame.awaitEvent(WI.Frame.Event.ResourceWasAdded);
+                const resource = event.data.resource;
+                InspectorTest.expectEqual(resource.type, WI.Resource.Type.XHR, "Resource should be XHR type.");
+                InspectorTest.expectEqual(resource.requestMethod, "GET", "Resource should be a GET request.");
+                await resource.awaitEvent(WI.Resource.Event.LoadingDidFinish);
+                const content = await resource.requestContentFromBackend()
+                resourceHandler(resource, content);
+            }
+        });
+    }
+
+    addTestCase({
+        name: "Resource.Type.XHR.Same.As.Main.Resource.Url",
+        description: "XHR with the same URL as main resource.",
+        expression: "triggerXHR()",
+        resourceHandler(resource, content) {
+            InspectorTest.expectEqual(resource.statusCode, 200, "Resource should have a 200 response.");
+            InspectorTest.expectThat(content.body.includes("Tests that XHRs with the same url as a main resource have correct type"), "Resource should receive main resource in response.");
+        }
+    });
+
+    addTestCase({
+        name: "Resource.Type.XHR.Another.Url",
+        description: "XHR with the same URL as main resource.",
+        expression: "triggerXHR('/inspector/network/resources/data.json')",
+        resourceHandler(resource, content) {
+            InspectorTest.expectEqual(resource.statusCode, 200, "Resource should have a 200 response.");
+            InspectorTest.expectEqual(content.body, `{"json": true, "value": 42}\n`, "Resource should receive json in response.");
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Tests that XHRs requests have type XHR, inlcuding those with the same URL as the main resource.</p>
+<a href="https://bugs.webkit.org/show_bug.cgi?id=68646">Bug 68646</a>
+<a href="https://bugs.webkit.org/show_bug.cgi?id=257407">Bug 257407</a>
+<div id="log"></div>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -1567,6 +1567,9 @@ http/tests/inspector/network/resource-response-inspector-override.html [ Skip ]
 inspector/network/intercept-aborted-request.html [ Skip ]
 inspector/network/interceptContinue.html [ Skip ]
 
+# Resource content retrival times out in WebKit1.
+webkit.org/b/257407 http/tests/inspector/network/xhr-request-type.html [ Skip ]
+
 webkit.org/b/164933 http/tests/misc/link-rel-icon-beforeload.html [ Failure ]
 
 # rdar://92764272 ([ Ventura wk1 ] 18 X fast/html/details (layout-tests) are flaky text failures)

diff --git a/Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp b/Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp
@@ -471,22 +471,6 @@ void InspectorNetworkAgent::willSendRequest(ResourceLoaderIdentifier identifier,
     auto loaderId = loaderIdentifier(loader);
     String targetId = request.initiatorIdentifier();
 
-    if (type == InspectorPageAgent::OtherResource) {
-        if (m_loadingXHRSynchronously || request.requester() == ResourceRequestRequester::XHR)
-            type = InspectorPageAgent::XHRResource;
-        else if (request.requester() == ResourceRequestRequester::Fetch)
-            type = InspectorPageAgent::FetchResource;
-        else if (loader && equalIgnoringFragmentIdentifier(request.url(), loader->url()) && !loader->isCommitted())
-            type = InspectorPageAgent::DocumentResource;
-        else if (loader) {
-            for (auto& linkIcon : loader->linkIcons()) {
-                if (equalIgnoringFragmentIdentifier(request.url(), linkIcon.url)) {
-                    type = InspectorPageAgent::ImageResource;
-                    break;
-                }
-            }
-        }
-    }
 
     m_resourcesData->resourceCreated(requestId, loaderId, type);
 
@@ -528,9 +512,27 @@ static InspectorPageAgent::ResourceType resourceTypeForLoadType(InspectorInstrum
 
 void InspectorNetworkAgent::willSendRequest(ResourceLoaderIdentifier identifier, DocumentLoader* loader, ResourceRequest& request, const ResourceResponse& redirectResponse, const CachedResource* cachedResource, ResourceLoader* resourceLoader)
 {
-    if (!cachedResource && loader)
-        cachedResource = InspectorPageAgent::cachedResource(loader->frame(), request.url());
-    willSendRequest(identifier, loader, request, redirectResponse, resourceTypeForCachedResource(cachedResource), resourceLoader);
+    InspectorPageAgent::ResourceType type = InspectorPageAgent::OtherResource;
+    if (m_loadingXHRSynchronously || request.requester() == ResourceRequestRequester::XHR)
+        type = InspectorPageAgent::XHRResource;
+    else if (request.requester() == ResourceRequestRequester::Fetch)
+        type = InspectorPageAgent::FetchResource;
+    else if (loader && equalIgnoringFragmentIdentifier(request.url(), loader->url()) && !loader->isCommitted())
+        type = InspectorPageAgent::DocumentResource;
+    else if (loader) {
+        for (auto& linkIcon : loader->linkIcons()) {
+            if (equalIgnoringFragmentIdentifier(request.url(), linkIcon.url)) {
+                type = InspectorPageAgent::ImageResource;
+                break;
+            }
+        }
+    }
+    if (type == InspectorPageAgent::OtherResource) {
+        if (!cachedResource && loader)
+            cachedResource = InspectorPageAgent::cachedResource(loader->frame(), request.url());
+        type = resourceTypeForCachedResource(cachedResource);
+    }
+    willSendRequest(identifier, loader, request, redirectResponse, type, resourceLoader);
 }
 
 void InspectorNetworkAgent::willSendRequestOfType(ResourceLoaderIdentifier identifier, DocumentLoader* loader, ResourceRequest& request, InspectorInstrumentation::LoadType loadType)