github/puppeteer
0
0
Fork 0
mirror of https://github.com/puppeteer/puppeteer synced 2024-06-14 14:02:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: don't emit an internal error when eval causes navigation (#3008)

Browse Source 
When an evaluation causes a navigation, for example:
```js
await page.evaluate(() => window.reload());
```
sometimes we process the ExecutionContextDestroyed event before the ack from the evaluate. When we do get the ack from the evaluate, we try to build a JSHandle for it, and try to find the execution by id. But it is gone, and we throw an error. This patch switches createJSHandle to accept an ExecutionContext instead of just an id.

This bug was making the test `should throw a nice error after a navigation` flaky.
This commit is contained in:
Joel Einbinder 2018-08-01 13:53:08 -07:00 committed by Andrey Lushnikov
parent e36a7ae677
commit 95d867aaac
2 changed files with 15 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
lib/FrameManager.js
View File

@ -182,7 +182,8 @@ class FrameManager extends EventEmitter {
_onExecutionContextCreated(contextPayload) { _onExecutionContextCreated(contextPayload) {
const frameId = contextPayload.auxData && contextPayload.auxData.isDefault ? contextPayload.auxData.frameId : null; const frameId = contextPayload.auxData && contextPayload.auxData.isDefault ? contextPayload.auxData.frameId : null;
const frame = frameId ? this._frames.get(frameId) : null; const frame = frameId ? this._frames.get(frameId) : null;
const context = new ExecutionContext(this._client, contextPayload, this.createJSHandle.bind(this, contextPayload.id), frame); /** @type {!ExecutionContext} */
const context = new ExecutionContext(this._client, contextPayload, obj => this.createJSHandle(context, obj), frame);
this._contextIdToContext.set(contextPayload.id, context); this._contextIdToContext.set(contextPayload.id, context);
if (frame) if (frame)
frame._setDefaultContext(context); frame._setDefaultContext(context);
@ -215,12 +216,20 @@ class FrameManager extends EventEmitter {
/** /**
* @param {number} contextId * @param {number} contextId
* @return {!ExecutionContext}
*/
executionContextById(contextId) {
const context = this._contextIdToContext.get(contextId);
assert(context, 'INTERNAL ERROR: missing context with id = ' + contextId);
return context;
}
/**
* @param {!ExecutionContext} context
* @param {!Protocol.Runtime.RemoteObject} remoteObject * @param {!Protocol.Runtime.RemoteObject} remoteObject
* @return {!JSHandle} * @return {!JSHandle}
*/ */
createJSHandle(contextId, remoteObject) { createJSHandle(context, remoteObject) {
const context = this._contextIdToContext.get(contextId);
assert(context, 'INTERNAL ERROR: missing context with id = ' + contextId);
if (remoteObject.subtype === 'node') if (remoteObject.subtype === 'node')
return new ElementHandle(context, this._client, remoteObject, this._page, this); return new ElementHandle(context, this._client, remoteObject, this._page, this);
return new JSHandle(context, this._client, remoteObject); return new JSHandle(context, this._client, remoteObject);

3
lib/Page.js
View File

@ -477,7 +477,8 @@ class Page extends EventEmitter {
* @param {!Protocol.Runtime.consoleAPICalledPayload} event * @param {!Protocol.Runtime.consoleAPICalledPayload} event
*/ */
async _onConsoleAPI(event) { async _onConsoleAPI(event) {
const values = event.args.map(arg => this._frameManager.createJSHandle(event.executionContextId, arg)); const context = this._frameManager.executionContextById(event.executionContextId);
const values = event.args.map(arg => this._frameManager.createJSHandle(context, arg));
this._addConsoleMessage(event.type, values); this._addConsoleMessage(event.type, values);
} }