-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: remove dependency on devtools-frontend NetworkRequest #5451
Changes from 6 commits
3848575
58e7c51
f35c56c
ade9def
05194ea
dd11851
5988b11
1c16232
aecb428
432b989
0324bc0
c5db54d
5836b4c
cee1f0e
4212ab6
7634cf8
21fe6df
1f0ac70
915449f
95551ee
92e8abf
b831cbb
911a268
d11d2e7
a2015af
e5bea12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,7 @@ class UsesRelPreconnectAudit extends Audit { | |
* @return {boolean} | ||
*/ | ||
static hasValidTiming(record) { | ||
return record._timing && record._timing.connectEnd > 0 && record._timing.connectStart > 0; | ||
return !!record._timing && record._timing.connectEnd > 0 && record._timing.connectStart > 0; | ||
} | ||
|
||
/** | ||
|
@@ -48,6 +48,7 @@ class UsesRelPreconnectAudit extends Audit { | |
*/ | ||
static hasAlreadyConnectedToOrigin(record) { | ||
return ( | ||
!!record._timing && | ||
record._timing.dnsEnd - record._timing.dnsStart === 0 && | ||
record._timing.connectEnd - record._timing.connectStart === 0 | ||
); | ||
|
@@ -90,7 +91,7 @@ class UsesRelPreconnectAudit extends Audit { | |
// filter out all resources where timing info was invalid | ||
!UsesRelPreconnectAudit.hasValidTiming(record) || | ||
// filter out all resources that are loaded by the document | ||
record.initiatorRequest() === mainResource || | ||
record._initiator.url === mainResource.url || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these aren't totally equivalent, fwiw. but the difference may not matter. And I suppose devtools was making this assumption previously. :p There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I was kinda shocked that the whole initiatorRequest logic is literally just first one with matching URL and manager 😮 |
||
// filter out urls that do not have an origin (data, ...) | ||
!record.parsedURL || !record.parsedURL.securityOrigin() || | ||
// filter out all resources that have the same origin | ||
|
@@ -118,6 +119,9 @@ class UsesRelPreconnectAudit extends Audit { | |
return (record.startTime < firstRecord.startTime) ? record: firstRecord; | ||
}); | ||
|
||
// Skip the origin if we don't have timing information | ||
if (!firstRecordOfOrigin._timing) return; | ||
|
||
const securityOrigin = firstRecordOfOrigin.parsedURL.securityOrigin(); | ||
|
||
// Approximate the connection time with the duration of TCP (+potentially SSL) handshake | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we just use one of these? this PR is switching to use the other already.
Also happy to fix the
_prop
use in a followup to minimize the diff.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah eliminated
_transferSize
, the dance was necessary for transferSize since the weird way the getter interacts with our simulation but the rest will be followup 👍