- Revision
- 293264
- Author
- pan...@apple.com
- Date
- 2022-04-22 16:19:24 -0700 (Fri, 22 Apr 2022)
Log Message
Web Inspector: Regression(r287684) Resources from the memory cache show empty content in Network, Sources, and Search tabs
https://bugs.webkit.org/show_bug.cgi?id=239667
Reviewed by Devin Rousso.
Source/WebCore:
Updated tests:
- http/tests/inspector/network/resource-response-source-disk-cache.html
- http/tests/inspector/network/resource-response-source-memory-cache.html
r287684 introduced a subtle bug when calling InspectorInstrumentation::didReceiveData. We rely
on there being a difference between real, but empty, data buffer and not having a data buffer,
but after r287684 an empty SharedBuffer would be passed in the later case. Instead, we should
continue to pass nullptr if there is no buffer so that InspectorNetworkAgent::didReceiveData
can distiguish between the two.
The bug is the result of having a non-nullptr `data` in `InspectorNetworkAgent::didReceiveData`,
which causes us to call `maybeAddResourceData`, which means by the time
`InspectorNetworkAgent::getResponseBody` is called, the ResourceData for the request will have
an empty, not non-existant, `content()`, which means we will return the empty content instead,
since we believe the response had actual content.
* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::didReceiveDataImpl):
* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::didReceiveData):
* loader/ResourceLoadNotifier.cpp:
(WebCore::ResourceLoadNotifier::dispatchDidReceiveData):
LayoutTests:
Add test steps to ensure that the resource has content, and that the base64Encoded value matches our
expectations. While only the memory cache was affected by this regression, add the test steps to both
the memory and disk caches to defend this functionality.
* http/tests/inspector/network/resource-response-source-memory-cache-expected.txt:
* http/tests/inspector/network/resource-response-source-memory-cache.html:
* http/tests/inspector/network/resource-response-source-disk-cache-expected.txt:
* http/tests/inspector/network/resource-response-source-disk-cache.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (293263 => 293264)
--- trunk/LayoutTests/ChangeLog 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/LayoutTests/ChangeLog 2022-04-22 23:19:24 UTC (rev 293264)
@@ -1,3 +1,19 @@
+2022-04-22 Patrick Angle <pan...@apple.com>
+
+ Web Inspector: Regression(r287684) Resources from the memory cache show empty content in Network, Sources, and Search tabs
+ https://bugs.webkit.org/show_bug.cgi?id=239667
+
+ Reviewed by Devin Rousso.
+
+ Add test steps to ensure that the resource has content, and that the base64Encoded value matches our
+ expectations. While only the memory cache was affected by this regression, add the test steps to both
+ the memory and disk caches to defend this functionality.
+
+ * http/tests/inspector/network/resource-response-source-memory-cache-expected.txt:
+ * http/tests/inspector/network/resource-response-source-memory-cache.html:
+ * http/tests/inspector/network/resource-response-source-disk-cache-expected.txt:
+ * http/tests/inspector/network/resource-response-source-disk-cache.html:
+
2022-04-22 Simon Fraser <simon.fra...@apple.com>
Focusing scroll container before scrolling breaks smooth scrolling
Modified: trunk/LayoutTests/http/tests/inspector/network/resource-response-source-disk-cache-expected.txt (293263 => 293264)
--- trunk/LayoutTests/http/tests/inspector/network/resource-response-source-disk-cache-expected.txt 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/LayoutTests/http/tests/inspector/network/resource-response-source-disk-cache-expected.txt 2022-04-22 23:19:24 UTC (rev 293264)
@@ -6,6 +6,8 @@
-- Running test case: PossibleNetworkLoad
PASS: Resource should be created.
PASS: Resource should receive a Response.
+PASS: Response `body` should not be empty.
+PASS: Response should be base64 encoded.
-- Running test case: Resource.ResponseSource.DiskCache
PASS: Resource should be created.
@@ -12,4 +14,6 @@
PASS: Resource should receive a Response.
PASS: statusCode should be 200
PASS: responseSource should be Symbol(disk-cache)
+PASS: Response `body` should not be empty.
+PASS: Response should be base64 encoded.
Modified: trunk/LayoutTests/http/tests/inspector/network/resource-response-source-disk-cache.html (293263 => 293264)
--- trunk/LayoutTests/http/tests/inspector/network/resource-response-source-disk-cache.html 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/LayoutTests/http/tests/inspector/network/resource-response-source-disk-cache.html 2022-04-22 23:19:24 UTC (rev 293264)
@@ -40,6 +40,10 @@
InspectorTest.expectEqual(resource.statusCode, statusCode, `statusCode should be ${statusCode}`);
if (responseSource)
InspectorTest.expectEqual(resource.responseSource, responseSource, `responseSource should be ${String(responseSource)}`);
+ return resource.requestContentFromBackend();
+ }).then((responseContent) => {
+ InspectorTest.expectTrue(responseContent.body.length, "Response `body` should not be empty.");
+ InspectorTest.expectTrue(responseContent.base64Encoded, "Response should be base64 encoded.");
}).then(resolve, reject);
}
});
Modified: trunk/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-expected.txt (293263 => 293264)
--- trunk/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-expected.txt 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-expected.txt 2022-04-22 23:19:24 UTC (rev 293264)
@@ -6,4 +6,6 @@
PASS: Resource should exist.
PASS: statusCode should be 200
PASS: responseSource should be Symbol(memory-cache)
+PASS: Response `body` should not be empty.
+PASS: Response should not be base64 encoded.
Modified: trunk/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache.html (293263 => 293264)
--- trunk/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache.html 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache.html 2022-04-22 23:19:24 UTC (rev 293264)
@@ -32,6 +32,10 @@
InspectorTest.expectThat(resource instanceof WI.Resource, "Resource should exist.");
InspectorTest.expectEqual(resource.statusCode, statusCode, `statusCode should be ${statusCode}`);
InspectorTest.expectEqual(resource.responseSource, responseSource, `responseSource should be ${String(responseSource)}`);
+ return resource.requestContentFromBackend();
+ }).then((responseContent) => {
+ InspectorTest.expectTrue(responseContent.body.length, "Response `body` should not be empty.");
+ InspectorTest.expectFalse(responseContent.base64Encoded, "Response should not be base64 encoded.");
}).then(resolve, reject);
}
});
Modified: trunk/Source/WebCore/ChangeLog (293263 => 293264)
--- trunk/Source/WebCore/ChangeLog 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/Source/WebCore/ChangeLog 2022-04-22 23:19:24 UTC (rev 293264)
@@ -1,3 +1,33 @@
+2022-04-22 Patrick Angle <pan...@apple.com>
+
+ Web Inspector: Regression(r287684) Resources from the memory cache show empty content in Network, Sources, and Search tabs
+ https://bugs.webkit.org/show_bug.cgi?id=239667
+
+ Reviewed by Devin Rousso.
+
+ Updated tests:
+ - http/tests/inspector/network/resource-response-source-disk-cache.html
+ - http/tests/inspector/network/resource-response-source-memory-cache.html
+
+ r287684 introduced a subtle bug when calling InspectorInstrumentation::didReceiveData. We rely
+ on there being a difference between real, but empty, data buffer and not having a data buffer,
+ but after r287684 an empty SharedBuffer would be passed in the later case. Instead, we should
+ continue to pass nullptr if there is no buffer so that InspectorNetworkAgent::didReceiveData
+ can distiguish between the two.
+
+ The bug is the result of having a non-nullptr `data` in `InspectorNetworkAgent::didReceiveData`,
+ which causes us to call `maybeAddResourceData`, which means by the time
+ `InspectorNetworkAgent::getResponseBody` is called, the ResourceData for the request will have
+ an empty, not non-existant, `content()`, which means we will return the empty content instead,
+ since we believe the response had actual content.
+
+ * inspector/InspectorInstrumentation.cpp:
+ (WebCore::InspectorInstrumentation::didReceiveDataImpl):
+ * inspector/InspectorInstrumentation.h:
+ (WebCore::InspectorInstrumentation::didReceiveData):
+ * loader/ResourceLoadNotifier.cpp:
+ (WebCore::ResourceLoadNotifier::dispatchDidReceiveData):
+
2022-04-22 Simon Fraser <simon.fra...@apple.com>
Focusing scroll container before scrolling breaks smooth scrolling
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (293263 => 293264)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2022-04-22 23:19:24 UTC (rev 293264)
@@ -628,10 +628,10 @@
networkAgent->didReceiveThreadableLoaderResponse(identifier, documentThreadableLoader);
}
-void InspectorInstrumentation::didReceiveDataImpl(InstrumentingAgents& instrumentingAgents, ResourceLoaderIdentifier identifier, const SharedBuffer& buffer, int encodedDataLength)
+void InspectorInstrumentation::didReceiveDataImpl(InstrumentingAgents& instrumentingAgents, ResourceLoaderIdentifier identifier, const SharedBuffer* buffer, int encodedDataLength)
{
if (auto* networkAgent = instrumentingAgents.enabledNetworkAgent())
- networkAgent->didReceiveData(identifier, &buffer, buffer.size(), encodedDataLength);
+ networkAgent->didReceiveData(identifier, buffer, buffer ? buffer->size() : 0, encodedDataLength);
}
void InspectorInstrumentation::didFinishLoadingImpl(InstrumentingAgents& instrumentingAgents, ResourceLoaderIdentifier identifier, DocumentLoader* loader, const NetworkLoadMetrics& networkLoadMetrics, ResourceLoader* resourceLoader)
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (293263 => 293264)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2022-04-22 23:19:24 UTC (rev 293264)
@@ -198,7 +198,7 @@
static void didLoadResourceFromMemoryCache(Page&, DocumentLoader*, CachedResource*);
static void didReceiveResourceResponse(Frame&, ResourceLoaderIdentifier, DocumentLoader*, const ResourceResponse&, ResourceLoader*);
static void didReceiveThreadableLoaderResponse(DocumentThreadableLoader&, ResourceLoaderIdentifier);
- static void didReceiveData(Frame*, ResourceLoaderIdentifier, const SharedBuffer&, int encodedDataLength);
+ static void didReceiveData(Frame*, ResourceLoaderIdentifier, const SharedBuffer*, int encodedDataLength);
static void didFinishLoading(Frame*, DocumentLoader*, ResourceLoaderIdentifier, const NetworkLoadMetrics&, ResourceLoader*);
static void didFailLoading(Frame*, DocumentLoader*, ResourceLoaderIdentifier, const ResourceError&);
@@ -422,7 +422,7 @@
static void didLoadResourceFromMemoryCacheImpl(InstrumentingAgents&, DocumentLoader*, CachedResource*);
static void didReceiveResourceResponseImpl(InstrumentingAgents&, ResourceLoaderIdentifier, DocumentLoader*, const ResourceResponse&, ResourceLoader*);
static void didReceiveThreadableLoaderResponseImpl(InstrumentingAgents&, DocumentThreadableLoader&, ResourceLoaderIdentifier);
- static void didReceiveDataImpl(InstrumentingAgents&, ResourceLoaderIdentifier, const SharedBuffer&, int encodedDataLength);
+ static void didReceiveDataImpl(InstrumentingAgents&, ResourceLoaderIdentifier, const SharedBuffer*, int encodedDataLength);
static void didFinishLoadingImpl(InstrumentingAgents&, ResourceLoaderIdentifier, DocumentLoader*, const NetworkLoadMetrics&, ResourceLoader*);
static void didFailLoadingImpl(InstrumentingAgents&, ResourceLoaderIdentifier, DocumentLoader*, const ResourceError&);
static void willLoadXHRSynchronouslyImpl(InstrumentingAgents&);
@@ -1104,7 +1104,7 @@
didReceiveThreadableLoaderResponseImpl(*agents, documentThreadableLoader, identifier);
}
-inline void InspectorInstrumentation::didReceiveData(Frame* frame, ResourceLoaderIdentifier identifier, const SharedBuffer& buffer, int encodedDataLength)
+inline void InspectorInstrumentation::didReceiveData(Frame* frame, ResourceLoaderIdentifier identifier, const SharedBuffer* buffer, int encodedDataLength)
{
FAST_RETURN_IF_NO_FRONTENDS(void());
if (auto* agents = instrumentingAgents(frame))
@@ -1114,7 +1114,7 @@
inline void InspectorInstrumentation::didReceiveData(WorkerOrWorkletGlobalScope& globalScope, ResourceLoaderIdentifier identifier, const SharedBuffer& buffer)
{
FAST_RETURN_IF_NO_FRONTENDS(void());
- didReceiveDataImpl(instrumentingAgents(globalScope), identifier, buffer, buffer.size());
+ didReceiveDataImpl(instrumentingAgents(globalScope), identifier, &buffer, buffer.size());
}
inline void InspectorInstrumentation::didFinishLoading(Frame* frame, DocumentLoader* loader, ResourceLoaderIdentifier identifier, const NetworkLoadMetrics& networkLoadMetrics, ResourceLoader* resourceLoader)
Modified: trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp (293263 => 293264)
--- trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp 2022-04-22 23:06:32 UTC (rev 293263)
+++ trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp 2022-04-22 23:19:24 UTC (rev 293264)
@@ -160,7 +160,7 @@
Ref<Frame> protect(m_frame);
m_frame.loader().client().dispatchDidReceiveContentLength(loader, identifier, expectedDataLength);
- InspectorInstrumentation::didReceiveData(&m_frame, identifier, buffer ? *buffer : SharedBuffer::create(), encodedDataLength);
+ InspectorInstrumentation::didReceiveData(&m_frame, identifier, buffer, encodedDataLength);
}
void ResourceLoadNotifier::dispatchDidFinishLoading(DocumentLoader* loader, ResourceLoaderIdentifier identifier, const NetworkLoadMetrics& networkLoadMetrics, ResourceLoader* resourceLoader)
