Skip to content

Commit

Permalink
Fix evaluateAsync in the face of aggressive Promise polyfills (Google…
Browse files Browse the repository at this point in the history
…Chrome#1178)

* Fix `driver.evaluateAsync` behavior when [zone.js](https://github.com/angular/zone.js) has mucked with Promise.

* Add zone.js to the smoke tests so regression won't happen again.

* Remove old testing promise-polyfill
  • Loading branch information
patrickhulce authored and andrewrota committed Jan 13, 2017
1 parent 7657d95 commit 8996aeb
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 270 deletions.
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>
8 changes: 4 additions & 4 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.
const thirdPartyPath = '../../../lighthouse-core/third_party';
absoluteFilePath = path.join(__dirname, `${thirdPartyPath}/promise-polyfill/promise.js`);
// We bring in an aggressive Promise polyfill (zone) to ensure we don't still fail.
const zonePath = '../../../node_modules/zone.js';
absoluteFilePath = path.join(__dirname, `${zonePath}/dist/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 explicitly wrap the raw expression for several purposes:
// 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);
});
}())`,
includeCommandLineAPI: true,
awaitPromise: true,
Expand Down
257 changes: 0 additions & 257 deletions lighthouse-core/third_party/promise-polyfill/promise.js

This file was deleted.

7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
"jsdom": "^9.0.0",
"mkdirp": "^0.5.1",
"mocha": "^2.3.3",
"walk": "^2.3.9"
"walk": "^2.3.9",
"zone.js": "^0.7.3"
},
"dependencies": {
"axe-core": "2.1.7",
Expand All @@ -52,13 +53,13 @@
"handlebars": "^4.0.5",
"json-stringify-safe": "^5.0.1",
"mkdirp": "^0.5.1",
"opn": "^4.0.2",
"rimraf": "^2.2.8",
"semver": ">=4.3.3",
"speedline": "1.0.3",
"whatwg-url": "^4.0.0",
"ws": "^1.1.1",
"yargs": "3.30.0",
"opn": "^4.0.2"
"yargs": "3.30.0"
},
"repository": "googlechrome/lighthouse",
"keywords": [
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2873,3 +2873,7 @@ yargs@~3.10.0:
cliui "^2.1.0"
decamelize "^1.0.0"
window-size "0.1.0"

zone.js@^0.7.3:
version "0.7.3"
resolved "https://registry.yarnpkg.com/zone.js/-/zone.js-0.7.3.tgz#d91432b6584f73c2c9e03ce4bb0870becc45d445"

0 comments on commit 8996aeb

Please sign in to comment.