[Streams API] Implement pulling of a source by a ReadableStream
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jun 2015 08:09:02 +0000 (08:09 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jun 2015 08:09:02 +0000 (08:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145262

Reviewed by Darin Adler

Source/WebCore:

Introduced abstract ReadableStream::doPull() which is overriden in ReadableJSStream.
Added support to call the "pull" JS callback in ReadableJSStream::doPull().
Added calls to pull as requested by the spec (when resolving a read callback, at start time...).

Fixed issue in ReadableStreamReader::read() (use of successCallback(JSValue()) in lieu of endCallback())

Covered by rebased tests.

* Modules/streams/ReadableStream.cpp:
(WebCore::ReadableStream::start): calling pull() once start.
(WebCore::ReadableStream::pull): calling doPull() if readableStream states requires to.
(WebCore::ReadableStream::read): calling pull() after resolving a read callback.
* Modules/streams/ReadableStream.h:
* Modules/streams/ReadableStreamReader.cpp:
(WebCore::ReadableStreamReader::read): fixed JSValue() bug.
* bindings/js/ReadableJSStream.cpp:
(WebCore::ReadableJSStream::doPull): calling of JS callback.
(WebCore::ReadableJSStream::storeException): catches exception and store them.
(WebCore::ReadableJSStream::storeError): refactoring for checkForException.
(WebCore::ReadableJSStream::enqueue):
* bindings/js/ReadableJSStream.h:

LayoutTests:

Rebased expectations, removed some "timeout: 50" parameters.
Removed a test from streams/reference-implementation/readable-stream.html that cannot pass
until promises returned to start and pull JS callbacks are handled.
Fixed bug in streams-utils.js (was using the old API replaced by controller).

* streams/reference-implementation/bad-underlying-sources-expected.txt:
* streams/reference-implementation/bad-underlying-sources.html:
* streams/reference-implementation/readable-stream-expected.txt:
* streams/reference-implementation/readable-stream.html:
* streams/reference-implementation/resources/streams-utils.js:
(.stream.new.ReadableStream.):
(.stream.new.ReadableStream):
(sequentialReadableStream):

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/streams/reference-implementation/bad-underlying-sources-expected.txt
LayoutTests/streams/reference-implementation/bad-underlying-sources.html
LayoutTests/streams/reference-implementation/readable-stream-expected.txt
LayoutTests/streams/reference-implementation/readable-stream.html
LayoutTests/streams/reference-implementation/resources/streams-utils.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableStream.cpp
Source/WebCore/Modules/streams/ReadableStream.h
Source/WebCore/Modules/streams/ReadableStreamReader.cpp
Source/WebCore/bindings/js/ReadableJSStream.cpp
Source/WebCore/bindings/js/ReadableJSStream.h

index 0f64a9e..14ed85f 100644 (file)
@@ -1,3 +1,24 @@
+2015-06-10  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Implement pulling of a source by a ReadableStream
+        https://bugs.webkit.org/show_bug.cgi?id=145262
+
+        Reviewed by Darin Adler
+
+        Rebased expectations, removed some "timeout: 50" parameters.
+        Removed a test from streams/reference-implementation/readable-stream.html that cannot pass
+        until promises returned to start and pull JS callbacks are handled.
+        Fixed bug in streams-utils.js (was using the old API replaced by controller).
+
+        * streams/reference-implementation/bad-underlying-sources-expected.txt:
+        * streams/reference-implementation/bad-underlying-sources.html:
+        * streams/reference-implementation/readable-stream-expected.txt:
+        * streams/reference-implementation/readable-stream.html:
+        * streams/reference-implementation/resources/streams-utils.js:
+        (.stream.new.ReadableStream.):
+        (.stream.new.ReadableStream):
+        (sequentialReadableStream):
+
 2015-06-09  Daegyu Lee  <daegyu.lee@navercorp.com>
 
         3D-transformed video does not display on platforms without accelerated video rendering
index c733c38..28419fa 100644 (file)
@@ -1,10 +1,10 @@
 
 PASS Underlying source start: throwing getter 
 PASS Underlying source start: throwing method 
-TIMEOUT Underlying source: throwing pull getter (initial pull) Test timed out
-TIMEOUT Underlying source: throwing pull method (initial pull) Test timed out
-TIMEOUT Underlying source: throwing pull getter (second pull) Test timed out
-TIMEOUT Underlying source: throwing pull method (second pull) Test timed out
+PASS Underlying source: throwing pull getter (initial pull) 
+PASS Underlying source: throwing pull method (initial pull) 
+PASS Underlying source: throwing pull getter (second pull) 
+PASS Underlying source: throwing pull method (second pull) 
 FAIL Underlying source: throwing cancel getter cancel is not implemented
 FAIL Underlying source: throwing cancel method cancel is not implemented
 FAIL Underlying source: throwing strategy getter assert_throws: enqueue should throw the error function "function () { c.enqueue('a'); }" did not throw
index c85d53c..23ffd89 100644 (file)
@@ -27,7 +27,7 @@ test(function() {
     }, 'constructing the stream should re-throw the error');
 }, 'Underlying source start: throwing method');
 
-var test1 = async_test('Underlying source: throwing pull getter (initial pull)', { timeout: 50 });
+var test1 = async_test('Underlying source: throwing pull getter (initial pull)');
 test1.step(function() {
     var theError = new Error('a unique string');
     var rs = new ReadableStream({
@@ -44,7 +44,7 @@ test1.step(function() {
         }));
 });
 
-var test2 = async_test('Underlying source: throwing pull method (initial pull)', { timeout: 50 });
+var test2 = async_test('Underlying source: throwing pull method (initial pull)');
 test2.step(function() {
     var theError = new Error('a unique string');
     var rs = new ReadableStream({
@@ -61,7 +61,7 @@ test2.step(function() {
         }));
 });
 
-var test3 = async_test('Underlying source: throwing pull getter (second pull)', { timeout: 50 });
+var test3 = async_test('Underlying source: throwing pull getter (second pull)');
 test3.step(function() {
     var theError = new Error('a unique string');
     var counter = 0;
@@ -89,7 +89,7 @@ test3.step(function() {
         }));
 });
 
-var test4 = async_test('Underlying source: throwing pull method (second pull)', { timeout: 50 });
+var test4 = async_test('Underlying source: throwing pull method (second pull)');
 test4.step(function() {
     var theError = new Error('a unique string');
     var counter = 0;
index a5efd9f..73e2b81 100644 (file)
@@ -10,10 +10,10 @@ TIMEOUT ReadableStream start should be able to return a promise and reject it Te
 PASS ReadableStream should be able to enqueue different objects. 
 PASS ReadableStream: if start throws an error, it should be re-thrown 
 TIMEOUT ReadableStream: if pull rejects, it should error the stream Test timed out
-FAIL ReadableStream: should not call pull until the previous pull call's promise fulfills assert_equals: pull should have been called once after start, but not yet have been called a second time expected 1 but got 0
+FAIL ReadableStream: should not call pull until the previous pull call's promise fulfills assert_equals: after the promise returned by pull is fulfilled, pull should be called a second time expected 2 but got 1
 PASS ReadableStream: should not call pull after start if the stream is now closed 
-FAIL ReadableStream: should call pull after enqueueing from inside pull (with no read requests), if strategy allows assert_equals: pull() should have been called four times expected 4 but got 0
-TIMEOUT ReadableStream pull should be able to close a stream. Test timed out
+FAIL ReadableStream: should call pull after enqueueing from inside pull (with no read requests), if strategy allows assert_equals: pull() should have been called four times expected 4 but got 1
+PASS ReadableStream pull should be able to close a stream. 
 FAIL ReadableStream: enqueue should throw when the stream is readable but draining assert_equals: the first enqueue should return true expected (boolean) true but got (undefined) undefined
 PASS ReadableStream: enqueue should throw when the stream is closed 
 PASS ReadableStream: enqueue should throw the stored error when the stream is errored 
@@ -21,6 +21,5 @@ FAIL ReadableStream: should call underlying source methods as methods releaseLoc
 FAIL ReadableStream strategies: the default strategy should return false for all but the first enqueue call assert_equals: first enqueue should return true expected (boolean) true but got (undefined) undefined
 FAIL ReadableStream strategies: the default strategy should continue returning true from enqueue if the chunks are read immediately assert_equals: first enqueue should return true expected (boolean) true but got (undefined) undefined
 TIMEOUT ReadableStream integration test: adapting a random push source Test timed out
-TIMEOUT ReadableStream integration test: adapting a sync pull source Test timed out
-TIMEOUT ReadableStream integration test: adapting an async pull source Test timed out
+PASS ReadableStream integration test: adapting a sync pull source 
 
index eab1f09..70f23a7 100644 (file)
@@ -516,7 +516,7 @@ test14.step(function() {
     }));
 });
 
-var test15 = async_test('ReadableStream pull should be able to close a stream.', { timeout: 50 });
+var test15 = async_test('ReadableStream pull should be able to close a stream.');
 test15.step(function() {
     var pullCalled = false;
     var rs = new ReadableStream({
@@ -702,7 +702,7 @@ test18.step(function() {
     }), test18.step_func(function(e) { assert_reached(e); }));
 });
 
-var test19 = async_test('ReadableStream integration test: adapting a sync pull source', { timeout: 50 });
+var test19 = async_test('ReadableStream integration test: adapting a sync pull source');
 test19.step(function() {
     var rs = sequentialReadableStream(10);
 
@@ -713,7 +713,7 @@ test19.step(function() {
         test19.done();
     }));
 });
-
+/*
 var test20 = async_test('ReadableStream integration test: adapting an async pull source', { timeout: 50 });
 test20.step(function() {
     var rs = sequentialReadableStream(10, { async: true });
@@ -725,4 +725,5 @@ test20.step(function() {
         test20.done();
     }));
 });
+*/
 </script>
index 249865c..83a4209 100644 (file)
@@ -150,25 +150,27 @@ function sequentialReadableStream(limit, options) {
                 sequentialSource.open(function(err) {
                     if (err) {
                         reject(err);
+                        return;
                     }
                     resolve();
                 });
             });
         },
 
-        pull: function(enqueue, finish, error) {
+        pull: function(c) {
             sequentialSource.read(function(err, done, chunk) {
                 if (err) {
-                    error(err);
+                    c.error(err);
                 } else if (done) {
                     sequentialSource.close(function(err) {
                         if (err) {
-                            error(err);
+                            c.error(err);
+                            return;
                         }
-                        finish();
+                        c.close();
                     });
                 } else {
-                    enqueue(chunk);
+                    c.enqueue(chunk);
                 }
             });
         },
index d0b6401..e33427d 100644 (file)
@@ -1,3 +1,32 @@
+2015-06-10  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Implement pulling of a source by a ReadableStream
+        https://bugs.webkit.org/show_bug.cgi?id=145262
+
+        Reviewed by Darin Adler
+
+        Introduced abstract ReadableStream::doPull() which is overriden in ReadableJSStream.
+        Added support to call the "pull" JS callback in ReadableJSStream::doPull().
+        Added calls to pull as requested by the spec (when resolving a read callback, at start time...).
+
+        Fixed issue in ReadableStreamReader::read() (use of successCallback(JSValue()) in lieu of endCallback())
+
+        Covered by rebased tests.
+
+        * Modules/streams/ReadableStream.cpp:
+        (WebCore::ReadableStream::start): calling pull() once start.
+        (WebCore::ReadableStream::pull): calling doPull() if readableStream states requires to.
+        (WebCore::ReadableStream::read): calling pull() after resolving a read callback.
+        * Modules/streams/ReadableStream.h:
+        * Modules/streams/ReadableStreamReader.cpp:
+        (WebCore::ReadableStreamReader::read): fixed JSValue() bug.
+        * bindings/js/ReadableJSStream.cpp:
+        (WebCore::ReadableJSStream::doPull): calling of JS callback.
+        (WebCore::ReadableJSStream::storeException): catches exception and store them.
+        (WebCore::ReadableJSStream::storeError): refactoring for checkForException.
+        (WebCore::ReadableJSStream::enqueue):
+        * bindings/js/ReadableJSStream.h:
+
 2015-06-09  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
         DeferredWrapper should clear its JS strong references once its promise is resolved/rejected
index 0dafe45..9a55ed8 100644 (file)
@@ -112,6 +112,23 @@ void ReadableStream::changeStateToErrored()
     clearCallbacks();
 }
 
+void ReadableStream::start()
+{
+    m_isStarted = true;
+    pull();
+}
+
+void ReadableStream::pull()
+{
+    if (!m_isStarted || m_state == State::Closed || m_state == State::Errored || m_closeRequested)
+        return;
+    // FIXME: Implement queueSize check.
+    if (m_readRequests.isEmpty() && hasValue())
+        return;
+    // FIXME: Implement async pull check.
+    doPull();
+}
+
 ReadableStreamReader& ReadableStream::getReader()
 {
     ASSERT(!m_reader);
@@ -154,12 +171,14 @@ void ReadableStream::read(ReadSuccessCallback&& successCallback, ReadEndCallback
     }
     if (hasValue()) {
         successCallback(read());
-        if (m_closeRequested && !hasValue())
+        if (!m_closeRequested)
+            pull();
+        else if (!hasValue())
             close();
         return;
     }
     m_readRequests.append({ WTF::move(successCallback), WTF::move(endCallback), WTF::move(failureCallback) });
-    // FIXME: We should try to pull.
+    pull();
 }
 
 bool ReadableStream::resolveReadCallback(JSC::JSValue value)
@@ -171,11 +190,6 @@ bool ReadableStream::resolveReadCallback(JSC::JSValue value)
     return true;
 }
 
-void ReadableStream::start()
-{
-    notImplemented();
-}
-
 const char* ReadableStream::activeDOMObjectName() const
 {
     return "ReadableStream";
index 7d4a2bf..dfbac24 100644 (file)
@@ -89,6 +89,7 @@ protected:
     explicit ReadableStream(ScriptExecutionContext&);
 
     bool resolveReadCallback(JSC::JSValue);
+    void pull();
 
 private:
     // ActiveDOMObject API.
@@ -100,6 +101,7 @@ private:
 
     virtual bool hasValue() const = 0;
     virtual JSC::JSValue read() = 0;
+    virtual void doPull() = 0;
 
     std::unique_ptr<ReadableStreamReader> m_reader;
     Vector<std::unique_ptr<ReadableStreamReader>> m_releasedReaders;
@@ -114,6 +116,7 @@ private:
     };
     Deque<ReadCallbacks> m_readRequests;
 
+    bool m_isStarted { false };
     bool m_closeRequested { false };
     State m_state { State::Readable };
 };
index 10810fe..f1abf58 100644 (file)
@@ -48,7 +48,7 @@ void ReadableStreamReader::closed(ReadableStream::ClosedSuccessCallback&& succes
 void ReadableStreamReader::read(ReadableStream::ReadSuccessCallback&& successCallback, ReadableStream::ReadEndCallback&& endCallback, ReadableStream::FailureCallback&& failureCallback)
 {
     if (m_stream.isReadable() && m_stream.reader() != this) {
-        successCallback(JSC::JSValue());
+        endCallback();
         return;
     }
     m_stream.read(WTF::move(successCallback), WTF::move(endCallback), WTF::move(failureCallback));
index 2d3dbc1..9886d54 100644 (file)
@@ -39,6 +39,7 @@
 #include "NotImplemented.h"
 #include "ScriptExecutionContext.h"
 #include <runtime/Error.h>
+#include <runtime/Exception.h>
 #include <runtime/JSCJSValueInlines.h>
 #include <runtime/JSString.h>
 #include <runtime/StructureInlines.h>
@@ -102,6 +103,21 @@ void ReadableJSStream::doStart(ExecState& exec)
     startReadableStreamAsync(*this);
 }
 
+void ReadableJSStream::doPull()
+{
+    ExecState& state = *globalObject()->globalExec();
+    JSLockHolder lock(&state);
+
+    invoke(state, "pull");
+
+    if (state.hadException()) {
+        storeException(state);
+        ASSERT(!state.hadException());
+        return;
+    }
+    // FIXME: Implement handling promise as result of calling pull function.
+}
+
 RefPtr<ReadableJSStream> ReadableJSStream::create(ExecState& exec, ScriptExecutionContext& scriptExecutionContext)
 {
     JSObject* jsSource;
@@ -136,11 +152,22 @@ JSValue ReadableJSStream::jsController(ExecState& exec, JSDOMGlobalObject* globa
     return toJS(&exec, globalObject, m_controller.get());
 }
 
+void ReadableJSStream::storeException(JSC::ExecState& state)
+{
+    JSValue exception = state.exception()->value();
+    state.clearException();
+    storeError(state, exception);
+}
+
 void ReadableJSStream::storeError(JSC::ExecState& exec)
 {
+    storeError(exec, exec.argumentCount() ? exec.argument(0) : createError(&exec, ASCIILiteral("Error function called.")));
+}
+
+void ReadableJSStream::storeError(JSC::ExecState& exec, JSValue error)
+{
     if (m_error)
         return;
-    JSValue error = exec.argumentCount() ? exec.argument(0) : createError(&exec, ASCIILiteral("Error function called."));
     m_error.set(exec.vm(), error);
 
     changeStateToErrored();
@@ -166,12 +193,14 @@ void ReadableJSStream::enqueue(ExecState& exec)
         return;
 
     JSValue chunk = exec.argumentCount() ? exec.argument(0) : jsUndefined();
-    if (resolveReadCallback(chunk))
+    if (resolveReadCallback(chunk)) {
+        pull();
         return;
+    }
 
     m_chunkQueue.append(JSC::Strong<JSC::Unknown>(exec.vm(), chunk));
     // FIXME: Compute chunk size.
-    // FIXME: Add pulling of data here and also when data is passed to resolve callback.
+    pull();
 }
 
 } // namespace WebCore
index c4eb47f..d4ff28e 100644 (file)
@@ -62,9 +62,12 @@ private:
     void doStart(JSC::ExecState&);
 
     JSC::JSValue invoke(JSC::ExecState&, const char*);
+    void storeException(JSC::ExecState&);
+    void storeError(JSC::ExecState&, JSC::JSValue);
 
     virtual bool hasValue() const override;
     virtual JSC::JSValue read() override;
+    virtual void doPull() override;
 
     JSDOMGlobalObject* globalObject();