Summary: | Resource Timing: Duration is 0 in many cases | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nic Jansma <nic> | ||||||||
Component: | Platform | Assignee: | Alex Christensen <achristensen> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, cdumez, ews-watchlist, japhet, nic, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Mac (Intel) | ||||||||||
OS: | macOS 10.15 | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=229516 | ||||||||||
Attachments: |
|
Description
Nic Jansma
2021-05-12 22:25:20 PDT
Created attachment 431947 [details]
Patch
Created attachment 431989 [details]
Patch
Created attachment 432000 [details]
Patch
Comment on attachment 432000 [details]
Patch
Now is not the right time to make a change like this. The status quo will have to suffice for a little while longer.
Comment on attachment 432000 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432000&action=review > Source/WebCore/loader/ResourceTimingInformation.cpp:43 > + return resource.resourceRequest().url().protocolIsInHTTPFamily() Note to my hopefully-not-too-far-in-the-future self: Making this change here doesn't just fix this test, but it allows this change to fix even more tests, like http://wpt.live/resource-timing/buffer-full-add-then-clear.html diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp index a261eaef4f24..ac1ab9783d9a 100644 --- a/Source/WebCore/loader/SubresourceLoader.cpp +++ b/Source/WebCore/loader/SubresourceLoader.cpp @@ -494,7 +494,11 @@ void SubresourceLoader::didReceiveResponse(const ResourceResponse& response, Com didFinishLoadingOnePart(emptyMetrics); } - checkForHTTPStatusCodeError(); + if (checkForHTTPStatusCodeError()) { + auto* metrics = this->response().deprecatedNetworkLoadMetricsOrNull(); + reportResourceTiming(metrics ? *metrics : NetworkLoadMetrics { }); + cancel(); + } if (m_inAsyncResponsePolicyCheck) m_policyForResponseCompletionHandler = completionHandlerCaller.release(); @@ -563,7 +567,6 @@ bool SubresourceLoader::checkForHTTPStatusCodeError() m_state = Finishing; m_resource->error(CachedResource::LoadError); - cancel(); return true; } Comment on attachment 432000 [details]
Patch
r=me
Note: r281123 probably wasn't necessary because bug 229167 probably introduced that crash in an early version of the patch that I'm fixing, but I think it doesn't hurt to be cautious to not introduce a null crash in the loading code. |