w3c / ServiceWorker

Service Workers
https://w3c.github.io/ServiceWorker/
Other
3.63k stars 313 forks source link

respondWith is not implementable as written #1168

Open bzbarsky opened 7 years ago

bzbarsky commented 7 years ago

https://w3c.github.io/ServiceWorker/v1/#fetch-event-respondwith step 10.3.1.2.5.1 calls https://fetch.spec.whatwg.org/#concept-get-reader which calls https://streams.spec.whatwg.org/#acquire-readable-stream-reader

This last expects to be called in the usual ES way, with a current Realm and whatnot. But the above-cited step is running "in parallel", and hence has no current Realm.

bzbarsky commented 7 years ago

@domenic @wanderview @bakulf

wanderview commented 7 years ago

I believe this is an issue with the ReadableStream integration into fetch and SW specs. @yutakahirano and @jungkees might be able to help?

At least in gecko we are planning to do something like this:

  1. If stream has a native inner source, then just consume that in native code directly.
  2. If stream has a js inner source, then pump the ReadableStream into a buffer on the js thread, possibly applying back pressure if the buffer fills up. The native code then reads from this buffer as one of its native streams.

I don't know if this helps develop safe spec language or not. Not sure what blink does here.

Edit: Note, implicit in my description above is that a "native stream" or "native inner source" can be read by the native code on any thread. The js thread no longer really matters at that point.

wanderview commented 7 years ago

Here is some pseudo code of what I am talking about in https://github.com/w3c/ServiceWorker/issues/1168#issuecomment-317031289. This is from our gecko bug where I'm trying to explain how we can pump the ReadableStream into our native pipe buffering stream. Its a two stage loops to handle blocking on either ReadableStream or the buffer being full.

1) The "outer" loop is doing:

  function outer() {
    // read next chunk
    return stream.reader().read().then(chunk => {
      // wait until we have pushed it into the pipe
      return inner(chunk);
    })
    // repeat
    .then(outer);
  }

2) The "inner" loop is doing:

  function inner(chunk) {
    return new Promise(resolve => {
      let remaining = chunk.length;
      let offset = 0;

      function writeSegment() {
        let written = pipeWriter.write(chunk + offset, remaining);
        remaining -= written;
        offset += written;

        // done
        if(remaining < 1) {
          resolve();
          return;
        }

        // wait for more room, then loop
        pipeWriter.asyncWait(writeSegment);
      }
    });
  };
yutakahirano commented 7 years ago

Sorry I assumed that we could use ES objects in "in parallel" blocks. Does that mean we cannot use promises in an "in parallel" block, too?

wanderview commented 7 years ago

Does that mean we cannot use promises in an "in parallel" block, too?

I bet there are a lot of places in the specs that "resolve the promise" in a parallel block. To be technically correct these should probably queue a task on the target realm to resolve the promise. I think.

bzbarsky commented 7 years ago

https://www.w3.org/2001/tag/doc/promises-guide#shorthand-manipulating defines "resolve a promise" as queuing a task to do so, when executed "in parallel". So using that particular shorthand in parallel is ok.