Skip to content

chore: make various progress instances "strict" #36349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 34 additions & 28 deletions packages/playwright-core/src/server/android/android.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import { chromiumSwitches } from '../chromium/chromiumSwitches';
import { CRBrowser } from '../chromium/crBrowser';
import { removeFolders } from '../utils/fileUtils';
import { helper } from '../helper';
import { SdkObject, serverSideCallMetadata } from '../instrumentation';
import { CallMetadata, SdkObject } from '../instrumentation';
import { gracefullyCloseSet } from '../utils/processLauncher';
import { ProgressController } from '../progress';
import { Progress, ProgressController } from '../progress';
import { registry } from '../registry';

import type { BrowserOptions, BrowserProcess } from '../browser';
Expand Down Expand Up @@ -122,6 +122,7 @@ export class AndroidDevice extends SdkObject {
this.model = model;
this.serial = backend.serial;
this._options = options;
this.logName = 'browser';
}

static async create(android: Android, backend: DeviceBackend, options: channels.AndroidDevicesOptions): Promise<AndroidDevice> {
Expand Down Expand Up @@ -258,18 +259,21 @@ export class AndroidDevice extends SdkObject {
this.emit(AndroidDevice.Events.Close);
}

async launchBrowser(pkg: string = 'com.android.chrome', options: channels.AndroidDeviceLaunchBrowserParams): Promise<BrowserContext> {
debug('pw:android')('Force-stopping', pkg);
await this._backend.runCommand(`shell:am force-stop ${pkg}`);
const socketName = isUnderTest() ? 'webview_devtools_remote_playwright_test' : ('playwright_' + createGuid() + '_devtools_remote');
const commandLine = this._defaultArgs(options, socketName).join(' ');
debug('pw:android')('Starting', pkg, commandLine);
// encode commandLine to base64 to avoid issues (bash encoding) with special characters
await this._backend.runCommand(`shell:echo "${Buffer.from(commandLine).toString('base64')}" | base64 -d > /data/local/tmp/chrome-command-line`);
await this._backend.runCommand(`shell:am start -a android.intent.action.VIEW -d about:blank ${pkg}`);
const browserContext = await this._connectToBrowser(socketName, options);
await this._backend.runCommand(`shell:rm /data/local/tmp/chrome-command-line`);
return browserContext;
async launchBrowser(metadata: CallMetadata, pkg: string = 'com.android.chrome', options: channels.AndroidDeviceLaunchBrowserParams): Promise<BrowserContext> {
const controller = new ProgressController(metadata, this, 'strict');
return controller.run(async progress => {
debug('pw:android')('Force-stopping', pkg);
await this._backend.runCommand(`shell:am force-stop ${pkg}`);
const socketName = isUnderTest() ? 'webview_devtools_remote_playwright_test' : ('playwright_' + createGuid() + '_devtools_remote');
const commandLine = this._defaultArgs(options, socketName).join(' ');
debug('pw:android')('Starting', pkg, commandLine);
// encode commandLine to base64 to avoid issues (bash encoding) with special characters
await progress.race(this._backend.runCommand(`shell:echo "${Buffer.from(commandLine).toString('base64')}" | base64 -d > /data/local/tmp/chrome-command-line`));
await progress.race(this._backend.runCommand(`shell:am start -a android.intent.action.VIEW -d about:blank ${pkg}`));
const browserContext = await this._connectToBrowser(progress, socketName, options);
await progress.race(this._backend.runCommand(`shell:rm /data/local/tmp/chrome-command-line`));
return browserContext;
});
}

private _defaultArgs(options: channels.AndroidDeviceLaunchBrowserParams, socketName: string): string[] {
Expand Down Expand Up @@ -301,25 +305,30 @@ export class AndroidDevice extends SdkObject {
return chromeArguments;
}

async connectToWebView(socketName: string): Promise<BrowserContext> {
const webView = this._webViews.get(socketName);
if (!webView)
throw new Error('WebView has been closed');
return await this._connectToBrowser(socketName);
async connectToWebView(metadata: CallMetadata, socketName: string): Promise<BrowserContext> {
const controller = new ProgressController(metadata, this, 'strict');
return controller.run(async progress => {
const webView = this._webViews.get(socketName);
if (!webView)
throw new Error('WebView has been closed');
return await this._connectToBrowser(progress, socketName);
});
}

private async _connectToBrowser(socketName: string, options: types.BrowserContextOptions = {}): Promise<BrowserContext> {
const socket = await this._waitForLocalAbstract(socketName);
private async _connectToBrowser(progress: Progress, socketName: string, options: types.BrowserContextOptions = {}): Promise<BrowserContext> {
const socket = await progress.race(this._waitForLocalAbstract(socketName));
const androidBrowser = new AndroidBrowser(this, socket);
await androidBrowser._init();
progress.cleanupWhenAborted(() => androidBrowser.close());
await progress.race(androidBrowser._init());
this._browserConnections.add(androidBrowser);

const artifactsDir = await fs.promises.mkdtemp(ARTIFACTS_FOLDER);
const artifactsDir = await progress.race(fs.promises.mkdtemp(ARTIFACTS_FOLDER));
const cleanupArtifactsDir = async () => {
const errors = (await removeFolders([artifactsDir])).filter(Boolean);
for (let i = 0; i < (errors || []).length; ++i)
debug('pw:android')(`exception while removing ${artifactsDir}: ${errors[i]}`);
};
progress.cleanupWhenAborted(cleanupArtifactsDir);
gracefullyCloseSet.add(cleanupArtifactsDir);
socket.on('close', async () => {
gracefullyCloseSet.delete(cleanupArtifactsDir);
Expand All @@ -341,12 +350,9 @@ export class AndroidDevice extends SdkObject {
};
validateBrowserContextOptions(options, browserOptions);

const browser = await CRBrowser.connect(this.attribution.playwright, androidBrowser, browserOptions);
const controller = new ProgressController(serverSideCallMetadata(), this);
const browser = await progress.race(CRBrowser.connect(this.attribution.playwright, androidBrowser, browserOptions));
const defaultContext = browser._defaultContext!;
await controller.run(async progress => {
await defaultContext._loadDefaultContextAsIs(progress);
});
await defaultContext._loadDefaultContextAsIs(progress);
return defaultContext;
}

Expand Down
32 changes: 18 additions & 14 deletions packages/playwright-core/src/server/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,12 @@ export abstract class BrowserContext extends SdkObject {
}

async resetForReuse(metadata: CallMetadata, params: channels.BrowserNewContextForReuseParams | null) {
const controller = new ProgressController(metadata, this);
const controller = new ProgressController(metadata, this, 'strict');
return controller.run(progress => this.resetForReuseImpl(progress, params));
}

async resetForReuseImpl(progress: Progress, params: channels.BrowserNewContextForReuseParams | null) {
await this.tracing.resetForReuse();
await progress.race(this.tracing.resetForReuse());

if (params) {
for (const key of paramsThatAllowContextReuse)
Expand All @@ -219,18 +219,22 @@ export abstract class BrowserContext extends SdkObject {
await page?.mainFrame().gotoImpl(progress, 'about:blank', {});

await this._resetStorage(progress);
await this.clock.resetForReuse();
// TODO: following can be optimized to not perform noops.
if (this._options.permissions)
await this.grantPermissions(this._options.permissions);
else
await this.clearPermissions();
await this.setExtraHTTPHeaders(this._options.extraHTTPHeaders || []);
await this.setGeolocation(this._options.geolocation);
await this.setOffline(!!this._options.offline);
await this.setUserAgent(this._options.userAgent);
await this.clearCache();
await this._resetCookies();

const resetOptions = async () => {
await this.clock.resetForReuse();
// TODO: following can be optimized to not perform noops.
if (this._options.permissions)
await this.grantPermissions(this._options.permissions);
else
await this.clearPermissions();
await this.setExtraHTTPHeaders(this._options.extraHTTPHeaders || []);
await this.setGeolocation(this._options.geolocation);
await this.setOffline(!!this._options.offline);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't all these protocol calls take progress (eventually) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not yet sure. There are some calls that need specific cleanup and so cannot just be generalized as "race this protocol call with a progress".

await this.setUserAgent(this._options.userAgent);
await this.clearCache();
await this._resetCookies();
};
await progress.race(resetOptions());

await page?.resetForReuse(progress);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ export class VideoRecorder {
if (!options.outputFile.endsWith('.webm'))
throw new Error('File must have .webm extension');

const controller = new ProgressController(serverSideCallMetadata(), page);
const controller = new ProgressController(serverSideCallMetadata(), page, 'strict');
controller.setLogName('browser');
return await controller.run(async progress => {
const recorder = new VideoRecorder(page, ffmpegPath, progress);
progress.cleanupWhenAborted(() => recorder.stop());
await recorder._launch(options);
return recorder;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,21 +160,21 @@ export class AndroidDeviceDispatcher extends Dispatcher<AndroidDevice, channels.
await this._object.push(params.file, params.path, params.mode);
}

async launchBrowser(params: channels.AndroidDeviceLaunchBrowserParams): Promise<channels.AndroidDeviceLaunchBrowserResult> {
async launchBrowser(params: channels.AndroidDeviceLaunchBrowserParams, metadata: CallMetadata): Promise<channels.AndroidDeviceLaunchBrowserResult> {
if (this.parentScope()._denyLaunch)
throw new Error(`Launching more browsers is not allowed.`);
const context = await this._object.launchBrowser(params.pkg, params);
const context = await this._object.launchBrowser(metadata, params.pkg, params);
return { context: BrowserContextDispatcher.from(this, context) };
}

async close(params: channels.AndroidDeviceCloseParams) {
await this._object.close();
}

async connectToWebView(params: channels.AndroidDeviceConnectToWebViewParams): Promise<channels.AndroidDeviceConnectToWebViewResult> {
async connectToWebView(params: channels.AndroidDeviceConnectToWebViewParams, metadata: CallMetadata): Promise<channels.AndroidDeviceConnectToWebViewResult> {
if (this.parentScope()._denyLaunch)
throw new Error(`Launching more browsers is not allowed.`);
return { context: BrowserContextDispatcher.from(this, await this._object.connectToWebView(params.socketName)) };
return { context: BrowserContextDispatcher.from(this, await this._object.connectToWebView(metadata, params.socketName)) };
}
}

Expand Down
22 changes: 10 additions & 12 deletions packages/playwright-core/src/server/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1370,9 +1370,9 @@ export class Frame extends SdkObject {
}

async ariaSnapshot(metadata: CallMetadata, selector: string, options: { forAI?: boolean } & types.TimeoutOptions): Promise<string> {
const controller = new ProgressController(metadata, this);
const controller = new ProgressController(metadata, this, 'strict');
return controller.run(async progress => {
return await this._retryWithProgressIfNotConnected(progress, selector, true /* strict */, true /* performActionPreChecks */, handle => handle.ariaSnapshot(options));
return await this._retryWithProgressIfNotConnected(progress, selector, true /* strict */, true /* performActionPreChecks */, handle => progress.race(handle.ariaSnapshot(options)));
}, options.timeout);
}

Expand All @@ -1391,7 +1391,7 @@ export class Frame extends SdkObject {
const start = timeout > 0 ? monotonicTime() : 0;

// Step 1: perform locator handlers checkpoint with a specified timeout.
await (new ProgressController(metadata, this)).run(async progress => {
await (new ProgressController(metadata, this, 'strict')).run(async progress => {
progress.log(`${renderTitleForCall(metadata)}${timeout ? ` with timeout ${timeout}ms` : ''}`);
if (selector)
progress.log(`waiting for ${this._asLocator(selector)}`);
Expand All @@ -1402,7 +1402,7 @@ export class Frame extends SdkObject {
// Supports the case of `expect(locator).toBeVisible({ timeout: 1 })`
// that should succeed when the locator is already visible.
try {
const resultOneShot = await (new ProgressController(metadata, this)).run(async progress => {
const resultOneShot = await (new ProgressController(metadata, this, 'strict')).run(async progress => {
return await this._expectInternal(progress, selector, options, lastIntermediateResult);
});
if (resultOneShot.matches !== options.isNot)
Expand All @@ -1420,7 +1420,7 @@ export class Frame extends SdkObject {
return { matches: options.isNot, log: compressCallLog(metadata.log), timedOut: true, received: lastIntermediateResult.received };

// Step 3: auto-retry expect with increasing timeouts. Bounded by the total remaining time.
return await (new ProgressController(metadata, this)).run(async progress => {
return await (new ProgressController(metadata, this, 'strict')).run(async progress => {
return await this.retryWithProgressAndTimeouts(progress, [100, 250, 500, 1000], async continuePolling => {
await this._page.performActionPreChecks(progress);
const { matches, received } = await this._expectInternal(progress, selector, options, lastIntermediateResult);
Expand Down Expand Up @@ -1448,16 +1448,14 @@ export class Frame extends SdkObject {
}

private async _expectInternal(progress: Progress, selector: string | undefined, options: FrameExpectParams, lastIntermediateResult: { received?: any, isSet: boolean }) {
const selectorInFrame = selector ? await this.selectors.resolveFrameForSelector(selector, { strict: true }) : undefined;
progress.throwIfAborted();
const selectorInFrame = selector ? await progress.race(this.selectors.resolveFrameForSelector(selector, { strict: true })) : undefined;

const { frame, info } = selectorInFrame || { frame: this, info: undefined };
const world = options.expression === 'to.have.property' ? 'main' : (info?.world ?? 'utility');
const context = await frame._context(world);
const injected = await context.injectedScript();
progress.throwIfAborted();
const context = await progress.race(frame._context(world));
const injected = await progress.race(context.injectedScript());

const { log, matches, received, missingReceived } = await injected.evaluate(async (injected, { info, options, callId }) => {
const { log, matches, received, missingReceived } = await progress.race(injected.evaluate(async (injected, { info, options, callId }) => {
const elements = info ? injected.querySelectorAll(info.parsed, document) : [];
if (callId)
injected.markTargetElements(new Set(elements), callId);
Expand All @@ -1470,7 +1468,7 @@ export class Frame extends SdkObject {
else if (elements.length)
log = ` locator resolved to ${injected.previewNode(elements[0])}`;
return { log, ...await injected.expect(elements[0], options, elements) };
}, { info, options, callId: progress.metadata.id });
}, { info, options, callId: progress.metadata.id }));

if (log)
progress.log(log);
Expand Down
54 changes: 27 additions & 27 deletions packages/playwright-core/src/server/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,10 +812,13 @@ export class Page extends SdkObject {
this._isServerSideOnly = true;
}

async snapshotForAI(metadata: CallMetadata): Promise<string> {
this.lastSnapshotFrameIds = [];
const snapshot = await snapshotFrameForAI(metadata, this.mainFrame(), 0, this.lastSnapshotFrameIds);
return snapshot.join('\n');
snapshotForAI(metadata: CallMetadata): Promise<string> {
const controller = new ProgressController(metadata, this, 'strict');
return controller.run(async progress => {
this.lastSnapshotFrameIds = [];
const snapshot = await snapshotFrameForAI(progress, this.mainFrame(), 0, this.lastSnapshotFrameIds);
return snapshot.join('\n');
});
}
}

Expand Down Expand Up @@ -991,29 +994,26 @@ class FrameThrottler {
}
}

async function snapshotFrameForAI(metadata: CallMetadata, frame: frames.Frame, frameOrdinal: number, frameIds: string[]): Promise<string[]> {
async function snapshotFrameForAI(progress: Progress, frame: frames.Frame, frameOrdinal: number, frameIds: string[]): Promise<string[]> {
// Only await the topmost navigations, inner frames will be empty when racing.
const controller = new ProgressController(metadata, frame);
const snapshot = await controller.run(progress => {
return frame.retryWithProgressAndTimeouts(progress, [1000, 2000, 4000, 8000], async continuePolling => {
try {
const context = await frame._utilityContext();
const injectedScript = await context.injectedScript();
const snapshotOrRetry = await injectedScript.evaluate((injected, refPrefix) => {
const node = injected.document.body;
if (!node)
return true;
return injected.ariaSnapshot(node, { forAI: true, refPrefix });
}, frameOrdinal ? 'f' + frameOrdinal : '');
if (snapshotOrRetry === true)
return continuePolling;
return snapshotOrRetry;
} catch (e) {
if (isAbortError(e) || isSessionClosedError(e) || js.isJavaScriptErrorInEvaluate(e))
throw e;
const snapshot = await frame.retryWithProgressAndTimeouts(progress, [1000, 2000, 4000, 8000], async continuePolling => {
try {
const context = await progress.race(frame._utilityContext());
const injectedScript = await progress.race(context.injectedScript());
const snapshotOrRetry = await progress.race(injectedScript.evaluate((injected, refPrefix) => {
const node = injected.document.body;
if (!node)
return true;
return injected.ariaSnapshot(node, { forAI: true, refPrefix });
}, frameOrdinal ? 'f' + frameOrdinal : ''));
if (snapshotOrRetry === true)
return continuePolling;
}
});
return snapshotOrRetry;
} catch (e) {
if (isAbortError(e) || isSessionClosedError(e) || js.isJavaScriptErrorInEvaluate(e))
throw e;
return continuePolling;
}
});

const lines = snapshot.split('\n');
Expand All @@ -1029,15 +1029,15 @@ async function snapshotFrameForAI(metadata: CallMetadata, frame: frames.Frame, f
const ref = match[2];
const frameSelector = `aria-ref=${ref} >> internal:control=enter-frame`;
const frameBodySelector = `${frameSelector} >> body`;
const child = await frame.selectors.resolveFrameForSelector(frameBodySelector, { strict: true });
const child = await progress.race(frame.selectors.resolveFrameForSelector(frameBodySelector, { strict: true }));
if (!child) {
result.push(line);
continue;
}
const frameOrdinal = frameIds.length + 1;
frameIds.push(child.frame._id);
try {
const childSnapshot = await snapshotFrameForAI(metadata, child.frame, frameOrdinal, frameIds);
const childSnapshot = await snapshotFrameForAI(progress, child.frame, frameOrdinal, frameIds);
result.push(line + ':', ...childSnapshot.map(l => leadingSpace + ' ' + l));
} catch {
result.push(line);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
timeout: 0,
}
});
const controller = new ProgressController(serverSideCallMetadata(), context._browser);
const controller = new ProgressController(serverSideCallMetadata(), context._browser, 'strict');
await controller.run(async progress => {
await context._browser._defaultContext!._loadDefaultContextAsIs(progress);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export async function openTraceViewerApp(url: string, browserName: string, optio
},
});

const controller = new ProgressController(serverSideCallMetadata(), context._browser);
const controller = new ProgressController(serverSideCallMetadata(), context._browser, 'strict');
await controller.run(async progress => {
await context._browser._defaultContext!._loadDefaultContextAsIs(progress);
});
Expand Down
Loading