Skip to content

Commit 71631b3

Browse files
Adam RaineDevtools-frontend LUCI CQ
authored andcommitted
[RPP Observations] Handle prerendered pages correctly
Clicking a link to a prerendered page could trip up the live metrics model if the frame from the original page is removed before live metrics handles the interaction event. This CL skips the step to check if a frame is the primary frame if it comes from the already known "good" execution context. Bug: 394918742 Change-Id: I8773d0771690a55a583afaf4ed3d97f4058346ee Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6284568 Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Adam Raine <[email protected]>
1 parent 75fdad2 commit 71631b3

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

front_end/models/live-metrics/LiveMetrics.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -401,20 +401,25 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
401401
// Async tasks can be performed while handling an event (e.g. resolving DOM node)
402402
// Use a mutex here to ensure the events are handled in the order they are received.
403403
await this.#mutex.run(async () => {
404-
const frame = await this.#getFrameForExecutionContextId(data.executionContextId);
405-
if (!frame?.isPrimaryFrame()) {
406-
return;
407-
}
408-
409404
const webVitalsEvent = JSON.parse(data.payload) as Spec.WebVitalsEvent;
410405

411-
// Previously injected scripts shouldn't persist, this is just a defensive measure.
412-
// Ensure we only handle events from the same execution context as the most recent "reset" event.
413-
// "reset" events are only emitted once when the script is injected or a bfcache restoration.
414-
if (webVitalsEvent.name === 'reset') {
406+
// This ensures that `#lastResetContextId` will always be an execution context on the
407+
// primary frame. If we receive events from this execution context then we automatically
408+
// know that they are for the primary frame.
409+
if (this.#lastResetContextId !== data.executionContextId) {
410+
if (webVitalsEvent.name !== 'reset') {
411+
return;
412+
}
413+
414+
// We should avoid calling this function for every event.
415+
// If an interaction triggers a pre-rendered navigation then the old primary frame could
416+
// be removed before we reach this point, and then it will hang forever.
417+
const frame = await this.#getFrameForExecutionContextId(data.executionContextId);
418+
if (!frame?.isPrimaryFrame()) {
419+
return;
420+
}
421+
415422
this.#lastResetContextId = data.executionContextId;
416-
} else if (this.#lastResetContextId !== data.executionContextId) {
417-
return;
418423
}
419424

420425
await this.#handleWebVitalsEvent(webVitalsEvent, data.executionContextId);

0 commit comments

Comments
 (0)