As discovered in #14264 we were severely excluding events in our processEvents array. And also frameEvents, frameTreeEvents, and mainThreadEvents.

The primary changes here are in trace-processor and this pr contains mostly bug fixes. I envision a second PR which refactors the traceprocessor functions and flow a bit (and wouldn't adjust test results). I can do that here, too, but i'll leave that to the reviewer to opt-in to.

Observations & changes:

• about:blank => website can undergo a process switch. the twitter.com repro makes this super obvious.
• findMainFrameIds() returns the "starting" pid, but that will not be the pid of the primary navigation necessarily. that needs to be found afterwards.
• frameIds are consistent across x-process navigations. (always have been, AFAIK)
• the FrameCommittedInBrowser event is great and spells out the potentially-new processId of the renderer in the new navigation.
• I tried to support the usecase of 'multiple navigations in one trace' (eg in timespans). I'm not convinced we support that well right now.
• Trace events explicitly associated with a frame are generally tagged with either args.data.frame or args.frame. No consistency. Where I saw our code depending on one location, I expanded it to both.

Followup work:

• Verify we handle metric calculation of multiple navigations correctly. (For timespan mode)
• Nearly certain that this fcpAllFrames calculation doesnt return the right value.
• 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.
• Clarify that frameEvents/frameTreeEvents are a subset of all events from that frame.
• Validate all uses of startingPid are using it correctly. (They're probably not)
• Review all uses of trace.traceEvents as there's potentially a mistake handling pids/frames.
• Refactor the traceprocessor flow. In short: determine the "inspected" pids/frames in one pass before doing all the subsetting.

This enables certain clients to calculate the performance metrics (and score) with just the trace. (Currently you need both trace and devtoolsLog.)

There are a few known limitations as the trace instrumentation for network requests isn't as complete as whats in the protocol. Those items are:

• initiator (and any relevant js callstack)
• request & response headers
• isLinkPreload
• referrerPolicy (eh)
• perhaps some signal about fromDiskCache/fromMemoryCache
• potentially others like if a request's priority changes over time.

The good news is that these signals are not at all needed for the metrics. Some are needed for non-metric perf audits, but those are not needed currently.

The feature is exposed via an additional sharedFlagSetting:

  /** By default, network records and timing come from the devtools protocol. If true, the trace will be used as the source of truth for network activity. */
  traceBasedNetworkRecords?: boolean;

While there's a lot of good work in here... and it's near complete.. We decided to not continue moving forward with the integration this feature was planned for.

We may dust this off in the future.. or perhaps use it when trying out #14282

tests(smoke): increase windows retries (#14022)

core: log requestedUrl with unexpected value (#14010)

core(image-elements): use execution context isolation (#14005)

renames

core: save native getBoundingClientRect to avoid overrides (#14002)

thurday edits

and more thursday edits

tests(config-helpers): restore process.cwd after mocking (#14036)

tests: use readJson instead of imports for json (#14020)

docs(user-flows): add instructions for DevTools (#14009)

tests(devtools): sync web tests (#14061)

tests(topbar): replace module mock with dependency injection (#14057)

i18n: reduce unnecessary message formats (#14030)

deps: chrome-launcher@0.15.1 (#14070)

core(source-maps): throw explicit error when map is missing required fields (#14060)

docs(architecture): update to reflect FR changes (#14017)

tests(smoke): fix ToT node id failures (#14077)

core(driver): fix legacy runner hanging oopifs in some cases (#14074)

tests: disable perf-diagnostics-third-party for FR (#14092)

misc(build): fix lightrider report generator bundle (#14031)

https://github.com/WPO-Foundation/wptagent/pull/557 https://github.com/WPO-Foundation/wptagent/pull/538#issuecomment-1176863942

cleanup

Revert "test: reformat trace fixtures"

This reverts commit e283d7635dd5448404b60ed4c8cab37a22549f2a.

cleanup

fix frame matching in responsiveness / work-during-interaction

I'm doing some work on the core of traceprocessor and several of our trace fixtures are all on 1 line or are mega-stringified. Makes it hard to debug.

Wrote a lil script to fixup the problematic ones (and confirm the new data deepStrictEqual the old stuff). I didn't include the script, but could.

Here's the rough version: https://github.com/paulirish/git-open/compare/v2.1.0...v3.0.0

i haven't written this up... (and would appreciate any help)

Brendan and I looked into it and found some issues. In short, our frameEvents is a small subset of all events happening on that frame... and more important to this issue.. our processEvents can pick the incorrect process if there was a cross-origin redirect during navigation. This latter item seems like a regression somehow at some point. We'll dig into it.

Thanks @mattzeunert mucho for reporting

You're right that Lighthouse is saying these are text-based resources and they are not.

Really, the assets here fell into a super edge case because its lossless uncompressed video assets. And turns out gzip would actually provide a win.

We'll consider reframing how we do this, but... it would only really apply for this sort of test asset.

test page: https://output.jsbin.com/lenelog/quiet w/ google maps embed

(also treemap breaks on our ToT lhrs cuz no lhr.requestedUrl)

but anyway, the iframe requests are lost somewhere (maybe in the bundles artifact?) and so script-treemap-data has no nodes.

But...

You don't want browsers launched in this context to be headless, and yet you have an HEADLESS env variable with a truthy value.. 🤷‍♂️

Can I ask what the purpose of that env variable is then?

Implementation-wise... clearly we could use CHROME_HEADLESS (or CHROME_LAUNCHER_HEADLESS) instead... but... it's just.. This seems like a weird edge case I'm not inclined to support.. 😕