Migrate evaluateAsync to Runtime.evaluate(awaitPromise)

[pavelfeldman]

edit by paulirish: fixes #580

[paulirish]

We were waiting until m54 hit stable to do this (#580), but as of this week, 54 is the new stable. \o/

There's a few issues that travis picked up in the PR:
https://travis-ci.org/GoogleChrome/lighthouse/jobs/168801013#L290

Aside from those, this is looking on point. Excited to simplify this stuff.

[pavelfeldman]

Made the tests happy

[brendankenny]

just a few things, half of which are fixing up what the code should have already been doing :)

[brendankenny]

const

[pavelfeldman]

Done.

[brendankenny]

rather than expose the resolve function external to the promise, you could instead do

function runA11yChecks() {
  return new Promise((resolve, reject) => {
    axe.a11yCheck(document, resolve);
  });
}

[pavelfeldman]

Done. We are using single line callback -> Promise conversion in devtools, but with your linting it does not fit a line, so your version works better in your source base.

[brendankenny]

while you're changing this, maybe we should just drop the catch handler here? It isn't providing additional feedback and a rejected promise from evaluateAsync is caught in afterPass anyways

[pavelfeldman]

Done.

[brendankenny]

While you're in here, would you mind adding a doc string? Maybe something like

/**
 * 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<*>}
 */

[pavelfeldman]

Done.

[brendankenny]

when does m54 go 100%? End of the week?

[addyosmani]

M54 is rolling out today so should be a few days. I've heard it occasionally takes up to a week to fully ramp up to 100% but given this is master and we're not tagging a release anytime soon, it seems okay to land.

[brendankenny]

LGTM. We can wait for @paulirish's +1 before merging

[paulirish]

So good.