GoogleChrome / GoogleChrome/lighthouse
trace processor - cleanup and fixes ☂️
- Dominant language
- JavaScript
- Stars
- 30.8k
- Forks
- 9.8k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 19
Description
Forking off from #14287 …
## arch
* Refactor the traceprocessor flow. In short: determine the "inspected" pids/frames in one pass before doing all the subsetting.
* Verify we handle metric calculation of multiple navigations correctly. (For timespan mode)
* While trace-processor could organize all processes found in the trace, I think it's better to just instantly whittle down the events to the "inspected" process tree and frame tree. Drop everything else, so no metric calculation code needs to filter for themselves.
- or... _don't_ do this since [the processFilter](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/tracing_handler.cc;l=819-841;drc=b3bdc5a24a0d87b4de66320c6d187aafd55840ca;bpv=1;bpt=1) is already doing this for us.
* handle the `pid` reuse case (however unlikely that is while tracing). Having just a map of `pid->tid` says nothing about the timing. Seems like we might need a temporal aspect to the tracking as well? Or just step though the trace, subsetting in chunks between any `FrameCommittedInBrowser` events.
## frames
* Clarify that frameEvents/frameTreeEvents are a subset of all events from that frame.
* Adopt [`isOutermostMainFrame`](https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3714815)?
### allframes metrics
* Nearly certain that [this fcpAllFrames calculation](https://github.com/GoogleChrome/lighthouse/blob/6909c6cc3a6b72b421fbedeabf06974734f7845d/core/lib/tracehouse/trace-processor.js#L723-L726) doesnt return the right value.
* #14760 is relevant..
## audit traceEvent usages
* Review all uses of `trace.traceEvents` as there's potentially a mistake handling pids/frames.
* Audit all uses of .args[.data].frame to see if there's a better way to ensure the data is reliably there.
* Validate all uses of startingPid are using it correctly. (They're probably not)
Contributor guide
Assessment
This issue has not been assessed yet.