Create a new ReadableStream in FetchEvent.respondWith #934
Conversation
|
I'm sorry that I'm not accustomed to edit this spec. What should I do for service_worker_1/index.bs? |
|
TODO: Define concept-read-chunk-from-reader and concept-cancel-stream-reader in the fetch spec. |
|
Thanks for working on this PR.
Don't worry about V1. Please just work on Nightly. If a backport is needed for V1, I'll cover it. |
| @@ -1784,14 +1790,22 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/ | |||
| <li>Let <var>potentialResponse</var> be a copy of <var>response</var>'s associated <a href="https://fetch.spec.whatwg.org/#concept-response-response">response</a>, except for its body.</li> | |||
| <li>If <var>response</var>'s body is non-null, run these substeps: | |||
There was a problem hiding this comment.
While here it might be good to make "body" link to https://fetch.spec.whatwg.org/#concept-response-body
| <li>Let <var>bytes</var> be an empty byte sequence. | ||
| <li>Let <var>end-of-body</var> be false. | ||
| <li>Let <var>done</var> be false. | ||
| <li>Let <var>targetRealm</var> be the <a>relevant Realm</a> of this <code>FetchEvent</code>. |
There was a problem hiding this comment.
This needs to be saved before you go "in parallel". Also the link doesn't seem to be working; not sure why.
There was a problem hiding this comment.
Use {{FetchEvent}} to get autolinking.
|
(I will fix ForeignFetchEvent.respondWith after FetchEvent.respondWith looks good.) |
|
LGTM |
|
Thanks, fixed ForeignFetch.respondWith. |
| @@ -1762,6 +1770,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/ | |||
| <li>Set the <a>stop propagation flag</a> and <a>stop immediate propagation flag</a>.</li> | |||
| <li>Set the <a href="#respond-with-entered-flag">respond-with entered flag</a>.</li> | |||
| <li>Set the <a href="#wait-to-respond-flag">wait to respond flag</a>.</li> | |||
| <li>Let <var>targetRealm</var> be the <a>relevant Realm</a> of this {{FetchEvent}}. | |||
There was a problem hiding this comment.
Can you please use "the context object" instead of "this {{FetchEvent}}" for consistency in this spec.
|
LGTM with the above nits and a rebase: we removed ServiceWorkerContainer.onerror in c240d9d. This patch is missing in this PR. Also, should I wait until your other PR to fetch gets landed? Or just merge this PR first? |
df3f571 to
94c481a
Compare
|
Thanks, I addressed comments, rebased and squashed commits. |
The current description pretends that a ReadableStream can be passed from one realm to another, which is wrong. This change changes the description so that it creates a new ReadableStream and passes Uint8Array objects from the old ReadableStream to the new one. This is discussed at whatwg/fetch#330. This change fixes w3c#850 as well.
94c481a to
6c2b3f1
Compare
This change adds some ReadableStream operations that are used in w3c/ServiceWorker#934.
The current description pretends that a ReadableStream can be passed from one realm to another, which is wrong. This change changes the description so that it creates a new ReadableStream and passes Uint8Array objects from the old ReadableStream to the new one.
This is discussed at whatwg/fetch#330. This change fixes #850 as well.