From 80ca262ac2626d6944bd9811b62ebce1a3f86d7f Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 18 Oct 2016 18:37:54 -0700 Subject: [PATCH] Migrate evaluateAsync to Runtime.evaluate(awaitPromise) --- lighthouse-core/gather/drivers/driver.js | 55 ++++++------------- .../gather/gatherers/accessibility.js | 7 +-- .../gather/gatherers/cache-contents.js | 9 +-- .../gather/gatherers/content-width.js | 4 +- .../gather/gatherers/geolocation-on-start.js | 4 +- .../gatherers/html-without-javascript.js | 5 +- 6 files changed, 29 insertions(+), 55 deletions(-) diff --git a/lighthouse-core/gather/drivers/driver.js b/lighthouse-core/gather/drivers/driver.js index d0695c96c1e4..cc71b63c4388 100644 --- a/lighthouse-core/gather/drivers/driver.js +++ b/lighthouse-core/gather/drivers/driver.js @@ -144,50 +144,29 @@ class Driver { }); } + /** + * Evaluate an expression in the context of the current page. Expression must + * evaluate to a Promise. Returns a promise that resolves on asyncExpression's + * resolved value. + * @param {string} asyncExpression + * @return {!Promise<*>} + */ evaluateAsync(asyncExpression) { return new Promise((resolve, reject) => { - let asyncTimeout; - - // Inject the call to capture inspection. - const expression = `(function() { - const __inspect = inspect; - const __returnResults = function(results) { - __inspect(JSON.stringify(results)); - }; - ${asyncExpression} - })()`; - - const inspectHandler = value => { - if (asyncTimeout !== undefined) { - clearTimeout(asyncTimeout); - } - - // If the returned object doesn't meet the expected pattern, bail with an undefined. - if (value === undefined || - value.object === undefined || - value.object.value === undefined) { - return resolve(); - } - - return resolve(JSON.parse(value.object.value)); - }; - - // COMPAT: Chrome 52 is when Runtime.inspectRequested became available - // https://codereview.chromium.org/1866213002 - // Previously, a similar-looking Inspector.inspect event was available, but unfortunately - // it will not fire in this scenario. - this.once('Runtime.inspectRequested', inspectHandler); - - this.sendCommand('Runtime.evaluate', { - expression, - includeCommandLineAPI: true - }).catch(reject); - // If this gets to 60s and it hasn't been resolved, reject the Promise. - asyncTimeout = setTimeout( + const asyncTimeout = setTimeout( (_ => reject(new Error('The asynchronous expression exceeded the allotted time of 60s'))), 60000 ); + this.sendCommand('Runtime.evaluate', { + expression: asyncExpression, + includeCommandLineAPI: true, + awaitPromise: true, + returnByValue: true + }).then(result => { + clearTimeout(asyncTimeout); + resolve(result.result.value); + }).catch(reject); }); } diff --git a/lighthouse-core/gather/gatherers/accessibility.js b/lighthouse-core/gather/gatherers/accessibility.js index d0427213602f..20292f4b9407 100644 --- a/lighthouse-core/gather/gatherers/accessibility.js +++ b/lighthouse-core/gather/gatherers/accessibility.js @@ -16,7 +16,7 @@ */ 'use strict'; -/* global document, __returnResults */ +/* global document */ const Gatherer = require('./gatherer'); const fs = require('fs'); @@ -27,9 +27,8 @@ const axe = fs.readFileSync( // This is run in the page, not Lighthouse itself. /* istanbul ignore next */ function runA11yChecks() { - axe.a11yCheck(document, function(results) { - // __returnResults is magically inserted by driver.evaluateAsync - __returnResults(results); + return new Promise((resolve, reject) => { + axe.a11yCheck(document, resolve); }); } diff --git a/lighthouse-core/gather/gatherers/cache-contents.js b/lighthouse-core/gather/gatherers/cache-contents.js index 112fb92898f5..597cae601269 100644 --- a/lighthouse-core/gather/gatherers/cache-contents.js +++ b/lighthouse-core/gather/gatherers/cache-contents.js @@ -16,7 +16,7 @@ */ 'use strict'; -/* global __returnResults, caches */ +/* global caches */ const Gatherer = require('./gatherer'); @@ -24,7 +24,7 @@ const Gatherer = require('./gatherer'); /* istanbul ignore next */ function getCacheContents() { // Get every cache by name. - caches.keys() + return caches.keys() // Open each one. .then(cacheNames => Promise.all(cacheNames.map(cacheName => caches.open(cacheName)))) @@ -39,11 +39,8 @@ function getCacheContents() { requests.push(...reqs.map(r => r.url)); }); })).then(_ => { - // __returnResults is magically inserted by driver.evaluateAsync - __returnResults(requests); + return requests; }); - }).catch(_ => { - __returnResults(undefined); }); } diff --git a/lighthouse-core/gather/gatherers/content-width.js b/lighthouse-core/gather/gatherers/content-width.js index 5ebbcb8d0c2b..e8152afe9e0a 100644 --- a/lighthouse-core/gather/gatherers/content-width.js +++ b/lighthouse-core/gather/gatherers/content-width.js @@ -18,13 +18,13 @@ const Gatherer = require('./gatherer'); -/* global window, __returnResults */ +/* global window */ /* istanbul ignore next */ function getContentWidth() { // window.innerWidth to get the scrollable size of the window (irrespective of zoom) // window.outerWidth to get the size of the visible area - __returnResults({ + return Promise.resolve({ scrollWidth: window.innerWidth, viewportWidth: window.outerWidth }); diff --git a/lighthouse-core/gather/gatherers/geolocation-on-start.js b/lighthouse-core/gather/gatherers/geolocation-on-start.js index 2231985220e7..3ffe4ed67c90 100644 --- a/lighthouse-core/gather/gatherers/geolocation-on-start.js +++ b/lighthouse-core/gather/gatherers/geolocation-on-start.js @@ -26,7 +26,7 @@ const Gatherer = require('./gatherer'); * @author Paul Lewis */ -/* global navigator, window, __returnResults */ +/* global navigator, window */ /* istanbul ignore next */ function overrideGeo() { @@ -39,7 +39,7 @@ function overrideGeo() { } function collectGeoState() { - __returnResults(window.__didNotCallGeo); + return Promise.resolve(window.__didNotCallGeo); } class GeolocationOnStart extends Gatherer { diff --git a/lighthouse-core/gather/gatherers/html-without-javascript.js b/lighthouse-core/gather/gatherers/html-without-javascript.js index 963398e8c7b4..02ccaf77d137 100644 --- a/lighthouse-core/gather/gatherers/html-without-javascript.js +++ b/lighthouse-core/gather/gatherers/html-without-javascript.js @@ -20,14 +20,13 @@ const Gatherer = require('./gatherer'); -/* global document, __returnResults */ +/* global document */ /* istanbul ignore next */ function getBodyText() { // note: we use innerText, not textContent, because textContent includes the content of