From 985faebd128def0eddbbc743f96115f2bcccee30 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 30 Jan 2020 17:30:47 -0800 Subject: [PATCH] fix: avoid unhandled promise rejection in WKSession.send (#770) --- src/webkit/wkConnection.ts | 9 ++++----- src/webkit/wkExecutionContext.ts | 20 ++++++++------------ src/webkit/wkPage.ts | 30 +++++++++++++++++++----------- src/webkit/wkPageProxy.ts | 9 ++++++--- src/webkit/wkProvisionalPage.ts | 3 ++- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/webkit/wkConnection.ts b/src/webkit/wkConnection.ts index 2f167d7526041..27a65505942bd 100644 --- a/src/webkit/wkConnection.ts +++ b/src/webkit/wkConnection.ts @@ -119,20 +119,19 @@ export class WKSession extends platform.EventEmitter { this.once = super.once; } - send( + async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { if (this._disposed) - return Promise.reject(new Error(`Protocol error (${method}): ${this.errorText}`)); + throw new Error(`Protocol error (${method}): ${this.errorText}`); const id = this.connection.nextMessageId(); const messageObj = { id, method, params }; platform.debug('pw:wrapped:' + this.sessionId)('SEND ► ' + JSON.stringify(messageObj, null, 2)); - const result = new Promise((resolve, reject) => { + this._rawSend(messageObj); + return new Promise((resolve, reject) => { this._callbacks.set(id, {resolve, reject, error: new Error(), method}); }); - this._rawSend(messageObj); - return result; } isDisposed(): boolean { diff --git a/src/webkit/wkExecutionContext.ts b/src/webkit/wkExecutionContext.ts index 69f3f53be035c..f70b12cf6d0e1 100644 --- a/src/webkit/wkExecutionContext.ts +++ b/src/webkit/wkExecutionContext.ts @@ -120,21 +120,17 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { throw error; } - let callFunctionOnPromise; - try { - callFunctionOnPromise = this._session.send('Runtime.callFunctionOn', { - functionDeclaration: functionText + '\n' + suffix + '\n', - objectId: thisObjectId, - arguments: serializableArgs.map((arg: any) => this._convertArgument(arg)), - returnByValue: false, - emulateUserGesture: true - }); - } catch (err) { + return this._session.send('Runtime.callFunctionOn', { + functionDeclaration: functionText + '\n' + suffix + '\n', + objectId: thisObjectId, + arguments: serializableArgs.map((arg: any) => this._convertArgument(arg)), + returnByValue: false, + emulateUserGesture: true + }).catch(err => { if (err instanceof TypeError && err.message.startsWith('Converting circular structure to JSON')) err.message += ' Are you passing a nested JSHandle?'; throw err; - } - return callFunctionOnPromise.then(response => { + }).then(response => { if (response.result.type === 'object' && response.result.className === 'Promise') { return Promise.race([ this._executionContextDestroyedPromise.then(() => contextDestroyedResult), diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 5ce6188e33403..6f0d8a0ca4d29 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -95,10 +95,24 @@ export class WKPage implements PageDelegate { // This method is called for provisional targets as well. The session passed as the parameter // may be different from the current session and may be destroyed without becoming current. async _initializeSession(session: WKSession, resourceTreeHandler: (r: Protocol.Page.getResourceTreeReturnValue) => void) { - const promises : Promise[] = [ + await this._initializeSessionMayThrow(session, resourceTreeHandler).catch(e => { + if (session.isDisposed()) + return; + // Swallow initialization errors due to newer target swap in, + // since we will reinitialize again. + if (this._session === session) + throw e; + }); + } + + private async _initializeSessionMayThrow(session: WKSession, resourceTreeHandler: (r: Protocol.Page.getResourceTreeReturnValue) => void) { + const [, frameTree] = await Promise.all([ // Page agent must be enabled before Runtime. session.send('Page.enable'), - session.send('Page.getResourceTree').then(resourceTreeHandler), + session.send('Page.getResourceTree'), + ]); + resourceTreeHandler(frameTree); + const promises : Promise[] = [ // Resource tree should be received before first execution context. session.send('Runtime.enable'), session.send('Page.createUserWorld', { name: UTILITY_WORLD_NAME }).catch(_ => {}), // Worlds are per-process @@ -129,19 +143,13 @@ export class WKPage implements PageDelegate { promises.push(session.send('Network.setExtraHTTPHeaders', { headers: this._page._state.extraHTTPHeaders })); if (this._page._state.hasTouch) promises.push(session.send('Page.setTouchEmulationEnabled', { enabled: true })); - await Promise.all(promises).catch(e => { - if (session.isDisposed()) - return; - // Swallow initialization errors due to newer target swap in, - // since we will reinitialize again. - if (this._session === session) - throw e; - }); + await Promise.all(promises); } - onProvisionalLoadStarted(provisionalSession: WKSession) { + initializeProvisionalPage(provisionalSession: WKSession) : Promise { assert(!this._provisionalPage); this._provisionalPage = new WKProvisionalPage(provisionalSession, this); + return this._provisionalPage.initializationPromise; } onProvisionalLoadCommitted(session: WKSession) { diff --git a/src/webkit/wkPageProxy.ts b/src/webkit/wkPageProxy.ts index 9e67adf757b99..69f1dcadcf315 100644 --- a/src/webkit/wkPageProxy.ts +++ b/src/webkit/wkPageProxy.ts @@ -139,10 +139,13 @@ export class WKPageProxy { } if (targetInfo.isProvisional) { (session as any)[isPovisionalSymbol] = true; - if (this._wkPage) - this._wkPage.onProvisionalLoadStarted(session); - if (targetInfo.isPaused) + if (this._wkPage) { + const provisionalPageInitialized = this._wkPage.initializeProvisionalPage(session); + if (targetInfo.isPaused) + provisionalPageInitialized.then(() => this._resumeTarget(targetInfo.targetId)); + } else if (targetInfo.isPaused) { this._resumeTarget(targetInfo.targetId); + } } else if (this._pagePromise) { assert(!this._pagePausedOnStart); // This is the first time page target is created, will resume diff --git a/src/webkit/wkProvisionalPage.ts b/src/webkit/wkProvisionalPage.ts index 899e1061c63d8..0511252dfe18f 100644 --- a/src/webkit/wkProvisionalPage.ts +++ b/src/webkit/wkProvisionalPage.ts @@ -24,6 +24,7 @@ export class WKProvisionalPage { private readonly _wkPage: WKPage; private _sessionListeners: RegisteredListener[] = []; private _mainFrameId: string | null = null; + readonly initializationPromise: Promise; constructor(session: WKSession, page: WKPage) { this._session = session; @@ -47,7 +48,7 @@ export class WKProvisionalPage { helper.addEventListener(session, 'Network.loadingFailed', overrideFrameId(e => wkPage._onLoadingFailed(e))), ]; - this._wkPage._initializeSession(session, ({frameTree}) => this._handleFrameTree(frameTree)); + this.initializationPromise = this._wkPage._initializeSession(session, ({frameTree}) => this._handleFrameTree(frameTree)); } dispose() {