core(page-dependency-graph): compute using URL artifact #13772
Conversation
| const sampleTrace = require('../fixtures/traces/progressive-app-m60.json'); | ||
| const sampleDevtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); | ||
| const sampleTrace = require('../fixtures/traces/iframe-m79.trace.json'); | ||
| const sampleDevtoolsLog = require('../fixtures/traces/iframe-m79.devtoolslog.json'); |
There was a problem hiding this comment.
The M60 trace has the old TracingStartedInPage which uses uses a memory address as the frame ID instead of the same ID in the DT log.
There was a problem hiding this comment.
I wonder if we should refresh many of these old traces. m60 is from 2017-07
There was a problem hiding this comment.
Not a bad idea, but wouldn't help this PR anymore.
After piping artifacts.URL everywhere, this change isn't really necessary.
| @@ -463,6 +448,34 @@ class PageDependencyGraph { | |||
| }); | |||
| } | |||
|
|
|||
| /** | |||
| * Should provide the same urls found on `artifacts.URL`. | |||
| * TODO: make `artifacts.URL` a dependency for this computed artifact. | |||
There was a problem hiding this comment.
I was hoping this would make it so we wouldn't need to refactor a bunch of test cases, but it seems like that's unavoidable since networkRecordsToDevtoolsLog doesn't currently add any Page.frameNavigated events.
|
Piping See below |
lighthouse-core/test/test-utils.js
Outdated
| * @param {LH.DevtoolsLog} devtoolsLog | ||
| * @return {LH.Artifacts['URL']} | ||
| */ | ||
| function getURLFromDevtoolsLog(devtoolsLog) { |
There was a problem hiding this comment.
nit: getURLArtifactFromDevtoolsLog ?
So pubads requires a backport that makes |
Removes to "first document request" path from
NetworkAnalyzer.findMainDocument. The document url is now a required parameter to find the main document. This improves our handling of JS redirects.I don't think anything here would be a breaking change,and it's only tangentially related to #13706Related
#8984
#13706