fix(oopifs): ignore events from stale frames (#8926)

This is a speculative fix for the following scenario:
- Main frame A creates a child frame B.
- B navigates cross-origin and forces an oopif.
- Target.attachedToTarget for B arrives.
- B loads and creates execution contexts.
- Process with A creates an execution context in the
  (still local) frame B (e.g. with document.write?)
- Process with A sends executionContextCreated and
  overwrites the execution context that came from B.
- Process with A finally sends frameDetached for B,
  and we ignore it since we already have the B target.
This sequence results in a stale execution context
from process A that is actually not present in B.

Overall, events coming from process A for the frame
that has already moved to an oopif B should be ignored.
Seems totally safe! This is also pure specultation
from analyzing protocol logs, no easy repro found.
This commit is contained in:
Dmitry Gozman 2021-09-15 10:26:02 -07:00 committed by GitHub
parent 04fa5e6b2d
commit 2ec82b9a5c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -136,7 +136,7 @@ export class CRPage implements PageDelegate {
}));
}
private _sessionForFrame(frame: frames.Frame): FrameSession {
_sessionForFrame(frame: frames.Frame): FrameSession {
// Frame id equals target id.
while (!this._sessions.has(frame._id)) {
const parent = frame.parentFrame();
@ -553,6 +553,8 @@ class FrameSession {
}
_onLifecycleEvent(event: Protocol.Page.lifecycleEventPayload) {
if (this._eventBelongsToStaleFrame(event.frameId))
return;
if (event.name === 'load')
this._page._frameManager.frameLifecycleEvent(event.frameId, 'load');
else if (event.name === 'DOMContentLoaded')
@ -560,6 +562,8 @@ class FrameSession {
}
_onFrameStoppedLoading(frameId: string) {
if (this._eventBelongsToStaleFrame(frameId))
return;
this._page._frameManager.frameStoppedLoading(frameId);
}
@ -573,6 +577,19 @@ class FrameSession {
this._handleFrameTree(child);
}
private _eventBelongsToStaleFrame(frameId: string) {
const frame = this._page._frameManager.frame(frameId);
// Subtree may be already gone because some ancestor navigation destroyed the oopif.
if (!frame)
return true;
// When frame goes remote, parent process may still send some events
// related to the local frame before it sends frameDetached.
// In this case, we already have a new session for this frame, so events
// in the old session should be ignored.
const session = this._crPage._sessionForFrame(frame);
return session && session !== this && !session._swappedIn;
}
_onFrameAttached(frameId: string, parentFrameId: string | null) {
const frameSession = this._crPage._sessions.get(frameId);
if (frameSession && frameId !== this._targetId) {
@ -595,19 +612,23 @@ class FrameSession {
}
_onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) {
if (!this._page._frameManager.frame(framePayload.id))
return; // Subtree may be already gone because some ancestor navigation destroyed the oopif.
if (this._eventBelongsToStaleFrame(framePayload.id))
return;
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url + (framePayload.urlFragment || ''), framePayload.name || '', framePayload.loaderId, initial);
if (!initial)
this._firstNonInitialNavigationCommittedFulfill();
}
_onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {
if (this._eventBelongsToStaleFrame(payload.frameId))
return;
if (payload.disposition === 'currentTab')
this._page._frameManager.frameRequestedNavigation(payload.frameId);
}
_onFrameNavigatedWithinDocument(frameId: string, url: string) {
if (this._eventBelongsToStaleFrame(frameId))
return;
this._page._frameManager.frameCommittedSameDocumentNavigation(frameId, url);
}
@ -633,7 +654,7 @@ class FrameSession {
_onExecutionContextCreated(contextPayload: Protocol.Runtime.ExecutionContextDescription) {
const frame = contextPayload.auxData ? this._page._frameManager.frame(contextPayload.auxData.frameId) : null;
if (!frame)
if (!frame || this._eventBelongsToStaleFrame(frame._id))
return;
const delegate = new CRExecutionContext(this._client, contextPayload);
let worldName: types.World|null = null;