From 2e6f544342a4fbd9c7159c051b5f7bf6ecaaaf0a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 14 Apr 2020 14:51:23 -0700 Subject: [PATCH] chore(webkit): stop using windowOpen signal to determine initial empty page (#1776) --- src/webkit/wkBrowser.ts | 20 +------------------- src/webkit/wkPage.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index cdca4cc826..cf4fa01caf 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -37,8 +37,6 @@ export class WKBrowser extends BrowserBase { readonly _contexts = new Map(); readonly _wkPages = new Map(); private readonly _eventListeners: RegisteredListener[]; - private _popupOpeners: string[] = []; - private _closeOverride?: () => Promise; private _firstPageCallback: () => void = () => {}; private readonly _firstPagePromise: Promise; @@ -60,7 +58,6 @@ export class WKBrowser extends BrowserBase { helper.addEventListener(this._browserSession, 'Playwright.pageProxyCreated', this._onPageProxyCreated.bind(this)), helper.addEventListener(this._browserSession, 'Playwright.pageProxyDestroyed', this._onPageProxyDestroyed.bind(this)), helper.addEventListener(this._browserSession, 'Playwright.provisionalLoadFailed', event => this._onProvisionalLoadFailed(event)), - helper.addEventListener(this._browserSession, 'Playwright.windowOpen', this._onWindowOpen.bind(this)), helper.addEventListener(this._browserSession, 'Playwright.downloadCreated', this._onDownloadCreated.bind(this)), helper.addEventListener(this._browserSession, 'Playwright.downloadFinished', this._onDownloadFinished.bind(this)), helper.addEventListener(this._browserSession, kPageProxyMessageReceived, this._onPageProxyMessageReceived.bind(this)), @@ -98,10 +95,6 @@ export class WKBrowser extends BrowserBase { return this._firstPagePromise; } - _onWindowOpen(payload: Protocol.Playwright.windowOpenPayload) { - this._popupOpeners.push(payload.pageProxyId); - } - _onDownloadCreated(payload: Protocol.Playwright.downloadCreatedPayload) { const page = this._wkPages.get(payload.pageProxyId); if (!page) @@ -132,18 +125,7 @@ export class WKBrowser extends BrowserBase { this._connection.rawSend({ ...message, pageProxyId }); }); const opener = pageProxyInfo.openerId ? this._wkPages.get(pageProxyInfo.openerId) : undefined; - let hasInitialAboutBlank = false; - if (pageProxyInfo.openerId) { - const openerIndex = this._popupOpeners.indexOf(pageProxyInfo.openerId); - if (openerIndex !== -1) { - this._popupOpeners.splice(openerIndex, 1); - // When this page is a result of window.open($url) call, we should have it's opener - // in the list of popup openers. In this case we know there is an initial - // about:blank navigation, followed by a navigation to $url. - hasInitialAboutBlank = true; - } - } - const wkPage = new WKPage(context, pageProxySession, opener || null, hasInitialAboutBlank); + const wkPage = new WKPage(context, pageProxySession, opener || null); this._wkPages.set(pageProxyId, wkPage); wkPage.pageOrError().then(async () => { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index d212e3c3ba..cbea2b0ce3 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -59,14 +59,12 @@ export class WKPage implements PageDelegate { private readonly _evaluateOnNewDocumentSources: string[] = []; readonly _browserContext: WKBrowserContext; private _initialized = false; - private _hasInitialAboutBlank: boolean; private _firstNonInitialNavigationCommittedPromise: Promise; private _firstNonInitialNavigationCommittedCallback = () => {}; - constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null, hasInitialAboutBlank: boolean) { + constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null) { this._pageProxySession = pageProxySession; this._opener = opener; - this._hasInitialAboutBlank = hasInitialAboutBlank; this.rawKeyboard = new RawKeyboardImpl(pageProxySession); this.rawMouse = new RawMouseImpl(pageProxySession); this._contextIdToContext = new Map(); @@ -262,8 +260,12 @@ export class WKPage implements PageDelegate { } if (targetInfo.isPaused) this._pageProxySession.send('Target.resume', { targetId: targetInfo.targetId }).catch(debugError); - if (this._hasInitialAboutBlank) + if (this._page.mainFrame().url() === '') { + // Initial empty page has an empty url. We should wait until the first real url has been loaded, + // even if that url is about:blank. This is especially important for popups, where we need the + // actual url before interacting with it. await this._firstNonInitialNavigationCommittedPromise; + } this._initialized = true; this._pagePromiseCallback(pageOrError); } else {