Skip to content

Commit 18a7136

Browse files
committed
Fix high event timing presentation delay on back-forward cache restores
https://bugs.webkit.org/show_bug.cgi?id=308224 rdar://171273031 Reviewed by Ryosuke Niwa. Finalizes and queues dispatch of event timing entries when suspending a page to back-forward cache. This finalizes their durations before suspension, as if they were painted (navigating occurring is some kind of user feedback, so this is ok). This avoids unreasonable large durations - which include all suspended time - when the events are later resurfaced as the page is restored. Some LocalDOMWindow methods dealing with event timing were renamed to better reflect what they do: * dispatchPendingEventTimingEntries() -> finalizeAndQueueEventTimingEntries() * finalizeEventTimingEntry() -> markEndOfProcessingForEventTiming() * initializeEventTimingEntry() -> initializeEventTiming() A non-WPT test was added, using [ UsesBackForwardCache=true ]. I was unable to create a WPT test for this due the simultaneous need for back-forward navigation and trusted events. Canonical link: https://commits.webkit.org/309144@main
1 parent d042d9b commit 18a7136

8 files changed

Lines changed: 105 additions & 9 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Tests that event timing entries do not have inflated durations after back-forward cache restore.
2+
3+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
4+
5+
6+
PASS Event timing entries have reasonable durations after back-forward restore
7+
PASS successfullyParsed is true
8+
9+
TEST COMPLETE
10+
Navigate
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<!-- webkit-test-runner [ UsesBackForwardCache=true ] -->
2+
<!DOCTYPE html>
3+
<html>
4+
<body>
5+
<a id="link" href="resources/go-back-after-delay.html" style="display:block; width:100px; height:100px;">Navigate</a>
6+
<script src="../../resources/js-test.js"></script>
7+
<script src="../../resources/ui-helper.js"></script>
8+
<script>
9+
description("Tests that event timing entries do not have inflated durations after back-forward cache restore.");
10+
window.jsTestIsAsync = true;
11+
12+
const collectedEntries = [];
13+
const observer = new PerformanceObserver(list => {
14+
for (const entry of list.getEntries()) {
15+
collectedEntries.push({
16+
name: entry.name,
17+
duration: entry.duration,
18+
});
19+
}
20+
});
21+
observer.observe({ type: 'event', durationThreshold: 16 });
22+
23+
const link = document.getElementById('link');
24+
link.addEventListener('click', () => {
25+
const target = performance.now() + 20;
26+
while (performance.now() < target);
27+
}, { once: true });
28+
29+
window.addEventListener("pageshow", function(event) {
30+
if (!event.persisted)
31+
return;
32+
33+
// Wait for any remaining entries to be dispatched after restoration.
34+
requestAnimationFrame(async () => {
35+
await new Promise(r => setTimeout(r, 0));
36+
await new Promise(r => requestAnimationFrame(r));
37+
await new Promise(r => setTimeout(r, 0));
38+
39+
let hasClickEntry = false;
40+
let hasInflatedDuration = false;
41+
for (const entry of collectedEntries) {
42+
if (entry.name === 'click')
43+
hasClickEntry = true;
44+
if (entry.duration >= 1500) {
45+
testFailed("'" + entry.name + "' entry has inflated duration: " + entry.duration + "ms");
46+
hasInflatedDuration = true;
47+
}
48+
}
49+
if (!hasClickEntry)
50+
testFailed("No 'click' event timing entry collected");
51+
else if (!hasInflatedDuration)
52+
testPassed("Event timing entries have reasonable durations after back-forward restore");
53+
finishJSTest();
54+
});
55+
});
56+
57+
window.addEventListener("pagehide", function(event) {
58+
if (!event.persisted) {
59+
testFailed("Page did not enter the page cache");
60+
finishJSTest();
61+
}
62+
});
63+
64+
window.addEventListener('load', async function() {
65+
// Click the link to generate event timing entries and navigate via
66+
// the link's default action. The navigation happens as part of click
67+
// event dispatch, so no rendering update can drain entries beforehand.
68+
await UIHelper.activateElement(link);
69+
});
70+
</script>
71+
</body>
72+
</html>
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE html>
2+
<p>This page navigates back after a delay.</p>
3+
<script>
4+
setTimeout(function() { history.back(); }, 2000);
5+
</script>

LayoutTests/platform/mac-site-isolation/TestExpectations

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ editing/selection/drag-in-iframe.html [ Failure ]
133133
editing/undo/undo-with-disconnected-editable-element-crash.html [ Failure ]
134134
fast/canvas/webgl/canvas-webgl-page-cache.html [ Failure ]
135135
fast/dom/window-load-crash.html [ Failure ]
136+
fast/events/event-timing-back-forward-cache-duration.html [ Failure ]
136137
fast/files/file-reader-back-forward-cache.html [ Failure ]
137138
fast/forms/form-attribute-elements.html [ Failure ]
138139
fast/forms/formmethod-attribute-button-html.html [ Failure ]

Source/WebCore/dom/Document.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4424,7 +4424,7 @@ void Document::enqueueEventTimingEntriesIfNeeded()
44244424
if (!window())
44254425
return;
44264426

4427-
protect(window())->dispatchPendingEventTimingEntries();
4427+
protect(window())->finalizeAndQueueEventTimingEntries();
44284428
}
44294429

44304430
ExceptionOr<void> Document::write(Document* entryDocument, SegmentedString&& text)

Source/WebCore/dom/EventDispatcher.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,10 @@ void EventDispatcher::dispatchEvent(Node& node, Event& event)
192192
RefPtr window = document->window();
193193
std::optional<PerformanceEventTimingCandidate> pendingEventTiming;
194194
if (typeInfo.isInCategory(EventCategory::EventTimingEligible) && window && document->settings().eventTimingEnabled() && event.isTrusted())
195-
pendingEventTiming = window->initializeEventTimingEntry(event, typeInfo.type());
195+
pendingEventTiming = window->initializeEventTiming(event, typeInfo.type());
196196
auto finalizeEntry(WTF::makeScopeExit([&, event = Ref(event)] {
197197
if (pendingEventTiming)
198-
window->finalizeEventTimingEntry(*pendingEventTiming, event, typeInfo.type());
198+
window->markEndOfProcessingForEventTiming(*pendingEventTiming, event, typeInfo.type());
199199
}));
200200

201201
bool targetOrRelatedTargetIsInShadowTree = node.isInShadowTree() || isInShadowTree(event.relatedTarget());

Source/WebCore/page/LocalDOMWindow.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,12 @@ void LocalDOMWindow::resetUnlessSuspendedForDocumentSuspension()
614614

615615
void LocalDOMWindow::suspendForBackForwardCache()
616616
{
617+
// Without this, entries queued just before navigation would have their
618+
// duration computed after restoration:
619+
if (m_performanceEventTimingCandidates.size())
620+
LOG_WITH_STREAM(PerformanceTimeline, stream << "Dispatching event timing entries before suspending to back-forward cache.");
621+
finalizeAndQueueEventTimingEntries();
622+
617623
SetForScope isSuspendingObservers(m_isSuspendingObservers, true);
618624
RELEASE_ASSERT(frame());
619625

@@ -2641,7 +2647,7 @@ void LocalDOMWindow::queueEventTimingCandidateForDispatch(PerformanceEventTiming
26412647
page->scheduleRenderingUpdate(RenderingUpdateStep::EventTiming);
26422648
}
26432649

2644-
PerformanceEventTimingCandidate LocalDOMWindow::initializeEventTimingEntry(Event& event, EventType type)
2650+
PerformanceEventTimingCandidate LocalDOMWindow::initializeEventTiming(Event& event, EventType type)
26452651
{
26462652
auto startTime = performance().relativeTimeFromTimeOriginInReducedResolutionSeconds(event.timeStamp());
26472653
auto processingStart = performance().nowInReducedResolutionSeconds();
@@ -2669,8 +2675,9 @@ PerformanceEventTimingCandidate LocalDOMWindow::initializeEventTimingEntry(Event
26692675
};
26702676
}
26712677

2672-
void LocalDOMWindow::finalizeEventTimingEntry(PerformanceEventTimingCandidate& entry, const Event& event, EventType type)
2678+
void LocalDOMWindow::markEndOfProcessingForEventTiming(PerformanceEventTimingCandidate& entry, const Event& event, EventType type)
26732679
{
2680+
// Maps to "Finalize event timing" in the spec.
26742681
auto processingEnd = performance().nowInReducedResolutionSeconds();
26752682
entry.processingEnd = processingEnd;
26762683
entry.target = event.target();
@@ -2738,8 +2745,9 @@ void LocalDOMWindow::finalizeEventTimingEntry(PerformanceEventTimingCandidate& e
27382745
}
27392746
}
27402747

2741-
void LocalDOMWindow::dispatchPendingEventTimingEntries()
2748+
void LocalDOMWindow::finalizeAndQueueEventTimingEntries()
27422749
{
2750+
// Maps to "Dispatch pending Event Timing entries" in the spec.
27432751
auto renderingTime = performance().nowInReducedResolutionSeconds();
27442752
if (m_pendingPointerDown && !m_pendingPointerDown->duration)
27452753
m_pendingPointerDown->duration = std::max(renderingTime - m_pendingPointerDown->startTime, Seconds::fromMilliseconds(1));

Source/WebCore/page/LocalDOMWindow.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,9 @@ class LocalDOMWindow final
284284
void finishedLoading();
285285

286286
// EventTiming API
287-
PerformanceEventTimingCandidate initializeEventTimingEntry(Event&, EventType);
288-
void finalizeEventTimingEntry(PerformanceEventTimingCandidate&, const Event&, EventType);
289-
void dispatchPendingEventTimingEntries();
287+
PerformanceEventTimingCandidate initializeEventTiming(Event&, EventType);
288+
void markEndOfProcessingForEventTiming(PerformanceEventTimingCandidate&, const Event&, EventType);
289+
void finalizeAndQueueEventTimingEntries();
290290
uint64_t interactionCount() { return m_interactionCount; }
291291
// Misleading function names that mirror the spec; see https://github.com/w3c/event-timing/issues/158 :
292292
bool hasDispatchedInputEvent() const { return m_hasDispatchedInputEvent; }

0 commit comments

Comments
 (0)