Skip to content

Commit

Permalink
fix(evaluate): awaitPromise when Promise is overwritten (#2759)
Browse files Browse the repository at this point in the history
Firefox and WebKit require native promises to provide awaitPromise
functionality. When the Promise is overwritten, all evaluations
in the main world produce wrong Promise, so we wrap with async
function to get a native promise instead.
  • Loading branch information
dgozman authored Jun 29, 2020
1 parent e154e08 commit 1fa9d30
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 6 deletions.
3 changes: 0 additions & 3 deletions src/common/utilityScriptSerializers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ export function serializeAsCallArgument(value: any, jsHandleSerializer: (value:
}

function serialize(value: any, jsHandleSerializer: (value: any) => { fallThrough?: any }, visited: Set<any>): any {
if (value && typeof value === 'object' && typeof value.then === 'function')
return value;

const result = jsHandleSerializer(value);
if ('fallThrough' in result)
value = result.fallThrough;
Expand Down
12 changes: 10 additions & 2 deletions src/injected/utilityScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,16 @@ export default class UtilityScript {
}
};

if (value && typeof value === 'object' && typeof value.then === 'function')
return value.then(safeJson);
if (value && typeof value === 'object' && typeof value.then === 'function') {
return (async () => {
// By using async function we ensure that return value is a native Promise,
// and not some overriden Promise in the page.
// This makes Firefox and WebKit debugging protocols recognize it as a Promise,
// properly await and return the value.
const promiseValue = await value;
return safeJson(promiseValue);
})();
}
return safeJson(value);
}
}
44 changes: 44 additions & 0 deletions test/evaluation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,50 @@ describe('Page.evaluate', function() {
const result = await page.evaluate(() => -Infinity);
expect(Object.is(result, -Infinity)).toBe(true);
});
it('should work with overwritten Promise', async({page, server}) => {
await page.evaluate(() => {
const originalPromise = window.Promise;
class Promise2 {
static all(...arg) {
return wrap(originalPromise.all(...arg));
}
static race(...arg) {
return wrap(originalPromise.race(...arg));
}
static resolve(...arg) {
return wrap(originalPromise.resolve(...arg));
}
constructor(f, r) {
this._promise = new originalPromise(f, r);
}
then(f, r) {
return wrap(this._promise.then(f, r));
}
catch(f) {
return wrap(this._promise.catch(f));
}
finally(f) {
return wrap(this._promise.finally(f));
}
};
const wrap = p => {
const result = new Promise2(() => {}, () => {});
result._promise = p;
return result;
};
window.Promise = Promise2;
window.__Promise2 = Promise2;
});

// Sanity check.
expect(await page.evaluate(() => {
const p = Promise.all([Promise.race([]), new Promise(() => {}).then(() => {})]);
return p instanceof window.__Promise2;
})).toBe(true);

// Now, the new promise should be awaitable.
expect(await page.evaluate(() => Promise.resolve(42))).toBe(42);
});
it('should throw when passed more than one parameter', async({page, server}) => {
const expectThrow = async f => {
let error;
Expand Down
2 changes: 1 addition & 1 deletion test/frame.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('Frame.evaluate', function() {
iframe.contentDocument.close();
});
// Main world should work.
expect(await page.frames()[1].evaluate(() => window.top.location.href)).toBe('http://localhost:8907/empty.html');
expect(await page.frames()[1].evaluate(() => window.top.location.href)).toBe(server.EMPTY_PAGE);
// Utility world should work.
expect(await page.frames()[1].$('div')).toBeTruthy(null);
});
Expand Down

0 comments on commit 1fa9d30

Please sign in to comment.