diff --git a/src/chromium/FrameManager.ts b/src/chromium/FrameManager.ts index 25937d51cb..2e09a499e0 100644 --- a/src/chromium/FrameManager.ts +++ b/src/chromium/FrameManager.ts @@ -426,7 +426,7 @@ export class FrameManager implements PageDelegate { backendNodeId, executionContextId: (to._delegate as ExecutionContextDelegate)._contextId, }).catch(debugError); - if (!result) + if (!result || result.object.subtype === 'null') throw new Error('Unable to adopt element handle from a different document'); return to._createHandle(result.object).asElement()!; } diff --git a/src/dom.ts b/src/dom.ts index f2aee71297..3b536f82d1 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -25,6 +25,33 @@ export class FrameExecutionContext extends js.ExecutionContext { return this._frame; } + async _evaluate(returnByValue: boolean, pageFunction: string | Function, ...args: any[]): Promise { + const needsAdoption = (value: any): boolean => { + return typeof value === 'object' && value instanceof ElementHandle && value._context !== this; + }; + + if (!args.some(needsAdoption)) { + // Only go through asynchronous calls if required. + return this._delegate.evaluate(this, returnByValue, pageFunction, ...args); + } + + const toDispose: Promise[] = []; + const adopted = await Promise.all(args.map(async arg => { + if (!needsAdoption(arg)) + return arg; + const adopted = this._frame._page._delegate.adoptElementHandle(arg, this); + toDispose.push(adopted); + return adopted; + })); + let result; + try { + result = await this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted); + } finally { + await Promise.all(toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose()))); + } + return result; + } + _createHandle(remoteObject: any): js.JSHandle | null { if (this._frame._page._delegate.isElementHandle(remoteObject)) return new ElementHandle(this, remoteObject); diff --git a/src/frames.ts b/src/frames.ts index f49f9b0ebf..1993cfbc54 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -638,20 +638,9 @@ export class Frame { async select(selector: string, value: string | dom.ElementHandle | SelectOption | string[] | dom.ElementHandle[] | SelectOption[] | undefined, options?: WaitForOptions): Promise { const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options, 'any'); - const toDispose: Promise[] = []; const values = value === undefined ? [] : Array.isArray(value) ? value : [value]; - const context = await this._utilityContext(); - const adoptedValues = await Promise.all(values.map(async value => { - if (value instanceof dom.ElementHandle && value._context !== context) { - const adopted = this._page._delegate.adoptElementHandle(value, context); - toDispose.push(adopted); - return adopted; - } - return value; - })); - const result = await handle.select(...adoptedValues); + const result = await handle.select(...values); await handle.dispose(); - await Promise.all(toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose()))); return result; } diff --git a/src/javascript.ts b/src/javascript.ts index 57190ee743..71b5d3207d 100644 --- a/src/javascript.ts +++ b/src/javascript.ts @@ -24,12 +24,16 @@ export class ExecutionContext { return null; } - evaluate: types.Evaluate = (pageFunction, ...args) => { - return this._delegate.evaluate(this, true /* returnByValue */, pageFunction, ...args); + _evaluate(returnByValue: boolean, pageFunction: string | Function, ...args: any[]): Promise { + return this._delegate.evaluate(this, returnByValue, pageFunction, ...args); } - evaluateHandle: types.EvaluateHandle = (pageFunction, ...args) => { - return this._delegate.evaluate(this, false /* returnByValue */, pageFunction, ...args); + evaluate: types.Evaluate = async (pageFunction, ...args) => { + return this._evaluate(true /* returnByValue */, pageFunction, ...args); + } + + evaluateHandle: types.EvaluateHandle = async (pageFunction, ...args) => { + return this._evaluate(false /* returnByValue */, pageFunction, ...args); } _createHandle(remoteObject: any): JSHandle { diff --git a/src/webkit/FrameManager.ts b/src/webkit/FrameManager.ts index 0263ca0f8d..80595dfe5a 100644 --- a/src/webkit/FrameManager.ts +++ b/src/webkit/FrameManager.ts @@ -424,7 +424,9 @@ export class FrameManager implements PageDelegate { const result = await this._session.send('DOM.resolveNode', { objectId: toRemoteObject(handle).objectId, executionContextId: (to._delegate as ExecutionContextDelegate)._contextId - }); + }).catch(debugError); + if (!result || result.object.subtype === 'null') + throw new Error('Unable to adopt element handle from a different document'); return to._createHandle(result.object) as dom.ElementHandle; } } diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js index ff95b9a8c6..7f3b96abed 100644 --- a/test/evaluation.spec.js +++ b/test/evaluation.spec.js @@ -219,14 +219,6 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { await page.evaluate(e => e.textContent, element).catch(e => error = e); expect(error.message).toContain('JSHandle is disposed'); }); - it('should throw if elementHandles are from other frames', async({page, server}) => { - await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE); - const bodyHandle = await page.frames()[1].$('body'); - let error = null; - await page.evaluate(body => body.innerHTML, bodyHandle).catch(e => error = e); - expect(error).toBeTruthy(); - expect(error.message).toContain('JSHandles can be evaluated only in the context they were created'); - }); it('should simulate a user gesture', async({page, server}) => { const result = await page.evaluate(() => { document.body.appendChild(document.createTextNode('test')); @@ -333,5 +325,34 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { await page.goto(server.CROSS_PROCESS_PREFIX + '/empty.html'); expect(await mainFrame.evaluate(() => window.location.href)).toContain('127'); }); + xit('should allow cross-frame js handles', async({page, server}) => { + // TODO: this should be possible because frames script each other, but + // protocol implementations do not support this. + await page.goto(server.PREFIX + '/frames/one-frame.html'); + const handle = await page.evaluateHandle(() => { + const iframe = document.querySelector('iframe'); + const foo = { bar: 'baz' }; + iframe.contentWindow.__foo = foo; + return foo; + }); + const childFrame = page.mainFrame().childFrames()[0]; + const childResult = await childFrame.evaluate(() => window.__foo); + expect(childResult).toEqual({ bar: 'baz' }); + const result = await childFrame.evaluate(foo => foo.bar, handle); + expect(result).toBe('baz'); + }); + it.skip(FFOX)('should allow cross-frame element handles', async({page, server}) => { + await page.goto(server.PREFIX + '/frames/one-frame.html'); + const bodyHandle = await page.mainFrame().childFrames()[0].$('body'); + const result = await page.evaluate(body => body.innerHTML, bodyHandle); + expect(result.trim()).toBe('
Hi, I\'m frame
'); + }); + it.skip(FFOX)('should not allow cross-frame element handles when frames do not script each other', async({page, server}) => { + await page.goto(server.EMPTY_PAGE); + const frame = await utils.attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/empty.html'); + const bodyHandle = await frame.$('body'); + const error = await page.evaluate(body => body.innerHTML, bodyHandle).catch(e => e); + expect(error.message).toContain('Unable to adopt element handle from a different document'); + }); }); };