From 678b8e85ad3c5ff87fb4d331d1d1d198809f793b Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Tue, 22 Jan 2019 17:55:33 -0500 Subject: [PATCH] fix(page): use secondary DOMWorld to drive page.select() (#3809) This patch starts creating secondary DOMWorld for every connected page and switches `page.select()` to run inside the secondary world. Fix #3327. --- lib/DOMWorld.js | 29 ++++---------- lib/ExecutionContext.js | 9 ++--- lib/FrameManager.js | 86 ++++++++++++++++++++++++++++------------- lib/Page.js | 2 +- lib/externs.d.ts | 2 + test/page.spec.js | 2 +- 6 files changed, 74 insertions(+), 56 deletions(-) diff --git a/lib/DOMWorld.js b/lib/DOMWorld.js index c466ceb6..f05949e5 100644 --- a/lib/DOMWorld.js +++ b/lib/DOMWorld.js @@ -44,6 +44,13 @@ class DOMWorld { this._detached = false; } + /** + * @return {!Puppeteer.Frame} + */ + frame() { + return this._frame; + } + /** * @param {?Puppeteer.ExecutionContext} context */ @@ -419,28 +426,6 @@ class DOMWorld { await handle.dispose(); } - /** - * @param {(string|number|Function)} selectorOrFunctionOrTimeout - * @param {!Object=} options - * @param {!Array<*>} args - * @return {!Promise} - */ - waitFor(selectorOrFunctionOrTimeout, options = {}, ...args) { - const xPathPattern = '//'; - - if (helper.isString(selectorOrFunctionOrTimeout)) { - const string = /** @type {string} */ (selectorOrFunctionOrTimeout); - if (string.startsWith(xPathPattern)) - return this.waitForXPath(string, options); - return this.waitForSelector(string, options); - } - if (helper.isNumber(selectorOrFunctionOrTimeout)) - return new Promise(fulfill => setTimeout(fulfill, /** @type {number} */ (selectorOrFunctionOrTimeout))); - if (typeof selectorOrFunctionOrTimeout === 'function') - return this.waitForFunction(selectorOrFunctionOrTimeout, options, ...args); - return Promise.reject(new Error('Unsupported target type: ' + (typeof selectorOrFunctionOrTimeout))); - } - /** * @param {string} selector * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options diff --git a/lib/ExecutionContext.js b/lib/ExecutionContext.js index feaadf15..b832039d 100644 --- a/lib/ExecutionContext.js +++ b/lib/ExecutionContext.js @@ -24,20 +24,19 @@ class ExecutionContext { /** * @param {!Puppeteer.CDPSession} client * @param {!Protocol.Runtime.ExecutionContextDescription} contextPayload - * @param {?Puppeteer.Frame} frame + * @param {?Puppeteer.DOMWorld} world */ - constructor(client, contextPayload, frame) { + constructor(client, contextPayload, world) { this._client = client; - this._frame = frame; + this._world = world; this._contextId = contextPayload.id; - this._isDefault = contextPayload.auxData ? !!contextPayload.auxData['isDefault'] : false; } /** * @return {?Puppeteer.Frame} */ frame() { - return this._frame; + return this._world ? this._world.frame() : null; } /** diff --git a/lib/FrameManager.js b/lib/FrameManager.js index b730bab4..a5c72ae0 100644 --- a/lib/FrameManager.js +++ b/lib/FrameManager.js @@ -17,10 +17,12 @@ const EventEmitter = require('events'); const {helper, assert} = require('./helper'); const {Events} = require('./Events'); -const {ExecutionContext} = require('./ExecutionContext'); +const {ExecutionContext, EVALUATION_SCRIPT_URL} = require('./ExecutionContext'); const {LifecycleWatcher} = require('./LifecycleWatcher'); const {DOMWorld} = require('./DOMWorld'); +const UTILITY_WORLD_NAME = '__puppeteer_utility_world__'; + class FrameManager extends EventEmitter { /** * @param {!Puppeteer.CDPSession} client @@ -38,6 +40,8 @@ class FrameManager extends EventEmitter { this._frames = new Map(); /** @type {!Map} */ this._contextIdToContext = new Map(); + /** @type {!Set} */ + this._isolatedWorlds = new Set(); this._client.on('Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)); this._client.on('Page.frameNavigated', event => this._onFrameNavigated(event.frame)); @@ -48,7 +52,6 @@ class FrameManager extends EventEmitter { this._client.on('Runtime.executionContextDestroyed', event => this._onExecutionContextDestroyed(event.executionContextId)); this._client.on('Runtime.executionContextsCleared', event => this._onExecutionContextsCleared()); this._client.on('Page.lifecycleEvent', event => this._onLifecycleEvent(event)); - this._handleFrameTree(frameTree); } @@ -244,6 +247,28 @@ class FrameManager extends EventEmitter { this.emit(Events.FrameManager.FrameNavigated, frame); } + async ensureSecondaryDOMWorld() { + await this._ensureIsolatedWorld(UTILITY_WORLD_NAME); + } + + /** + * @param {string} name + */ + async _ensureIsolatedWorld(name) { + if (this._isolatedWorlds.has(name)) + return; + this._isolatedWorlds.add(name); + await this._client.send('Page.addScriptToEvaluateOnNewDocument', { + source: `//# sourceURL=${EVALUATION_SCRIPT_URL}`, + worldName: name, + }), + await Promise.all(this.frames().map(frame => this._client.send('Page.createIsolatedWorld', { + frameId: frame._id, + grantUniveralAccess: true, + worldName: name, + }))); + } + /** * @param {string} frameId * @param {string} url @@ -269,11 +294,20 @@ class FrameManager extends EventEmitter { _onExecutionContextCreated(contextPayload) { const frameId = contextPayload.auxData ? contextPayload.auxData.frameId : null; const frame = this._frames.get(frameId) || null; + let world = null; + if (frame) { + if (contextPayload.auxData && !!contextPayload.auxData['isDefault']) + world = frame._mainWorld; + else if (contextPayload.name === UTILITY_WORLD_NAME) + world = frame._secondaryWorld; + } + if (contextPayload.auxData && contextPayload.auxData['type'] === 'isolated') + this._isolatedWorlds.add(contextPayload.name); /** @type {!ExecutionContext} */ - const context = new ExecutionContext(this._client, contextPayload, frame); + const context = new ExecutionContext(this._client, contextPayload, world); + if (world) + world._setContext(context); this._contextIdToContext.set(contextPayload.id, context); - if (frame) - frame._addExecutionContext(context); } /** @@ -284,14 +318,14 @@ class FrameManager extends EventEmitter { if (!context) return; this._contextIdToContext.delete(executionContextId); - if (context.frame()) - context.frame()._removeExecutionContext(context); + if (context._world) + context._world._setContext(null); } _onExecutionContextsCleared() { for (const context of this._contextIdToContext.values()) { - if (context.frame()) - context.frame()._removeExecutionContext(context); + if (context._world) + context._world._setContext(null); } this._contextIdToContext.clear(); } @@ -340,6 +374,7 @@ class Frame { /** @type {!Set} */ this._lifecycleEvents = new Set(); this._mainWorld = new DOMWorld(frameManager, this); + this._secondaryWorld = new DOMWorld(frameManager, this); /** @type {!Set} */ this._childFrames = new Set(); @@ -347,22 +382,6 @@ class Frame { this._parentFrame._childFrames.add(this); } - /** - * @param {!ExecutionContext} context - */ - _addExecutionContext(context) { - if (context._isDefault) - this._mainWorld._setContext(context); - } - - /** - * @param {!ExecutionContext} context - */ - _removeExecutionContext(context) { - if (context._isDefault) - this._mainWorld._setContext(null); - } - /** * @param {string} url * @param {!{referer?: string, timeout?: number, waitUntil?: string|!Array}=} options @@ -543,7 +562,7 @@ class Frame { * @return {!Promise>} */ select(selector, ...values){ - return this._mainWorld.select(selector, ...values); + return this._secondaryWorld.select(selector, ...values); } /** @@ -569,7 +588,19 @@ class Frame { * @return {!Promise} */ waitFor(selectorOrFunctionOrTimeout, options = {}, ...args) { - return this._mainWorld.waitFor(selectorOrFunctionOrTimeout, options, ...args); + const xPathPattern = '//'; + + if (helper.isString(selectorOrFunctionOrTimeout)) { + const string = /** @type {string} */ (selectorOrFunctionOrTimeout); + if (string.startsWith(xPathPattern)) + return this.waitForXPath(string, options); + return this.waitForSelector(string, options); + } + if (helper.isNumber(selectorOrFunctionOrTimeout)) + return new Promise(fulfill => setTimeout(fulfill, /** @type {number} */ (selectorOrFunctionOrTimeout))); + if (typeof selectorOrFunctionOrTimeout === 'function') + return this.waitForFunction(selectorOrFunctionOrTimeout, options, ...args); + return Promise.reject(new Error('Unsupported target type: ' + (typeof selectorOrFunctionOrTimeout))); } /** @@ -643,6 +674,7 @@ class Frame { _detach() { this._detached = true; this._mainWorld._detach(); + this._secondaryWorld._detach(); if (this._parentFrame) this._parentFrame._childFrames.delete(this); this._parentFrame = null; diff --git a/lib/Page.js b/lib/Page.js index 455789aa..63e63d67 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -50,7 +50,7 @@ class Page extends EventEmitter { client.send('Target.setAutoAttach', {autoAttach: true, waitForDebuggerOnStart: false}), client.send('Page.setLifecycleEventsEnabled', { enabled: true }), client.send('Network.enable', {}), - client.send('Runtime.enable', {}), + client.send('Runtime.enable', {}).then(() => page._frameManager.ensureSecondaryDOMWorld()), client.send('Security.enable', {}), client.send('Performance.enable', {}), client.send('Log.enable', {}), diff --git a/lib/externs.d.ts b/lib/externs.d.ts index 956278f5..9ddb2f2e 100644 --- a/lib/externs.d.ts +++ b/lib/externs.d.ts @@ -6,6 +6,7 @@ import {TaskQueue as RealTaskQueue} from './TaskQueue.js'; import {Mouse as RealMouse, Keyboard as RealKeyboard, Touchscreen as RealTouchscreen} from './Input.js'; import {Frame as RealFrame, FrameManager as RealFrameManager} from './FrameManager.js'; import {JSHandle as RealJSHandle, ElementHandle as RealElementHandle} from './JSHandle.js'; +import {DOMWorld as RealDOMWorld} from './DOMWorld.js'; import {ExecutionContext as RealExecutionContext} from './ExecutionContext.js'; import { NetworkManager as RealNetworkManager, Request as RealRequest, Response as RealResponse } from './NetworkManager.js'; import * as child_process from 'child_process'; @@ -28,6 +29,7 @@ declare global { export class NetworkManager extends RealNetworkManager {} export class ElementHandle extends RealElementHandle {} export class JSHandle extends RealJSHandle {} + export class DOMWorld extends RealDOMWorld {} export class ExecutionContext extends RealExecutionContext {} export class Page extends RealPage { } export class Response extends RealResponse { } diff --git a/test/page.spec.js b/test/page.spec.js index 89d89a9a..335d64a4 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -956,7 +956,7 @@ module.exports.addTests = function({testRunner, expect, headless}) { expect(error.message).toContain('Values must be strings'); }); // @see https://github.com/GoogleChrome/puppeteer/issues/3327 - xit('should work when re-defining top-level Event class', async({page, server}) => { + it('should work when re-defining top-level Event class', async({page, server}) => { await page.goto(server.PREFIX + '/input/select.html'); await page.evaluate(() => window.Event = null); await page.select('select', 'blue');