Skip to content

Commit

Permalink
cherry-pick(#18997): fix(reuse): stop pending operations upon reuse/d…
Browse files Browse the repository at this point in the history
…isconnect

SHA 503f8f5
  • Loading branch information
dgozman authored and aslushnikov committed Nov 23, 2022
1 parent 76dc43e commit 9ede66a
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 1 deletion.
2 changes: 2 additions & 0 deletions packages/playwright-core/src/remote/playwrightConnection.ts
Expand Up @@ -208,6 +208,8 @@ export class PlaywrightConnection {
for (const context of browser.contexts()) {
if (!context.pages().length)
await context.close(serverSideCallMetadata());
else
await context.stopPendingOperations();
}
if (!browser.contexts())
await browser.close();
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/server/browser.ts
Expand Up @@ -106,6 +106,7 @@ export abstract class Browser extends SdkObject {
this._contextForReuse = { context: await this.newContext(metadata, params), hash };
return { context: this._contextForReuse.context, needsReset: false };
}
await this._contextForReuse.context.stopPendingOperations();
return { context: this._contextForReuse.context, needsReset: true };
}

Expand Down
8 changes: 7 additions & 1 deletion packages/playwright-core/src/server/browserContext.ts
Expand Up @@ -26,7 +26,7 @@ import { helper } from './helper';
import * as network from './network';
import type { PageDelegate } from './page';
import { Page, PageBinding } from './page';
import type { Progress } from './progress';
import type { Progress, ProgressController } from './progress';
import type { Selectors } from './selectors';
import type * as types from './types';
import type * as channels from '@protocol/channels';
Expand Down Expand Up @@ -56,6 +56,7 @@ export abstract class BrowserContext extends SdkObject {

readonly _timeoutSettings = new TimeoutSettings();
readonly _pageBindings = new Map<string, PageBinding>();
readonly _activeProgressControllers = new Set<ProgressController>();
readonly _options: channels.BrowserNewContextParams;
_requestInterceptor?: network.RouteHandler;
private _isPersistentContext: boolean;
Expand Down Expand Up @@ -145,6 +146,11 @@ export abstract class BrowserContext extends SdkObject {
return true;
}

async stopPendingOperations() {
for (const controller of this._activeProgressControllers)
controller.abort(new Error(`Context was reset for reuse.`));
}

static reusableContextHash(params: channels.BrowserNewContextForReuseParams): string {
const paramsCopy = { ...params };

Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/server/fetch.ts
Expand Up @@ -78,6 +78,7 @@ export abstract class APIRequestContext extends SdkObject {
readonly fetchResponses: Map<string, Buffer> = new Map();
readonly fetchLog: Map<string, string[]> = new Map();
protected static allInstances: Set<APIRequestContext> = new Set();
readonly _activeProgressControllers = new Set<ProgressController>();

static findResponseBody(guid: string): Buffer | undefined {
for (const request of APIRequestContext.allInstances) {
Expand Down
6 changes: 6 additions & 0 deletions packages/playwright-core/src/server/progress.ts
Expand Up @@ -63,6 +63,10 @@ export class ProgressController {
return this._lastIntermediateResult;
}

abort(error: Error) {
this._forceAbortPromise.reject(error);
}

async run<T>(task: (progress: Progress) => Promise<T>, timeout?: number): Promise<T> {
if (timeout) {
this._timeout = timeout;
Expand All @@ -71,6 +75,7 @@ export class ProgressController {

assert(this._state === 'before');
this._state = 'running';
this.sdkObject.attribution.context?._activeProgressControllers.add(this);

const progress: Progress = {
log: message => {
Expand Down Expand Up @@ -117,6 +122,7 @@ export class ProgressController {
await Promise.all(this._cleanups.splice(0).map(runCleanup));
throw e;
} finally {
this.sdkObject.attribution.context?._activeProgressControllers.delete(this);
clearTimeout(timer);
}
}
Expand Down
24 changes: 24 additions & 0 deletions tests/playwright-test/playwright.reuse.spec.ts
Expand Up @@ -374,3 +374,27 @@ test('should reuse context with beforeunload', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
});

test('should cancel pending operations upon reuse', async ({ runInlineTest }) => {
const result = await runInlineTest({
'src/reuse.test.ts': `
const { test } = pwt;
test('one', async ({ page }) => {
await Promise.race([
page.getByText('click me').click().catch(e => {}),
page.waitForTimeout(2000),
]);
});
test('two', async ({ page }) => {
await page.setContent('<button onclick="window._clicked=true">click me</button>');
// Give it time to erroneously click.
await page.waitForTimeout(2000);
expect(await page.evaluate('window._clicked')).toBe(undefined);
});
`,
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: '1' });

expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
});

0 comments on commit 9ede66a

Please sign in to comment.