-
Notifications
You must be signed in to change notification settings - Fork 37
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
Expose transferSize, encodedBodySize, decodedBodySize #266
Conversation
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.
Thanks for pushing this!
index.html
Outdated
<li><p>If <a>this</a>'s <a data-for="PerformanceResourceTiming">cache mode</a> is | ||
"<code>local</code>", then return 0. | ||
<li><p>Return <a>this</a>'s | ||
<a data-for="PerformanceResourceTiming">timing info</a>'s [=fetch timing info/encoded body size=]. |
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.
If we add a fixed size here for the headers, we can close #238 (comment)
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.
Sure, I can do that, what value should I add?
index.html
Outdated
<li><a>Setup the resource timing entry</a> for |entry|, given |initiatorType|, |requestedURL| and | ||
|timingInfo|. | ||
<li><a>Setup the resource timing entry</a> for |entry|, given |initiatorType|, |requestedURL|, | ||
|timingInfo|, and the empty string or "<code>local</code>" |cacheMode|. |
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.
If we're just passing |cacheMode|, I think you can drop the "empty string or local" part
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.
LGTM % comments
index.html
Outdated
<li><a>Setup the resource timing entry</a> for |entry|, given |initiatorType|, |requestedURL| and | ||
|timingInfo|. | ||
<li><a>Setup the resource timing entry</a> for |entry|, given |initiatorType|, |requestedURL|, | ||
|timingInfo|, |cacheMode|. |
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.
s/,/, and/
PR requests addressed. Can merge? |
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.
LGTM
And hand processResponseDone a response. Resource Timing PR: w3c/resource-timing#266. Closes #1201. Follow-up: #1215.
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.
C
transferSize is based on the cache mode of the resource.
Closes #238
Preview | Diff