Skip to content

Commit

Permalink
fix(agent): use default context if mainframe
Browse files Browse the repository at this point in the history
  • Loading branch information
soundofspace committed Oct 10, 2024
1 parent c3e876e commit dbfff42
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 18 deletions.
25 changes: 15 additions & 10 deletions agent/main/lib/Frame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IF
}

public get page(): Page {
return this.#framesManager.page;
return this.#framesManager?.page;
}

public interactor: Interactor;
Expand Down Expand Up @@ -259,14 +259,14 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IF
expression: string,
options?: {
isolateFromWebPageEnvironment?: boolean;
usePageDefaultContextId?: boolean;
shouldAwaitExpression?: boolean;
retriesWaitingForLoad?: number;
returnByValue?: boolean;
includeCommandLineAPI?: boolean;
timeoutMs?: number;
},
): Promise<T> {
if (this.isClosing) return;
// can't run javascript if active dialog!
if (this.page.activeDialog) {
throw new Error('Cannot run frame.evaluate while an active dialog is present!!');
Expand All @@ -277,16 +277,19 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IF
}

const isolateFromWebPageEnvironment = options?.isolateFromWebPageEnvironment ?? false;
const contextId = options?.usePageDefaultContextId
? undefined
: await this.waitForContextId(isolateFromWebPageEnvironment);
let contextId: number | undefined;

try {
if (!contextId && !options?.usePageDefaultContextId) {
const notFound: any = new Error('Could not find a valid context for this request');
notFound.code = ContextNotFoundCode;
throw notFound;
// We only need to find context id when running in isolated mode, or in a frame other than the main frame.
if (isolateFromWebPageEnvironment || this !== this.page?.mainFrame) {
contextId = await this.waitForContextId(isolateFromWebPageEnvironment);
if (!contextId) {
const notFound: any = new Error('Could not find a valid context for this request');
notFound.code = ContextNotFoundCode;
throw notFound;
}
}

const result: Protocol.Runtime.EvaluateResponse = await this.devtoolsSession.send(
'Runtime.evaluate',
{
Expand All @@ -308,6 +311,8 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IF
if (remote.objectId) this.devtoolsSession.disposeRemoteObject(remote);
return remote.value as T;
} catch (err) {
if (this.isClosing) return;

const isNotFoundError =
err.code === ContextNotFoundCode ||
(err as ProtocolError).remoteError?.code === ContextNotFoundCode;
Expand Down Expand Up @@ -790,7 +795,7 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IF
try {
if (!this.isAttached) return;

// If an isolated world with the same worldName already exists chromium will reuse that world,
// If an isolated world with the same worldName already exists chromium will reuse that world,
// so calling this multiple times is safe, and can be used as creative way to get id of existing context.
// We need this because our isolated world is created with `Page.addScriptToEvaluateOnNewDocument`
// of which we don't know the contextId (since we are running with Runtime disabled to prevent detection).
Expand Down
2 changes: 1 addition & 1 deletion agent/main/lib/FramesManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ export default class FramesManager extends TypedEventEmitter<IFrameManagerEvents
refresh?: boolean;
}): Promise<number | undefined> {
const devtoolsSession = opts.devtoolsSession ?? this.devtoolsSession;
const contextId = new Resolvable<number>(3e3);
const contextId = new Resolvable<number>(10e3);

const subcriber = this.events.on(devtoolsSession, 'Runtime.executionContextCreated', event => {
const { context } = event;
Expand Down
9 changes: 3 additions & 6 deletions agent/main/lib/Page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ export default class Page extends TypedEventEmitter<IPageLevelEvents> implements
}

public get mainFrame(): Frame {
return this.framesManager.main;
return this.framesManager?.main;
}

public get frames(): Frame[] {
return this.framesManager.activeFrames;
return this.framesManager?.activeFrames;
}

public get workers(): Worker[] {
Expand Down Expand Up @@ -275,10 +275,7 @@ export default class Page extends TypedEventEmitter<IPageLevelEvents> implements
expression: string,
options?: { timeoutMs?: number; isolatedFromWebPageEnvironment?: boolean },
): Promise<T> {
return this.mainFrame.evaluate<T>(expression, {
...options,
usePageDefaultContextId: !options?.isolatedFromWebPageEnvironment,
});
return this.mainFrame.evaluate<T>(expression, options);
}

async navigate(url: string, options: { referrer?: string } = {}): Promise<{ loaderId: string }> {
Expand Down
13 changes: 12 additions & 1 deletion agent/main/test/Frames.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Browser, BrowserContext, Page } from '../index';
import Agent from '../lib/Agent';
import { attachFrame, setContent, waitForExists } from './_pageTestUtils';
import { TestServer } from './server';
import Frame from '../lib/Frame';

describe('Frames', () => {
let server: TestServer;
Expand Down Expand Up @@ -50,6 +51,16 @@ describe('Frames', () => {
expect(await page.frames[1].evaluate('window.FOO')).toBe('bar');
});

it('should not fetch context id for evaluate in mainframe', async () => {
await page.goto(server.emptyPage);
await page.waitForLoad('AllContentLoaded');
const mainFrame = page.mainFrame;

const spy = jest.spyOn<any, any>(mainFrame, 'waitForContextId');
await mainFrame.evaluate('window.location.href');
expect(spy).toHaveBeenCalledTimes(0);
});

it('should have correct execution contexts', async () => {
await page.goto(`${server.baseUrl}/frames/one-frame.html`);
await page.waitForLoad('AllContentLoaded');
Expand Down Expand Up @@ -94,7 +105,7 @@ describe('Frames', () => {
await page.waitOn('frame-created', ev => ev.frame.id === 'frame1', 2000);
}
await expect(
page.frames[1].waitForLoad({ loadStatus: 'JavascriptReady', timeoutMs:1000 }),
page.frames[1].waitForLoad({ loadStatus: 'JavascriptReady', timeoutMs: 1000 }),
).resolves.toBeTruthy();
});

Expand Down

0 comments on commit dbfff42

Please sign in to comment.