[Streams API] ReadableStream constructor start function should be able to close the...
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Apr 2015 13:31:59 +0000 (13:31 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Apr 2015 13:31:59 +0000 (13:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143363

Reviewed by Benjamin Poulain.

Source/WebCore:

Implements https://streams.spec.whatwg.org/#close-readable-stream.
When the "close" JS function is called, the stream is getting closed.
The stream state is changed to close and if it has a reader, the reader gets closed as well:
The reader resolves the closed promise and releases the stream.

Enabled the possibility to resolve a promise with any JS value.
This is used to resolve closed promise with jsUndefined and will be used for read promises in
the future as well, though of course it is not restricted to Streams.

Covered by reference tests that are now passing.

* Modules/streams/ReadableStream.h:
* Modules/streams/ReadableStream.cpp:
(WebCore::ReadableStream::changeStateToClosed): Called by the JS function 'close'.
* Modules/streams/ReadableStreamReader.cpp:
(WebCore::ReadableStreamReader::ReadableStreamReader): Initialized stream.
(WebCore::ReadableStreamReader::initialize): Added to handle state change at constructor time
in particular closed/errored state.
(WebCore::ReadableStreamReader::releaseStream): Releases the stream.
(WebCore::ReadableStreamReader::closed): Stores the promise callbacks and invokes success
immediately if the stream is already closed.
(WebCore::ReadableStreamReader::changeStateToClosed): Changes the internal state to closed,
resolves the promise and releases the stream.
* Modules/streams/ReadableStreamReader.h:
(WebCore::ReadableStreamReader::State): Added.
* bindings/js/JSDOMPromise.h:
(WebCore::DeferredWrapper::resolve<JSC::JSValue>): Adds the ability to resolve a promise with
a custom JS value.
* bindings/js/JSReadableStreamControllerCustom.cpp:
(WebCore::JSReadableStreamController::close): Not "notImplemented" anymore. Now it closes the
stream.
* bindings/js/JSReadableStreamReaderCustom.cpp:
(WebCore::JSReadableStreamReader::closed): Resolves the promise with undefined.

LayoutTests:

Updated expectations with new passes.

* streams/reference-implementation/bad-underlying-sources-expected.txt:
* streams/reference-implementation/readable-stream-reader-expected.txt:
* streams/reference-implementation/readable-stream-templated-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183395 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/streams/reference-implementation/bad-underlying-sources-expected.txt
LayoutTests/streams/reference-implementation/readable-stream-reader-expected.txt
LayoutTests/streams/reference-implementation/readable-stream-templated-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableStream.cpp
Source/WebCore/Modules/streams/ReadableStream.h
Source/WebCore/Modules/streams/ReadableStreamReader.cpp
Source/WebCore/Modules/streams/ReadableStreamReader.h
Source/WebCore/bindings/js/JSDOMPromise.h
Source/WebCore/bindings/js/JSReadableStreamControllerCustom.cpp
Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp

index c347cb0..97d5e13 100644 (file)
@@ -1,3 +1,16 @@
+2015-04-27  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [Streams API] ReadableStream constructor start function should be able to close the stream
+        https://bugs.webkit.org/show_bug.cgi?id=143363
+
+        Reviewed by Benjamin Poulain.
+
+        Updated expectations with new passes.
+
+        * streams/reference-implementation/bad-underlying-sources-expected.txt:
+        * streams/reference-implementation/readable-stream-reader-expected.txt:
+        * streams/reference-implementation/readable-stream-templated-expected.txt:
+
 2015-04-27  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
         Synchronous XMLHttpRequest should get access to AppCache resources stored as flat files
index e32a734..13069a9 100644 (file)
@@ -17,11 +17,11 @@ FAIL Underlying source: throwing strategy.shouldApplyBackpressure method assert_
 FAIL Underlying source: strategy.size returning NaN assert_unreached: enqueue didn't throw Reached unreachable code
 FAIL Underlying source: strategy.size returning -Infinity assert_unreached: enqueue didn't throw Reached unreachable code
 FAIL Underlying source: strategy.size returning +Infinity assert_unreached: enqueue didn't throw Reached unreachable code
-TIMEOUT Underlying source: calling close twice on an empty stream should throw the second time Test timed out
+PASS Underlying source: calling close twice on an empty stream should throw the second time 
 FAIL Underlying source: calling close twice on a non-empty stream should throw the second time read is not implemented
 FAIL Underlying source: calling close on an empty canceled stream should not throw cancel is not implemented
 FAIL Underlying source: calling close on a non-empty canceled stream should not throw cancel is not implemented
 TIMEOUT Underlying source: calling close after error should throw Test timed out
 TIMEOUT Underlying source: calling error twice should throw the second time Test timed out
-TIMEOUT Underlying source: calling error after close should throw Test timed out
+PASS Underlying source: calling error after close should throw 
 
index 291b447..759f13c 100644 (file)
@@ -11,7 +11,7 @@ PASS Constructing a ReadableStreamReader directly should be OK if the stream is
 PASS Constructing a ReadableStreamReader directly should be OK if the stream is errored 
 FAIL Reading from a reader for an empty stream will wait until a chunk is available read is not implemented
 FAIL cancel() on a reader releases the reader before calling through cancel is not implemented
-TIMEOUT closed should be fulfilled after stream is closed (.closed access before acquiring) Test timed out
+PASS closed should be fulfilled after stream is closed (.closed access before acquiring) 
 FAIL closed should be fulfilled after reader releases its lock (multiple stream locks) releaseLock is not implemented
 FAIL Cannot use an already-released reader to unlock a stream again releaseLock is not implemented
 FAIL Getting a second reader after erroring the stream should succeed read is not implemented
index 9212dee..a71698d 100644 (file)
@@ -12,10 +12,10 @@ FAIL canceling via the stream should fail cancel is not implemented
 PASS Running templatedRSClosed with ReadableStream (closed via call in start) 
 FAIL cancel() should return a distinct fulfilled promise each time cancel is not implemented
 PASS getReader() should be OK 
-FAIL should be able to acquire multiple readers, since they are all auto-released ReadableStream is locked
+PASS should be able to acquire multiple readers, since they are all auto-released 
 PASS Running templatedRSClosedReader with ReadableStream (closed via call in start) reader 
 FAIL read() should fulfill with { value: undefined, done: true } read is not implemented
-TIMEOUT closed should fulfill with undefined Test timed out
+PASS closed should fulfill with undefined 
 PASS Running templatedRSClosed with ReadableStream (closed via cancel) 
 FAIL cancel() should return a distinct fulfilled promise each time cancel is not implemented
 FAIL getReader() should be OK cancel is not implemented
index 2fd21b7..ee88f67 100644 (file)
@@ -1,3 +1,44 @@
+2015-04-27  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [Streams API] ReadableStream constructor start function should be able to close the stream
+        https://bugs.webkit.org/show_bug.cgi?id=143363
+
+        Reviewed by Benjamin Poulain.
+
+        Implements https://streams.spec.whatwg.org/#close-readable-stream.
+        When the "close" JS function is called, the stream is getting closed.
+        The stream state is changed to close and if it has a reader, the reader gets closed as well:
+        The reader resolves the closed promise and releases the stream.
+
+        Enabled the possibility to resolve a promise with any JS value.
+        This is used to resolve closed promise with jsUndefined and will be used for read promises in
+        the future as well, though of course it is not restricted to Streams.
+
+        Covered by reference tests that are now passing.
+
+        * Modules/streams/ReadableStream.h:
+        * Modules/streams/ReadableStream.cpp:
+        (WebCore::ReadableStream::changeStateToClosed): Called by the JS function 'close'.
+        * Modules/streams/ReadableStreamReader.cpp:
+        (WebCore::ReadableStreamReader::ReadableStreamReader): Initialized stream.
+        (WebCore::ReadableStreamReader::initialize): Added to handle state change at constructor time
+        in particular closed/errored state.
+        (WebCore::ReadableStreamReader::releaseStream): Releases the stream.
+        (WebCore::ReadableStreamReader::closed): Stores the promise callbacks and invokes success
+        immediately if the stream is already closed.
+        (WebCore::ReadableStreamReader::changeStateToClosed): Changes the internal state to closed,
+        resolves the promise and releases the stream.
+        * Modules/streams/ReadableStreamReader.h:
+        (WebCore::ReadableStreamReader::State): Added.
+        * bindings/js/JSDOMPromise.h:
+        (WebCore::DeferredWrapper::resolve<JSC::JSValue>): Adds the ability to resolve a promise with
+        a custom JS value.
+        * bindings/js/JSReadableStreamControllerCustom.cpp:
+        (WebCore::JSReadableStreamController::close): Not "notImplemented" anymore. Now it closes the
+        stream.
+        * bindings/js/JSReadableStreamReaderCustom.cpp:
+        (WebCore::JSReadableStreamReader::closed): Resolves the promise with undefined.
+
 2015-04-27  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         Fix the !ENABLE(CSS_GRID_LAYOUT) build after r183370
index 0f5fe03..21be6bd 100644 (file)
@@ -58,6 +58,16 @@ ReadableStream::~ReadableStream()
 #endif
 }
 
+void ReadableStream::changeStateToClosed()
+{
+    if (m_state != State::Readable)
+        return;
+    m_state = State::Closed;
+    if (m_reader)
+        m_reader->changeStateToClosed();
+    ASSERT(!m_reader);
+}
+
 void ReadableStream::start()
 {
     notImplemented();
index 4e92693..5b5727b 100644 (file)
@@ -68,6 +68,7 @@ public:
     State internalState() { return m_state; }
 
     void start();
+    void changeStateToClosed();
 
 protected:
     ReadableStream(ScriptExecutionContext&, Ref<ReadableStreamSource>&&);
index cd6f942..3c45b1a 100644 (file)
@@ -41,14 +41,13 @@ DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, readableStreamReaderCounter
 
 ReadableStreamReader::ReadableStreamReader(ReadableStream& stream)
     : ActiveDOMObject(stream.scriptExecutionContext())
+    , m_stream(&stream)
 {
 #ifndef NDEBUG
     readableStreamReaderCounter.increment();
 #endif
     suspendIfNeeded();
-    ASSERT_WITH_MESSAGE(!stream.reader(), "A ReadableStream cannot be locked by two readers at the same time.");
-    m_stream = &stream;
-    stream.lock(*this);
+    initialize();
 }
 
 ReadableStreamReader::~ReadableStreamReader()
@@ -62,9 +61,44 @@ ReadableStreamReader::~ReadableStreamReader()
     }
 }
 
-void ReadableStreamReader::closed(ClosedSuccessCallback, ClosedErrorCallback)
+void ReadableStreamReader::initialize()
 {
-    notImplemented();    
+    ASSERT_WITH_MESSAGE(!m_stream->isLocked(), "A ReadableStream cannot be locked by two readers at the same time.");
+    m_stream->lock(*this);
+    if (m_stream->internalState() == ReadableStream::State::Closed) {
+        changeStateToClosed();
+        return;
+    }
+}
+
+void ReadableStreamReader::releaseStream()
+{
+    ASSERT(m_stream);
+    m_stream->release();
+    m_stream = nullptr;
+}
+
+void ReadableStreamReader::closed(ClosedSuccessCallback successCallback, ClosedErrorCallback)
+{
+    if (m_state == State::Closed) {
+        successCallback();
+        return;
+    }
+    m_closedSuccessCallback = WTF::move(successCallback);
+}
+
+void ReadableStreamReader::changeStateToClosed()
+{
+    ASSERT(m_state == State::Readable);
+    m_state = State::Closed;
+
+    if (m_closedSuccessCallback) {
+        ClosedSuccessCallback closedSuccessCallback = WTF::move(m_closedSuccessCallback);
+        closedSuccessCallback();
+    }
+    ASSERT(!m_closedSuccessCallback);
+    releaseStream();
+    // FIXME: Implement read promise fulfilling.
 }
 
 const char* ReadableStreamReader::activeDOMObjectName() const
index f6014b3..d547d2c 100644 (file)
@@ -46,6 +46,12 @@ namespace WebCore {
 // See https://streams.spec.whatwg.org/#reader-class for more information.
 class ReadableStreamReader : public ActiveDOMObject, public ScriptWrappable, public RefCounted<ReadableStreamReader> {
 public:
+    enum class State {
+        Readable,
+        Closed,
+        Errored
+    };
+
     virtual ~ReadableStreamReader();
 
     ReadableStream* stream() { return m_stream.get(); }
@@ -54,15 +60,23 @@ public:
     typedef std::function<void()> ClosedErrorCallback;
     void closed(ClosedSuccessCallback, ClosedErrorCallback);
 
+    void changeStateToClosed();
+
 protected:
     ReadableStreamReader(ReadableStream&);
 
+    void releaseStream();
+
 private:
     // ActiveDOMObject API.
     const char* activeDOMObjectName() const override;
     bool canSuspendForPageCache() const override;
+    void initialize();
 
     RefPtr<ReadableStream> m_stream;
+    State m_state { State::Readable };
+
+    ClosedSuccessCallback m_closedSuccessCallback;
 };
 
 }
index d956812..592eb0c 100644 (file)
@@ -98,6 +98,13 @@ inline void DeferredWrapper::resolve<bool>(const bool& result)
 }
 
 template<>
+inline void DeferredWrapper::resolve<JSC::JSValue>(const JSC::JSValue& value)
+{
+    JSC::ExecState* exec = m_globalObject->globalExec();
+    JSC::JSLockHolder locker(exec);
+    resolve(exec, value);
+}
+template<>
 inline void DeferredWrapper::resolve<Vector<unsigned char>>(const Vector<unsigned char>& result)
 {
     JSC::ExecState* exec = m_globalObject->globalExec();
index 15f7a4a..846ac70 100644 (file)
@@ -49,7 +49,7 @@ JSValue JSReadableStreamController::close(ExecState* exec)
     // FIXME: Handle the case of draining.
     if (stream->internalState() != ReadableStream::State::Readable)
         return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Calling close on a stream which is not readable")));
-    notImplemented();
+    stream->changeStateToClosed();
     return jsUndefined();
 }
 
index f06cb02..e4adda7 100644 (file)
@@ -71,9 +71,8 @@ JSValue JSReadableStreamReader::closed(ExecState* exec) const
 {
     JSPromiseDeferred* promiseDeferred = getOrCreatePromiseDeferredFromObject(exec, this, globalObject(), closedPromiseSlotName());
     DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
-    auto successCallback = [this, wrapper]() mutable {
-        // FIXME: return jsUndefined().
-        wrapper.resolve(&impl());
+    auto successCallback = [wrapper]() mutable {
+        wrapper.resolve(jsUndefined());
     };
     auto failureCallback = [this, wrapper]() mutable {
         // FIXME: return stored error.