Permalink
Commits on Feb 17, 2017
  1. Execute strategySize first in write() (#680)

    A strategy size() function can re-entrantly call controller, writer or writable
    methods, leaving the stream in an unexpected state. To ensure predictable
    behaviour, call size() before observing or modifying any state.
    
    Previously, if size() threw an exception, writer.write() would reject with that
    exception. After this change, it rejects with a TypeError instead.
    
    Also update the web-platform-test to include the new
    streams/writable-streams/reentrant-strategy.js tests.
    ricea committed on GitHub Feb 17, 2017
Commits on Feb 15, 2017
  1. Correct bad variable in PromiseInvokeOrNoop

    Closes #682.
    ricea committed with domenic Feb 15, 2017
Commits on Feb 14, 2017
  1. Fix queue total size tracking logic

    Previously, we believed that the spec's inefficient-but-clear method, of adding up all the chunk sizes whenever it wanted to get the total queue size, was equivalent to keeping a running count. This was discussed in #582; note also #491 where we changed the reference implementation to the running-count strategy for speed reasons. As discovered in #582 (comment), and further elaborated on in #657, these two methods are not in fact equivalent, due to the vagaries of floating-point arithmetic. (In particular, a is not always equal to a + b - b.)
    
    As such, this commit switches the spec to the more efficient running-count method, as that's realistically the only implementable version.
    
    This commit also includes a few cleanups in related areas:
    
    - It introduces the ResetQueue abstract operation, to ensure that the queue total size gets reset to zero when the queue is cleared. This should not matter because no code paths check the queue's total size after it has been cleared, but keeping the two slots in sync seems virtuous.
    
    - It causes DequeueValue to ensure that [[queueTotalSize]] never goes negative, despite these issues with floating-point arithmetic.
    
    - It causes the desiredSize getter to always return null for errored streams, and zero for closed streams.
    
    - It updates the internal slot name for ReadableByteStreamController instances from [[totalQueuedBytes]] to [[queueTotalSize]], to be consistent with those for the other queue-containers. We do not yet use the queue-with-sizes abstract operations (except ResetQueue) on ReadableByteStreamController instances as the queue management is significantly more complicated there. But aligning makes the spec easier to read.
    
    Closes #582. Closes #657.
    domenic committed on GitHub Feb 14, 2017
Commits on Feb 13, 2017
  1. Various WritableStream fixes

    - Fixes a slot reference in WritableStreamDefaultWriterWrite step 14.
    - Makes a small simplification in WritableStreamDefaultControllerError, by using the controller parameter instead of doing a slot lookup on stream in step 6.b.
    - Fixes a spec bug in WritableStreamDefaultControllerError: as in the reference implementation, the conditional in step 8 of WritableStreamDefaultControllerError should require both controller.[[writing]] and controller.[[inClose]] to be false instead of either of them. Additionally, the "controller is undefined" check doesn't make sense, as step 7 requires controller to not be undefined already.
    tschneidereit committed with domenic Feb 13, 2017
Commits on Jan 26, 2017
  1. Fix readable byte streams' autoAllocateChunkSize

    It looks like having more than one read request on a byte stream
    with autoAllocateChunkSize has never actually worked, ever since
    it was first added in e601d69.
    
    var rs = new ReadableStream({type:'bytes', autoAllocateChunkSize:1});
    var reader = rs.getReader();
    var p1 = reader.read();
    var p2 = reader.read(); // assertion failure
    isonmad committed with domenic Jan 26, 2017
Commits on Jan 25, 2017
  1. WritableStream abort logic clean up and fix

    This patch factors out the logic to handle fulfillment/rejection of `sink.write()` and `sink.close()` operation into separate methods to make it easier to follow the code and check correctness while fixing a few bugs.
    
    This patch also clarifies the interface boundary, i.e. factoring out operations that are exposed by the WritableStream to controllers.
    
    This PR is recreation of #640.
    
    Principles behind this change:
    
    - `writer.closed` rejection is delayed while there's any pending `writer.write()` or `writer.close()`
      - rejection reason picking order:
        - 1. the error passed to `controller.error()` made during the pending operation if made
        - 2. the rejection reason of the pending operation if rejected
        - 3. a TypeError indicating a `writer.abort()` call was made if made
      - the picked reason will be set to `[[storedError]]`
    - `writer.ready` rejects immediately in response to `controller.error()` and `writer.abort()` with the error passed to the method
    - `writer.write()` calls made after `writer.abort()` rejects with:
      - if it's made before `writer.abort()` call and is staying in `[[writeRequests]]`: `[[storedError]]`
      - if it's made after `writer.abort()` but before `controller.error()` if any: an error indicating a `writer.abort()` call was made
      - if it's made after `controller.error()` if any: the error passed to the `controller.error()`
      - if it's made after the pending operation finishes: `[[storedError]]` which stores the reason picked by following the steps above
    - `writer.abort()` fulfills if the pending `writer.close()` succeeds, and `writer.closed` rejects
      - Note: This patch didn't change this behavior but changed the message.
    
    Bug fixes:
    
    - Fixed `sink.close()` rejection handling logic to reject `writer.closed` even if the stream has been `writer.abort()`-ed or `controller.error()`-ed #656
      - Before this fix, `WritableStreamDefaultControllerErrorIfNeeded()` was used which is no-op if `[[state]]` has already been set to `"errored"`
    - Fixed `writer.releaseLock()` to reject `writer.closed` when there's a pending write or close since closed rejection is delayed. Just checking `[[pendingAbortRequest]]` is not enough.
    
    Other visible changes:
    
    - Now `writer.abort()` doesn't set the `[[state]]` of the stream to `"errored"` when there's pending `sink.write()` or `sink.close()` but prevents further operations on writer by setting `[[pendingAbortRequest]]` to non-**undefined** value.
      - Note that `writer.ready` still rejects immediately
      - Now `controller.error()` succeeds even after `writer.abort()`.
        - So, `writer.abort()` and `writer.closed` reject with the error passed to `controller.error()`
        - Note that `writer.ready` is kept rejected with an error indicating abort even after `controller.error()`.
    - Now `sink.close()` rejection handling logic doesn't change the rejection reason of `writer.ready` if it's already rejected by `writer.abort()` or `controller.error()`
      - Question to be discussed: Is this ok? When a writer is re-obtained, the ready promise is set to be rejected with the latest error.
    - Now `sink.write()` rejection handling logic doesn't change the `[[storedError]]` to the rejection reason of `sink.write()` if it's already `controller.error()`-ed
      - Same about `writer.ready`
    
    Fulfillment/rejection order changes:
    
    - Rejection order on `sink.write()` completion:
      - Before: write(), closed, abort()
      - After: write(), abort(), closed
    - Rejection order on `sink.write()` rejection:
      - Before: write(), closed, abort(), ready
      - After: write(), abort(), ready, closed
    - `controller.error()` rejects:
      - Before: closed then ready
      - After: ready then closed
    - `writer.abort()` rejects
      - Before: closed, ready
      - After: ready, closed
    
    Refactoring:
    
    - Refactored `writer.abort()` finalization steps to clarify where a certain process is scheduled to run
    - Factored out `[[pendingWriteRequest]]` setting logic as a WritableStream abstract operation exposed to controllers
    - Removed redundant `[[queue]]` clearing from `sink.write()` rejection handling logic
    - Factored out:
      - `WritableStreamDefaultControllerUpdateBackpressureIfNeeded()`
      - `WritableStreamRejectClosedPromiseIfAny()`
      - `WritableStreamDefaultWriterEnsureReadyPromiseRejectedWith()`
    - Call what's needed directly in `sink.write()` rejection than using `WritableStreamDefaultControllerErrorIfNeeded()`
    - Don't clear `[[queue]]` on `sink.close()` rejection. It's guaranteed to be empty as we're processing closure.
    - Finish calculating conditions at the top of methods to avoid thinking about where a certain state has been updated or not yet (e.g. `[[error]]`)
    - Have redundant call to promise rejection helper method if needed to keep rejection order consistent
    
    Closes #639, #656
    
    This patch also includes some additional work as follows:
    
    - Update worker excluding pattern in test runner
      - w3c/web-platform-tests#4589 has changed the naming rule of streams test files.
      - Update the condition in run-web-platform-tests.js to exclude worker tests to follow the new rule.
    - Sync wpt submodule to the current HEAD to run the updated tests
    - Fix bikeshed option: Inline Github Issues
      - bikeshed API has been updated to reject true for the Inline Github Issues option.
      - To pass the "push" check on Travis CI, we need to fix this to make deploy.sh run successfully
      - Tentatively set it to one of the new valid values, "title".
    tyoshino committed Jan 6, 2017
Commits on Jan 23, 2017
  1. Link "transforming _p_ with a fulfillment handler" (#665)

    Replace "transforming * by a fulfillment handler" with "transforming *
    with a fulfullment handler" and link the word "transforming" to
    https://www.w3.org/2001/tag/doc/promises-guide/#transforming-by to make
    the exact steps explicit.
    
    Closes #622.
    ricea committed on GitHub Jan 23, 2017
  2. Minor fixes to TransformStream w-p-ts (#666)

    Rename the "transform-stream" test directory to "transform-streams" for
    consistency with "writable-streams" and "readable-streams".
    
    Also add the missing "done();" to the end of errors.js so that it can
    work correctly in Workers.
    ricea committed on GitHub Jan 23, 2017
Commits on Jan 20, 2017
  1. Mark pipeTo promise created by pipeThrough as handled

    Closes #652.
    domenic committed on GitHub Jan 20, 2017
Commits on Jan 19, 2017
  1. Update web-platform-test running infrastructure

    Previously it would sync the tests before running them, which overwrote any local changes. Instead this separates the syncing process out into a separate command.
    domenic committed Jan 19, 2017
  2. Move to the upstream versions of WPTs

    In w3c/web-platform-tests@55e9a48 I upstreamed our current test corpus, minus transform streams, to the web-platform-tests repository. This PR deletes those upstreamed tests from our tree, and moves to using a Git submodule for pulling them in, to better control our ability to track the version we work with.
    domenic committed on GitHub Jan 19, 2017
Commits on Jan 18, 2017
Commits on Jan 17, 2017
  1. Meta: enable running the HTML checker on the spec

    This ensures that the resulting spec is conformant HTML.
    annevk committed with domenic Jan 17, 2017
Commits on Jan 14, 2017
  1. Meta: use dfn.js instead of Bikeshed's definition panels

    This is for consistency with the rest of the WHATWG spec ecosystem. We may consider moving back in the future, but we should do so in concert with the rest. See whatwg/meta#2.
    domenic committed Jan 14, 2017
Commits on Jan 13, 2017
  1. add assert to byte stream controller [[Pull]]() (#649)

    It isn't immediately obvious, but the only caller of [[Pull]]()
    is ReadableStreamDefaultReaderRead(), and it depends on that.
    isonmad committed with ricea Jan 13, 2017
Commits on Jan 12, 2017
  1. Meta: add a service worker to the spec

    This allows the spec to be viewed offline.
    
    We use a cache-then-network strategy for all subresources, but a network-then-cache strategy for the main document, since it is imperative you see the latest version of living standards. In the future we may explore more complicated ways of speeding up the main resource fetch, e.g. #637 (comment).
    domenic committed on GitHub Jan 12, 2017
  2. Correctly handle releaseLock() during close() (#645)

    Code which calls writer.releaseLock() while writer.close() is in progress can cause an exception in WritableStreamFinishClose because stream.[[writer]] is undefined.
    
    Existing tests did not find the issue because it is quite hard to detect.
    
    Add undefined checks to fix this. Also add tests to verify the correct behavior.
    ricea committed with domenic Jan 12, 2017
Commits on Jan 11, 2017
  1. Fix emptyView construction

    It should be constructed from the data stored in pullIntoDescriptor, instead of from nonexistant slots on the view TypedArray.
    tschneidereit committed with domenic Jan 10, 2017
  2. Fix ReadableByteStreamControllerRespondWithNewView comparison

    It should be comparing firstDescriptor.[[byteLength]] with view.[[ByteLength]] instead of with view.[[ByteOffset]].
    tschneidereit committed with domenic Jan 7, 2017
Commits on Jan 10, 2017
  1. Include server-worker include in pipe-through test (#642)

    pipe-through.https.html uses worker_test() and therefore needs the /service-workers/service-worker/resources/test-helpers.sub.js script included. Add it.
    ricea committed on GitHub Jan 10, 2017
Commits on Jan 6, 2017
  1. Remove fallback from underlying sink abort to close

    See discussion in #620 and in particular the conclusion at #620 (comment).
    domenic committed on GitHub Jan 6, 2017
  2. Move controller type check to ReadableStreamBYOBReader constructor

    Previously the check was in ReadableStream's getReader, which erroneously allowed construction of ReadableStreamBYOBReader instances given non-ReadableStreams. This also adds tests for other aspects of the ReadableStreamBYOBReader constructor.
    tschneidereit committed with domenic Jan 6, 2017
  3. When underlyingSink.close() fulfills, assert state

    Previously, WritableStreamDefaultControllerProcessClose checked the state was "closing" or "errored" when underlyingSink.close() fulfills. In fact it can't be anything else, so make it an assert instead.
    ricea committed with domenic Jan 6, 2017
Commits on Jan 5, 2017
Commits on Dec 21, 2016
  1. Don't invoke [[strategySize]] as method

    The spec text says to Call it with *this* as undefined. That wasn't what the reference implementation was doing.
    isonmad committed with domenic Dec 21, 2016
  2. Editorial: a variety of writable stream tidy-ups

    - Alphabetize internal slots in their tables
    - Initialize all internal slots in the constructors
    - Fix a few instances of alias variables not being used, or not being created
    - Remove now-unused abstract operation WritableStreamFulfillWriteRequest
    - Modify a test for colliding abort errors and underlying sink write() errors for more coverage
    domenic committed Dec 20, 2016
Commits on Dec 16, 2016
  1. Editorial: fix a few typos

    domenic committed Dec 16, 2016
  2. Fix missing comma in example

    tkrotoff committed with domenic Dec 16, 2016
  3. Allow running web platform tests with glob filters

    Closes #625 (this is an alternate implementation).
    domenic committed Dec 16, 2016
Commits on Dec 8, 2016
  1. Reject the correct writer.closed promise on errored release

    Prior to #619, abort() would reject the closed promise immediately. Now it waits for queued sink operations to finish. This means that there can be a window when the stream is errored but the closed promise has not been rejected. If releaseLock() was called during the window it would incorrectly create a new closed promise on the assumption it was already rejected.
    
    Instead, when an abort() is pending, reject the promise rather than creating a new one.
    ricea committed with domenic Dec 8, 2016
Commits on Dec 5, 2016
  1. Editorial: remove ZERO WIDTH JOINER characters

    The standard text contains several ZERO WIDTH JOINER characters (U+200D, https://en.wikipedia.org/wiki/Zero-width_joiner). These don't appear to have any functional purpose. Remove them.
    ricea committed with domenic Dec 5, 2016
  2. Make write() and close() non-interruptible (#619)

    The underlying abort() method will now not be called until any pending
    underlying write() has finished. If an underlying close() is in progress
    then abort() will no longer be called at all.
    
    Other behavioural changes:
    
    * If the underlying operation has started, then the writer's write() and
    close() methods will always reflect the result of the underlying operation,
    rather than being rejected because abort() was called.
    
    * Consistently with the above, if an underlying operation calls
    controller.error() then the writer method will still reflect the result from
    the underlying operation.
    
    * If a call to writer.abort() has to wait for an underlying write() or
    close() to complete, and that underlying operation rejects, then the
    underlying abort() will not be called, and writer.abort() will return the
    rejection from the failed operation.
    ricea committed on GitHub Dec 5, 2016
Commits on Nov 28, 2016
  1. Do not assert() when writer is detached during close() (#621)

    When writer.releaseLock() was called during us.close(), an assert would
    fire in WritableStreamFinishClose() as it expected the writer to still
    be attached.
    
    Since detaching the writer during close() is permitted, remove the
    assert.
    
    Also add tests that this works correctly.
    ricea committed on GitHub Nov 28, 2016