Skip to content

Commit

Permalink
fix: restore OOPiF state (#2381)
Browse files Browse the repository at this point in the history
Addressing #2362. 

NodeJS runner nor chromedriver don't support re-connecting, so tested via puppeteer.
  • Loading branch information
sadym-chromium authored Jul 4, 2024
1 parent d3dafc2 commit 3e9855c
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 14 deletions.
21 changes: 15 additions & 6 deletions src/bidiMapper/modules/cdp/CdpTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,21 @@ export class CdpTarget {

#restoreFrameTreeState(frameTree: Protocol.Page.FrameTree) {
const frame = frameTree.frame;
if (
this.#browsingContextStorage.findContext(frame.id) === undefined &&
frame.parentId !== undefined
) {
// Can restore only not yet known nested frames. The top-level frame is created
// when the target is attached.
const maybeContext = this.#browsingContextStorage.findContext(frame.id);
if (maybeContext !== undefined) {
// Restoring parent of already known browsing context. This means the target is
// OOPiF and the BiDi session was connected to already existing browser instance.
if (
maybeContext.parentId === null &&
frame.parentId !== null &&
frame.parentId !== undefined
) {
maybeContext.parentId = frame.parentId;
}
}
if (maybeContext === undefined && frame.parentId !== undefined) {
// Restore not yet known nested frames. The top-level frame is created when the
// target is attached.
const parentBrowsingContext = this.#browsingContextStorage.getContext(
frame.parentId
);
Expand Down
49 changes: 41 additions & 8 deletions src/bidiMapper/modules/context/BrowsingContextImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class BrowsingContextImpl {
* The ID of the parent browsing context.
* If null, this is a top-level context.
*/
readonly #parentId: BrowsingContext.BrowsingContext | null;
#parentId: BrowsingContext.BrowsingContext | null = null;

/** Direct children browsing contexts. */
readonly #children = new Set<BrowsingContext.BrowsingContext>();
Expand Down Expand Up @@ -154,13 +154,30 @@ export class BrowsingContextImpl {
context.parent!.addChild(context.id);
}

eventManager.registerEvent(
{
type: 'event',
method: ChromiumBidi.BrowsingContext.EventNames.ContextCreated,
params: context.serializeToBidiValue(),
},
context.id
// Hold on the `contextCreated` event until the target is unblocked. This is required,
// as the parent of the context can be set later in case of reconnecting to an
// existing browser instance + OOPiF.
eventManager.registerPromiseEvent(
context.targetUnblockedOrThrow().then(
() => {
return {
kind: 'success',
value: {
type: 'event',
method: ChromiumBidi.BrowsingContext.EventNames.ContextCreated,
params: context.serializeToBidiValue(),
},
};
},
(error) => {
return {
kind: 'error',
error,
};
}
),
context.id,
ChromiumBidi.BrowsingContext.EventNames.ContextCreated
);

return context;
Expand Down Expand Up @@ -226,6 +243,22 @@ export class BrowsingContextImpl {
return this.#parentId;
}

/** Sets the parent context ID and updates parent's children. */
set parentId(parentId: BrowsingContext.BrowsingContext | null) {
if (this.#parentId !== null) {
this.#logger?.(LogType.debugError, 'Parent context already set');
// Cannot do anything except logging, as throwing will stop event processing. So
// just return,
return;
}

this.#parentId = parentId;

if (!this.isTopLevelContext()) {
this.parent!.addChild(this.id);
}
}

/** Returns the parent context. */
get parent(): BrowsingContextImpl | null {
if (this.parentId === null) {
Expand Down

0 comments on commit 3e9855c

Please sign in to comment.