Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: append sourceURL comment to eval code #13754

Merged
merged 9 commits into from
Mar 30, 2022
6 changes: 4 additions & 2 deletions lighthouse-core/gather/driver/execution-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ class ExecutionContext {
.catch(${pageFunctions.wrapRuntimeEvalErrorInBrowserString})
.then(resolve);
});
}())`,
}())
//# sourceURL=_lighthouse-eval.js`,
includeCommandLineAPI: true,
awaitPromise: true,
returnByValue: true,
Expand Down Expand Up @@ -195,7 +196,8 @@ class ExecutionContext {
${ExecutionContext._cachedNativesPreamble};
${depsSerialized};
(${mainFn})(${argsSerialized});
})()`;
})()
//# sourceURL=_lighthouse-eval.js`;

await this._session.sendCommand('Page.addScriptToEvaluateOnNewDocument', {source: expression});
}
Expand Down
13 changes: 7 additions & 6 deletions lighthouse-core/gather/gatherers/js-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ class JsUsage extends FRGatherer {
const usageByScriptId = {};

for (const scriptUsage of this._scriptUsages) {
// If `url` is blank, that means the script was anonymous (eval, new Function, onload, ...).
// Or, it's because it was code Lighthouse over the protocol via `Runtime.evaluate`.
// We currently don't consider coverage of anonymous scripts, and we definitely don't want
// coverage of code Lighthouse ran to inspect the page, so we ignore this ScriptCoverage if
// url is blank.
if (scriptUsage.url === '') {
// If `url` is blank, that means the script was dynamically
// created (eval, new Function, onload, ...)
if (scriptUsage.url === '' || scriptUsage.url === '_lighthouse-eval.js') {
// We currently don't consider coverage of dynamic scripts, and we definitely don't want
// coverage of code Lighthouse ran to inspect the page, so we ignore this ScriptCoverage.
// Audits would work the same without this, it is only an optimization (not tracking coverage
// for scripts we don't care about).
continue;
}

Expand Down
22 changes: 20 additions & 2 deletions lighthouse-core/gather/gatherers/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,23 @@ async function runInSeriesOrParallel(values, promiseMapper, runInSeries) {
}
}

/**
* Returns true if the script was created via our own calls
* to Runtime.evaluate.
* @param {LH.Crdp.Debugger.ScriptParsedEvent} script
*/
function isLighthouseRuntimeEvaluateScript(script) {
// Scripts created by Runtime.evaluate that run on the main session/frame
// result in an empty string for the embedderName.
// Or, it means the script was dynamically created (eval, new Function, onload, ...)
if (!script.embedderName) return true;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

// Otherwise, when running our own code inside other frames, the embedderName
// is set to the frame's url. In that case, we rely on the special sourceURL that
// we set.
return script.hasSourceURL && script.url === '_lighthouse-eval.js';
}

/**
* @fileoverview Gets JavaScript file contents.
*/
Expand Down Expand Up @@ -68,8 +85,9 @@ class Scripts extends FRGatherer {
// it also blocks scripts from the same origin but that happen to run in a different process,
// like a worker.
if (event.method === 'Debugger.scriptParsed' && !sessionId) {
// Events without an embedderName (read: a url) are for JS that we ran over the protocol.
if (event.params.embedderName) this._scriptParsedEvents.push(event.params);
if (!isLighthouseRuntimeEvaluateScript(event.params)) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: now can we add to the condition above ;)

this._scriptParsedEvents.push(event.params);
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/test/gather/driver/execution-context-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ const fetch = globalThis.__nativeFetch || globalThis.fetch;
})
.then(resolve);
});
}())`.trim();
}())
//# sourceURL=_lighthouse-eval.js`.trim();
expect(trimTrailingWhitespace(expression)).toBe(trimTrailingWhitespace(expected));
expect(await eval(expression)).toBe(1);
});
Expand Down