Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,6 @@ <h2>Do better web tester page</h2>
}
</script>

<script src="/promise_polyfill.js"></script>
<script src="/zone.js"></script>
</body>
</html>
6 changes: 3 additions & 3 deletions lighthouse-cli/test/fixtures/static-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ function requestHandler(request, response) {
const queryString = requestUrl.search;
let absoluteFilePath = path.join(__dirname, filePath);

if (filePath === '/promise_polyfill.js') {
if (filePath === '/zone.js') {
// evaluateAsync previously had a bug that LH would fail if a page polyfilled Promise.
// We bring in a third-party Promise polyfill to ensure we don't still fail.
// We bring in an aggressive Promise polyfill (zone) to ensure we don't still fail.
const thirdPartyPath = '../../../lighthouse-core/third_party';
absoluteFilePath = path.join(__dirname, `${thirdPartyPath}/promise-polyfill/promise.js`);
absoluteFilePath = path.join(__dirname, `${thirdPartyPath}/zone/zone.js`);
}

fs.exists(absoluteFilePath, fsExistsCallback);
Expand Down
14 changes: 9 additions & 5 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,19 @@ class Driver {
);

this.sendCommand('Runtime.evaluate', {
// We need to wrap the raw expression for several purposes
// We need to expliticly wrap the raw expression for several purposes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicitly

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, done :)

// 1. Ensure that the expression will be a native Promise and not a polyfill/non-Promise.
// 2. Ensure that errors captured in the Promise are converted into plain-old JS Objects
// 2. Ensure that errors in the expression are captured by the Promise.
// 3. Ensure that errors captured in the Promise are converted into plain-old JS Objects
// so that they can be serialized properly b/c JSON.stringify(new Error('foo')) === '{}'
expression: `(function wrapInNativePromise() {
const __nativePromise = window.__nativePromise || Promise;
return __nativePromise.resolve()
.then(_ => ${expression})
.catch(${wrapRuntimeEvalErrorInBrowser.toString()});
return new __nativePromise(function (resolve) {
return __nativePromise.resolve()
.then(_ => ${expression})
.catch(${wrapRuntimeEvalErrorInBrowser.toString()})
.then(resolve);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a few permutations of this and it looks like this is about the only decent one that works :) Reason in #1173 (comment).

For anyone following along, the nice thing about this form is that it lets the polyfill handle resolving to a value (or error), and then all we have to assume is that our __nativePromise constructor is intact, not our earlier assumption that its prototype chain had also not been altered

}())`,
includeCommandLineAPI: true,
awaitPromise: true,
Expand Down
257 changes: 0 additions & 257 deletions lighthouse-core/third_party/promise-polyfill/promise.js

This file was deleted.

Loading