From 0cc270a15dce5b25160f778f300fba46e6bd1721 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 21 Jan 2020 17:10:38 -0800 Subject: [PATCH] fix(webkit): always push state changes to the provisional page --- src/webkit/wkPage.ts | 74 +++++++++++++++++++++++++++++++-------- src/webkit/wkPageProxy.ts | 33 +++++------------ test/interception.spec.js | 13 +++++++ 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 329f8b24bf..0f877c0c28 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -16,7 +16,7 @@ */ import * as frames from '../frames'; -import { debugError, helper, RegisteredListener } from '../helper'; +import { debugError, helper, RegisteredListener, assert } from '../helper'; import * as dom from '../dom'; import * as network from '../network'; import { WKSession } from './wkConnection'; @@ -33,6 +33,7 @@ import * as types from '../types'; import * as accessibility from '../accessibility'; import * as platform from '../platform'; import { getAccessibilityTree } from './wkAccessibility'; +import { WKProvisionalPage } from './wkProvisionalPage'; const UTILITY_WORLD_NAME = '__playwright_utility_world__'; const BINDING_CALL_MESSAGE = '__playwright_binding_call__'; @@ -41,6 +42,7 @@ export class WKPage implements PageDelegate { readonly rawMouse: RawMouseImpl; readonly rawKeyboard: RawKeyboardImpl; _session: WKSession; + private _provisionalPage: WKProvisionalPage | null = null; readonly _page: Page; private readonly _pageProxySession: WKSession; private readonly _requestIdToRequest = new Map(); @@ -79,8 +81,6 @@ export class WKPage implements PageDelegate { this.rawKeyboard.setSession(session); this._addSessionListeners(); this._workers.setSession(session); - // New bootstrap scripts may have been added during provisional load, push them - // again to be on the safe side. if (this._bootstrapScripts.length) this._setBootstrapScripts(session).catch(e => debugError(e)); } @@ -121,8 +121,10 @@ export class WKPage implements PageDelegate { promises.push(session.send('Page.overrideUserAgent', { value: contextOptions.userAgent })); if (this._page._state.mediaType || this._page._state.colorScheme) promises.push(WKPage._setEmulateMedia(session, this._page._state.mediaType, this._page._state.colorScheme)); - if (isProvisional) - promises.push(this._setBootstrapScripts(session)); + if (isProvisional && this._bootstrapScripts.length) { + const source = this._bootstrapScripts.join(';'); + promises.push(session.send('Page.setBootstrapScript', { source })); + } if (contextOptions.bypassCSP) promises.push(session.send('Page.setBypassCSP', { enabled: true })); if (this._page._state.extraHTTPHeaders !== null) @@ -139,6 +141,30 @@ export class WKPage implements PageDelegate { }); } + onProvisionalLoadStarted(provisionalSession: WKSession) { + assert(!this._provisionalPage); + this._provisionalPage = new WKProvisionalPage(provisionalSession, this); + } + + onProvisionalLoadCommitted(session: WKSession) { + assert(this._provisionalPage); + assert(this._provisionalPage!._session === session); + this._provisionalPage!.commit(); + this._provisionalPage!.dispose(); + this._provisionalPage = null; + this.setSession(session); + } + + onSessionDestroyed(session: WKSession, crashed: boolean) { + if (this._provisionalPage && this._provisionalPage._session === session) { + this._provisionalPage.dispose(); + this._provisionalPage = null; + return; + } + if (this._session === session && crashed) + this.didClose(crashed); + } + didClose(crashed: boolean) { helper.removeEventListeners(this._sessionListeners); this.disconnectFromTarget(); @@ -148,7 +174,11 @@ export class WKPage implements PageDelegate { this._page._didClose(); } - didDisconnect() { + dispose() { + if (this._provisionalPage) { + this._provisionalPage.dispose(); + this._provisionalPage = null; + } this._page._didDisconnect(); } @@ -188,6 +218,22 @@ export class WKPage implements PageDelegate { this._contextIdToContext.clear(); } + private async _updateState( + method: T, + params?: Protocol.CommandParameters[T] + ): Promise { + const promises = [ + this._session.send(method, params) + ]; + // If the state changes during provisional load, push it to the provisional page + // as well to always be in sync with the backend. + if (this._provisionalPage) + promises.push(this._provisionalPage._session.send(method, params)); + for (const p of promises) + p.catch(debugError); + await Promise.all(promises); + } + _onFrameStoppedLoading(frameId: string) { this._page._frameManager.frameStoppedLoading(frameId); } @@ -344,21 +390,21 @@ export class WKPage implements PageDelegate { this._page._state.hasTouch = !!viewport.isMobile; await Promise.all([ this._pageProxySession.send('Emulation.setDeviceMetricsOverride', {width, height, fixedLayout, deviceScaleFactor }), - this._session.send('Page.setTouchEmulationEnabled', { enabled: !!viewport.isMobile }), + this._updateState('Page.setTouchEmulationEnabled', { enabled: !!viewport.isMobile }), ]); } async setCacheEnabled(enabled: boolean): Promise { const disabled = !enabled; - await this._session.send('Network.setResourceCachingDisabled', { disabled }); + await this._updateState('Network.setResourceCachingDisabled', { disabled }); } async setRequestInterception(enabled: boolean): Promise { - await this._session.send('Network.setInterceptionEnabled', { enabled, interceptRequests: enabled }); + await this._updateState('Network.setInterceptionEnabled', { enabled, interceptRequests: enabled }); } async setOfflineMode(offline: boolean) { - await this._session.send('Network.setEmulateOfflineState', { offline }); + await this._updateState('Network.setEmulateOfflineState', { offline }); } async authenticate(credentials: types.Credentials | null) { @@ -388,18 +434,18 @@ export class WKPage implements PageDelegate { async exposeBinding(name: string, bindingFunction: string): Promise { const script = `self.${name} = (param) => console.debug('${BINDING_CALL_MESSAGE}', {}, param); ${bindingFunction}`; this._bootstrapScripts.unshift(script); - await this._setBootstrapScripts(this._session); + await this._setBootstrapScripts(); await Promise.all(this._page.frames().map(frame => frame.evaluate(script).catch(debugError))); } async evaluateOnNewDocument(script: string): Promise { this._bootstrapScripts.push(script); - await this._setBootstrapScripts(this._session); + await this._setBootstrapScripts(); } - private async _setBootstrapScripts(session: WKSession) { + private async _setBootstrapScripts() { const source = this._bootstrapScripts.join(';'); - await session.send('Page.setBootstrapScript', { source }); + await this._updateState('Page.setBootstrapScript', { source }); } async closePage(runBeforeUnload: boolean): Promise { diff --git a/src/webkit/wkPageProxy.ts b/src/webkit/wkPageProxy.ts index a516145f32..5bfdc2d79a 100644 --- a/src/webkit/wkPageProxy.ts +++ b/src/webkit/wkPageProxy.ts @@ -22,7 +22,6 @@ import { WKSession } from './wkConnection'; import { WKPage } from './wkPage'; import { RegisteredListener, helper, assert, debugError } from '../helper'; import { Events } from '../events'; -import { WKProvisionalPage } from './wkProvisionalPage'; const isPovisionalSymbol = Symbol('isPovisional'); @@ -31,7 +30,6 @@ export class WKPageProxy { readonly _browserContext: BrowserContext; private _pagePromise: Promise | null = null; private _wkPage: WKPage | null = null; - private _provisionalPage: WKProvisionalPage | null = null; private readonly _firstTargetPromise: Promise; private _firstTargetCallback?: () => void; private readonly _sessions = new Map(); @@ -68,12 +66,8 @@ export class WKPageProxy { for (const session of this._sessions.values()) session.dispose(); this._sessions.clear(); - if (this._provisionalPage) { - this._provisionalPage.dispose(); - this._provisionalPage = null; - } if (this._wkPage) - this._wkPage.didDisconnect(); + this._wkPage.dispose(); } dispatchMessageToSession(message: any) { @@ -145,10 +139,8 @@ export class WKPageProxy { } if (targetInfo.isProvisional) (session as any)[isPovisionalSymbol] = true; - if (targetInfo.isProvisional && this._wkPage) { - assert(!this._provisionalPage); - this._provisionalPage = new WKProvisionalPage(session, this._wkPage); - } + if (targetInfo.isProvisional && this._wkPage) + this._wkPage.onProvisionalLoadStarted(session); if (targetInfo.isPaused) this._pageProxySession.send('Target.resume', { targetId: targetInfo.targetId }).catch(debugError); } @@ -156,15 +148,11 @@ export class WKPageProxy { private _onTargetDestroyed(event: Protocol.Target.targetDestroyedPayload) { const { targetId, crashed } = event; const session = this._sessions.get(targetId); - if (session) - session.dispose(); + assert(session, 'Unknown target destroyed: ' + targetId); + session!.dispose(); this._sessions.delete(targetId); - if (this._provisionalPage && this._provisionalPage._session === session) { - this._provisionalPage.dispose(); - this._provisionalPage = null; - } - if (this._wkPage && this._wkPage._session === session && crashed) - this._wkPage.didClose(crashed); + if (this._wkPage) + this._wkPage.onSessionDestroyed(session!, crashed); } private _onDispatchMessageFromTarget(event: Protocol.Target.dispatchMessageFromTargetPayload) { @@ -183,12 +171,7 @@ export class WKPageProxy { // TODO: make some calls like screenshot catch swapped out error and retry. oldSession!.errorText = 'Target was swapped out.'; (newSession as any)[isPovisionalSymbol] = undefined; - if (this._provisionalPage) { - this._provisionalPage.commit(); - this._provisionalPage.dispose(); - this._provisionalPage = null; - } if (this._wkPage) - this._wkPage.setSession(newSession!); + this._wkPage.onProvisionalLoadCommitted(newSession!); } } diff --git a/test/interception.spec.js b/test/interception.spec.js index b11d0309d1..a347113151 100644 --- a/test/interception.spec.js +++ b/test/interception.spec.js @@ -561,6 +561,19 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p await page.setOfflineMode(false); expect(await page.evaluate(() => window.navigator.onLine)).toBe(true); }); + it('should continue if the interception gets disabled during provisional load', async({page, server}) => { + await page.goto(server.EMPTY_PAGE); + await page.setRequestInterception(true); + expect(await page.evaluate(() => navigator.onLine)).toBe(true); + let intercepted; + page.on('request', async request => { + intercepted = true; + await page.setRequestInterception(false); + }); + const response = await page.goto(server.CROSS_PROCESS_PREFIX + '/empty.html'); + expect(intercepted).toBe(true); + expect(response.status()).toBe(200); + }); }); describe('Interception vs isNavigationRequest', () => {