-
Notifications
You must be signed in to change notification settings - Fork 351
processResponseDone should receive a response #1202
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
cdd8cae
processResponseDone should receive a response
noamr 8088493
Nits
noamr 8e6950b
Assign fetch timing to response also for CORS failure
noamr a7399bf
Use real export from resource timing
noamr d128656
Remove CORS part
noamr 9f994a8
Send cache-state to resource timing
noamr 7a1c008
Use ref for now
noamr 04d3a9b
Fix PR nits
noamr 638862d
More PR nits
noamr ab08fc0
Undo unwanted responseObject changes
noamr 9b79ae7
More nits
noamr 2f668b6
Amend
noamr cef771d
nits
noamr e61e0a8
Update note
noamr 51dd0e3
Use real ref for mark
noamr 764e995
Fix line length
noamr b455c91
nits
annevk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1932,7 +1932,8 @@ message as HTTP/2 does not support them. | |
<dfn export for=response id=concept-response-cache-state>cache state</dfn> (the empty string or | ||
"<code>local</code>"). Unlesss stated otherwise, it is the empty string. | ||
|
||
<p class=note>This is intended solely for usage by service workers. [[SW]] | ||
<p class=note>This is intended for usage by <cite>Service Workers</cite> and | ||
<cite>Resource Timing</cite>. [[SW]] [[RESOURCE-TIMING]] | ||
<!-- If we ever expand the utility of this we need to carefully consider whether filtered responses | ||
need to mask it, whether the cache API needs to store it, etc. --> | ||
|
||
|
@@ -3512,7 +3513,7 @@ representing the number of bytes transmitted. If given, <var>processRequestEndOf | |
an algorithm accepting no arguments. If given, <var>processResponse</var> must be an algorithm | ||
accepting a <a for=/>response</a>. If given, <var>processResponseEndOfBody</var> must be an | ||
algorithm accepting a <a for=/>response</a> and null, failure, or a <a for=/>byte sequence</a>. If | ||
given, <var>processResponseDone</var> must be an algorithm accepting no arguments. | ||
given, <var>processResponseDone</var> must be an algorithm accepting a <a for=/>response</a>. | ||
|
||
<p>An ongoing <a for=/>fetch</a> can be | ||
<dfn export for=fetch id=concept-fetch-terminate>terminated</dfn> with flag <var>aborted</var>, | ||
|
@@ -3986,9 +3987,9 @@ steps: | |
<a for=request>done flag</a>. | ||
|
||
<li><p>If <var>fetchParams</var>'s <a for="fetch params">process response done</a> is not null, | ||
then <a>queue a fetch task</a> given <var>fetchParams</var>'s | ||
<a for="fetch params">process response done</a> and <var>fetchParams</var>'s | ||
<a for="fetch params">task destination</a>. | ||
then <a>queue a fetch task</a> to run <var>fetchParams</var>'s | ||
<a for="fetch params">process response done</a> given <var>response</var>, | ||
with <var>fetchParams</var>'s <a for="fetch params">task destination</a>. | ||
</ol> | ||
|
||
<p>To <dfn export>finalize and report timing</dfn> given a <a for=/>response</a> | ||
|
@@ -4003,13 +4004,21 @@ steps: | |
|
||
<li><p>Let <var>timingInfo</var> be <var>response</var>'s <a for=response>timing info</a>. | ||
|
||
<li><p>Let <var>cacheState</var> be <var>response</var>'s <a for=response>cache state</a>. | ||
|
||
<li><p>If <var>timingInfo</var> is null, then return. | ||
|
||
<li><p>If <var>response</var>'s <a for=response>timing allow passed flag</a> is not set, then set | ||
<var>timingInfo</var> to a new <a for=/>fetch timing info</a> whose | ||
<a for="fetch timing info">start time</a> and | ||
<a for="fetch timing info">post-redirect start time</a> are <var>timingInfo</var>'s | ||
<a for="fetch timing info">start time</a>. | ||
<li> | ||
<p>If <var>response</var>'s <a for=response>timing allow passed flag</a> is not set, then: | ||
|
||
<ol> | ||
annevk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<li><p>Set <var>timingInfo</var> to a new <a for=/>fetch timing info</a> whose | ||
<a for="fetch timing info">start time</a> and | ||
<a for="fetch timing info">post-redirect start time</a> are <var>timingInfo</var>'s | ||
<a for="fetch timing info">start time</a>. | ||
|
||
<li><p>Set <var>cacheState</var> to the empty string. | ||
</ol> | ||
|
||
<li><p>Set <var>timingInfo</var>'s <a for="fetch timing info">end time</a> to the | ||
<a for=/>coarsened shared current time</a> given <var>global</var>'s | ||
|
@@ -4018,9 +4027,8 @@ steps: | |
|
||
<li><p>Set <var>response</var>'s <a for="response">timing info</a> to <var>timingInfo</var>. | ||
|
||
<li><p><a href="https://github.com/w3c/resource-timing/pull/261">Mark resource timing</a> for | ||
<var>timingInfo</var>, <var>originalURL</var>, <var>initiatorType</var>, and <var>global</var>. | ||
<!-- TODO --> | ||
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. Let's restore the TODO. 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. Done |
||
<li><p><a for=/>Mark resource timing</a> for <var>timingInfo</var>, <var>originalURL</var>, | ||
<var>initiatorType</var>, <var>global</var>, and <var>cacheState</var>. | ||
</ol> | ||
|
||
|
||
|
@@ -7269,8 +7277,9 @@ method steps are: | |
<li><p><a lt=terminated for=fetch>Terminate</a> the ongoing fetch with the aborted flag set. | ||
</ol> | ||
|
||
<li><p>Let <var>handleFetchDone</var> be to <a>finalize and report timing</a> with | ||
<var>response</var>, <var>globalObject</var>, and "<code>fetch</code>". | ||
<li><p>Let <var>handleFetchDone</var> given <a for=/>response</a> <var>response</var> be to | ||
<a>finalize and report timing</a> with <var>response</var>, <var>globalObject</var>, and | ||
"<code>fetch</code>". | ||
annevk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
<li> | ||
<p><a for=/>Fetch</a> <var>request</var> with <a for=fetch><i>processResponseDone</i></a> set to | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think we should keep this including the comment and instead say something like
I guess for Resource Timing we also need opaque responses (that have TAO) to leak this information. I'll have a look if that needs more work.
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.
Done