From 290f1b2389007143fef25af51b728a7343cf35cd Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 27 Dec 2021 16:27:07 +0300 Subject: [PATCH] gh-6037 --- src/browser/connection/gateway.ts | 11 +++++ src/browser/connection/index.ts | 2 + .../provider/built-in/dedicated/base.js | 7 +++ .../dedicated/chrome/cdp-client/index.ts | 47 +++++++++++++++++-- src/browser/provider/index.ts | 4 ++ src/browser/provider/plugin-host.js | 4 ++ src/client/browser/index.js | 7 +++ src/client/driver/driver.js | 9 +++- src/client/test-run/index.js.mustache | 3 +- src/test-run/index.ts | 1 + .../fixtures/multiple-windows/test.js | 7 +++ .../testcafe-fixtures/i6037.js | 13 +++++ 12 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 test/functional/fixtures/multiple-windows/testcafe-fixtures/i6037.js diff --git a/src/browser/connection/gateway.ts b/src/browser/connection/gateway.ts index 4091464f899..a95305b2853 100644 --- a/src/browser/connection/gateway.ts +++ b/src/browser/connection/gateway.ts @@ -64,6 +64,8 @@ export default class BrowserConnectionGateway { this._dispatch('/browser/init-script/{id}', proxy, BrowserConnectionGateway._onInitScriptResponse, 'POST'); this._dispatch('/browser/active-window-id/{id}', proxy, BrowserConnectionGateway._onGetActiveWindowIdRequest); this._dispatch('/browser/active-window-id/{id}', proxy, BrowserConnectionGateway._onSetActiveWindowIdRequest, 'POST'); + this._dispatch('/browser/close-window/{id}', proxy, BrowserConnectionGateway._onCloseWindowRequest, 'POST'); + proxy.GET(SERVICE_ROUTES.connect, (req: IncomingMessage, res: ServerResponse) => this._connectNextRemoteBrowser(req, res)); proxy.GET(SERVICE_ROUTES.connectWithTrailingSlash, (req: IncomingMessage, res: ServerResponse) => this._connectNextRemoteBrowser(req, res)); @@ -184,6 +186,15 @@ export default class BrowserConnectionGateway { } } + private static _onCloseWindowRequest (req: IncomingMessage, res: ServerResponse, connection: BrowserConnection): void { + if (BrowserConnectionGateway._ensureConnectionReady(res, connection)) { + connection.provider.closeBrowserChildWindow(connection.id) + .then(() => { + respondWithJSON(res); + }); + } + } + private async _connectNextRemoteBrowser (req: IncomingMessage, res: ServerResponse): Promise { preventCaching(res); diff --git a/src/browser/connection/index.ts b/src/browser/connection/index.ts index 5112b65e514..24ec3cc3214 100644 --- a/src/browser/connection/index.ts +++ b/src/browser/connection/index.ts @@ -100,6 +100,7 @@ export default class BrowserConnection extends EventEmitter { private readonly heartbeatUrl: string; private readonly statusUrl: string; public readonly activeWindowIdUrl: string; + public readonly closeWindowUrl: string; private statusDoneUrl: string; private readonly debugLogger: debug.Debugger; @@ -156,6 +157,7 @@ export default class BrowserConnection extends EventEmitter { this.statusRelativeUrl = `/browser/status/${this.id}`; this.statusDoneRelativeUrl = `/browser/status-done/${this.id}`; this.activeWindowIdUrl = `/browser/active-window-id/${this.id}`; + this.closeWindowUrl = `/browser/close-window/${this.id}`; this.heartbeatUrl = `${gateway.domain}${this.heartbeatRelativeUrl}`; this.statusUrl = `${gateway.domain}${this.statusRelativeUrl}`; diff --git a/src/browser/provider/built-in/dedicated/base.js b/src/browser/provider/built-in/dedicated/base.js index bc7311dc4e4..e12600d21b7 100644 --- a/src/browser/provider/built-in/dedicated/base.js +++ b/src/browser/provider/built-in/dedicated/base.js @@ -128,4 +128,11 @@ export default { return browserClient.switchToMainWindow(); }, + + async closeBrowserChildWindow (browserId) { + const runtimeInfo = this.openedBrowsers[browserId]; + const browserClient = this._getBrowserProtocolClient(runtimeInfo); + + return browserClient.closeBrowserChildWindow(); + }, }; diff --git a/src/browser/provider/built-in/dedicated/chrome/cdp-client/index.ts b/src/browser/provider/built-in/dedicated/chrome/cdp-client/index.ts index 49fce0dc76e..c2bbec7686c 100644 --- a/src/browser/provider/built-in/dedicated/chrome/cdp-client/index.ts +++ b/src/browser/provider/built-in/dedicated/chrome/cdp-client/index.ts @@ -27,14 +27,25 @@ import ClientFunctionExecutor from './client-function-executor'; import { SwitchToIframeCommand } from '../../../../../../test-run/commands/actions'; import ExecutionContext from './execution-context'; import * as clientsManager from './clients-manager'; +import delay from '../../../../../../utils/delay'; const DEBUG_SCOPE = (id: string): string => `testcafe:browser:provider:built-in:chrome:browser-client:${id}`; const DOWNLOADS_DIR = path.join(os.homedir(), 'Downloads'); const debugLog = debug('testcafe:browser:provider:built-in:dedicated:chrome'); +class ProtocolApiInfo { + public inactive: boolean; + public client: remoteChrome.ProtocolApi; + + public constructor (client: remoteChrome.ProtocolApi) { + this.client = client; + this.inactive = false; + } +} + export class BrowserClient { - private _clients: Dictionary = {}; + private _clients: Dictionary = {}; private _runtimeInfo: RuntimeInfo; private readonly _proxyless: boolean; private _parentTarget?: remoteChrome.TargetInfo; @@ -92,7 +103,7 @@ export class BrowserClient { const client = await remoteChrome({ target, port: this._runtimeInfo.cdpPort }); const { Page, Network, Runtime } = client; - this._clients[this._clientKey] = client; + this._clients[this._clientKey] = new ProtocolApiInfo(client); await guardTimeExecution( async () => await Page.enable(), @@ -222,10 +233,20 @@ export class BrowserClient { return !!this._parentTarget && this._config.headless; } + public async setClientInactive (): Promise { + // NOTE: ensure client exists + await this.getActiveClient(); + + const client = this._clients[this._clientKey]; + + if (client) + client.inactive = true; + } + public async getActiveClient (): Promise { try { if (!this._clients[this._clientKey]) - this._clients[this._clientKey] = await this._createClient(); + await this._createClient(); } catch (err) { debugLog(err); @@ -233,7 +254,12 @@ export class BrowserClient { return void 0; } - return this._clients[this._clientKey]; + const info = this._clients[this._clientKey]; + + if (info.inactive) + return void 0; + + return info.client; } public async init (): Promise { @@ -271,8 +297,12 @@ export class BrowserClient { const client = await this.getActiveClient(); - if (!client) + if (!client) { + // NOTE: required for https://github.com/DevExpress/testcafe/issues/6037 + await delay(0); + return Buffer.alloc(0); + } if (fullPage) { const { contentSize, visualViewport } = await client.Page.getLayoutMetrics(); @@ -385,4 +415,11 @@ export class BrowserClient { public switchToMainWindow (): void { ExecutionContext.switchToMainWindow(); } + + public async closeBrowserChildWindow (): Promise { + await this.setClientInactive(); + + // NOTE: delay browser window closing + await delay(100); + } } diff --git a/src/browser/provider/index.ts b/src/browser/provider/index.ts index 92e7279d28b..18655700449 100644 --- a/src/browser/provider/index.ts +++ b/src/browser/provider/index.ts @@ -442,4 +442,8 @@ export default class BrowserProvider { public setActiveWindowId (browserId: string, val: string): void { this.plugin.setActiveWindowId(browserId, val); } + + public async closeBrowserChildWindow (browserId: string): Promise { + await this.plugin.closeBrowserChildWindow(browserId); + } } diff --git a/src/browser/provider/plugin-host.js b/src/browser/provider/plugin-host.js index 2aeccf91471..6a696d9261d 100644 --- a/src/browser/provider/plugin-host.js +++ b/src/browser/provider/plugin-host.js @@ -153,4 +153,8 @@ export default class BrowserProviderPluginHost { getConfig (value) { return value; } + + closeBrowserChildWindow (/*browserId*/) { + return Promise.resolve(); + } } diff --git a/src/client/browser/index.js b/src/client/browser/index.js index 949a982d071..ef795a187c3 100644 --- a/src/client/browser/index.js +++ b/src/client/browser/index.js @@ -172,3 +172,10 @@ export function setActiveWindowId (activeWindowIdUrl, createXHR, windowId) { data: JSON.stringify({ windowId }), //eslint-disable-line no-restricted-globals }); } + +export function closeWindow (closeWindowUrl, createXHR, windowId) { + return sendXHR(closeWindowUrl, createXHR, { + method: 'POST', + data: JSON.stringify({ windowId }), //eslint-disable-line no-restricted-globals + }); +} diff --git a/src/client/driver/driver.js b/src/client/driver/driver.js index 9ffaaffac54..81bfd7bde9f 100644 --- a/src/client/driver/driver.js +++ b/src/client/driver/driver.js @@ -168,6 +168,7 @@ export default class Driver extends serviceUtils.EventEmitter { this.browserStatusUrl = communicationUrls.status; this.browserStatusDoneUrl = communicationUrls.statusDone; this.browserActiveWindowId = communicationUrls.activeWindowId; + this.browserCloseWindowUrl = communicationUrls.closeWindow; this.userAgent = runInfo.userAgent; this.fixtureName = runInfo.fixtureName; this.testName = runInfo.testName; @@ -633,11 +634,17 @@ export default class Driver extends serviceUtils.EventEmitter { }); } - _closeWindowAndWait (childWindowToClose, msg) { + async _closeWindowAndWait (childWindowToClose, msg) { const waitWindowForClose = this._createWaitForEventPromise(CHILD_WINDOW_CLOSED_EVENT, CHILD_WINDOW_CLOSED_EVENT_TIMEOUT); childWindowToClose.ignoreMasterSwitching = !msg.isCurrentWindow; + if (!this.closing) { + this.closing = true; + + await browser.closeWindow(this.browserCloseWindowUrl, hammerhead.createNativeXHR, this.windowId); + } + childWindowToClose.driverWindow.close(); return waitWindowForClose; diff --git a/src/client/test-run/index.js.mustache b/src/client/test-run/index.js.mustache index ac999e34e05..13bc1e307af 100644 --- a/src/client/test-run/index.js.mustache +++ b/src/client/test-run/index.js.mustache @@ -19,6 +19,7 @@ var browserStatusUrl = origin + {{{browserStatusRelativeUrl}}}; var browserStatusDoneUrl = origin + {{{browserStatusDoneRelativeUrl}}}; var browserActiveWindowIdUrl = origin + {{{browserActiveWindowIdUrl}}}; + var browserCloseWindowUrl = origin + {{{browserCloseWindowUrl}}}; var skipJsErrors = {{{skipJsErrors}}}; var dialogHandler = {{{dialogHandler}}}; var userAgent = {{{userAgent}}}; @@ -28,7 +29,7 @@ var ClientDriver = window['%testCafeDriver%']; var driver = new ClientDriver(testRunId, - { heartbeat: browserHeartbeatUrl, status: browserStatusUrl, statusDone: browserStatusDoneUrl, activeWindowId: browserActiveWindowIdUrl }, + { heartbeat: browserHeartbeatUrl, status: browserStatusUrl, statusDone: browserStatusDoneUrl, activeWindowId: browserActiveWindowIdUrl, closeWindow: browserCloseWindowUrl }, { userAgent: userAgent, fixtureName: fixtureName, testName: testName }, { selectorTimeout: selectorTimeout, diff --git a/src/test-run/index.ts b/src/test-run/index.ts index 200bf3024be..783e162e656 100644 --- a/src/test-run/index.ts +++ b/src/test-run/index.ts @@ -535,6 +535,7 @@ export default class TestRun extends AsyncEventEmitter { browserStatusRelativeUrl: JSON.stringify(this.browserConnection.statusRelativeUrl), browserStatusDoneRelativeUrl: JSON.stringify(this.browserConnection.statusDoneRelativeUrl), browserActiveWindowIdUrl: JSON.stringify(this.browserConnection.activeWindowIdUrl), + browserCloseWindowUrl: JSON.stringify(this.browserConnection.closeWindowUrl), userAgent: JSON.stringify(this.browserConnection.userAgent), testName: JSON.stringify(this.test.name), fixtureName: JSON.stringify((this.test.fixture as Fixture).name), diff --git a/test/functional/fixtures/multiple-windows/test.js b/test/functional/fixtures/multiple-windows/test.js index 5d8f4f7b56e..9e93d8e0482 100644 --- a/test/functional/fixtures/multiple-windows/test.js +++ b/test/functional/fixtures/multiple-windows/test.js @@ -156,6 +156,13 @@ describe('Multiple windows', () => { return runTests('testcafe-fixtures/i6085.js'); }); + it('Should not hang on close window whide video is recording', () => { + return runTests('testcafe-fixtures/i6037.js', '', { + only: 'chrome', + setVideoPath: true, + }); + }); + describe('API', () => { it('Open child window', () => { return runTests('testcafe-fixtures/api/api-test.js', 'Open child window', { only: 'chrome' }); diff --git a/test/functional/fixtures/multiple-windows/testcafe-fixtures/i6037.js b/test/functional/fixtures/multiple-windows/testcafe-fixtures/i6037.js new file mode 100644 index 00000000000..ea00050ac67 --- /dev/null +++ b/test/functional/fixtures/multiple-windows/testcafe-fixtures/i6037.js @@ -0,0 +1,13 @@ +const parent = 'http://localhost:3000/fixtures/multiple-windows/pages/api/parent.html'; +const child = 'http://localhost:3000/fixtures/multiple-windows/pages/api/child-2.html'; + +fixture('Should not hang on close window whide video is recording') + .page(parent); + +for (let i = 0; i < 10; i++) { + test(`attempt ${i}`, async t => { + await t + .openWindow(child) + .closeWindow(); + }); +}