diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index b461a5d35f..01cac268dd 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -152,10 +152,6 @@ export class CRBrowser extends BrowserBase { const opener = targetInfo.openerId ? this._crPages.get(targetInfo.openerId) || null : null; const crPage = new CRPage(session, targetInfo.targetId, context, opener, this._isHeadful); this._crPages.set(targetInfo.targetId, crPage); - if (opener && opener._initializedPage) { - for (const signalBarrier of opener._initializedPage._frameManager._signalBarriers) - signalBarrier.addPopup(crPage.pageOrError()); - } crPage.pageOrError().then(() => { context!.emit(CommonEvents.BrowserContext.Page, crPage._page); if (opener) { diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 146d08a40e..565551e3c5 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -99,10 +99,6 @@ export class FFBrowser extends BrowserBase { const ffPage = new FFPage(session, context, opener); this._ffPages.set(targetId, ffPage); - if (opener && opener._initializedPage) { - for (const signalBarrier of opener._initializedPage._frameManager._signalBarriers) - signalBarrier.addPopup(ffPage.pageOrError()); - } ffPage.pageOrError().then(async () => { const page = ffPage._page; context.emit(Events.BrowserContext.Page, page); diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index ff67ec9481..a5891c95ad 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -62,6 +62,7 @@ export class FFPage implements PageDelegate { this._page = new Page(this, browserContext); this._networkManager = new FFNetworkManager(session, this._page); this._page.on(Events.Page.FrameDetached, frame => this._removeContextsForFrame(frame)); + // TODO: remove Page.willOpenNewWindowAsynchronously from the protocol. this._eventListeners = [ helper.addEventListener(this._session, 'Page.eventFired', this._onEventFired.bind(this)), helper.addEventListener(this._session, 'Page.frameAttached', this._onFrameAttached.bind(this)), @@ -73,7 +74,6 @@ export class FFPage implements PageDelegate { helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)), helper.addEventListener(this._session, 'Page.linkClicked', event => this._onLinkClicked(event.phase)), - helper.addEventListener(this._session, 'Page.willOpenNewWindowAsynchronously', this._onWillOpenNewWindowAsynchronously.bind(this)), helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)), helper.addEventListener(this._session, 'Runtime.console', this._onConsole.bind(this)), helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)), @@ -136,11 +136,6 @@ export class FFPage implements PageDelegate { this._page._frameManager.frameDidPotentiallyRequestNavigation(); } - _onWillOpenNewWindowAsynchronously() { - for (const barrier of this._page._frameManager._signalBarriers) - barrier.expectPopup(); - } - _onNavigationStarted(params: Protocol.Page.navigationStartedPayload) { this._page._frameManager.frameRequestedNavigation(params.frameId, params.navigationId); } diff --git a/src/frames.ts b/src/frames.ts index ddd9789579..0c7d6b8fb1 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -954,10 +954,8 @@ function selectorToString(selector: string, state: 'attached' | 'detached' | 'vi } export class SignalBarrier { - private _frameIds = new Map(); private _options: types.NavigatingActionWaitOptions; private _protectCount = 0; - private _expectedPopups = 0; private _promise: Promise; private _promiseCallback = () => {}; private _deadline: number; @@ -981,23 +979,6 @@ export class SignalBarrier { this.release(); } - async expectPopup() { - ++this._expectedPopups; - } - - async unexpectPopup() { - --this._expectedPopups; - this._maybeResolve(); - } - - async addPopup(pageOrError: Promise) { - if (this._expectedPopups) - --this._expectedPopups; - this.retain(); - await pageOrError; - this.release(); - } - retain() { ++this._protectCount; } @@ -1008,7 +989,7 @@ export class SignalBarrier { } private async _maybeResolve() { - if (!this._protectCount && !this._expectedPopups && !this._frameIds.size) + if (!this._protectCount) this._promiseCallback(); } } diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 4543be21c9..b712c287f9 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -142,10 +142,6 @@ export class WKBrowser extends BrowserBase { const wkPage = new WKPage(context, pageProxySession, opener || null); this._wkPages.set(pageProxyId, wkPage); - if (opener && opener._initializedPage) { - for (const signalBarrier of opener._initializedPage._frameManager._signalBarriers) - signalBarrier.addPopup(wkPage.pageOrError()); - } wkPage.pageOrError().then(async () => { const page = wkPage._page; context!.emit(Events.BrowserContext.Page, page); diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index caa17250c6..10db5b1915 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -309,6 +309,7 @@ export class WKPage implements PageDelegate { } private _addSessionListeners() { + // TODO: remove Page.willRequestOpenWindow and Page.didRequestOpenWindow from the protocol. this._sessionListeners = [ helper.addEventListener(this._session, 'Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)), helper.addEventListener(this._session, 'Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url)), @@ -318,8 +319,6 @@ export class WKPage implements PageDelegate { helper.addEventListener(this._session, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)), helper.addEventListener(this._session, 'Page.loadEventFired', event => this._onLifecycleEvent(event.frameId, 'load')), helper.addEventListener(this._session, 'Page.domContentEventFired', event => this._onLifecycleEvent(event.frameId, 'domcontentloaded')), - helper.addEventListener(this._session, 'Page.willRequestOpenWindow', event => this._onWillRequestOpenWindow()), - helper.addEventListener(this._session, 'Page.didRequestOpenWindow', event => this._onDidRequestOpenWindow(event)), helper.addEventListener(this._session, 'Runtime.executionContextCreated', event => this._onExecutionContextCreated(event.context)), helper.addEventListener(this._session, 'Console.messageAdded', event => this._onConsoleMessage(event)), helper.addEventListener(this._session, 'Console.messageRepeatCountUpdated', event => this._onConsoleRepeatCountUpdated(event)), @@ -363,18 +362,6 @@ export class WKPage implements PageDelegate { this._page._frameManager.frameLifecycleEvent(frameId, event); } - private _onWillRequestOpenWindow() { - for (const barrier of this._page._frameManager._signalBarriers) - barrier.expectPopup(); - } - - private _onDidRequestOpenWindow(event: Protocol.Page.didRequestOpenWindowPayload) { - if (!event.opened) { - for (const barrier of this._page._frameManager._signalBarriers) - barrier.unexpectPopup(); - } - } - private _handleFrameTree(frameTree: Protocol.Page.FrameResourceTree) { this._onFrameAttached(frameTree.frame.id, frameTree.frame.parentId || null); this._onFrameNavigated(frameTree.frame, true); diff --git a/test/autowaiting.spec.js b/test/autowaiting.spec.js index 6bc0ba6182..6f0e36bde8 100644 --- a/test/autowaiting.spec.js +++ b/test/autowaiting.spec.js @@ -34,26 +34,6 @@ describe('Auto waiting', () => { ]); expect(messages.join('|')).toBe('route|navigated|click'); }); - it('should await popup when clicking anchor', async function({page, server}) { - await page.goto(server.EMPTY_PAGE); - await page.setContent('link'); - const messages = []; - await Promise.all([ - page.waitForEvent('popup').then(() => messages.push('popup')), - page.click('a').then(() => messages.push('click')), - ]); - expect(messages.join('|')).toBe('popup|click'); - }); - it('should await popup when clicking anchor with noopener', async function({page, server}) { - await page.goto(server.EMPTY_PAGE); - await page.setContent('link'); - const messages = []; - await Promise.all([ - page.waitForEvent('popup').then(() => messages.push('popup')), - page.click('a').then(() => messages.push('click')), - ]); - expect(messages.join('|')).toBe('popup|click'); - }); it('should await cross-process navigation when clicking anchor', async({page, server}) => { const messages = []; server.setRoute('/empty.html', async (req, res) => { @@ -150,15 +130,6 @@ describe('Auto waiting', () => { ]); expect(messages.join('|')).toBe('route|navigated|evaluate'); }); - it('should await new popup when evaluating', async function({page, server}) { - await page.goto(server.EMPTY_PAGE); - const messages = []; - await Promise.all([ - page.waitForEvent('popup').then(() => messages.push('popup')), - page.evaluate(() => window._popup = window.open(window.location.href)).then(() => messages.push('evaluate')), - ]); - expect(messages.join('|')).toBe('popup|evaluate'); - }); it('should await navigating specified target', async({page, server}) => { const messages = []; server.setRoute('/empty.html', async (req, res) => { @@ -254,5 +225,12 @@ describe('Auto waiting should not hang when', () => { popup.close(); }); }); + it('opening a popup', async function({page, server}) { + await page.goto(server.EMPTY_PAGE); + await Promise.all([ + page.waitForEvent('popup'), + page.evaluate(() => window._popup = window.open(window.location.href)), + ]); + }); });