GoogleChrome / GoogleChrome/lighthouse

Reduce `Debugger.*` friction between gatherers

Open
#14,747 0 comments 0 reactions 0 assignees View on GitHub
P2
Dominant language
JavaScript
Stars
30.8k
Forks
9.8k
Avg merge
1d 20h
Merged PRs (30d)
19

Description

From https://github.com/GoogleChrome/lighthouse/pull/14717:

> This PR has become a bit of a rabbit hole. While I do think it would be good to land this in the long term, it opens the door for some more complex challenges relating to scripts gathering and third parties.
>
> Incoming brain dump...
>
> Calling `Debugger.enable` when the domain is disabled will flush any `Debugger.scriptParsed` events that the session hadn't emitted yet. Since this only happens when the domain was in the disabled state, the events are emitted on the *first* invocation of `Debugger.enable` which previously happened in `prepare.js` as part of our navigation/timespan prework.
>
> This PR moves that invocation into the `DevtoolsLog` gatherer, which moves the *new* first invocation of `Debugger.enable` into the `Scripts` gatherer. This doesn't mean anything for navigation mode, since the domain will always be enabled while we are on `about:blank` before the navigation starts. However, timespan mode can start on an already loaded page, which means there could be `Debugger.scriptParsed` events waiting to emit that will now be caught by the `Scripts` gatherer since the first invocation of `Debugger.enable` happens there. This means timespan mode will collect all scripts that are loaded during the timespan, *as well as* any scripts that were on the page at the start of the timespan. In theory this is a better way of collecting scripts for an arbitrary timespan however...
>
> With the introduction of the new 3P system, we will throw an error if we encounter a url that doesn't exist in the network records. Now we are collecting scripts that weren't fetched during the timespan, [which can cause this error to be thrown](https://github.com/GoogleChrome/lighthouse/actions/runs/4018736613/jobs/6904726470) if we attempt to use a url from one of the pre-timespan scripts in say the `valid-source-map` audit:
>
> https://github.com/GoogleChrome/lighthouse/blob/6de2ed25cdf2da46ed1bc261486d638f7a5adc49/core/audits/valid-source-maps.js#L56-L62
>
> In the future, I imagine we will classify the urls in `Scripts` but for now I think the easiest workaround is to call `Debugger.enable` in the navigation/timespan prework so all the `Debugger.scriptParsed` events are flushed before gathering starts. Or we could just do https://github.com/GoogleChrome/lighthouse/pull/14716 w/ @brendankenny's suggestions.

To try and summarize:
1. We should start using separate sessions for different gatherers/tasks that require the debugger for different things.
2. I wanted to move the async stack handlers to the DevTools log gatherer without worrying about how it affects other gatherers like `Scripts`
3. `Scripts` can then collect scripts that were on the page at the start of timespan mode, however...
4. Our 3P system will need to support urls that are *not* seen in any network request. This probably means we need to feed `Scripts` into `EntityClassification`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.