core(root-causes): grab from trace insights rather than use protocol #16352
Conversation
There was a problem hiding this comment.
No url for injected iframe root cause
No render blocking requests root cause
Losing render blocking requests as a root cause doesn't really bother me. It would be nice to have the iframe node id in the TE result somehow, but this PR is still an upgrade since iframe root causes were disabled before.
| // { | ||
| // node: {selector: 'body > div#blue'}, | ||
| // // TODO: We can't get nodes from non-main frames yet. See runRootCauseAnalysis. | ||
| // subItems: {items: [{cause: /iframe/, extra: undefined}]}, |
There was a problem hiding this comment.
We should get an iframe root cause now. The iframe root causes were only disabled for the old root causes engine.
There was a problem hiding this comment.
It's not there. Keep in mind the implementations of these differ.
There was a problem hiding this comment.
What implementation difference? If this isn't showing up then maybe we have a bug in the TE?
There was a problem hiding this comment.
i don't know. it'd be a lot of work to understand this and I think I can't prioritize looking into it.
| /** @type {LH.Artifacts.TraceEngineRootCauses} */ | ||
| const allRootCauses = { | ||
| layoutShifts: new Map(), | ||
| }; |
There was a problem hiding this comment.
nit: can't we just do const layoutShifts = new Map()
This gatherer is one of the top CPU costs in PSI. Instead of using the protocol to gather root causes, lets just defer to the trace engine now that its there.
This regressed
layout-shiftsa bit:However, we're soon to drop that audit and replacing with an insight audit.