From 80ffd92552e6476f68365992439b5b8a41ed0e27 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 7 Jan 2020 17:16:27 -0800 Subject: [PATCH] fix(webkit): move UI process agents to page proxy (#416) --- package.json | 2 +- src/webkit/wkBrowser.ts | 2 +- src/webkit/wkConnection.ts | 18 ++++++--- src/webkit/wkInput.ts | 23 ++++++----- src/webkit/wkNetworkManager.ts | 22 ++++++----- src/webkit/wkPage.ts | 70 ++++++++++++++++++---------------- src/webkit/wkPageProxy.ts | 27 +++++-------- 7 files changed, 87 insertions(+), 77 deletions(-) diff --git a/package.json b/package.json index 733a3d62f9..c47ffd37d9 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "playwright": { "chromium_revision": "724623", "firefox_revision": "1009", - "webkit_revision": "1068" + "webkit_revision": "1070" }, "scripts": { "unit": "node test/test.js", diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 3e2d6a0949..82670173a4 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -96,7 +96,7 @@ export class WKBrowser extends browser.Browser { } if (!context) context = this._defaultContext; - const pageProxy = new WKPageProxy(this, session, context); + const pageProxy = new WKPageProxy(session, context); this._pageProxies.set(pageProxyInfo.pageProxyId, pageProxy); if (pageProxyInfo.openerId) { diff --git a/src/webkit/wkConnection.ts b/src/webkit/wkConnection.ts index 1ddfe2e888..25ae96ef7b 100644 --- a/src/webkit/wkConnection.ts +++ b/src/webkit/wkConnection.ts @@ -139,8 +139,9 @@ export const WKTargetSessionEvents = { export class WKPageProxySession extends platform.EventEmitter { _connection: WKConnection; private readonly _sessions = new Map(); - private readonly _callbacks = new Map void, reject: (e: Error) => void, error: Error, method: string}>(); private readonly _pageProxyId: string; + private readonly _closePromise: Promise; + private _closePromiseCallback: () => void; on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; addListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; off: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -151,6 +152,7 @@ export class WKPageProxySession extends platform.EventEmitter { super(); this._connection = connection; this._pageProxyId = pageProxyId; + this._closePromise = new Promise(r => this._closePromiseCallback = r); } send( @@ -159,11 +161,10 @@ export class WKPageProxySession extends platform.EventEmitter { ): Promise { if (!this._connection) return Promise.reject(new Error(`Protocol error (${method}): Session closed. Most likely the pageProxy has been closed.`)); - return this._connection.send(method, params, this._pageProxyId).catch(e => { - // There is a possible race of the connection closure. We may have received - // targetDestroyed notification before response for the command, in that - // case it's safe to swallow the exception. - }) as Promise; + return Promise.race([ + this._closePromise.then(() => { throw new Error('Page proxy closed'); }), + this._connection.send(method, params, this._pageProxyId) + ]); } _dispatchEvent(object: {method: string, params: any, pageProxyId?: string}, wrappedMessage: string) { @@ -205,11 +206,16 @@ export class WKPageProxySession extends platform.EventEmitter { } } + isClosed() { + return !this._connection; + } + dispose() { for (const session of this._sessions.values()) session._onClosed(); this._sessions.clear(); + this._closePromiseCallback(); this._connection = null; } } diff --git a/src/webkit/wkInput.ts b/src/webkit/wkInput.ts index abaf4058f3..0fad71c917 100644 --- a/src/webkit/wkInput.ts +++ b/src/webkit/wkInput.ts @@ -18,7 +18,7 @@ import * as input from '../input'; import { helper } from '../helper'; import { macEditingCommands } from '../usKeyboardLayout'; -import { WKTargetSession } from './wkConnection'; +import { WKPageProxySession, WKTargetSession } from './wkConnection'; function toModifiersMask(modifiers: Set): number { // From Source/WebKit/Shared/WebEvent.h @@ -35,8 +35,13 @@ function toModifiersMask(modifiers: Set): number { } export class RawKeyboardImpl implements input.RawKeyboard { + private readonly _pageProxySession: WKPageProxySession; private _session: WKTargetSession; + constructor(session: WKPageProxySession) { + this._pageProxySession = session; + } + setSession(session: WKTargetSession) { this._session = session; } @@ -52,7 +57,7 @@ export class RawKeyboardImpl implements input.RawKeyboard { let commands = macEditingCommands[shortcut]; if (helper.isString(commands)) commands = [commands]; - await this._session.send('Input.dispatchKeyEvent', { + await this._pageProxySession.send('Input.dispatchKeyEvent', { type: 'keyDown', modifiers: toModifiersMask(modifiers), windowsVirtualKeyCode: keyCode, @@ -67,7 +72,7 @@ export class RawKeyboardImpl implements input.RawKeyboard { } async keyup(modifiers: Set, code: string, keyCode: number, keyCodeWithoutLocation: number, key: string, location: number): Promise { - await this._session.send('Input.dispatchKeyEvent', { + await this._pageProxySession.send('Input.dispatchKeyEvent', { type: 'keyUp', modifiers: toModifiersMask(modifiers), key, @@ -83,14 +88,14 @@ export class RawKeyboardImpl implements input.RawKeyboard { } export class RawMouseImpl implements input.RawMouse { - private _client: WKTargetSession; + private readonly _pageProxySession: WKPageProxySession; - setSession(client: WKTargetSession) { - this._client = client; + constructor(session: WKPageProxySession) { + this._pageProxySession = session; } async move(x: number, y: number, button: input.Button | 'none', buttons: Set, modifiers: Set): Promise { - await this._client.send('Input.dispatchMouseEvent', { + await this._pageProxySession.send('Input.dispatchMouseEvent', { type: 'move', button, x, @@ -100,7 +105,7 @@ export class RawMouseImpl implements input.RawMouse { } async down(x: number, y: number, button: input.Button, buttons: Set, modifiers: Set, clickCount: number): Promise { - await this._client.send('Input.dispatchMouseEvent', { + await this._pageProxySession.send('Input.dispatchMouseEvent', { type: 'down', button, x, @@ -111,7 +116,7 @@ export class RawMouseImpl implements input.RawMouse { } async up(x: number, y: number, button: input.Button, buttons: Set, modifiers: Set, clickCount: number): Promise { - await this._client.send('Input.dispatchMouseEvent', { + await this._pageProxySession.send('Input.dispatchMouseEvent', { type: 'up', button, x, diff --git a/src/webkit/wkNetworkManager.ts b/src/webkit/wkNetworkManager.ts index 6cf7edb9e3..479b28e4c6 100644 --- a/src/webkit/wkNetworkManager.ts +++ b/src/webkit/wkNetworkManager.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { WKTargetSession } from './wkConnection'; +import { WKTargetSession, WKPageProxySession } from './wkConnection'; import { Page } from '../page'; import { helper, RegisteredListener, assert } from '../helper'; import { Protocol } from './protocol'; @@ -25,14 +25,20 @@ import * as types from '../types'; import * as platform from '../platform'; export class WKNetworkManager { + private readonly _page: Page; + private readonly _pageProxySession: WKPageProxySession; private _session: WKTargetSession; - _page: Page; - private _requestIdToRequest = new Map(); + private readonly _requestIdToRequest = new Map(); private _userCacheDisabled = false; private _sessionListeners: RegisteredListener[] = []; - constructor(page: Page) { + constructor(page: Page, pageProxySession: WKPageProxySession) { this._page = page; + this._pageProxySession = pageProxySession; + } + + async initializePageProxySession(credentials: types.Credentials | null) { + await this.authenticate(credentials); } setSession(session: WKTargetSession) { @@ -47,17 +53,13 @@ export class WKNetworkManager { ]; } - async initializeSession(session: WKTargetSession, interceptNetwork: boolean | null, offlineMode: boolean | null, credentials: types.Credentials | null) { + async initializeSession(session: WKTargetSession, interceptNetwork: boolean | null, offlineMode: boolean | null) { const promises = []; promises.push(session.send('Network.enable')); if (interceptNetwork) promises.push(session.send('Network.setInterceptionEnabled', { enabled: true })); if (offlineMode) promises.push(session.send('Network.setEmulateOfflineState', { offline: true })); - if (credentials) - promises.push(session.send('Emulation.setAuthCredentials', { ...credentials })); - else - promises.push(session.send('Emulation.setAuthCredentials', { username: '', password: '' })); await Promise.all(promises); } @@ -160,7 +162,7 @@ export class WKNetworkManager { } async authenticate(credentials: types.Credentials | null) { - await this._session.send('Emulation.setAuthCredentials', { ...(credentials || {}) }); + await this._pageProxySession.send('Emulation.setAuthCredentials', { ...(credentials || { username: '', password: '' }) }); } async setOfflineMode(value: boolean): Promise { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index d7b044af09..80f9406018 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -19,7 +19,7 @@ import * as frames from '../frames'; import { debugError, helper, RegisteredListener } from '../helper'; import * as dom from '../dom'; import * as network from '../network'; -import { WKTargetSession, WKTargetSessionEvents } from './wkConnection'; +import { WKTargetSession, WKTargetSessionEvents, WKPageProxySession } from './wkConnection'; import { Events } from '../events'; import { WKExecutionContext, EVALUATION_SCRIPT_URL } from './wkExecutionContext'; import { WKNetworkManager } from './wkNetworkManager'; @@ -43,31 +43,43 @@ export class WKPage implements PageDelegate { readonly rawKeyboard: RawKeyboardImpl; _session: WKTargetSession; readonly _page: Page; - private _browser: WKBrowser; - private _networkManager: WKNetworkManager; - private _workers: WKWorkers; - private _contextIdToContext: Map; + private readonly _pageProxySession: WKPageProxySession; + private readonly _networkManager: WKNetworkManager; + private readonly _workers: WKWorkers; + private readonly _contextIdToContext: Map; private _isolatedWorlds: Set; private _sessionListeners: RegisteredListener[] = []; - private _bootstrapScripts: string[] = []; + private readonly _bootstrapScripts: string[] = []; - constructor(browser: WKBrowser, browserContext: BrowserContext) { - this._browser = browser; - this.rawKeyboard = new RawKeyboardImpl(); - this.rawMouse = new RawMouseImpl(); + constructor(browserContext: BrowserContext, pageProxySession: WKPageProxySession) { + this._pageProxySession = pageProxySession; + this.rawKeyboard = new RawKeyboardImpl(pageProxySession); + this.rawMouse = new RawMouseImpl(pageProxySession); this._contextIdToContext = new Map(); this._isolatedWorlds = new Set(); this._page = new Page(this, browserContext); - this._networkManager = new WKNetworkManager(this._page); + this._networkManager = new WKNetworkManager(this._page, pageProxySession); this._workers = new WKWorkers(this._page); } + async _initializePageProxySession() { + const promises : Promise[] = [ + this._pageProxySession.send('Dialog.enable'), + this._networkManager.initializePageProxySession(this._page._state.credentials) + ]; + const contextOptions = this._page.browserContext()._options; + if (contextOptions.javaScriptEnabled === false) + promises.push(this._pageProxySession.send('Emulation.setJavaScriptEnabled', { enabled: false })); + if (this._page._state.viewport) + promises.push(this.setViewport(this._page._state.viewport)); + await Promise.all(promises); + } + setSession(session: WKTargetSession) { helper.removeEventListeners(this._sessionListeners); this.disconnectFromTarget(); this._session = session; this.rawKeyboard.setSession(session); - this.rawMouse.setSession(session); this._addSessionListeners(); this._networkManager.setSession(session); this._workers.setSession(session); @@ -90,30 +102,28 @@ export class WKPage implements PageDelegate { session.send('Runtime.enable').then(() => this._ensureIsolatedWorld(UTILITY_WORLD_NAME)), session.send('Console.enable'), session.send('Page.setInterceptFileChooserDialog', { enabled: true }), - this._networkManager.initializeSession(session, this._page._state.interceptNetwork, this._page._state.offlineMode, this._page._state.credentials), - this._workers.initializeSession(session) + this._networkManager.initializeSession(session, this._page._state.interceptNetwork, this._page._state.offlineMode), + this._workers.initializeSession(session), ]; - if (!session.isProvisional()) { - // FIXME: move dialog agent to web process. - // Dialog agent resides in the UI process and should not be re-enabled on navigation. - promises.push(session.send('Dialog.enable')); - } const contextOptions = this._page.browserContext()._options; if (contextOptions.userAgent) promises.push(session.send('Page.overrideUserAgent', { value: contextOptions.userAgent })); if (this._page._state.mediaType || this._page._state.colorScheme) promises.push(this._setEmulateMedia(session, this._page._state.mediaType, this._page._state.colorScheme)); - if (contextOptions.javaScriptEnabled === false) - promises.push(session.send('Emulation.setJavaScriptEnabled', { enabled: false })); if (session.isProvisional()) promises.push(this._setBootstrapScripts(session)); if (contextOptions.bypassCSP) promises.push(session.send('Page.setBypassCSP', { enabled: true })); if (this._page._state.extraHTTPHeaders !== null) promises.push(this._setExtraHTTPHeaders(session, this._page._state.extraHTTPHeaders)); - if (this._page._state.viewport) - promises.push(WKPage._setViewport(session, this._page._state.viewport)); - await Promise.all(promises); + await Promise.all(promises).catch(e => { + if (session.isClosed()) + return; + // Swallow initialization errors due to newer target swap in, + // since we will reinitialize again. + if (this._session === session) + throw e; + }); } didClose(crashed: boolean) { @@ -137,7 +147,7 @@ export class WKPage implements PageDelegate { helper.addEventListener(this._session, 'Page.domContentEventFired', event => this._onLifecycleEvent(event.frameId, 'domcontentloaded')), 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, 'Dialog.javascriptDialogOpening', event => this._onDialog(event)), + helper.addEventListener(this._pageProxySession, 'Dialog.javascriptDialogOpening', event => this._onDialog(event)), helper.addEventListener(this._session, 'Page.fileChooserOpened', event => this._onFileChooserOpened(event)), helper.addEventListener(this._session, WKTargetSessionEvents.Disconnected, event => this._page._didDisconnect()), ]; @@ -259,7 +269,7 @@ export class WKPage implements PageDelegate { event.type as dialog.DialogType, event.message, async (accept: boolean, promptText?: string) => { - await this._session.send('Dialog.handleJavaScriptDialog', { accept, promptText }); + await this._pageProxySession.send('Dialog.handleJavaScriptDialog', { accept, promptText }); }, event.defaultPrompt)); } @@ -307,15 +317,11 @@ export class WKPage implements PageDelegate { } async setViewport(viewport: types.Viewport): Promise { - return WKPage._setViewport(this._session, viewport); - } - - private static async _setViewport(session: WKTargetSession, viewport: types.Viewport): Promise { if (viewport.isMobile || viewport.isLandscape || viewport.hasTouch) throw new Error('Not implemented'); const width = viewport.width; const height = viewport.height; - await session.send('Emulation.setDeviceMetricsOverride', { width, height, fixedLayout: false, deviceScaleFactor: viewport.deviceScaleFactor || 1 }); + await this._pageProxySession.send('Emulation.setDeviceMetricsOverride', { width, height, fixedLayout: false, deviceScaleFactor: viewport.deviceScaleFactor || 1 }); } setCacheEnabled(enabled: boolean): Promise { @@ -402,7 +408,7 @@ export class WKPage implements PageDelegate { } async resetViewport(oldSize: types.Size): Promise { - await this._session.send('Emulation.setDeviceMetricsOverride', { ...oldSize, fixedLayout: false, deviceScaleFactor: 0 }); + await this._pageProxySession.send('Emulation.setDeviceMetricsOverride', { ...oldSize, fixedLayout: false, deviceScaleFactor: 0 }); } async getContentFrame(handle: dom.ElementHandle): Promise { diff --git a/src/webkit/wkPageProxy.ts b/src/webkit/wkPageProxy.ts index fc9d2ce48d..1503736065 100644 --- a/src/webkit/wkPageProxy.ts +++ b/src/webkit/wkPageProxy.ts @@ -7,12 +7,10 @@ import { Page } from '../page'; import { Protocol } from './protocol'; import { WKPageProxySession, WKPageProxySessionEvents, WKTargetSession } from './wkConnection'; import { WKPage } from './wkPage'; -import { WKBrowser } from './wkBrowser'; import { RegisteredListener, helper, assert, debugError } from '../helper'; import { Events } from '../events'; export class WKPageProxy { - private readonly _browser: WKBrowser; private readonly _pageProxySession: WKPageProxySession; readonly _browserContext: BrowserContext; private _pagePromise: Promise | null = null; @@ -22,8 +20,7 @@ export class WKPageProxy { private readonly _targetSessions = new Map(); private readonly _eventListeners: RegisteredListener[]; - constructor(browser: WKBrowser, session: WKPageProxySession, browserContext: BrowserContext) { - this._browser = browser; + constructor(session: WKPageProxySession, browserContext: BrowserContext) { this._pageProxySession = session; this._browserContext = browserContext; this._firstTargetPromise = new Promise(r => this._firstTargetCallback = r); @@ -35,6 +32,8 @@ export class WKPageProxy { // Intercept provisional targets during cross-process navigation. this._pageProxySession.send('Target.setPauseOnStart', { pauseOnStart: true }).catch(e => { + if (this._pageProxySession.isClosed()) + return; debugError(e); throw e; }); @@ -68,23 +67,15 @@ export class WKPageProxy { } } assert(session, 'One non-provisional target session must exist'); - this._wkPage = new WKPage(this._browser, this._browserContext); + this._wkPage = new WKPage(this._browserContext, this._pageProxySession); this._wkPage.setSession(session); - await this._initializeSession(session); + await Promise.all([ + this._wkPage._initializePageProxySession(), + this._wkPage._initializeSession(session) + ]); return this._wkPage._page; } - private _initializeSession(session: WKTargetSession) : Promise { - return this._wkPage._initializeSession(session).catch(e => { - if (session.isClosed()) - return; - // Swallow initialization errors due to newer target swap in, - // since we will reinitialize again. - if (this._wkPage._session === session) - throw e; - }); - } - private _onTargetCreated(session: WKTargetSession, targetInfo: Protocol.Target.TargetInfo) { assert(targetInfo.type === 'page', 'Only page targets are expected in WebKit, received: ' + targetInfo.type); this._targetSessions.set(targetInfo.targetId, session); @@ -93,7 +84,7 @@ export class WKPageProxy { this._firstTargetCallback = null; } if (targetInfo.isProvisional && this._wkPage) - this._initializeSession(session); + this._wkPage._initializeSession(session); if (targetInfo.isPaused) this._pageProxySession.send('Target.resume', { targetId: targetInfo.targetId }).catch(debugError); }