Skip to content

Commit

Permalink
fix: wait for default realm to be created before evaluating script (#…
Browse files Browse the repository at this point in the history
…2294)

Addressing
#2275.
Cannot reproduce in nodejs or chromedriver runners.
  • Loading branch information
sadym-chromium authored Jun 7, 2024
1 parent 20f46f7 commit 444e728
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 14 deletions.
45 changes: 31 additions & 14 deletions src/bidiMapper/modules/context/BrowsingContextImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ export class BrowsingContextImpl {
readonly #realmStorage: RealmStorage;
#loaderId?: Protocol.Network.LoaderId;
#cdpTarget: CdpTarget;
#maybeDefaultRealm?: Realm;
// The deferred will be resolved when the default realm is created.
#defaultRealmDeferred = new Deferred<Realm>();
readonly #logger?: LoggerFn;
// Keeps track of the previously set viewport.
#previousViewport: {width: number; height: number} = {width: 0, height: 0};
Expand Down Expand Up @@ -241,14 +242,6 @@ export class BrowsingContextImpl {
this.directChildren.map((child) => child.dispose());
}

get #defaultRealm(): Realm {
assert(
this.#maybeDefaultRealm,
`No default realm for browsing context ${this.#id}`
);
return this.#maybeDefaultRealm;
}

get cdpTarget(): CdpTarget {
return this.#cdpTarget;
}
Expand All @@ -275,7 +268,8 @@ export class BrowsingContextImpl {

async getOrCreateSandbox(sandbox: string | undefined): Promise<Realm> {
if (sandbox === undefined || sandbox === '') {
return this.#defaultRealm;
// Default realm is not guaranteed to be created at this point, so return a deferred.
return await this.#defaultRealmDeferred;
}

let maybeSandboxes = this.#realmStorage.findRealms({
Expand Down Expand Up @@ -462,7 +456,16 @@ export class BrowsingContextImpl {
sandbox = name;
// Sandbox should have the same origin as the context itself, but in CDP
// it has an empty one.
origin = this.#defaultRealm.origin;
if (!this.#defaultRealmDeferred.isFinished) {
this.#logger?.(
LogType.debugError,
'Unexpectedly, isolated realm created before the default one'
);
}
origin = this.#defaultRealmDeferred.isFinished
? this.#defaultRealmDeferred.result.origin
: // This fallback is not expected to be ever reached.
'';
break;
case 'default':
origin = serializeOrigin(params.context.origin);
Expand All @@ -484,7 +487,7 @@ export class BrowsingContextImpl {
);

if (auxData.isDefault) {
this.#maybeDefaultRealm = realm;
this.#defaultRealmDeferred.resolve(realm);

// Initialize ChannelProxy listeners for all the channels of all the
// preload scripts related to this BrowsingContext.
Expand All @@ -503,6 +506,14 @@ export class BrowsingContextImpl {
this.#cdpTarget.cdpClient.on(
'Runtime.executionContextDestroyed',
(params) => {
if (
this.#defaultRealmDeferred.isFinished &&
this.#defaultRealmDeferred.result.executionContextId ===
params.executionContextId
) {
this.#defaultRealmDeferred = new Deferred<Realm>();
}

this.#realmStorage.deleteRealms({
cdpSessionId: this.#cdpTarget.cdpSessionId,
executionContextId: params.executionContextId,
Expand All @@ -511,6 +522,12 @@ export class BrowsingContextImpl {
);

this.#cdpTarget.cdpClient.on('Runtime.executionContextsCleared', () => {
if (!this.#defaultRealmDeferred.isFinished) {
this.#defaultRealmDeferred.reject(
new UnknownErrorException('execution contexts cleared')
);
}
this.#defaultRealmDeferred = new Deferred<Realm>();
this.#realmStorage.deleteRealms({
cdpSessionId: this.#cdpTarget.cdpSessionId,
});
Expand Down Expand Up @@ -1029,7 +1046,7 @@ export class BrowsingContextImpl {
): Promise<BrowsingContext.LocateNodesResult> {
// TODO: create a dedicated sandbox instead of `#defaultRealm`.
return await this.#locateNodesByLocator(
this.#defaultRealm,
await this.#defaultRealmDeferred,
params.locator,
params.startNodes ?? [],
params.maxNodeCount,
Expand Down Expand Up @@ -1263,7 +1280,7 @@ export class BrowsingContextImpl {

// The next two commands cause a11y caches for the target to be
// preserved. We probably do not need to disable them if the
// client is using a11y features but we could by calling
// client is using a11y features, but we could by calling
// Accessibility.disable.
await Promise.all([
this.#cdpTarget.cdpClient.sendCommand('Accessibility.enable'),
Expand Down
15 changes: 15 additions & 0 deletions src/utils/Deferred.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,20 @@ describe('Deferred', () => {

await expect(deferredFinally).to.eventually.equal('done');
});

describe('result', () => {
it('should throw if not finished', () => {
const deferred = new Deferred<string>();
expect(() => {
deferred.result;
}).to.throw('Deferred is not finished yet');
});

it('should return when finished', () => {
const deferred = new Deferred<string>();
deferred.resolve('done');
expect(deferred.result).to.equal('done');
});
});
});
});
9 changes: 9 additions & 0 deletions src/utils/Deferred.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@
export class Deferred<T> implements Promise<T> {
#isFinished = false;
#promise: Promise<T>;
#result: T | undefined;
#resolve!: (value: T) => void;
#reject!: (reason: unknown) => void;

get isFinished(): boolean {
return this.#isFinished;
}

get result(): T {
if (!this.#isFinished) {
throw new Error('Deferred is not finished yet');
}
return this.#result!;
}

constructor() {
this.#promise = new Promise((resolve, reject) => {
this.#resolve = resolve;
Expand All @@ -51,6 +59,7 @@ export class Deferred<T> implements Promise<T> {
}

resolve(value: T) {
this.#result = value;
if (!this.#isFinished) {
this.#isFinished = true;
this.#resolve(value);
Expand Down

0 comments on commit 444e728

Please sign in to comment.