[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>
Tue, 14 Apr 2015 13:52:59 +0000 (13:52 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Apr 2015 13:52:59 +0000 (13:52 +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 release 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.

Covered by reference tests that are now passing.

* Modules/streams/ReadableStream.cpp:
(WebCore::ReadableStream::changeStateToClosed): Called by the JS function 'close'.
* Modules/streams/ReadableStream.h:
* Modules/streams/ReadableStreamReader.cpp:
(WebCore::ReadableStreamReader::ReadableStreamReader):
(WebCore::ReadableStreamReader::initialize): Added to handle state change at constructor time (in particular closed/errored state).
(WebCore::ReadableStreamReader::releaseStream):
(WebCore::ReadableStreamReader::closed): Storing the closed promise callbacks.
(WebCore::ReadableStreamReader::changeStateToClosed): Resolution of closed promise.
* Modules/streams/ReadableStreamReader.h:
* bindings/js/JSDOMPromise.h:
(WebCore::DeferredWrapper::resolve<JSC::JSValue>):
* bindings/js/JSReadableStreamReaderCustom.cpp:
(WebCore::JSReadableStreamReader::closed):
* bindings/js/ReadableStreamJSSource.cpp:
(WebCore::readableStreamSlotName):
(WebCore::getReadableJSStream): Helper function to retrieve the stream from the exec state.
(WebCore::closeReadableStreamFunction):
(WebCore::createReadableStreamCloseFunction):
(WebCore::createReadableStreamController):
(WebCore::ReadableStreamJSSource::start):
(WebCore::ReadableJSStreamReader::ReadableJSStreamReader): Deleted.

LayoutTests:

Rebased tests as some are now passing.

* 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@182794 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
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/JSReadableStreamReaderCustom.cpp
Source/WebCore/bindings/js/ReadableStreamJSSource.cpp

index 1d3c3f54bc267d4bcfe6489b479e41bb5e157904..5f6cb31e702a4420db21610d855fd4a24cb9c865 100644 (file)
@@ -1,3 +1,15 @@
+2015-04-14  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.
+
+        Rebased tests as some are now passing.
+
+        * streams/reference-implementation/readable-stream-reader-expected.txt:
+        * streams/reference-implementation/readable-stream-templated-expected.txt:
+
 2015-04-14  Marcos Chavarría Teijeiro  <chavarria1991@gmail.com>
 
         [GTK] Gardening 14th April
index c3e5142d469951e9d3a62b65cfb434ab175e3543..4c952c83b78cb7d191ab7eb60888b4fbc15295b0 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 Multiple readers can access the stream in sequence read is not implemented
 FAIL Cannot use an already-released reader to unlock a stream again releaseLock is not implemented
index d74271063df84b8f461f41744a805299ecaa5feb..a4e247891f27ca2c4643c9645d1c7c82b798d28d 100644 (file)
@@ -15,10 +15,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 
 FAIL cancel() should return a distinct fulfilled promise each time cancel is not implemented
 PASS Running templatedRSClosed with ReadableStream (closed via cancel) 
 FAIL cancel() should return a distinct fulfilled promise each time cancel is not implemented
index 6da7353a3136612973fe3775d31a8c1e1d44e1d5..6a76c7488ccc8211c9b2a72b7517f7432f11b5c0 100644 (file)
@@ -1,3 +1,43 @@
+2015-04-14  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 release 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.
+
+        Covered by reference tests that are now passing.
+
+        * Modules/streams/ReadableStream.cpp:
+        (WebCore::ReadableStream::changeStateToClosed): Called by the JS function 'close'.
+        * Modules/streams/ReadableStream.h:
+        * Modules/streams/ReadableStreamReader.cpp:
+        (WebCore::ReadableStreamReader::ReadableStreamReader):
+        (WebCore::ReadableStreamReader::initialize): Added to handle state change at constructor time (in particular closed/errored state).
+        (WebCore::ReadableStreamReader::releaseStream):
+        (WebCore::ReadableStreamReader::closed): Storing the closed promise callbacks.
+        (WebCore::ReadableStreamReader::changeStateToClosed): Resolution of closed promise.
+        * Modules/streams/ReadableStreamReader.h:
+        * bindings/js/JSDOMPromise.h:
+        (WebCore::DeferredWrapper::resolve<JSC::JSValue>):
+        * bindings/js/JSReadableStreamReaderCustom.cpp:
+        (WebCore::JSReadableStreamReader::closed):
+        * bindings/js/ReadableStreamJSSource.cpp:
+        (WebCore::readableStreamSlotName):
+        (WebCore::getReadableJSStream): Helper function to retrieve the stream from the exec state.
+        (WebCore::closeReadableStreamFunction):
+        (WebCore::createReadableStreamCloseFunction):
+        (WebCore::createReadableStreamController):
+        (WebCore::ReadableStreamJSSource::start):
+        (WebCore::ReadableJSStreamReader::ReadableJSStreamReader): Deleted.
+
 2015-04-13  Joonghun Park  <jh718.park@samsung.com>
 
         Use modern for-loops in Document
index 0f5fe0301a6615e649e5966dba993c144e9ba230..21be6bd93fc6a451f2841f9bec8bba27e06501ec 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 4e9269318491cbbbb0e28c1eabc9b79bd61d88d4..5b5727bde8bcaf7f5a3bbd287154f50553f68202 100644 (file)
@@ -68,6 +68,7 @@ public:
     State internalState() { return m_state; }
 
     void start();
+    void changeStateToClosed();
 
 protected:
     ReadableStream(ScriptExecutionContext&, Ref<ReadableStreamSource>&&);
index cd6f942d276e8fadc37c2698a551c5b04adf3385..3c45b1a612a796334acf78d1b00261e67237233b 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 f6014b3951ac320208a402b028bf015ed59e3c18..d547d2ccb0582f529a55adbd33e0ecca5acf9f96 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 d956812c43e231ce6a508f549180bfd0f295ed4d..592eb0c7a3f3b87152899bb2fd4fd151afbaa65e 100644 (file)
@@ -97,6 +97,13 @@ inline void DeferredWrapper::resolve<bool>(const bool& result)
     resolve(exec, JSC::jsBoolean(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)
 {
index f06cb022b4431875df0129a09df31b9e08f9916b..e4adda7015907e0ed37b303e8e2c154f74808745 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.
index 6f9d7ba79fe419c5e522d4af3ad977b83f5d9110..b83234bf3120a8983feea6158d4592a093fc0af1 100644 (file)
@@ -64,6 +64,21 @@ JSValue getInternalSlotFromObject(ExecState* exec, JSValue objectValue, PrivateN
     return propertySlot.getValue(exec, propertyName);
 }
 
+// This slot name is used to store the JSReadableStream in created JS functions (enqueue, close...).
+// This allows retrieving the corresponding JSReadableStream when executing the JS function.
+static JSC::PrivateName& readableStreamSlotName()
+{
+    static NeverDestroyed<JSC::PrivateName> readableStreamSlotName("readableStream");
+    return readableStreamSlotName;
+}
+
+static ReadableJSStream& getReadableJSStream(ExecState* exec)
+{
+    JSReadableStream* jsReadableStream = jsDynamicCast<JSReadableStream*>(getInternalSlotFromObject(exec, exec->callee(), readableStreamSlotName()));
+    ASSERT(jsReadableStream);
+    return static_cast<ReadableJSStream&>(jsReadableStream->impl());
+}
+
 static inline JSValue getPropertyFromObject(ExecState* exec, JSObject* object, const char* identifier)
 {
     return object->get(exec, Identifier::fromString(exec, identifier));
@@ -106,9 +121,17 @@ static inline JSFunction* createReadableStreamEnqueueFunction(ExecState* exec)
     return JSFunction::create(exec->vm(), exec->callee()->globalObject(), 1, String(), notImplementedFunction);
 }
 
-static inline JSFunction* createReadableStreamCloseFunction(ExecState* exec)
+static EncodedJSValue JSC_HOST_CALL closeReadableStreamFunction(ExecState* exec)
+{
+    getReadableJSStream(exec).changeStateToClosed();
+    return JSValue::encode(jsUndefined());
+}
+
+static inline JSFunction* createReadableStreamCloseFunction(ExecState* exec, JSReadableStream* readableStream)
 {
-    return JSFunction::create(exec->vm(), exec->callee()->globalObject(), 0, String(), notImplementedFunction);
+    JSFunction* closeFunction = JSFunction::create(exec->vm(), exec->callee()->globalObject(), 0, String(), closeReadableStreamFunction);
+    setInternalSlotToObject(exec, closeFunction, readableStreamSlotName(), readableStream);
+    return closeFunction;
 }
 
 static inline JSFunction* createReadableStreamErrorFunction(ExecState* exec)
@@ -124,10 +147,10 @@ static void startReadableStreamAsync(ReadableStream& readableStream)
     });
 }
 
-static inline JSObject* createReadableStreamController(JSC::ExecState* exec)
+static inline JSObject* createReadableStreamController(JSC::ExecState* exec, JSReadableStream* readableStream)
 {
     JSFunction* enqueueFunction = createReadableStreamEnqueueFunction(exec);
-    JSFunction* closeFunction = createReadableStreamCloseFunction(exec);
+    JSFunction* closeFunction = createReadableStreamCloseFunction(exec, readableStream);
     JSFunction* errorFunction = createReadableStreamErrorFunction(exec);
 
     JSObject* controller =  JSFinalObject::create(exec->vm(), JSFinalObject::createStructure(exec->vm(), exec->callee()->globalObject(), jsNull(), 3));
@@ -141,7 +164,7 @@ void ReadableStreamJSSource::start(JSC::ExecState* exec, JSReadableStream* reada
 {
     JSLockHolder lock(exec);
 
-    m_controller.set(exec->vm(), createReadableStreamController(exec));
+    m_controller.set(exec->vm(), createReadableStreamController(exec, readableStream));
 
     JSValue startFunction = getPropertyFromObject(exec, m_source.get(), "start");
     if (!startFunction.isFunction()) {