[Streams API] ReadableStream constructor start function should be able to error the...
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 May 2015 10:44:06 +0000 (10:44 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 May 2015 10:44:06 +0000 (10:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141162

Reviewed by Darin Adler.

Source/WebCore:

This patch implements the functionality of the ReadableStreamController error function.
It basically changes the state of the stream to errored, resolves the ready promise and rejects the closed promise.
Adding support to reject promise with any JSValue.

Support for storing the error is added to both reader and stream.

Test: streams/readable-stream-controller-error.html and rebased tests

* Modules/streams/ReadableStream.cpp:
(WebCore::ReadableStream::changeStateToErrored):
* Modules/streams/ReadableStream.h:
* Modules/streams/ReadableStreamReader.cpp:
(WebCore::ReadableStreamReader::initialize):
(WebCore::ReadableStreamReader::closed):
(WebCore::ReadableStreamReader::changeStateToClosed):
(WebCore::ReadableStreamReader::changeStateToErrored):
* Modules/streams/ReadableStreamReader.h:
* bindings/js/JSDOMPromise.h:
(WebCore::DeferredWrapper::reject):
* bindings/js/JSReadableStreamControllerCustom.cpp:
(WebCore::JSReadableStreamController::error):
* bindings/js/JSReadableStreamReaderCustom.cpp:
(WebCore::JSReadableStreamReader::closed):
* bindings/js/ReadableJSStream.cpp:
(WebCore::ReadableJSStream::createReader):
(WebCore::ReadableJSStream::storeError):
(WebCore::ReadableJSStream::Reader::storeError):
(WebCore::ReadableJSStream::jsController): Deleted.
* bindings/js/ReadableJSStream.h:

LayoutTests:

* streams/readable-stream-controller-error-expected.txt: Added.
* streams/readable-stream-controller-error.html: Added.
* streams/reference-implementation/bad-underlying-sources-expected.txt:
* streams/reference-implementation/readable-stream-templated-expected.txt:

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/streams/readable-stream-controller-error-expected.txt [new file with mode: 0644]
LayoutTests/streams/readable-stream-controller-error.html [new file with mode: 0644]
LayoutTests/streams/reference-implementation/bad-underlying-sources-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
Source/WebCore/bindings/js/ReadableJSStream.cpp
Source/WebCore/bindings/js/ReadableJSStream.h

index d0ee3599e78cccb911b51e960605bef893b3ad68..8baf310b193976ac3e0ea2af607f3d9f71889485 100644 (file)
@@ -1,3 +1,15 @@
+2015-05-08  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet <youenn.fablet@crf.canon.fr>
+
+        [Streams API] ReadableStream constructor start function should be able to error the stream
+        https://bugs.webkit.org/show_bug.cgi?id=141162
+
+        Reviewed by Darin Adler.
+
+        * streams/readable-stream-controller-error-expected.txt: Added.
+        * streams/readable-stream-controller-error.html: Added.
+        * streams/reference-implementation/bad-underlying-sources-expected.txt:
+        * streams/reference-implementation/readable-stream-templated-expected.txt:
+
 2015-05-08  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r183985.
diff --git a/LayoutTests/streams/readable-stream-controller-error-expected.txt b/LayoutTests/streams/readable-stream-controller-error-expected.txt
new file mode 100644 (file)
index 0000000..f0336e0
--- /dev/null
@@ -0,0 +1,5 @@
+
+PASS Erroring a ReadableStream should reject ReadableStreamReader close promise 
+PASS Erroring a ReadableStream should reject ReadableStreamReader close promise 
+PASS Erroring a ReadableStream without any value 
+
diff --git a/LayoutTests/streams/readable-stream-controller-error.html b/LayoutTests/streams/readable-stream-controller-error.html
new file mode 100644 (file)
index 0000000..0ce7aa5
--- /dev/null
@@ -0,0 +1,69 @@
+<!DOCTYPE html>
+<script src='../resources/testharness.js'></script>
+<script src='../resources/testharnessreport.js'></script>
+<script>
+
+t2 = async_test('Erroring a ReadableStream should reject ReadableStreamReader close promise');
+t2.step(function() {
+    var controller;
+    var rs = new ReadableStream({
+        start: function(c) {
+            controller = c;
+        }
+    });
+
+   rs.getReader().closed.then(t2.step_func(function() {
+        assert_unreached("closed promise should not be resolved when stream is errored");
+    }), t2.step_func(function(err) {
+        assert_equals(rsError, err);
+        t2.done();
+    }));
+
+    var rsError = "my error";
+    controller.error(rsError);
+});
+
+t3 = async_test('Erroring a ReadableStream should reject ReadableStreamReader close promise');
+t3.step(function() {
+    var controller;
+    var rs = new ReadableStream({
+        start: function(c) {
+            controller = c;
+        }
+    });
+
+    var rsError = "my error";
+    controller.error(rsError);
+
+    // Let's call getReader twice to ensure that stream is not locked to a reader.
+    rs.getReader();
+    rs.getReader().closed.then(t3.step_func(function() {
+        assert_unreached("closed promise should not be resolved when stream is errored");
+    }), t3.step_func(function(err) {
+        assert_equals(rsError, err);
+        t3.done();
+    }));
+});
+
+t4 = async_test('Erroring a ReadableStream without any value');
+t4.step(function() {
+    var controller;
+    var rs = new ReadableStream({
+        start: function(c) {
+            controller = c;
+        }
+    });
+
+    rs.getReader().closed.then(t3.step_func(function() {
+        assert_unreached("closed promise should not be resolved when stream is errored");
+    }), t4.step_func(function(err) {
+        // Error generated by JS engine.
+        assert_true(typeof err == 'object');
+        t4.done();
+    }));
+
+    controller.error();
+});
+
+
+</script>
index 13069a9c3977d3786d45859c4eeaf4b34e03610a..19ca35559574e313099fc4882cadd1412a9a2280 100644 (file)
@@ -21,7 +21,7 @@ PASS Underlying source: calling close twice on an empty stream should throw the
 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
+PASS Underlying source: calling close after error should throw 
+PASS Underlying source: calling error twice should throw the second time 
 PASS Underlying source: calling error after close should throw 
 
index a71698d3cc95665d84201b105b3ebfcb6d497e2c..238b0f7fb75082b7b96a83ba4d19d8f4a1d8ef7b 100644 (file)
@@ -28,7 +28,7 @@ FAIL getReader() should return a reader that acts errored read is not implemente
 PASS Running templatedRSErroredSyncOnly with ReadableStream (errored via call in start) 
 FAIL cancel() should return a distinct rejected promise each time cancel is not implemented
 FAIL reader cancel() should return a distinct rejected promise each time cancel is not implemented
-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 templatedRSErrored with ReadableStream (errored via returning a rejected promise in start) 
 FAIL getReader() should return a reader that acts errored read is not implemented
 PASS Running templatedRSErroredReader with ReadableStream (errored via returning a rejected promise in start) reader 
index c0c6a920d5e0dfdd999c5a8413dd110326a82398..83a6d2e92e6d02f06ba673801e70edb503668d62 100644 (file)
@@ -1,3 +1,40 @@
+2015-05-08  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet <youenn.fablet@crf.canon.fr>
+
+        [Streams API] ReadableStream constructor start function should be able to error the stream
+        https://bugs.webkit.org/show_bug.cgi?id=141162
+
+        Reviewed by Darin Adler.
+
+        This patch implements the functionality of the ReadableStreamController error function.
+        It basically changes the state of the stream to errored, resolves the ready promise and rejects the closed promise.
+        Adding support to reject promise with any JSValue.
+
+        Support for storing the error is added to both reader and stream.
+
+        Test: streams/readable-stream-controller-error.html and rebased tests
+
+        * Modules/streams/ReadableStream.cpp:
+        (WebCore::ReadableStream::changeStateToErrored):
+        * Modules/streams/ReadableStream.h:
+        * Modules/streams/ReadableStreamReader.cpp:
+        (WebCore::ReadableStreamReader::initialize):
+        (WebCore::ReadableStreamReader::closed):
+        (WebCore::ReadableStreamReader::changeStateToClosed):
+        (WebCore::ReadableStreamReader::changeStateToErrored):
+        * Modules/streams/ReadableStreamReader.h:
+        * bindings/js/JSDOMPromise.h:
+        (WebCore::DeferredWrapper::reject):
+        * bindings/js/JSReadableStreamControllerCustom.cpp:
+        (WebCore::JSReadableStreamController::error):
+        * bindings/js/JSReadableStreamReaderCustom.cpp:
+        (WebCore::JSReadableStreamReader::closed):
+        * bindings/js/ReadableJSStream.cpp:
+        (WebCore::ReadableJSStream::createReader):
+        (WebCore::ReadableJSStream::storeError):
+        (WebCore::ReadableJSStream::Reader::storeError):
+        (WebCore::ReadableJSStream::jsController): Deleted.
+        * bindings/js/ReadableJSStream.h:
+
 2015-05-08  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r183985.
index 21be6bd93fc6a451f2841f9bec8bba27e06501ec..4d69ee687baf60c8a23cda266a64c2947f0c72da 100644 (file)
@@ -68,6 +68,16 @@ void ReadableStream::changeStateToClosed()
     ASSERT(!m_reader);
 }
 
+void ReadableStream::changeStateToErrored()
+{
+    if (m_state != State::Readable)
+        return;
+    m_state = State::Errored;
+    if (m_reader)
+        m_reader->changeStateToErrored();
+    ASSERT(!m_reader);
+}
+
 void ReadableStream::start()
 {
     notImplemented();
index 2e5888d456a953535280149a8c2ac739d0e36db9..00ac96c81e8d8934fa5d8d2d09bdb0c904507c10 100644 (file)
@@ -69,6 +69,7 @@ public:
 
     void start();
     void changeStateToClosed();
+    void changeStateToErrored();
 
     ReadableStreamSource& source() { return m_source.get(); }
 
index 3c45b1a612a796334acf78d1b00261e67237233b..065f5d7fab8da332561dc98b331dc18831347b04 100644 (file)
@@ -69,6 +69,10 @@ void ReadableStreamReader::initialize()
         changeStateToClosed();
         return;
     }
+    if (m_stream->internalState() == ReadableStream::State::Errored) {
+        changeStateToErrored();
+        return;
+    }
 }
 
 void ReadableStreamReader::releaseStream()
@@ -78,13 +82,18 @@ void ReadableStreamReader::releaseStream()
     m_stream = nullptr;
 }
 
-void ReadableStreamReader::closed(ClosedSuccessCallback successCallback, ClosedErrorCallback)
+void ReadableStreamReader::closed(ClosedSuccessCallback successCallback, ClosedErrorCallback errorCallback)
 {
     if (m_state == State::Closed) {
         successCallback();
         return;
     }
+    if (m_state == State::Errored) {
+        errorCallback();
+        return;
+    }
     m_closedSuccessCallback = WTF::move(successCallback);
+    m_closedErrorCallback = WTF::move(errorCallback);
 }
 
 void ReadableStreamReader::changeStateToClosed()
@@ -97,8 +106,28 @@ void ReadableStreamReader::changeStateToClosed()
         closedSuccessCallback();
     }
     ASSERT(!m_closedSuccessCallback);
-    releaseStream();
+    m_closedErrorCallback = nullptr;
+
     // FIXME: Implement read promise fulfilling.
+
+    releaseStream();
+}
+
+void ReadableStreamReader::changeStateToErrored()
+{
+    ASSERT(m_state == State::Readable);
+    m_state = State::Errored;
+
+    if (m_closedErrorCallback) {
+        ClosedErrorCallback closedErrorCallback = WTF::move(m_closedErrorCallback);
+        closedErrorCallback();
+    }
+    ASSERT(!m_closedErrorCallback);
+    m_closedSuccessCallback = nullptr;
+
+    // FIXME: Implement read promise rejection.
+
+    releaseStream();
 }
 
 const char* ReadableStreamReader::activeDOMObjectName() const
index d547d2ccb0582f529a55adbd33e0ecca5acf9f96..7ddaf02323e98218f4fb8c47a93120c63ccf5928 100644 (file)
@@ -36,6 +36,7 @@
 #include "ReadableStream.h"
 #include "ScriptWrappable.h"
 #include <functional>
+#include <runtime/JSCJSValue.h>
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
 
@@ -61,6 +62,9 @@ public:
     void closed(ClosedSuccessCallback, ClosedErrorCallback);
 
     void changeStateToClosed();
+    void changeStateToErrored();
+
+    virtual JSC::JSValue error() = 0;
 
 protected:
     ReadableStreamReader(ReadableStream&);
@@ -77,6 +81,7 @@ private:
     State m_state { State::Readable };
 
     ClosedSuccessCallback m_closedSuccessCallback;
+    ClosedErrorCallback m_closedErrorCallback;
 };
 
 }
index 592eb0c7a3f3b87152899bb2fd4fd151afbaa65e..9d40899c84a87d927ca2880f02d991fee77a4575 100644 (file)
@@ -81,6 +81,14 @@ inline void DeferredWrapper::reject(const std::nullptr_t&)
     reject(exec, JSC::jsNull());
 }
 
+template<>
+inline void DeferredWrapper::reject(const JSC::JSValue& value)
+{
+    JSC::ExecState* exec = m_globalObject->globalExec();
+    JSC::JSLockHolder locker(exec);
+    reject(exec, value);
+}
+
 template<>
 inline void DeferredWrapper::resolve<String>(const String& result)
 {
index 695cf29e87de46f867849c7285da2b67084c7e57..ba9423677285fec04c755de6f3523c9951a82c24 100644 (file)
@@ -62,7 +62,7 @@ JSValue JSReadableStreamController::error(ExecState* exec)
     ReadableJSStream& stream = impl().stream();
     if (stream.internalState() != ReadableStream::State::Readable)
         return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Calling error on a stream which is not readable")));
-    notImplemented();
+    stream.storeError(*exec);
     return jsUndefined();
 }
 
index 48db54f90cb2ea3a7e82ba9eab90e465187c6508..d31d885b01a14f690d6db2f345ccb2fb3d659851 100644 (file)
@@ -75,8 +75,7 @@ JSValue JSReadableStreamReader::closed(ExecState* exec) const
         wrapper.resolve(jsUndefined());
     };
     auto failureCallback = [this, wrapper]() mutable {
-        // FIXME: return stored error.
-        wrapper.reject(&impl());
+        wrapper.reject(impl().error());
     };
 
     impl().closed(WTF::move(successCallback), WTF::move(failureCallback));
index f067d169395f4b6fd8cc89a3de68bf1de89d2e61..0a91300b1b3ef4e9187809478cff6fea99a43c3e 100644 (file)
@@ -141,7 +141,10 @@ Ref<ReadableJSStream> ReadableJSStream::create(ExecState& exec, ScriptExecutionC
 
 Ref<ReadableStreamReader> ReadableJSStream::createReader()
 {
-    return Reader::create(*this);
+    Ref<Reader> reader = Reader::create(*this);
+    if (m_error)
+        reader->storeError(*jsSource().globalObject()->globalExec(), m_error.get());
+    return static_reference_cast<ReadableStreamReader>(reader);
 }
 
 ReadableJSStream::ReadableJSStream(ScriptExecutionContext& scriptExecutionContext, Ref<ReadableJSStream::Source>&& source)
@@ -161,6 +164,19 @@ JSValue ReadableJSStream::jsController(ExecState& exec, JSDOMGlobalObject* globa
     return toJS(&exec, globalObject, m_controller.get());
 }
 
+void ReadableJSStream::storeError(JSC::ExecState& exec)
+{
+    if (m_error)
+        return;
+    JSValue error = exec.argumentCount() ? exec.argument(0) : createError(&exec, ASCIILiteral("Error function called."));
+    m_error.set(exec.vm(), error);
+
+    if (auto reader = static_cast<Reader*>(this->reader()))
+        reader->storeError(exec, error);
+
+    changeStateToErrored();
+}
+
 Ref<ReadableJSStream::Reader> ReadableJSStream::Reader::create(ReadableJSStream& stream)
 {
     return adoptRef(*new Reader(stream));
@@ -171,6 +187,11 @@ ReadableJSStream::Reader::Reader(ReadableJSStream& readableStream)
 {
 }
 
+void ReadableJSStream::Reader::storeError(JSC::ExecState& exec, JSValue error)
+{
+    m_error.set(exec.vm(), error);
+}
+
 } // namespace WebCore
 
 #endif
index be7f2bd235a62568e191b1ac1c92134a41577d80..900aed6ecff84585d9acdac1b5bf4203810911a5 100644 (file)
@@ -51,8 +51,13 @@ private:
     class Reader: public ReadableStreamReader {
     public:
         static Ref<Reader> create(ReadableJSStream&);
+        void storeError(JSC::ExecState&, JSC::JSValue);
+        JSC::JSValue error() { return m_error.get(); }
+
     private:
         explicit Reader(ReadableJSStream&);
+
+        JSC::Strong<JSC::Unknown> m_error;
     };
 
     class Source: public ReadableStreamSource {
@@ -75,10 +80,14 @@ public:
     ReadableJSStream::Source& jsSource();
     JSC::JSValue jsController(JSC::ExecState&, JSDOMGlobalObject*);
 
+    void storeError(JSC::ExecState&);
+    JSC::JSValue error() { return m_error.get(); }
+
 private:
     ReadableJSStream(ScriptExecutionContext&, Ref<ReadableJSStream::Source>&&);
 
     std::unique_ptr<ReadableStreamController> m_controller;
+    JSC::Strong<JSC::Unknown> m_error;
 };
 
 void setInternalSlotToObject(JSC::ExecState*, JSC::JSValue, JSC::PrivateName&, JSC::JSValue);