From 90018b2c3a3a2caf7f67bfc285059326fadc3586 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 19 May 2020 18:35:30 -0700 Subject: [PATCH 01/21] core(driver): create page code using structured interface --- lighthouse-core/gather/driver.js | 10 ++- lighthouse-core/gather/fetcher.js | 3 +- .../gather/gatherers/link-elements.js | 16 ++--- .../gather/gatherers/meta-elements.js | 39 +++++++---- .../gather/gatherers/script-elements.js | 24 +++---- .../gather/gatherers/seo/tap-targets.js | 68 +++++++++---------- .../gather/gatherers/trace-elements.js | 39 ++++++----- lighthouse-core/lib/page-functions.js | 34 +++++++++- lighthouse-core/report/html/renderer/dom.js | 6 +- 9 files changed, 140 insertions(+), 99 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 016e5f40eec8..71209bf4676c 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -423,11 +423,15 @@ class Driver { * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. * Returns a promise that resolves on the expression's value. - * @param {string} expression - * @param {{useIsolation?: boolean}=} options - * @return {Promise<*>} + * @template T, R + * @param {string | ((...args: T[]) => R)} expression + * @param {{useIsolation?: boolean, mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}=} options + * @return {Promise} */ async evaluateAsync(expression, options = {}) { + if (typeof expression !== 'string') { + expression = pageFunctions.createEvalCode(expression, options); + } const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; try { diff --git a/lighthouse-core/gather/fetcher.js b/lighthouse-core/gather/fetcher.js index 061a0f51b5e9..4f9faf73561d 100644 --- a/lighthouse-core/gather/fetcher.js +++ b/lighthouse-core/gather/fetcher.js @@ -156,7 +156,8 @@ class Fetcher { document.body.appendChild(iframe); } - await this.driver.evaluateAsync(`${injectIframe}(${JSON.stringify(url)})`, { + await this.driver.evaluateAsync(injectIframe, { + args: [url], useIsolation: true, }); diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index 4f04706aec7c..becb852b0ede 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -9,7 +9,7 @@ const Gatherer = require('./gatherer.js'); const URL = require('../../lib/url-shim.js').URL; const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); const LinkHeader = require('http-link-header'); -const {getElementsInDocumentString} = require('../../lib/page-functions.js'); +const {getElementsInDocument} = require('../../lib/page-functions.js'); /* globals HTMLLinkElement */ @@ -48,9 +48,7 @@ function getCrossoriginFromHeader(value) { */ /* istanbul ignore next */ function getLinkElementsInDOM() { - /** @type {Array} */ - // @ts-ignore - getElementsInDocument put into scope via stringification - const browserElements = getElementsInDocument('link'); // eslint-disable-line no-undef + const browserElements = getElementsInDocument('link'); /** @type {LH.Artifacts['LinkElements']} */ const linkElements = []; @@ -81,12 +79,10 @@ class LinkElements extends Gatherer { static getLinkElementsInDOM(passContext) { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return passContext.driver.evaluateAsync(`(() => { - ${getElementsInDocumentString}; - ${getLinkElementsInDOM}; - - return getLinkElementsInDOM(); - })()`, {useIsolation: true}); + return passContext.driver.evaluateAsync(getLinkElementsInDOM, { + useIsolation: true, + deps: [getElementsInDocument], + }); } /** diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index a1b67b00d6cc..8f4558963807 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -6,7 +6,27 @@ 'use strict'; const Gatherer = require('./gatherer.js'); -const getElementsInDocumentString = require('../../lib/page-functions.js').getElementsInDocumentString; // eslint-disable-line max-len +const {getElementsInDocument} = require('../../lib/page-functions.js'); + +/* istanbul ignore next */ +function collectMetaElements() { + const metas = /** @type {HTMLMetaElement[]} */ (getElementsInDocument('head meta')); + return metas.map(meta => { + /** @param {string} name */ + const getAttribute = name => { + const attr = meta.attributes.getNamedItem(name); + if (!attr) return; + return attr.value; + }; + return { + name: meta.name.toLowerCase(), + content: meta.content, + property: getAttribute('property'), + httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined, + charset: getAttribute('charset'), + }; + }); +} class MetaElements extends Gatherer { /** @@ -18,19 +38,10 @@ class MetaElements extends Gatherer { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return driver.evaluateAsync(`(() => { - ${getElementsInDocumentString}; - - return getElementsInDocument('head meta').map(meta => { - return { - name: meta.name.toLowerCase(), - content: meta.content, - property: meta.attributes.property ? meta.attributes.property.value : undefined, - httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined, - charset: meta.attributes.charset ? meta.attributes.charset.value : undefined, - }; - }); - })()`, {useIsolation: true}); + return driver.evaluateAsync(collectMetaElements, { + useIsolation: true, + deps: [getElementsInDocument], + }); } } diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 7a4bca6982dd..7c0aef6e5ce2 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -8,19 +8,15 @@ const Gatherer = require('./gatherer.js'); const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); const NetworkRequest = require('../../lib/network-request.js'); -const getElementsInDocumentString = require('../../lib/page-functions.js').getElementsInDocumentString; // eslint-disable-line max-len +const {getElementsInDocument, getNodePath} = require('../../lib/page-functions.js'); const pageFunctions = require('../../lib/page-functions.js'); -/* global getNodePath */ - /** * @return {LH.Artifacts['ScriptElements']} */ /* istanbul ignore next */ -function collectAllScriptElements() { - /** @type {HTMLScriptElement[]} */ - // @ts-ignore - getElementsInDocument put into scope via stringification - const scripts = getElementsInDocument('script'); // eslint-disable-line no-undef +function collectScriptElements() { + const scripts = getElementsInDocument('script'); return scripts.map(script => { return { @@ -30,7 +26,6 @@ function collectAllScriptElements() { async: script.async, defer: script.defer, source: /** @type {'head'|'body'} */ (script.closest('head') ? 'head' : 'body'), - // @ts-ignore - getNodePath put into scope via stringification devtoolsNodePath: getNodePath(script), content: script.src ? null : script.text, requestId: null, @@ -72,12 +67,13 @@ class ScriptElements extends Gatherer { const driver = passContext.driver; const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url); - /** @type {LH.Artifacts['ScriptElements']} */ - const scripts = await driver.evaluateAsync(`(() => { - ${getElementsInDocumentString} - ${pageFunctions.getNodePathString}; - return (${collectAllScriptElements.toString()})(); - })()`, {useIsolation: true}); + const scripts = await driver.evaluateAsync(collectScriptElements, { + useIsolation: true, + deps: [ + getElementsInDocument, + pageFunctions.getNodePathString, + ], + }); for (const script of scripts) { if (script.content) script.requestId = mainResource.requestId; diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index ab7571b9f3e5..175d9a4587f9 100644 --- a/lighthouse-core/gather/gatherers/seo/tap-targets.js +++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js @@ -38,7 +38,7 @@ const tapTargetsSelector = TARGET_SELECTORS.join(','); /** * @param {HTMLElement} element - * @returns {boolean} + * @return {boolean} */ /* istanbul ignore next */ function elementIsVisible(element) { @@ -47,7 +47,7 @@ function elementIsVisible(element) { /** * @param {Element} element - * @returns {LH.Artifacts.Rect[]} + * @return {LH.Artifacts.Rect[]} */ /* istanbul ignore next */ function getClientRects(element) { @@ -69,17 +69,18 @@ function getClientRects(element) { /** * @param {Element} element - * @returns {boolean} + * @param {string} tapTargetsSelector + * @return {boolean} */ /* istanbul ignore next */ -function elementHasAncestorTapTarget(element) { +function elementHasAncestorTapTarget(element, tapTargetsSelector) { if (!element.parentElement) { return false; } if (element.parentElement.matches(tapTargetsSelector)) { return true; } - return elementHasAncestorTapTarget(element.parentElement); + return elementHasAncestorTapTarget(element.parentElement, tapTargetsSelector); } /** @@ -123,7 +124,7 @@ function hasTextNodeSiblingsFormingTextBlock(element) { * Makes a reasonable guess, but for example gets it wrong if the element is surrounded by other * HTML elements instead of direct text nodes. * @param {Element} element - * @returns {boolean} + * @return {boolean} */ /* istanbul ignore next */ function elementIsInTextBlock(element) { @@ -177,7 +178,7 @@ function elementCenterIsAtZAxisTop(el, elCenterPoint) { /** * Finds all position sticky/absolute elements on the page and adds a class * that disables pointer events on them. - * @returns {() => void} - undo function to re-enable pointer events + * @return {() => void} - undo function to re-enable pointer events */ /* istanbul ignore next */ function disableFixedAndStickyElementPointerEvents() { @@ -202,10 +203,11 @@ function disableFixedAndStickyElementPointerEvents() { } /** - * @returns {LH.Artifacts.TapTarget[]} + * @param {string} tapTargetsSelector + * @return {LH.Artifacts.TapTarget[]} */ /* istanbul ignore next */ -function gatherTapTargets() { +function gatherTapTargets(tapTargetsSelector) { /** @type {LH.Artifacts.TapTarget[]} */ const targets = []; @@ -223,7 +225,7 @@ function gatherTapTargets() { const tapTargetsWithClientRects = []; tapTargetElements.forEach(tapTargetElement => { // Filter out tap targets that are likely to cause false failures: - if (elementHasAncestorTapTarget(tapTargetElement)) { + if (elementHasAncestorTapTarget(tapTargetElement, tapTargetsSelector)) { // This is usually intentional, either the tap targets trigger the same action // or there's a child with a related action (like a delete button for an item) return; @@ -305,30 +307,28 @@ class TapTargets extends Gatherer { * @return {Promise} All visible tap targets with their positions and sizes */ afterPass(passContext) { - const expression = `(function() { - const tapTargetsSelector = "${tapTargetsSelector}"; - ${pageFunctions.getElementsInDocumentString}; - ${disableFixedAndStickyElementPointerEvents.toString()}; - ${elementIsVisible.toString()}; - ${elementHasAncestorTapTarget.toString()}; - ${elementCenterIsAtZAxisTop.toString()} - ${truncate.toString()}; - ${getClientRects.toString()}; - ${hasTextNodeSiblingsFormingTextBlock.toString()}; - ${elementIsInTextBlock.toString()}; - ${getRectArea.toString()}; - ${getLargestRect.toString()}; - ${getRectCenterPoint.toString()}; - ${rectContains.toString()}; - ${pageFunctions.getNodePathString}; - ${pageFunctions.getNodeSelectorString}; - ${pageFunctions.getNodeLabelString}; - ${gatherTapTargets.toString()}; - - return gatherTapTargets(); - })()`; - - return passContext.driver.evaluateAsync(expression, {useIsolation: true}); + return passContext.driver.evaluateAsync(gatherTapTargets, { + args: [tapTargetsSelector], + useIsolation: true, + deps: [ + pageFunctions.getElementsInDocument, + disableFixedAndStickyElementPointerEvents, + elementIsVisible, + elementHasAncestorTapTarget, + elementCenterIsAtZAxisTop, + truncate, + getClientRects, + hasTextNodeSiblingsFormingTextBlock, + elementIsInTextBlock, + getRectArea, + getLargestRect, + getRectCenterPoint, + rectContains, + pageFunctions.getNodePath, + pageFunctions.getNodeSelectorString, + pageFunctions.getNodeLabelString, + ], + }); } } diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index 32a5437b8554..9a7af1ad581c 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -11,10 +11,13 @@ * We take the backend nodeId from the trace and use it to find the corresponding element in the DOM. */ +/* global document */ + const Gatherer = require('./gatherer.js'); -const pageFunctions = require('../../lib/page-functions.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); +const {createEvalCode, getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = + require('../../lib/page-functions.js'); /** * @this {HTMLElement} @@ -23,19 +26,16 @@ const RectHelpers = require('../../lib/rect-helpers.js'); */ /* istanbul ignore next */ function setAttributeMarker(metricName) { - const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; // eslint-disable-line no-undef + const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; let traceElement; if (elem) { + const nodeLabel = getNodeLabel(elem); traceElement = { metricName, - // @ts-ignore - put into scope via stringification - devtoolsNodePath: getNodePath(elem), // eslint-disable-line no-undef - // @ts-ignore - put into scope via stringification - selector: getNodeSelector(elem), // eslint-disable-line no-undef - // @ts-ignore - put into scope via stringification - nodeLabel: getNodeLabel(elem), // eslint-disable-line no-undef - // @ts-ignore - put into scope via stringification - snippet: getOuterHTMLSnippet(elem), // eslint-disable-line no-undef + devtoolsNodePath: getNodePath(elem), + selector: getNodeSelector(elem), + nodeLabel: nodeLabel ? nodeLabel : undefined, + snippet: getOuterHTMLSnippet(elem), }; } return traceElement; @@ -138,16 +138,19 @@ class TraceElements extends Gatherer { const resolveNodeResponse = await driver.sendCommand('DOM.resolveNode', {backendNodeId: backendNodeIds[i]}); const objectId = resolveNodeResponse.object.objectId; + // TODO(cjamcl): create driver.evaluateFunctionOn.. const response = await driver.sendCommand('Runtime.callFunctionOn', { objectId, - functionDeclaration: `function () { - ${setAttributeMarker.toString()}; - ${pageFunctions.getNodePathString}; - ${pageFunctions.getNodeSelectorString}; - ${pageFunctions.getNodeLabelString}; - ${pageFunctions.getOuterHTMLSnippetString}; - return setAttributeMarker.call(this, '${metricName}'); - }`, + functionDeclaration: createEvalCode(setAttributeMarker, { + mode: 'function', + deps: [ + getNodePath, + getNodeSelector, + getNodeLabel, + getOuterHTMLSnippet, + ], + args: [metricName], + }), returnByValue: true, awaitPromise: true, }); diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index f1fbc3a8fde2..8141689a384c 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -6,8 +6,34 @@ // @ts-nocheck 'use strict'; +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ + /* global window document Node ShadowRoot */ +/** + * @template T, R + * @param {(...args: T[]) => R} mainFn + * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ + */ +function createEvalCode(mainFn, {mode, args, deps} = {}) { + const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; + const depsSerialized = deps ? deps.join('\n') : ''; + + if (!mode || mode === 'iffe') { + return `(() => { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}(${argsSerialized}); + })()`; + } else { + return `function () { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}.call(this, ${argsSerialized}); + }`; + } +} + /** * Helper functions that are passed by `toString()` by Driver to be evaluated in target page. */ @@ -78,9 +104,10 @@ function checkTimeSinceLastLongTask() { } /** - * @param {string=} selector Optional simple CSS selector to filter nodes on. + * @template {string} T + * @param {T} selector Optional simple CSS selector to filter nodes on. * Combinators are not supported. - * @return {Array} + * @return {Array} */ /* istanbul ignore next */ function getElementsInDocument(selector) { @@ -304,14 +331,17 @@ function getNodeLabel(node) { } module.exports = { + createEvalCode, wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(), registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(), checkTimeSinceLastLongTaskString: checkTimeSinceLastLongTask.toString(), + getElementsInDocument, getElementsInDocumentString: getElementsInDocument.toString(), getOuterHTMLSnippetString: getOuterHTMLSnippet.toString(), getOuterHTMLSnippet: getOuterHTMLSnippet, ultradumbBenchmark: ultradumbBenchmark, ultradumbBenchmarkString: ultradumbBenchmark.toString(), + getNodePath, getNodePathString: getNodePath.toString(), getNodeSelectorString: getNodeSelector.toString(), getNodeSelector: getNodeSelector, diff --git a/lighthouse-core/report/html/renderer/dom.js b/lighthouse-core/report/html/renderer/dom.js index e712d69930b2..4f5889f51446 100644 --- a/lighthouse-core/report/html/renderer/dom.js +++ b/lighthouse-core/report/html/renderer/dom.js @@ -18,7 +18,7 @@ /* globals URL self Util */ -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ class DOM { /** @@ -38,7 +38,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementByTagName[T]} + * @return {HTMLElementTagNameMapWithDefault[T]} */ createElement(name, className, attrs = {}) { const element = this._document.createElement(name); @@ -69,7 +69,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementByTagName[T]} + * @return {HTMLElementTagNameMapWithDefault[T]} */ createChildOf(parentElem, elementName, className, attrs) { const element = this.createElement(elementName, className, attrs); From f7e38f5a5ceb9a2ed4cc9ad922d1bf3167edf9b1 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 12:24:37 -0700 Subject: [PATCH 02/21] rename type --- lighthouse-core/lib/page-functions.js | 4 ++-- lighthouse-core/report/html/renderer/dom.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8141689a384c..d678dbd88fe3 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -6,7 +6,7 @@ // @ts-nocheck 'use strict'; -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ /* global window document Node ShadowRoot */ @@ -107,7 +107,7 @@ function checkTimeSinceLastLongTask() { * @template {string} T * @param {T} selector Optional simple CSS selector to filter nodes on. * Combinators are not supported. - * @return {Array} + * @return {Array} */ /* istanbul ignore next */ function getElementsInDocument(selector) { diff --git a/lighthouse-core/report/html/renderer/dom.js b/lighthouse-core/report/html/renderer/dom.js index 4f5889f51446..e712d69930b2 100644 --- a/lighthouse-core/report/html/renderer/dom.js +++ b/lighthouse-core/report/html/renderer/dom.js @@ -18,7 +18,7 @@ /* globals URL self Util */ -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ class DOM { /** @@ -38,7 +38,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementTagNameMapWithDefault[T]} + * @return {HTMLElementByTagName[T]} */ createElement(name, className, attrs = {}) { const element = this._document.createElement(name); @@ -69,7 +69,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementTagNameMapWithDefault[T]} + * @return {HTMLElementByTagName[T]} */ createChildOf(parentElem, elementName, className, attrs) { const element = this.createElement(elementName, className, attrs); From 78f386ac2c5e4b0673618a65965f28a7146633f1 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 12:55:15 -0700 Subject: [PATCH 03/21] pr feedback --- lighthouse-core/gather/driver.js | 77 +++++++++++++++++-- lighthouse-core/gather/fetcher.js | 2 +- .../gather/gatherers/link-elements.js | 2 +- .../gather/gatherers/meta-elements.js | 2 +- .../gather/gatherers/script-elements.js | 2 +- .../gather/gatherers/seo/tap-targets.js | 2 +- .../gather/gatherers/trace-elements.js | 23 +++--- lighthouse-core/lib/page-functions.js | 9 ++- 8 files changed, 92 insertions(+), 27 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 71209bf4676c..dddb1eeecd3d 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -418,21 +418,86 @@ class Driver { }); } + /** + * Deprecated: renamed to `evaluate`. + * @param {string} expression + * @param {{useIsolation?: boolean}=} options + * @return {Promise<*>} + */ + async evaluateAsync(expression, options = {}) { + return this.evaluate(expression, options); + } + /** * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. * Returns a promise that resolves on the expression's value. + * See the documentation for `createEvalCode` in `page-functions.js`. * @template T, R - * @param {string | ((...args: T[]) => R)} expression - * @param {{useIsolation?: boolean, mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}=} options + * @param {string | ((...args: T[]) => R)} expressionOrMainFn + * @param {{useIsolation?: boolean, args?: T[], deps?: (Function|string)[]}=} options * @return {Promise} */ - async evaluateAsync(expression, options = {}) { - if (typeof expression !== 'string') { - expression = pageFunctions.createEvalCode(expression, options); + async evaluate(expressionOrMainFn, options = {}) { + if (typeof expressionOrMainFn !== 'string') { + expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, {mode: 'iffe', ...options}); + } + return this._evaluate(expressionOrMainFn, options); + } + + /** + * Evaluate a statement in the context of a JavaScript object on the current page. + * Returns a promise that resolves on the expression's value. + * See the documentation for `createEvalCode` in `page-functions.js`. + * @template T, R + * @param {string | ((...args: T[]) => R)} statementOrMainFn + * @param {{objectId: string, args?: T[], deps?: (Function|string)[]}} options + * @return {Promise} + */ + async evaluateFunctionOnObject(statementOrMainFn, options) { + if (typeof statementOrMainFn !== 'string') { + statementOrMainFn = pageFunctions.createEvalCode(statementOrMainFn, { + mode: 'function', + ...options, + }); + } + + const response = await this.sendCommand('Runtime.callFunctionOn', { + objectId: options.objectId, + functionDeclaration: statementOrMainFn, + returnByValue: true, + awaitPromise: true, + }); + + if (response.exceptionDetails) { + // An error occurred before we could even create a Promise, should be *very* rare. + // Also occurs when the expression is not valid JavaScript. + const errorMessage = response.exceptionDetails.exception ? + response.exceptionDetails.exception.description : + response.exceptionDetails.text; + return Promise.reject(new Error(`Evaluation exception: ${errorMessage}`)); + } + + // TODO: check if __failedInBrowser happens here too. + if (response && response.result && response.result.value) { + return response.result.value; } - const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; + return Promise.reject(); + } + + /** + * Evaluate an expression in the context of the current page. If useIsolation is true, the expression + * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state + * is completely separate. + * Returns a promise that resolves on the expression's value. + * @param {string} expression + * @param {{useIsolation?: boolean}} options + * @return {Promise<*>} + */ + async _evaluate(expression, options) { + const contextId = + options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; try { // `await` is not redundant here because we want to `catch` the async errors diff --git a/lighthouse-core/gather/fetcher.js b/lighthouse-core/gather/fetcher.js index 4f9faf73561d..c048b1f0427c 100644 --- a/lighthouse-core/gather/fetcher.js +++ b/lighthouse-core/gather/fetcher.js @@ -156,7 +156,7 @@ class Fetcher { document.body.appendChild(iframe); } - await this.driver.evaluateAsync(injectIframe, { + await this.driver.evaluate(injectIframe, { args: [url], useIsolation: true, }); diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index becb852b0ede..caa1b2cec21c 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -79,7 +79,7 @@ class LinkElements extends Gatherer { static getLinkElementsInDOM(passContext) { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return passContext.driver.evaluateAsync(getLinkElementsInDOM, { + return passContext.driver.evaluate(getLinkElementsInDOM, { useIsolation: true, deps: [getElementsInDocument], }); diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index 8f4558963807..6d4f5c39d3c9 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -38,7 +38,7 @@ class MetaElements extends Gatherer { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return driver.evaluateAsync(collectMetaElements, { + return driver.evaluate(collectMetaElements, { useIsolation: true, deps: [getElementsInDocument], }); diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 7c0aef6e5ce2..4d964c1c1e91 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -67,7 +67,7 @@ class ScriptElements extends Gatherer { const driver = passContext.driver; const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url); - const scripts = await driver.evaluateAsync(collectScriptElements, { + const scripts = await driver.evaluate(collectScriptElements, { useIsolation: true, deps: [ getElementsInDocument, diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index 175d9a4587f9..28dc8fa51c7c 100644 --- a/lighthouse-core/gather/gatherers/seo/tap-targets.js +++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js @@ -307,7 +307,7 @@ class TapTargets extends Gatherer { * @return {Promise} All visible tap targets with their positions and sizes */ afterPass(passContext) { - return passContext.driver.evaluateAsync(gatherTapTargets, { + return passContext.driver.evaluate(gatherTapTargets, { args: [tapTargetsSelector], useIsolation: true, deps: [ diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index 9a7af1ad581c..91e946958d70 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -16,7 +16,7 @@ const Gatherer = require('./gatherer.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); -const {createEvalCode, getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = +const {getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = require('../../lib/page-functions.js'); /** @@ -138,11 +138,11 @@ class TraceElements extends Gatherer { const resolveNodeResponse = await driver.sendCommand('DOM.resolveNode', {backendNodeId: backendNodeIds[i]}); const objectId = resolveNodeResponse.object.objectId; - // TODO(cjamcl): create driver.evaluateFunctionOn.. - const response = await driver.sendCommand('Runtime.callFunctionOn', { - objectId, - functionDeclaration: createEvalCode(setAttributeMarker, { - mode: 'function', + if (!objectId) continue; + + try { + const element = await driver.evaluateFunctionOnObject(setAttributeMarker, { + objectId, deps: [ getNodePath, getNodeSelector, @@ -150,14 +150,9 @@ class TraceElements extends Gatherer { getOuterHTMLSnippet, ], args: [metricName], - }), - returnByValue: true, - awaitPromise: true, - }); - - if (response && response.result && response.result.value) { - traceElements.push(response.result.value); - } + }); + if (element) traceElements.push(element); + } catch (_) {} } return traceElements; diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index d678dbd88fe3..e47bdcf7ae3c 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -11,9 +11,14 @@ /* global window document Node ShadowRoot */ /** + * Creates valid JavaScript code given functions, strings of valid code, and arguments. * @template T, R - * @param {(...args: T[]) => R} mainFn - * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ + * @param {(...args: T[]) => R} mainFn The main function to call. It's return value will be the return value + * of `createEvalCode`, wrapped in a Promise. + * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ Set mode to `iffe` to create a self- + * executing function expression, set to `function` to create just a function declaration statement. Args should match + * the args of `mainFn`, and can be any serializable value. `deps` are functions or string of valid code that must be + * defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; From 32856e756867fc5882139388493769555b477aff Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 15:18:10 -0700 Subject: [PATCH 04/21] no strings --- lighthouse-core/gather/driver.js | 4 ++-- lighthouse-core/gather/gatherers/script-elements.js | 2 +- lighthouse-core/gather/gatherers/seo/tap-targets.js | 4 ++-- lighthouse-core/lib/page-functions.js | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index dddb1eeecd3d..23e06413713b 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -436,7 +436,7 @@ class Driver { * See the documentation for `createEvalCode` in `page-functions.js`. * @template T, R * @param {string | ((...args: T[]) => R)} expressionOrMainFn - * @param {{useIsolation?: boolean, args?: T[], deps?: (Function|string)[]}=} options + * @param {{useIsolation?: boolean, args?: T[], deps?: Function[]}=} options * @return {Promise} */ async evaluate(expressionOrMainFn, options = {}) { @@ -452,7 +452,7 @@ class Driver { * See the documentation for `createEvalCode` in `page-functions.js`. * @template T, R * @param {string | ((...args: T[]) => R)} statementOrMainFn - * @param {{objectId: string, args?: T[], deps?: (Function|string)[]}} options + * @param {{objectId: string, args?: T[], deps?: Function[]}} options * @return {Promise} */ async evaluateFunctionOnObject(statementOrMainFn, options) { diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 4d964c1c1e91..ddacfb8fee6c 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -71,7 +71,7 @@ class ScriptElements extends Gatherer { useIsolation: true, deps: [ getElementsInDocument, - pageFunctions.getNodePathString, + pageFunctions.getNodePath, ], }); diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index 28dc8fa51c7c..56b9572e067c 100644 --- a/lighthouse-core/gather/gatherers/seo/tap-targets.js +++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js @@ -325,8 +325,8 @@ class TapTargets extends Gatherer { getRectCenterPoint, rectContains, pageFunctions.getNodePath, - pageFunctions.getNodeSelectorString, - pageFunctions.getNodeLabelString, + pageFunctions.getNodeSelector, + pageFunctions.getNodeLabel, ], }); } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index e47bdcf7ae3c..5ae0e5116348 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -15,10 +15,10 @@ * @template T, R * @param {(...args: T[]) => R} mainFn The main function to call. It's return value will be the return value * of `createEvalCode`, wrapped in a Promise. - * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ Set mode to `iffe` to create a self- - * executing function expression, set to `function` to create just a function declaration statement. Args should match - * the args of `mainFn`, and can be any serializable value. `deps` are functions or string of valid code that must be - * defined for `mainFn` to work. + * @param {{mode?: 'iffe'|'function', args?: T[], deps?: Function[]}} _ Set mode to `iffe` to + * create a self-executing function expression, set to `function` to create just a function + * declaration statement. Args should match the args of `mainFn`, and can be any serializable + * value. `deps` are functions that must be defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; From e2dd63dac018768cdd384028d2310c2906ef79e6 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 15:41:35 -0700 Subject: [PATCH 05/21] test --- lighthouse-core/gather/driver.js | 5 +- .../test/lib/page-functions-test.js | 85 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 23e06413713b..92c5cd5969b3 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -441,7 +441,10 @@ class Driver { */ async evaluate(expressionOrMainFn, options = {}) { if (typeof expressionOrMainFn !== 'string') { - expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, {mode: 'iffe', ...options}); + expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, { + mode: 'iffe', + ...options, + }); } return this._evaluate(expressionOrMainFn, options); } diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index bea55964c0f5..4125ba2fd79d 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -12,6 +12,91 @@ const pageFunctions = require('../../lib/page-functions.js'); /* eslint-env jest */ +describe('createEvalCode', () => { + it('iffe basic', () => { + function mainFn() { + return true; + } + const code = pageFunctions.createEvalCode(mainFn, { + mode: 'iffe', + }); + expect(code).toMatchInlineSnapshot(` + "(() => { + + function mainFn() { + return true; + } + return mainFn(); + })()" + `); + expect(eval(code)).toEqual(true); + }); + + it('iffe complex', () => { + function mainFn({a, b}, passThru) { + return {a: abs(a), b: square(b), passThru}; + } + function abs(val) { + return Math.abs(val); + } + function square(val) { + return val * val; + } + const code = pageFunctions.createEvalCode(mainFn, { + args: [{a: -5, b: 10}, 'hello'], + deps: [abs, square], + mode: 'iffe', + }); + expect(code).toMatchInlineSnapshot(` + "(() => { + function abs(val) { + return Math.abs(val); + } + function square(val) { + return val * val; + } + function mainFn({ + a, + b + }, passThru) { + return { + a: abs(a), + b: square(b), + passThru + }; + } + return mainFn({\\"a\\":-5,\\"b\\":10},\\"hello\\"); + })()" + `); + expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); + }); + + it('function', () => { + function mainFn(a, b) { + return adder(a, b); + } + function adder(a, b) { + return a + b; + } + const code = pageFunctions.createEvalCode(mainFn, { + args: [1, 2], + mode: 'function', + deps: [adder], + }); + expect(code).toMatchInlineSnapshot(` + "function () { + function adder(a, b) { + return a + b; + } + function mainFn(a, b) { + return adder(a, b); + } + return mainFn.call(this, 1,2); + }" + `); + }); +}); + describe('Page Functions', () => { let dom; From ce9839b165b388b55fe990ab293a860dcdfe3089 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 3 Jun 2020 19:35:02 -0700 Subject: [PATCH 06/21] test --- lighthouse-core/test/gather/gatherers/link-elements-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/gather/gatherers/link-elements-test.js b/lighthouse-core/test/gather/gatherers/link-elements-test.js index 39dfdaf9e92d..fedf3c3cc21a 100644 --- a/lighthouse-core/test/gather/gatherers/link-elements-test.js +++ b/lighthouse-core/test/gather/gatherers/link-elements-test.js @@ -30,7 +30,7 @@ describe('Link Elements gatherer', () => { function getPassData({linkElementsInDOM = [], headers = []}) { const url = 'https://example.com'; const loadData = {networkRecords: [{url, responseHeaders: headers, resourceType: 'Document'}]}; - const driver = {evaluateAsync: () => Promise.resolve(linkElementsInDOM)}; + const driver = {evaluate: () => Promise.resolve(linkElementsInDOM)}; const passContext = {driver, url}; return [passContext, loadData]; } From 4c903def0c4149895a628eadc7e0466eb128f350 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 5 Jun 2020 13:45:33 -0700 Subject: [PATCH 07/21] restrucutre --- lighthouse-core/gather/driver.js | 35 ++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 92c5cd5969b3..1c455b8ff356 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -419,21 +419,38 @@ class Driver { } /** - * Deprecated: renamed to `evaluate`. + * Evaluate an expression in the context of the current page. If useIsolation is true, the expression + * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state + * is completely separate. + * Returns a promise that resolves on the expression's value. + * @template T * @param {string} expression - * @param {{useIsolation?: boolean}=} options + * @param {{useIsolation?: boolean, args?: T[], deps?: Function[]}=} options * @return {Promise<*>} */ async evaluateAsync(expression, options = {}) { - return this.evaluate(expression, options); + const contextId = + options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; + + try { + // `await` is not redundant here because we want to `catch` the async errors + return await this._evaluateInContext(expression, contextId); + } catch (err) { + // If we were using isolation and the context disappeared on us, retry one more time. + if (contextId && err.message.includes('Cannot find context')) { + this._clearIsolatedContextId(); + const freshContextId = await this._getOrCreateIsolatedContextId(); + return this._evaluateInContext(expression, freshContextId); + } + + throw err; + } } /** - * Evaluate an expression in the context of the current page. If useIsolation is true, the expression - * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state - * is completely separate. - * Returns a promise that resolves on the expression's value. - * See the documentation for `createEvalCode` in `page-functions.js`. + * Evaluate an expression (optionally defined in a structured manner, see `createEvalCode` + * in `page-functions.js`). + * See `evaluateAsync`. * @template T, R * @param {string | ((...args: T[]) => R)} expressionOrMainFn * @param {{useIsolation?: boolean, args?: T[], deps?: Function[]}=} options @@ -446,7 +463,7 @@ class Driver { ...options, }); } - return this._evaluate(expressionOrMainFn, options); + return this.evaluateAsync(expressionOrMainFn, options); } /** From 02dc2153fb6d72b20d2360a1f594240395213c23 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 16 Jun 2020 17:50:27 -0700 Subject: [PATCH 08/21] remove obj --- lighthouse-core/gather/driver.js | 40 -------------- .../gather/gatherers/trace-elements.js | 53 ++++++++++--------- 2 files changed, 27 insertions(+), 66 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index a6e07f3738f4..488dac004ffc 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -466,46 +466,6 @@ class Driver { return this.evaluateAsync(expressionOrMainFn, options); } - /** - * Evaluate a statement in the context of a JavaScript object on the current page. - * Returns a promise that resolves on the expression's value. - * See the documentation for `createEvalCode` in `page-functions.js`. - * @template T, R - * @param {string | ((...args: T[]) => R)} statementOrMainFn - * @param {{objectId: string, args?: T[], deps?: Function[]}} options - * @return {Promise} - */ - async evaluateFunctionOnObject(statementOrMainFn, options) { - if (typeof statementOrMainFn !== 'string') { - statementOrMainFn = pageFunctions.createEvalCode(statementOrMainFn, { - mode: 'function', - ...options, - }); - } - - const response = await this.sendCommand('Runtime.callFunctionOn', { - objectId: options.objectId, - functionDeclaration: statementOrMainFn, - returnByValue: true, - awaitPromise: true, - }); - - if (response.exceptionDetails) { - // An error occurred before we could even create a Promise, should be *very* rare. - // Also occurs when the expression is not valid JavaScript. - const errorMessage = response.exceptionDetails.exception ? - response.exceptionDetails.exception.description : - response.exceptionDetails.text; - return Promise.reject(new Error(`Evaluation exception: ${errorMessage}`)); - } - - // TODO: check if __failedInBrowser happens here too. - if (response && response.result && response.result.value) { - return response.result.value; - } - return Promise.reject(); - } - /** * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index 91e946958d70..c3450d9e3417 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -11,13 +11,10 @@ * We take the backend nodeId from the trace and use it to find the corresponding element in the DOM. */ -/* global document */ - const Gatherer = require('./gatherer.js'); +const pageFunctions = require('../../lib/page-functions.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); -const {getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = - require('../../lib/page-functions.js'); /** * @this {HTMLElement} @@ -26,16 +23,19 @@ const {getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = */ /* istanbul ignore next */ function setAttributeMarker(metricName) { - const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; + const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; // eslint-disable-line no-undef let traceElement; if (elem) { - const nodeLabel = getNodeLabel(elem); traceElement = { metricName, - devtoolsNodePath: getNodePath(elem), - selector: getNodeSelector(elem), - nodeLabel: nodeLabel ? nodeLabel : undefined, - snippet: getOuterHTMLSnippet(elem), + // @ts-ignore - put into scope via stringification + devtoolsNodePath: getNodePath(elem), // eslint-disable-line no-undef + // @ts-ignore - put into scope via stringification + selector: getNodeSelector(elem), // eslint-disable-line no-undef + // @ts-ignore - put into scope via stringification + nodeLabel: getNodeLabel(elem), // eslint-disable-line no-undef + // @ts-ignore - put into scope via stringification + snippet: getOuterHTMLSnippet(elem), // eslint-disable-line no-undef }; } return traceElement; @@ -135,24 +135,25 @@ class TraceElements extends Gatherer { for (let i = 0; i < backendNodeIds.length; i++) { const metricName = lcpNodeId === backendNodeIds[i] ? 'largest-contentful-paint' : 'cumulative-layout-shift'; - const resolveNodeResponse = - await driver.sendCommand('DOM.resolveNode', {backendNodeId: backendNodeIds[i]}); - const objectId = resolveNodeResponse.object.objectId; + const objectId = await driver.resolveNodeIdToObjectId(backendNodeIds[i]); if (!objectId) continue; + const response = await driver.sendCommand('Runtime.callFunctionOn', { + objectId, + functionDeclaration: `function () { + ${setAttributeMarker.toString()}; + ${pageFunctions.getNodePathString}; + ${pageFunctions.getNodeSelectorString}; + ${pageFunctions.getNodeLabelString}; + ${pageFunctions.getOuterHTMLSnippetString}; + return setAttributeMarker.call(this, '${metricName}'); + }`, + returnByValue: true, + awaitPromise: true, + }); - try { - const element = await driver.evaluateFunctionOnObject(setAttributeMarker, { - objectId, - deps: [ - getNodePath, - getNodeSelector, - getNodeLabel, - getOuterHTMLSnippet, - ], - args: [metricName], - }); - if (element) traceElements.push(element); - } catch (_) {} + if (response && response.result && response.result.value) { + traceElements.push(response.result.value); + } } return traceElements; From 70d821014fd253b7a5177ac19e3e00c2c0b4b41d Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 5 Nov 2020 13:59:59 -0600 Subject: [PATCH 09/21] rm dead code --- lighthouse-core/gather/driver.js | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index fe8eca18a7ed..fa5ab41416e2 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -438,6 +438,7 @@ class Driver { } /** + * Note: Prefer `evaluate` instead. * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. @@ -485,34 +486,6 @@ class Driver { return this.evaluateAsync(expressionOrMainFn, options); } - /** - * Evaluate an expression in the context of the current page. If useIsolation is true, the expression - * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state - * is completely separate. - * Returns a promise that resolves on the expression's value. - * @param {string} expression - * @param {{useIsolation?: boolean}} options - * @return {Promise<*>} - */ - async _evaluate(expression, options) { - const contextId = - options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; - - try { - // `await` is not redundant here because we want to `catch` the async errors - return await this._evaluateInContext(expression, contextId); - } catch (err) { - // If we were using isolation and the context disappeared on us, retry one more time. - if (contextId && err.message.includes('Cannot find context')) { - this._clearIsolatedContextId(); - const freshContextId = await this._getOrCreateIsolatedContextId(); - return this._evaluateInContext(expression, freshContextId); - } - - throw err; - } - } - /** * Evaluate an expression in the given execution context; an undefined contextId implies the main * page without isolation. From 4e594763c2b199d44ea4df80f0aee9b30b19ca28 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:10:09 -0600 Subject: [PATCH 10/21] adam feedback --- lighthouse-core/lib/page-functions.js | 2 +- lighthouse-core/test/lib/page-functions-test.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8e2fe96a27af..66ba77c05e73 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -21,7 +21,7 @@ * value. `deps` are functions that must be defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { - const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; + const argsSerialized = args.map(arg => JSON.stringify(arg)).join(','); const depsSerialized = deps ? deps.join('\n') : ''; if (!mode || mode === 'iife') { diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 34580c1ee469..9b5f18e998c6 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -13,11 +13,12 @@ const pageFunctions = require('../../lib/page-functions.js'); /* eslint-env jest */ describe('createEvalCode', () => { - it('iffe basic', () => { + it('iife basic', () => { function mainFn() { return true; } const code = pageFunctions.createEvalCode(mainFn, { + args: [], mode: 'iife', }); expect(code).toMatchInlineSnapshot(` @@ -32,7 +33,7 @@ describe('createEvalCode', () => { expect(eval(code)).toEqual(true); }); - it('iffe complex', () => { + it('iife complex', () => { function mainFn({a, b}, passThru) { return {a: abs(a), b: square(b), passThru}; } @@ -94,6 +95,7 @@ describe('createEvalCode', () => { return mainFn.call(this, 1,2); }" `); + expect(eval(`(${code})()`)).toEqual(3); }); }); From 147b98c277e0f1f35313fdb262b986b0d696e1d1 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:23:54 -0600 Subject: [PATCH 11/21] fix tests --- lighthouse-core/gather/driver.js | 2 +- lighthouse-core/gather/gatherers/meta-elements.js | 7 +++++-- lighthouse-core/lib/page-functions.js | 2 +- lighthouse-core/test/lib/page-functions-test.js | 1 - 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index fa5ab41416e2..6812a38682b2 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -445,7 +445,7 @@ class Driver { * Returns a promise that resolves on the expression's value. * @template T * @param {string} expression - * @param {{useIsolation?: boolean, args?: T[], deps?: Array}=} options + * @param {{useIsolation?: boolean}=} options * @return {Promise<*>} */ async evaluateAsync(expression, options = {}) { diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index 6d4f5c39d3c9..bb90e9195116 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -6,10 +6,13 @@ 'use strict'; const Gatherer = require('./gatherer.js'); -const {getElementsInDocument} = require('../../lib/page-functions.js'); +const pageFunctions = require('../../lib/page-functions.js'); + +/* global getElementsInDocument */ /* istanbul ignore next */ function collectMetaElements() { + // @ts-expect-error - getElementsInDocument put into scope via stringification const metas = /** @type {HTMLMetaElement[]} */ (getElementsInDocument('head meta')); return metas.map(meta => { /** @param {string} name */ @@ -40,7 +43,7 @@ class MetaElements extends Gatherer { // the values like access from JavaScript does. return driver.evaluate(collectMetaElements, { useIsolation: true, - deps: [getElementsInDocument], + deps: [pageFunctions.getElementsInDocument], }); } } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 66ba77c05e73..8e2fe96a27af 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -21,7 +21,7 @@ * value. `deps` are functions that must be defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { - const argsSerialized = args.map(arg => JSON.stringify(arg)).join(','); + const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; const depsSerialized = deps ? deps.join('\n') : ''; if (!mode || mode === 'iife') { diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 9b5f18e998c6..7b3427bebb49 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -18,7 +18,6 @@ describe('createEvalCode', () => { return true; } const code = pageFunctions.createEvalCode(mainFn, { - args: [], mode: 'iife', }); expect(code).toMatchInlineSnapshot(` From a4051a1218122c0d449a1bb34e09b57ec8967397 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:37:20 -0600 Subject: [PATCH 12/21] fix mangle issues --- lighthouse-core/gather/gatherers/link-elements.js | 7 ++++--- lighthouse-core/gather/gatherers/meta-elements.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index 2362519e7431..5b409af99f3b 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -9,7 +9,7 @@ const LinkHeader = require('http-link-header'); const Gatherer = require('./gatherer.js'); const {URL} = require('../../lib/url-shim.js'); const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); -const {getElementsInDocument, getNodeDetailsString} = require('../../lib/page-functions.js'); +const pageFunctions = require('../../lib/page-functions.js'); /* globals HTMLLinkElement getNodeDetails */ @@ -49,6 +49,7 @@ function getCrossoriginFromHeader(value) { /* istanbul ignore next */ function getLinkElementsInDOM() { /** @type {Array} */ + // @ts-expect-error - getElementsInDocument put into scope via stringification const browserElements = getElementsInDocument('link'); // eslint-disable-line no-undef /** @type {LH.Artifacts['LinkElements']} */ const linkElements = []; @@ -88,8 +89,8 @@ class LinkElements extends Gatherer { return passContext.driver.evaluate(getLinkElementsInDOM, { useIsolation: true, deps: [ - getNodeDetailsString, - getElementsInDocument, + pageFunctions.getNodeDetailsString, + pageFunctions.getElementsInDocument, ], }); } diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index bb90e9195116..5ccb609a914b 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -8,7 +8,7 @@ const Gatherer = require('./gatherer.js'); const pageFunctions = require('../../lib/page-functions.js'); -/* global getElementsInDocument */ +/* globals getElementsInDocument */ /* istanbul ignore next */ function collectMetaElements() { From 77afd884fee6d27caaa8ce7a13dfa44cf6a91834 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 13:31:55 -0600 Subject: [PATCH 13/21] fix --- lighthouse-core/gather/gatherers/script-elements.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index e2d19008b260..0bcc2d5cc2dc 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -8,7 +8,7 @@ const Gatherer = require('./gatherer.js'); const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); const NetworkRequest = require('../../lib/network-request.js'); -const {getElementsInDocument, getNodeDetailsString} = require('../../lib/page-functions.js'); +const pageFunctions = require('../../lib/page-functions.js'); /* global getNodeDetails */ @@ -18,6 +18,7 @@ const {getElementsInDocument, getNodeDetailsString} = require('../../lib/page-fu /* istanbul ignore next */ function collectAllScriptElements() { /** @type {HTMLScriptElement[]} */ + // @ts-expect-error - getElementsInDocument put into scope via stringification const scripts = getElementsInDocument('script'); // eslint-disable-line no-undef return scripts.map(script => { @@ -73,8 +74,8 @@ class ScriptElements extends Gatherer { const scripts = await driver.evaluate(collectAllScriptElements, { useIsolation: true, deps: [ - getNodeDetailsString, - getElementsInDocument, + pageFunctions.getNodeDetailsString, + pageFunctions.getElementsInDocument, ], }); From fd945646c51100a1fa11d680be3016ca37291dc0 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 15:01:16 -0600 Subject: [PATCH 14/21] fix nasty types by using tuples --- lighthouse-core/gather/driver.js | 11 +++++++---- lighthouse-core/lib/page-functions.js | 10 +++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 6812a38682b2..f2584be0d6bf 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -443,7 +443,6 @@ class Driver { * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. * Returns a promise that resolves on the expression's value. - * @template T * @param {string} expression * @param {{useIsolation?: boolean}=} options * @return {Promise<*>} @@ -467,13 +466,17 @@ class Driver { } } + /** + * @typedef {any[]} TExtendsArray + */ + /** * Evaluate an expression (optionally defined in a structured manner, see `createEvalCode` * in `page-functions.js`). * See `evaluateAsync`. - * @template T, R - * @param {string | ((...args: T[]) => R)} expressionOrMainFn - * @param {{useIsolation?: boolean, args?: T[], deps?: Array}=} options + * @template {TExtendsArray} T, R + * @param {string | ((...args: T) => R)} expressionOrMainFn + * @param {{useIsolation?: boolean, args?: T, deps?: Array}=} options * @return {Promise} */ async evaluate(expressionOrMainFn, options = {}) { diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8e2fe96a27af..a74a0e3fe669 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -10,12 +10,16 @@ /* global window document Node ShadowRoot */ +/** + * @typedef {any[]} TExtendsArray + */ + /** * Creates valid JavaScript code given functions, strings of valid code, and arguments. - * @template T, R - * @param {(...args: T[]) => R} mainFn The main function to call. It's return value will be the return value + * @template {TExtendsArray} T, R + * @param {(...args: T) => R} mainFn The main function to call. It's return value will be the return value * of `createEvalCode`, wrapped in a Promise. - * @param {{mode?: 'iife'|'function', args?: T[], deps?: Array}} _ Set mode to `iife` to + * @param {{mode?: 'iife'|'function', args?: T, deps?: Array}} _ Set mode to `iife` to * create a self-executing function expression, set to `function` to create just a function * declaration statement. Args should match the args of `mainFn`, and can be any serializable * value. `deps` are functions that must be defined for `mainFn` to work. From 9e7644e1007460b97d5f9ce64ca40e73c93b6819 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 15:19:49 -0600 Subject: [PATCH 15/21] no snapshot --- lighthouse-core/gather/driver.js | 3 +- .../test/lib/page-functions-test.js | 76 +++++++++---------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index f2584be0d6bf..2dfe7e5fc0b8 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -448,8 +448,7 @@ class Driver { * @return {Promise<*>} */ async evaluateAsync(expression, options = {}) { - const contextId = - options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; + const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; try { // `await` is not redundant here because we want to `catch` the async errors diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 7b3427bebb49..6606b82dc860 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -20,15 +20,13 @@ describe('createEvalCode', () => { const code = pageFunctions.createEvalCode(mainFn, { mode: 'iife', }); - expect(code).toMatchInlineSnapshot(` - "(() => { - - function mainFn() { - return true; - } - return mainFn(); - })()" - `); + expect(code).toEqual(`(() => { + + function mainFn() { + return true; + } + return mainFn(); + })()`); expect(eval(code)).toEqual(true); }); @@ -47,27 +45,25 @@ describe('createEvalCode', () => { deps: [abs, square], mode: 'iife', }); - expect(code).toMatchInlineSnapshot(` - "(() => { - function abs(val) { - return Math.abs(val); - } - function square(val) { - return val * val; - } - function mainFn({ - a, - b - }, passThru) { - return { - a: abs(a), - b: square(b), - passThru - }; - } - return mainFn({\\"a\\":-5,\\"b\\":10},\\"hello\\"); - })()" - `); + expect(code).toEqual(`(() => { + function abs(val) { + return Math.abs(val); + } +function square(val) { + return val * val; + } + function mainFn({ + a, + b + }, passThru) { + return { + a: abs(a), + b: square(b), + passThru + }; + } + return mainFn({"a":-5,"b":10},"hello"); + })()`); expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); }); @@ -83,17 +79,15 @@ describe('createEvalCode', () => { mode: 'function', deps: [adder], }); - expect(code).toMatchInlineSnapshot(` - "function () { - function adder(a, b) { - return a + b; - } - function mainFn(a, b) { - return adder(a, b); - } - return mainFn.call(this, 1,2); - }" - `); + expect(code).toEqual(`function () { + function adder(a, b) { + return a + b; + } + function mainFn(a, b) { + return adder(a, b); + } + return mainFn.call(this, 1,2); + }`); expect(eval(`(${code})()`)).toEqual(3); }); }); From b1fb5d3e674858d2fc31864f834554596462c91f Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 11 Nov 2020 18:49:46 -0600 Subject: [PATCH 16/21] fix pr --- lighthouse-core/gather/driver.js | 8 +- lighthouse-core/lib/eval.js | 43 +++++++++ lighthouse-core/lib/page-functions.js | 47 +++------- lighthouse-core/test/gather/driver-test.js | 19 ++++ lighthouse-core/test/lib/eval-test.js | 90 +++++++++++++++++++ .../test/lib/page-functions-test.js | 80 ----------------- tmp.txt | 25 ++++++ 7 files changed, 192 insertions(+), 120 deletions(-) create mode 100644 lighthouse-core/lib/eval.js create mode 100644 lighthouse-core/test/lib/eval-test.js create mode 100644 tmp.txt diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 2dfe7e5fc0b8..0143d5c1e9a5 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -14,6 +14,7 @@ const NetworkRequest = require('../lib/network-request.js'); const EventEmitter = require('events').EventEmitter; const i18n = require('../lib/i18n/i18n.js'); const URL = require('../lib/url-shim.js'); +const {createEvalCode} = require('../lib/eval.js'); const constants = require('../config/constants.js'); const log = require('lighthouse-logger'); @@ -471,8 +472,9 @@ class Driver { /** * Evaluate an expression (optionally defined in a structured manner, see `createEvalCode` - * in `page-functions.js`). - * See `evaluateAsync`. + * in `eval.js`). + * @see createEvalCode + * @see evaluateAsync * @template {TExtendsArray} T, R * @param {string | ((...args: T) => R)} expressionOrMainFn * @param {{useIsolation?: boolean, args?: T, deps?: Array}=} options @@ -480,7 +482,7 @@ class Driver { */ async evaluate(expressionOrMainFn, options = {}) { if (typeof expressionOrMainFn !== 'string') { - expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, { + expressionOrMainFn = createEvalCode(expressionOrMainFn, { mode: 'iife', ...options, }); diff --git a/lighthouse-core/lib/eval.js b/lighthouse-core/lib/eval.js new file mode 100644 index 000000000000..273a21ce55e6 --- /dev/null +++ b/lighthouse-core/lib/eval.js @@ -0,0 +1,43 @@ +/** + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/** + * @typedef {any[]} TExtendsArray + */ + +/** + * Creates valid JavaScript code given functions, strings of valid code, and arguments. + * @template {TExtendsArray} T, R + * @param {(...args: T) => R} mainFn The main function to call. It's return value will be the return value + * of `createEvalCode`, wrapped in a Promise. + * @param {{mode?: 'iife'|'function', args?: T, deps?: Array}} _ Set mode to `iife` to + * create a self-executing function expression, set to `function` to create just a function + * declaration statement. Args should match the args of `mainFn`, and can be any serializable + * value. `deps` are functions that must be defined for `mainFn` to work. + */ +function createEvalCode(mainFn, {mode, args, deps} = {}) { + const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; + const depsSerialized = deps ? deps.join('\n') : ''; + + if (!mode || mode === 'iife') { + return `(() => { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}(${argsSerialized}); + })()`; + } else { + return `function () { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}.call(this, ${argsSerialized}); + }`; + } +} + +module.exports = { + createEvalCode, +}; diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index a74a0e3fe669..44e8d1fbecf0 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -6,46 +6,20 @@ // @ts-nocheck 'use strict'; -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ - -/* global window document Node ShadowRoot */ - /** - * @typedef {any[]} TExtendsArray + * @fileoverview + * Helper functions that are passed by `toString()` by Driver to be evaluated in target page. + * + * Important: this module should only be imported like this: + * const pageFunctions = require('...'); + * Never like this: + * const {justWhatINeed} = require('...'); + * Otherwise, minification will mangle the variable names and break usage. */ -/** - * Creates valid JavaScript code given functions, strings of valid code, and arguments. - * @template {TExtendsArray} T, R - * @param {(...args: T) => R} mainFn The main function to call. It's return value will be the return value - * of `createEvalCode`, wrapped in a Promise. - * @param {{mode?: 'iife'|'function', args?: T, deps?: Array}} _ Set mode to `iife` to - * create a self-executing function expression, set to `function` to create just a function - * declaration statement. Args should match the args of `mainFn`, and can be any serializable - * value. `deps` are functions that must be defined for `mainFn` to work. - */ -function createEvalCode(mainFn, {mode, args, deps} = {}) { - const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; - const depsSerialized = deps ? deps.join('\n') : ''; - - if (!mode || mode === 'iife') { - return `(() => { - ${depsSerialized} - ${mainFn} - return ${mainFn.name}(${argsSerialized}); - })()`; - } else { - return `function () { - ${depsSerialized} - ${mainFn} - return ${mainFn.name}.call(this, ${argsSerialized}); - }`; - } -} +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ -/** - * Helper functions that are passed by `toString()` by Driver to be evaluated in target page. - */ +/* global window document Node ShadowRoot */ /** * The `exceptionDetails` provided by the debugger protocol does not contain the useful @@ -499,7 +473,6 @@ const getNodeDetailsString = `function getNodeDetails(elem) { }`; module.exports = { - createEvalCode, wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(), registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(), checkTimeSinceLastLongTaskString: checkTimeSinceLastLongTask.toString(), diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 4dbe76b1f78b..c125253457c9 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -8,6 +8,7 @@ const Driver = require('../../gather/driver.js'); const Connection = require('../../gather/connections/connection.js'); const LHElement = require('../../lib/lh-element.js'); +const {createEvalCode} = require('../../lib/eval.js'); const {protocolGetVersionResponse} = require('./fake-driver.js'); const {createMockSendCommandFn, createMockOnceFn} = require('./mock-commands.js'); @@ -305,6 +306,24 @@ describe('.evaluateAsync', () => { }); }); +describe('.evaluate', () => { + it('transforms parameters into an expression', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Runtime.evaluate', {result: {value: 1}}); + + /** @param {number} value */ + function main(value) { + return value; + } + const value = await driver.evaluate(main, {args: [1]}); + expect(value).toEqual(1); + + const {expression} = connectionStub.sendCommand.findInvocation('Runtime.evaluate'); + const expected = createEvalCode(main, {args: [1]}); + expect(expression.includes(expected)).toBeTruthy(); + }); +}); + describe('.sendCommand', () => { it('.sendCommand timesout when commands take too long', async () => { const mockTimeout = 5000; diff --git a/lighthouse-core/test/lib/eval-test.js b/lighthouse-core/test/lib/eval-test.js new file mode 100644 index 000000000000..8a2d0bdb99d5 --- /dev/null +++ b/lighthouse-core/test/lib/eval-test.js @@ -0,0 +1,90 @@ +/** + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/* eslint-env jest */ + +const {createEvalCode} = require('../../lib/eval.js'); + +describe('createEvalCode', () => { + it('iife basic', () => { + function mainFn() { + return true; + } + const code = createEvalCode(mainFn, { + mode: 'iife', + }); + expect(code).toEqual(`(() => { + + function mainFn() { + return true; + } + return mainFn(); + })()`); + expect(eval(code)).toEqual(true); + }); + + it('iife complex', () => { + function mainFn({a, b}, passThru) { + return {a: abs(a), b: square(b), passThru}; + } + function abs(val) { + return Math.abs(val); + } + function square(val) { + return val * val; + } + const code = createEvalCode(mainFn, { + args: [{a: -5, b: 10}, 'hello'], + deps: [abs, square], + mode: 'iife', + }); + expect(code).toEqual(`(() => { + function abs(val) { + return Math.abs(val); + } +function square(val) { + return val * val; + } + function mainFn({ + a, + b + }, passThru) { + return { + a: abs(a), + b: square(b), + passThru + }; + } + return mainFn({"a":-5,"b":10},"hello"); + })()`); + expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); + }); + + it('function', () => { + function mainFn(a, b) { + return adder(a, b); + } + function adder(a, b) { + return a + b; + } + const code = createEvalCode(mainFn, { + args: [1, 2], + mode: 'function', + deps: [adder], + }); + expect(code).toEqual(`function () { + function adder(a, b) { + return a + b; + } + function mainFn(a, b) { + return adder(a, b); + } + return mainFn.call(this, 1,2); + }`); + expect(eval(`(${code})()`)).toEqual(3); + }); +}); diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 6606b82dc860..90e8be1c2b25 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -12,86 +12,6 @@ const pageFunctions = require('../../lib/page-functions.js'); /* eslint-env jest */ -describe('createEvalCode', () => { - it('iife basic', () => { - function mainFn() { - return true; - } - const code = pageFunctions.createEvalCode(mainFn, { - mode: 'iife', - }); - expect(code).toEqual(`(() => { - - function mainFn() { - return true; - } - return mainFn(); - })()`); - expect(eval(code)).toEqual(true); - }); - - it('iife complex', () => { - function mainFn({a, b}, passThru) { - return {a: abs(a), b: square(b), passThru}; - } - function abs(val) { - return Math.abs(val); - } - function square(val) { - return val * val; - } - const code = pageFunctions.createEvalCode(mainFn, { - args: [{a: -5, b: 10}, 'hello'], - deps: [abs, square], - mode: 'iife', - }); - expect(code).toEqual(`(() => { - function abs(val) { - return Math.abs(val); - } -function square(val) { - return val * val; - } - function mainFn({ - a, - b - }, passThru) { - return { - a: abs(a), - b: square(b), - passThru - }; - } - return mainFn({"a":-5,"b":10},"hello"); - })()`); - expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); - }); - - it('function', () => { - function mainFn(a, b) { - return adder(a, b); - } - function adder(a, b) { - return a + b; - } - const code = pageFunctions.createEvalCode(mainFn, { - args: [1, 2], - mode: 'function', - deps: [adder], - }); - expect(code).toEqual(`function () { - function adder(a, b) { - return a + b; - } - function mainFn(a, b) { - return adder(a, b); - } - return mainFn.call(this, 1,2); - }`); - expect(eval(`(${code})()`)).toEqual(3); - }); -}); - describe('Page Functions', () => { let dom; diff --git a/tmp.txt b/tmp.txt new file mode 100644 index 000000000000..ec1d1ceae7e5 --- /dev/null +++ b/tmp.txt @@ -0,0 +1,25 @@ +(function wrapInNativePromise() { + const __nativePromise = window.__nativePromise || Promise; + const URL = window.__nativeURL || window.URL; + return new __nativePromise(function (resolve) { + return __nativePromise.resolve() + .then(_ => (() => { + + function main(value) { + return value; + } + return main(1); + })()) + .catch(function wrapRuntimeEvalErrorInBrowser(err) { + err = err || new Error(); + const fallbackMessage = typeof err === 'string' ? err : 'unknown error'; + return { + __failedInBrowser: true, + name: err.name || 'Error', + message: err.message || fallbackMessage, + stack: err.stack || new Error().stack + }; +}) + .then(resolve); + }); + }()) \ No newline at end of file From c4d3a58d7ff3f11488f95a901a74cc3cb4ed8335 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 11 Nov 2020 18:50:21 -0600 Subject: [PATCH 17/21] oops --- tmp.txt | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 tmp.txt diff --git a/tmp.txt b/tmp.txt deleted file mode 100644 index ec1d1ceae7e5..000000000000 --- a/tmp.txt +++ /dev/null @@ -1,25 +0,0 @@ -(function wrapInNativePromise() { - const __nativePromise = window.__nativePromise || Promise; - const URL = window.__nativeURL || window.URL; - return new __nativePromise(function (resolve) { - return __nativePromise.resolve() - .then(_ => (() => { - - function main(value) { - return value; - } - return main(1); - })()) - .catch(function wrapRuntimeEvalErrorInBrowser(err) { - err = err || new Error(); - const fallbackMessage = typeof err === 'string' ? err : 'unknown error'; - return { - __failedInBrowser: true, - name: err.name || 'Error', - message: err.message || fallbackMessage, - stack: err.stack || new Error().stack - }; -}) - .then(resolve); - }); - }()) \ No newline at end of file From ee6f930bd749062ed497390bca1e6a1ae43b5531 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 12 Nov 2020 18:10:16 -0600 Subject: [PATCH 18/21] require empty args array --- lighthouse-core/gather/driver.js | 10 +++------- lighthouse-core/gather/gatherers/link-elements.js | 1 + lighthouse-core/gather/gatherers/meta-elements.js | 1 + lighthouse-core/gather/gatherers/script-elements.js | 1 + lighthouse-core/lib/eval.js | 13 +++++-------- lighthouse-core/test/lib/eval-test.js | 1 + 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 0143d5c1e9a5..0c786291ddf2 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -466,21 +466,17 @@ class Driver { } } - /** - * @typedef {any[]} TExtendsArray - */ - /** * Evaluate an expression (optionally defined in a structured manner, see `createEvalCode` * in `eval.js`). * @see createEvalCode * @see evaluateAsync - * @template {TExtendsArray} T, R + * @template {any[]} T, R * @param {string | ((...args: T) => R)} expressionOrMainFn - * @param {{useIsolation?: boolean, args?: T, deps?: Array}=} options + * @param {{args: T, useIsolation?: boolean, deps?: Array}} options * @return {Promise} */ - async evaluate(expressionOrMainFn, options = {}) { + async evaluate(expressionOrMainFn, options) { if (typeof expressionOrMainFn !== 'string') { expressionOrMainFn = createEvalCode(expressionOrMainFn, { mode: 'iife', diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index 5b409af99f3b..058adde691c1 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -87,6 +87,7 @@ class LinkElements extends Gatherer { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. return passContext.driver.evaluate(getLinkElementsInDOM, { + args: [], useIsolation: true, deps: [ pageFunctions.getNodeDetailsString, diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index 5ccb609a914b..c2a38b86e1c0 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -42,6 +42,7 @@ class MetaElements extends Gatherer { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. return driver.evaluate(collectMetaElements, { + args: [], useIsolation: true, deps: [pageFunctions.getElementsInDocument], }); diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 0bcc2d5cc2dc..2a2622d212d9 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -72,6 +72,7 @@ class ScriptElements extends Gatherer { const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url); const scripts = await driver.evaluate(collectAllScriptElements, { + args: [], useIsolation: true, deps: [ pageFunctions.getNodeDetailsString, diff --git a/lighthouse-core/lib/eval.js b/lighthouse-core/lib/eval.js index 273a21ce55e6..d23021d91ab1 100644 --- a/lighthouse-core/lib/eval.js +++ b/lighthouse-core/lib/eval.js @@ -5,22 +5,19 @@ */ 'use strict'; -/** - * @typedef {any[]} TExtendsArray - */ - /** * Creates valid JavaScript code given functions, strings of valid code, and arguments. - * @template {TExtendsArray} T, R + * @template {any[]} T, R * @param {(...args: T) => R} mainFn The main function to call. It's return value will be the return value * of `createEvalCode`, wrapped in a Promise. - * @param {{mode?: 'iife'|'function', args?: T, deps?: Array}} _ Set mode to `iife` to + * @param {{args: T, mode?: 'iife'|'function', deps?: Array}} options Set mode to `iife` to * create a self-executing function expression, set to `function` to create just a function * declaration statement. Args should match the args of `mainFn`, and can be any serializable * value. `deps` are functions that must be defined for `mainFn` to work. */ -function createEvalCode(mainFn, {mode, args, deps} = {}) { - const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; +function createEvalCode(mainFn, options) { + const {mode, args, deps} = options; + const argsSerialized = args.map(arg => JSON.stringify(arg)).join(','); const depsSerialized = deps ? deps.join('\n') : ''; if (!mode || mode === 'iife') { diff --git a/lighthouse-core/test/lib/eval-test.js b/lighthouse-core/test/lib/eval-test.js index 8a2d0bdb99d5..b9fa33bb6edb 100644 --- a/lighthouse-core/test/lib/eval-test.js +++ b/lighthouse-core/test/lib/eval-test.js @@ -16,6 +16,7 @@ describe('createEvalCode', () => { } const code = createEvalCode(mainFn, { mode: 'iife', + args: [], }); expect(code).toEqual(`(() => { From 6be38ebc24847ed8619393f85e952f92a1520c36 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 12 Nov 2020 18:58:41 -0600 Subject: [PATCH 19/21] inline --- lighthouse-core/gather/driver.js | 37 +++---- lighthouse-core/lib/eval.js | 40 -------- lighthouse-core/test/gather/driver-test.js | 106 ++++++++++++++++++++- lighthouse-core/test/lib/eval-test.js | 91 ------------------ 4 files changed, 121 insertions(+), 153 deletions(-) delete mode 100644 lighthouse-core/lib/eval.js delete mode 100644 lighthouse-core/test/lib/eval-test.js diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 0c786291ddf2..daab87b544dd 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -14,7 +14,6 @@ const NetworkRequest = require('../lib/network-request.js'); const EventEmitter = require('events').EventEmitter; const i18n = require('../lib/i18n/i18n.js'); const URL = require('../lib/url-shim.js'); -const {createEvalCode} = require('../lib/eval.js'); const constants = require('../config/constants.js'); const log = require('lighthouse-logger'); @@ -439,7 +438,6 @@ class Driver { } /** - * Note: Prefer `evaluate` instead. * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. @@ -467,23 +465,26 @@ class Driver { } /** - * Evaluate an expression (optionally defined in a structured manner, see `createEvalCode` - * in `eval.js`). - * @see createEvalCode - * @see evaluateAsync + * Evaluate a function in the context of the current page. + * If `useIsolation` is true, the function will be evaluated in a content script that has + * access to the page's DOM but whose JavaScript state is completely separate. + * Returns a promise that resolves on a value of `mainFn`'s return type. * @template {any[]} T, R - * @param {string | ((...args: T) => R)} expressionOrMainFn - * @param {{args: T, useIsolation?: boolean, deps?: Array}} options + * @param {((...args: T) => R)} mainFn The main function to call. + * @param {{args: T, useIsolation?: boolean, deps?: Array}} options `args` should + * match the args of `mainFn`, and can be any serializable value. `deps` are functions that must be + * defined for `mainFn` to work. * @return {Promise} */ - async evaluate(expressionOrMainFn, options) { - if (typeof expressionOrMainFn !== 'string') { - expressionOrMainFn = createEvalCode(expressionOrMainFn, { - mode: 'iife', - ...options, - }); - } - return this.evaluateAsync(expressionOrMainFn, options); + async evaluate(mainFn, options) { + const argsSerialized = options.args.map(arg => JSON.stringify(arg)).join(','); + const depsSerialized = options.deps ? options.deps.join('\n') : ''; + const expression = `(() => { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}(${argsSerialized}); + })()`; + return this.evaluateAsync(expression, options); } /** @@ -505,8 +506,8 @@ class Driver { // 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; - const URL = window.__nativeURL || window.URL; + const __nativePromise = globalThis.__nativePromise || Promise; + const URL = globalThis.__nativeURL || globalThis.URL; return new __nativePromise(function (resolve) { return __nativePromise.resolve() .then(_ => ${expression}) diff --git a/lighthouse-core/lib/eval.js b/lighthouse-core/lib/eval.js deleted file mode 100644 index d23021d91ab1..000000000000 --- a/lighthouse-core/lib/eval.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - */ -'use strict'; - -/** - * Creates valid JavaScript code given functions, strings of valid code, and arguments. - * @template {any[]} T, R - * @param {(...args: T) => R} mainFn The main function to call. It's return value will be the return value - * of `createEvalCode`, wrapped in a Promise. - * @param {{args: T, mode?: 'iife'|'function', deps?: Array}} options Set mode to `iife` to - * create a self-executing function expression, set to `function` to create just a function - * declaration statement. Args should match the args of `mainFn`, and can be any serializable - * value. `deps` are functions that must be defined for `mainFn` to work. - */ -function createEvalCode(mainFn, options) { - const {mode, args, deps} = options; - const argsSerialized = args.map(arg => JSON.stringify(arg)).join(','); - const depsSerialized = deps ? deps.join('\n') : ''; - - if (!mode || mode === 'iife') { - return `(() => { - ${depsSerialized} - ${mainFn} - return ${mainFn.name}(${argsSerialized}); - })()`; - } else { - return `function () { - ${depsSerialized} - ${mainFn} - return ${mainFn.name}.call(this, ${argsSerialized}); - }`; - } -} - -module.exports = { - createEvalCode, -}; diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index c125253457c9..866875e30cd9 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -8,7 +8,6 @@ const Driver = require('../../gather/driver.js'); const Connection = require('../../gather/connections/connection.js'); const LHElement = require('../../lib/lh-element.js'); -const {createEvalCode} = require('../../lib/eval.js'); const {protocolGetVersionResponse} = require('./fake-driver.js'); const {createMockSendCommandFn, createMockOnceFn} = require('./mock-commands.js'); @@ -307,7 +306,7 @@ describe('.evaluateAsync', () => { }); describe('.evaluate', () => { - it('transforms parameters into an expression', async () => { + it('transforms parameters into an expression given to Runtime.evaluate', async () => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Runtime.evaluate', {result: {value: 1}}); @@ -319,8 +318,107 @@ describe('.evaluate', () => { expect(value).toEqual(1); const {expression} = connectionStub.sendCommand.findInvocation('Runtime.evaluate'); - const expected = createEvalCode(main, {args: [1]}); - expect(expression.includes(expected)).toBeTruthy(); + const expected = ` +(function wrapInNativePromise() { + const __nativePromise = globalThis.__nativePromise || Promise; + const URL = globalThis.__nativeURL || globalThis.URL; + return new __nativePromise(function (resolve) { + return __nativePromise.resolve() + .then(_ => (() => { + + function main(value) { + return value; + } + return main(1); + })()) + .catch(function wrapRuntimeEvalErrorInBrowser(err) { + err = err || new Error(); + const fallbackMessage = typeof err === 'string' ? err : 'unknown error'; + return { + __failedInBrowser: true, + name: err.name || 'Error', + message: err.message || fallbackMessage, + stack: err.stack || new Error().stack + }; +}) + .then(resolve); + }); + }())`.trim(); + expect(expression).toBe(expected); + expect(await eval(expression)).toBe(1); + }); + + it('transforms parameters into an expression (basic)', async () => { + const mockFn = driver._evaluateInContext = jest.fn() + .mockImplementation(() => Promise.resolve()); + + /** @param {number} value */ + function mainFn(value) { + return value; + } + await driver.evaluate(mainFn, {args: [1]}); + + const code = mockFn.mock.calls[0][0]; + expect(code).toBe(`(() => { + + function mainFn(value) { + return value; + } + return mainFn(1); + })()`); + expect(eval(code)).toEqual(1); + }); + + it('transforms parameters into an expression (complex)', async () => { + const mockFn = driver._evaluateInContext = jest.fn() + .mockImplementation(() => Promise.resolve()); + + /** + * @param {{a: number, b: number}} _ + * @param {any} passThru + */ + function mainFn({a, b}, passThru) { + return {a: abs(a), b: square(b), passThru}; + } + /** + * @param {number} val + */ + function abs(val) { + return Math.abs(val); + } + /** + * @param {number} val + */ + function square(val) { + return val * val; + } + + await driver.evaluate(mainFn, { + args: [{a: -5, b: 10}, 'hello'], + deps: [abs, square], + }); + + const code = mockFn.mock.calls[0][0]; + expect(code).toEqual(`(() => { + function abs(val) { + return Math.abs(val); + } +function square(val) { + return val * val; + } + function mainFn({ + a, + b + }, passThru) { + return { + a: abs(a), + b: square(b), + passThru + }; + } + return mainFn({"a":-5,"b":10},"hello"); + })()`); + expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); }); }); diff --git a/lighthouse-core/test/lib/eval-test.js b/lighthouse-core/test/lib/eval-test.js deleted file mode 100644 index b9fa33bb6edb..000000000000 --- a/lighthouse-core/test/lib/eval-test.js +++ /dev/null @@ -1,91 +0,0 @@ -/** - * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - */ -'use strict'; - -/* eslint-env jest */ - -const {createEvalCode} = require('../../lib/eval.js'); - -describe('createEvalCode', () => { - it('iife basic', () => { - function mainFn() { - return true; - } - const code = createEvalCode(mainFn, { - mode: 'iife', - args: [], - }); - expect(code).toEqual(`(() => { - - function mainFn() { - return true; - } - return mainFn(); - })()`); - expect(eval(code)).toEqual(true); - }); - - it('iife complex', () => { - function mainFn({a, b}, passThru) { - return {a: abs(a), b: square(b), passThru}; - } - function abs(val) { - return Math.abs(val); - } - function square(val) { - return val * val; - } - const code = createEvalCode(mainFn, { - args: [{a: -5, b: 10}, 'hello'], - deps: [abs, square], - mode: 'iife', - }); - expect(code).toEqual(`(() => { - function abs(val) { - return Math.abs(val); - } -function square(val) { - return val * val; - } - function mainFn({ - a, - b - }, passThru) { - return { - a: abs(a), - b: square(b), - passThru - }; - } - return mainFn({"a":-5,"b":10},"hello"); - })()`); - expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); - }); - - it('function', () => { - function mainFn(a, b) { - return adder(a, b); - } - function adder(a, b) { - return a + b; - } - const code = createEvalCode(mainFn, { - args: [1, 2], - mode: 'function', - deps: [adder], - }); - expect(code).toEqual(`function () { - function adder(a, b) { - return a + b; - } - function mainFn(a, b) { - return adder(a, b); - } - return mainFn.call(this, 1,2); - }`); - expect(eval(`(${code})()`)).toEqual(3); - }); -}); From 97b052519a1b81ebc1cafaed3739b543c30fab1e Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 13 Nov 2020 13:42:42 -0600 Subject: [PATCH 20/21] last bits --- lighthouse-core/test/gather/driver-test.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 866875e30cd9..aa7ed4fd4f86 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -349,6 +349,8 @@ describe('.evaluate', () => { }); it('transforms parameters into an expression (basic)', async () => { + // Mock so the argument can be intercepted, and the generated code + // can be evaluated without the error catching code. const mockFn = driver._evaluateInContext = jest.fn() .mockImplementation(() => Promise.resolve()); @@ -356,7 +358,8 @@ describe('.evaluate', () => { function mainFn(value) { return value; } - await driver.evaluate(mainFn, {args: [1]}); + /** @type {number} */ + const value = await driver.evaluate(mainFn, {args: [1]}); // eslint-disable-line no-unused-vars const code = mockFn.mock.calls[0][0]; expect(code).toBe(`(() => { @@ -370,6 +373,8 @@ describe('.evaluate', () => { }); it('transforms parameters into an expression (complex)', async () => { + // Mock so the argument can be intercepted, and the generated code + // can be evaluated without the error catching code. const mockFn = driver._evaluateInContext = jest.fn() .mockImplementation(() => Promise.resolve()); @@ -393,7 +398,8 @@ describe('.evaluate', () => { return val * val; } - await driver.evaluate(mainFn, { + /** @type {{a: number, b: number, passThru: any}} */ + const value = await driver.evaluate(mainFn, { // eslint-disable-line no-unused-vars args: [{a: -5, b: 10}, 'hello'], deps: [abs, square], }); From 3c9f5738190b92179773b8e2ba5d3e2ee0abcfc4 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 1 Dec 2020 18:56:21 -0600 Subject: [PATCH 21/21] tests --- .../gather/driver/execution-context.js | 6 +- lighthouse-core/test/gather/driver-test.js | 123 ----------------- .../gather/driver/execution-context-test.js | 129 ++++++++++++++++++ 3 files changed, 132 insertions(+), 126 deletions(-) diff --git a/lighthouse-core/gather/driver/execution-context.js b/lighthouse-core/gather/driver/execution-context.js index 8ab27f76b1d4..8964219c39cb 100644 --- a/lighthouse-core/gather/driver/execution-context.js +++ b/lighthouse-core/gather/driver/execution-context.js @@ -83,9 +83,9 @@ class ExecutionContext { // 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; - const URL = window.__nativeURL || window.URL; - window.__lighthouseExecutionContextId = ${contextId}; + const __nativePromise = globalThis.__nativePromise || Promise; + const URL = globalThis.__nativeURL || globalThis.URL; + globalThis.__lighthouseExecutionContextId = ${contextId}; return new __nativePromise(function (resolve) { return __nativePromise.resolve() .then(_ => ${expression}) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 4e5a39ccfe62..d5177aff27f8 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -150,129 +150,6 @@ describe('.evaluateAsync', () => { }); }); -describe('.evaluate', () => { - it('transforms parameters into an expression given to Runtime.evaluate', async () => { - connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Runtime.evaluate', {result: {value: 1}}); - - /** @param {number} value */ - function main(value) { - return value; - } - const value = await driver.evaluate(main, {args: [1]}); - expect(value).toEqual(1); - - const {expression} = connectionStub.sendCommand.findInvocation('Runtime.evaluate'); - const expected = ` -(function wrapInNativePromise() { - const __nativePromise = globalThis.__nativePromise || Promise; - const URL = globalThis.__nativeURL || globalThis.URL; - return new __nativePromise(function (resolve) { - return __nativePromise.resolve() - .then(_ => (() => { - - function main(value) { - return value; - } - return main(1); - })()) - .catch(function wrapRuntimeEvalErrorInBrowser(err) { - err = err || new Error(); - const fallbackMessage = typeof err === 'string' ? err : 'unknown error'; - return { - __failedInBrowser: true, - name: err.name || 'Error', - message: err.message || fallbackMessage, - stack: err.stack || new Error().stack - }; -}) - .then(resolve); - }); - }())`.trim(); - expect(expression).toBe(expected); - expect(await eval(expression)).toBe(1); - }); - - it('transforms parameters into an expression (basic)', async () => { - // Mock so the argument can be intercepted, and the generated code - // can be evaluated without the error catching code. - const mockFn = driver._evaluateInContext = jest.fn() - .mockImplementation(() => Promise.resolve()); - - /** @param {number} value */ - function mainFn(value) { - return value; - } - /** @type {number} */ - const value = await driver.evaluate(mainFn, {args: [1]}); // eslint-disable-line no-unused-vars - - const code = mockFn.mock.calls[0][0]; - expect(code).toBe(`(() => { - - function mainFn(value) { - return value; - } - return mainFn(1); - })()`); - expect(eval(code)).toEqual(1); - }); - - it('transforms parameters into an expression (complex)', async () => { - // Mock so the argument can be intercepted, and the generated code - // can be evaluated without the error catching code. - const mockFn = driver._evaluateInContext = jest.fn() - .mockImplementation(() => Promise.resolve()); - - /** - * @param {{a: number, b: number}} _ - * @param {any} passThru - */ - function mainFn({a, b}, passThru) { - return {a: abs(a), b: square(b), passThru}; - } - /** - * @param {number} val - */ - function abs(val) { - return Math.abs(val); - } - /** - * @param {number} val - */ - function square(val) { - return val * val; - } - - /** @type {{a: number, b: number, passThru: any}} */ - const value = await driver.evaluate(mainFn, { // eslint-disable-line no-unused-vars - args: [{a: -5, b: 10}, 'hello'], - deps: [abs, square], - }); - - const code = mockFn.mock.calls[0][0]; - expect(code).toEqual(`(() => { - function abs(val) { - return Math.abs(val); - } -function square(val) { - return val * val; - } - function mainFn({ - a, - b - }, passThru) { - return { - a: abs(a), - b: square(b), - passThru - }; - } - return mainFn({"a":-5,"b":10},"hello"); - })()`); - expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); - }); -}); - describe('.sendCommand', () => { it('.sendCommand timesout when commands take too long', async () => { const mockTimeout = 5000; diff --git a/lighthouse-core/test/gather/driver/execution-context-test.js b/lighthouse-core/test/gather/driver/execution-context-test.js index f75cc188a4c2..afe3a74c2bc2 100644 --- a/lighthouse-core/test/gather/driver/execution-context-test.js +++ b/lighthouse-core/test/gather/driver/execution-context-test.js @@ -171,3 +171,132 @@ describe('.evaluateAsync', () => { expect(value).toEqual('mocked value'); }); }); + +describe('.evaluate', () => { + /** @type {LH.Gatherer.FRProtocolSession} */ + let sessionMock; + /** @type {ExecutionContext} */ + let executionContext; + + beforeEach(() => { + sessionMock = createMockSession(); + sessionMock.on = jest.fn(); + executionContext = new ExecutionContext(sessionMock); + }); + + it('transforms parameters into an expression given to Runtime.evaluate', async () => { + const mockFn = sessionMock.sendCommand = createMockSendCommandFn() + .mockResponse('Runtime.evaluate', {result: {value: 1}}); + + /** @param {number} value */ + function main(value) { + return value; + } + const value = await executionContext.evaluate(main, {args: [1]}); + expect(value).toEqual(1); + + const {expression} = mockFn.findInvocation('Runtime.evaluate'); + const expected = ` +(function wrapInNativePromise() { + const __nativePromise = globalThis.__nativePromise || Promise; + const URL = globalThis.__nativeURL || globalThis.URL; + globalThis.__lighthouseExecutionContextId = undefined; + return new __nativePromise(function (resolve) { + return __nativePromise.resolve() + .then(_ => (() => { + + function main(value) { + return value; + } + return main(1); + })()) + .catch(function wrapRuntimeEvalErrorInBrowser(err) { + err = err || new Error(); + const fallbackMessage = typeof err === 'string' ? err : 'unknown error'; + + return { + __failedInBrowser: true, + name: err.name || 'Error', + message: err.message || fallbackMessage, + stack: err.stack || (new Error()).stack, + }; +}) + .then(resolve); + }); + }())`.trim(); + expect(expression).toBe(expected); + expect(await eval(expression)).toBe(1); + }); + + it('transforms parameters into an expression (basic)', async () => { + // Mock so the argument can be intercepted, and the generated code + // can be evaluated without the error catching code. + const mockFn = executionContext._evaluateInContext = jest.fn() + .mockImplementation(() => Promise.resolve()); + + /** @param {number} value */ + function mainFn(value) { + return value; + } + /** @type {number} */ + const value = await executionContext.evaluate(mainFn, {args: [1]}); // eslint-disable-line no-unused-vars + + const code = mockFn.mock.calls[0][0]; + expect(code).toBe(`(() => { + + function mainFn(value) { + return value; + } + return mainFn(1); + })()`); + expect(eval(code)).toEqual(1); + }); + + it('transforms parameters into an expression (complex)', async () => { + // Mock so the argument can be intercepted, and the generated code + // can be evaluated without the error catching code. + const mockFn = executionContext._evaluateInContext = jest.fn() + .mockImplementation(() => Promise.resolve()); + + /** + * @param {{a: number, b: number}} _ + * @param {any} passThru + */ + function mainFn({a, b}, passThru) { + return {a: abs(a), b: square(b), passThru}; + } + /** + * @param {number} val + */ + function abs(val) { + return Math.abs(val); + } + /** + * @param {number} val + */ + function square(val) { + return val * val; + } + + /** @type {{a: number, b: number, passThru: any}} */ + const value = await executionContext.evaluate(mainFn, { // eslint-disable-line no-unused-vars + args: [{a: -5, b: 10}, 'hello'], + deps: [abs, square], + }); + + const code = mockFn.mock.calls[0][0]; + expect(code).toEqual(`(() => { + function abs(val) { + return Math.abs(val); + } +function square(val) { + return val * val; + } + function mainFn({a, b}, passThru) { + return {a: abs(a), b: square(b), passThru}; + } + return mainFn({"a":-5,"b":10},"hello"); + })()`); + expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); + }); +});