Skip to content

Commit 1c91278

Browse files
bnhambnham
committed
Fix nested resource load tracepoints
https://bugs.webkit.org/show_bug.cgi?id=228242 Reviewed by Alex Christensen. Source/WebCore: The main resource and subresource load tracepoints aren't interpreted correctly if there are multiple resource loads in flight at the same time. To fix this, we pass the pointer to the FrameLoader or SubresourceLoader to ktrace so that the tracing tool can use that pointer to properly nest overlapping resource loading trace intervals. One additional change I made is to move the SubresourceLoadWillStart tracepoint to after the call to willSendRequestInternal. The reason for this is because I wanted to log the resource ID associated with the subresource request, but we only generate that ID after the call to willSendRequestInternal. * loader/FrameLoader.cpp: (WebCore::FrameLoader::provisionalLoadStarted): (WebCore::FrameLoader::checkLoadCompleteForThisFrame): * loader/SubresourceLoader.cpp: (WebCore::SubresourceLoader::willSendRequestInternal): (WebCore::SubresourceLoader::didFinishLoading): (WebCore::SubresourceLoader::didFail): (WebCore::SubresourceLoader::didCancel): Tools: Use page and resource ids to properly handle nested main resource and subresource load tracepoint intervals. * Tracing/SystemTracePoints.plist: Canonical link: https://commits.webkit.org/240400@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280859 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent c44db5f commit 1c91278

5 files changed

Lines changed: 83 additions & 6 deletions

File tree

‎Source/WebCore/ChangeLog‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
2021-08-10 Ben Nham <nham@apple.com>
2+
3+
Fix nested resource load tracepoints
4+
https://bugs.webkit.org/show_bug.cgi?id=228242
5+
6+
Reviewed by Alex Christensen.
7+
8+
The main resource and subresource load tracepoints aren't interpreted correctly if there are
9+
multiple resource loads in flight at the same time. To fix this, we pass the pointer to the
10+
FrameLoader or SubresourceLoader to ktrace so that the tracing tool can use that pointer to
11+
properly nest overlapping resource loading trace intervals.
12+
13+
One additional change I made is to move the SubresourceLoadWillStart tracepoint to after the
14+
call to willSendRequestInternal. The reason for this is because I wanted to log the resource
15+
ID associated with the subresource request, but we only generate that ID after the call to
16+
willSendRequestInternal.
17+
18+
* loader/FrameLoader.cpp:
19+
(WebCore::FrameLoader::provisionalLoadStarted):
20+
(WebCore::FrameLoader::checkLoadCompleteForThisFrame):
21+
* loader/SubresourceLoader.cpp:
22+
(WebCore::SubresourceLoader::willSendRequestInternal):
23+
(WebCore::SubresourceLoader::didFinishLoading):
24+
(WebCore::SubresourceLoader::didFail):
25+
(WebCore::SubresourceLoader::didCancel):
26+
127
2021-08-10 Chris Dumez <cdumez@apple.com>
228

329
Document.baseURI is inaccurate for iframe srcdoc documents

‎Source/WebCore/loader/FrameLoader.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,7 @@ void FrameLoader::provisionalLoadStarted()
10631063
m_client->provisionalLoadStarted();
10641064

10651065
if (m_frame.isMainFrame()) {
1066-
tracePoint(MainResourceLoadDidStartProvisional);
1066+
tracePoint(MainResourceLoadDidStartProvisional, PAGE_ID);
10671067

10681068
if (auto* page = m_frame.page())
10691069
page->didStartProvisionalLoad();
@@ -2570,7 +2570,7 @@ void FrameLoader::checkLoadCompleteForThisFrame()
25702570
Page* page = m_frame.page();
25712571
if (page) {
25722572
if (m_frame.isMainFrame()) {
2573-
tracePoint(MainResourceLoadDidEnd);
2573+
tracePoint(MainResourceLoadDidEnd, PAGE_ID);
25742574
page->didFinishLoad();
25752575
}
25762576
}

‎Source/WebCore/loader/SubresourceLoader.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest&& newRequest, co
196196
}
197197

198198
if (newRequest.requester() != ResourceRequestBase::Requester::Main) {
199-
tracePoint(SubresourceLoadWillStart);
200199
ResourceLoadObserver::shared().logSubresourceLoading(m_frame.get(), newRequest, redirectResponse,
201200
(isScriptLikeDestination(options().destination) ? ResourceLoadObserver::FetchDestinationIsScriptLike::Yes : ResourceLoadObserver::FetchDestinationIsScriptLike::No));
202201
}
@@ -211,6 +210,8 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest&& newRequest, co
211210
}
212211

213212
ResourceLoader::willSendRequestInternal(WTFMove(newRequest), redirectResponse, [this, protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler), redirectResponse] (ResourceRequest&& request) mutable {
213+
tracePoint(SubresourceLoadWillStart, identifier(), PAGE_ID, FRAME_ID);
214+
214215
if (reachedTerminalState()) {
215216
SUBRESOURCELOADER_RELEASE_LOG("willSendRequestInternal: reached terminal state; calling completion handler");
216217
return completionHandler(WTFMove(request));
@@ -740,7 +741,7 @@ void SubresourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMe
740741
}
741742

742743
if (m_resource->type() != CachedResource::Type::MainResource)
743-
tracePoint(SubresourceLoadDidEnd);
744+
tracePoint(SubresourceLoadDidEnd, identifier());
744745

745746
m_state = Finishing;
746747
m_resource->finishLoading(resourceData(), networkLoadMetrics);
@@ -786,7 +787,7 @@ void SubresourceLoader::didFail(const ResourceError& error)
786787
m_state = Finishing;
787788

788789
if (m_resource->type() != CachedResource::Type::MainResource)
789-
tracePoint(SubresourceLoadDidEnd);
790+
tracePoint(SubresourceLoadDidEnd, identifier());
790791

791792
if (m_resource->resourceToRevalidate())
792793
MemoryCache::singleton().revalidationFailed(*m_resource);
@@ -839,7 +840,7 @@ void SubresourceLoader::didCancel(const ResourceError&)
839840
ASSERT(m_resource);
840841

841842
if (m_resource->type() != CachedResource::Type::MainResource)
842-
tracePoint(SubresourceLoadDidEnd);
843+
tracePoint(SubresourceLoadDidEnd, identifier());
843844

844845
m_resource->cancelLoad();
845846
notifyDone(LoadCompletionType::Cancel);

‎Tools/ChangeLog‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2021-08-10 Ben Nham <nham@apple.com>
2+
3+
Fix nested resource load tracepoints
4+
https://bugs.webkit.org/show_bug.cgi?id=228242
5+
6+
Reviewed by Alex Christensen.
7+
8+
Use page and resource ids to properly handle nested main resource and subresource load
9+
tracepoint intervals.
10+
11+
* Tracing/SystemTracePoints.plist:
12+
113
2021-08-10 Kimmo Kinnunen <kkinnunen@apple.com>
214

315
Scripts/generate-derived-sources.sh: line 19: [: binary operator expected while building Source/WebKit

‎Tools/Tracing/SystemTracePoints.plist‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,21 @@
6666
<string>5001</string>
6767
<key>CodeEnd</key>
6868
<string>5002</string>
69+
<key>ArgNamesBegin</key>
70+
<dict>
71+
<key>Arg1</key>
72+
<string>Page ID</string>
73+
</dict>
74+
<key>ArgValueTypesBegin</key>
75+
<dict>
76+
<key>Arg1</key>
77+
<string>UInt64</string>
78+
</dict>
79+
<key>EventsMatchedBy</key>
80+
<array>
81+
<string>Thread</string>
82+
<string>Arg1</string>
83+
</array>
6984
</dict>
7085
<dict>
7186
<key>Name</key>
@@ -78,6 +93,29 @@
7893
<string>5003</string>
7994
<key>CodeEnd</key>
8095
<string>5004</string>
96+
<key>ArgNamesBegin</key>
97+
<dict>
98+
<key>Arg1</key>
99+
<string>Resource ID</string>
100+
<key>Arg2</key>
101+
<string>Page ID</string>
102+
<key>Arg3</key>
103+
<string>Frame ID</string>
104+
</dict>
105+
<key>ArgValueTypesBegin</key>
106+
<dict>
107+
<key>Arg1</key>
108+
<string>UInt64</string>
109+
<key>Arg2</key>
110+
<string>UInt64</string>
111+
<key>Arg3</key>
112+
<string>UInt64</string>
113+
</dict>
114+
<key>EventsMatchedBy</key>
115+
<array>
116+
<string>Thread</string>
117+
<string>Arg1</string>
118+
</array>
81119
</dict>
82120
<dict>
83121
<key>Name</key>

0 commit comments

Comments
 (0)