[Streams API] Implement ReadableStream js source "'cancel" callback
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Jun 2015 11:29:32 +0000 (11:29 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Jun 2015 11:29:32 +0000 (11:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=146204

Reviewed by Darin Adler.

Source/WebCore:

Calling "cancel" JS function when web app is cancelling a JS readable stream.
Handling of promise returned value in case of async cancel.

Covered by rebased tests.

* bindings/js/ReadableJSStream.cpp:
(WebCore::ReadableJSStream::invoke): Refactoring to pass cancel reason or controller to the JS function.
(WebCore::ReadableJSStream::doStart): Ditto.
(WebCore::startReadableStreamAsync): Renaming readableStream as protectedStream.
(WebCore::createPullResultFulfilledFunction): Ditto.
(WebCore::ReadableJSStream::doPull): Refactoring to pass cancel reason or controller to the JS function.
(WebCore::createCancelResultFulfilledFunction): Cancel promise fullfil callback.
(WebCore::createCancelResultRejectedFunction): Cancel promise reject callback.
(WebCore::ReadableJSStream::doCancel): Calling cancel JS callback and handling promise returned value.
* bindings/js/ReadableJSStream.h: Refactoring to pass cancel reason or controller to the JS function.

LayoutTests:

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

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

LayoutTests/ChangeLog
LayoutTests/streams/reference-implementation/bad-underlying-sources-expected.txt
LayoutTests/streams/reference-implementation/readable-stream-cancel-expected.txt
LayoutTests/streams/reference-implementation/readable-stream-expected.txt
LayoutTests/streams/reference-implementation/readable-stream-reader-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/ReadableJSStream.cpp
Source/WebCore/bindings/js/ReadableJSStream.h

index a6acafc..b87be73 100644 (file)
@@ -1,3 +1,15 @@
+2015-06-23  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Implement ReadableStream js source "'cancel" callback
+        https://bugs.webkit.org/show_bug.cgi?id=146204
+
+        Reviewed by Darin Adler.
+
+        * streams/reference-implementation/bad-underlying-sources-expected.txt:
+        * streams/reference-implementation/readable-stream-cancel-expected.txt:
+        * streams/reference-implementation/readable-stream-expected.txt:
+        * streams/reference-implementation/readable-stream-reader-expected.txt:
+
 2015-06-23  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
 
         [EFL] Unreviewed, gardening.
index 1bcfa79..f76d9a9 100644 (file)
@@ -5,8 +5,8 @@ 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 assert_unreached: cancel should not fulfill Reached unreachable code
-FAIL Underlying source: throwing cancel method assert_unreached: cancel should not fulfill Reached unreachable code
+PASS Underlying source: throwing cancel getter 
+PASS Underlying source: throwing cancel method 
 PASS Underlying source: calling enqueue on an empty canceled stream should not throw 
 PASS Underlying source: calling enqueue on a non-empty canceled stream should not throw 
 PASS Underlying source: calling enqueue on a closed stream should throw 
index 5a33722..83ebd91 100644 (file)
@@ -1,12 +1,12 @@
 
-FAIL ReadableStream cancellation: integration test on an infinite stream derived from a random push source assert_equals: it returns a promise that is fulfilled when the cancellation finishes expected true but got false
-FAIL ReadableStream cancellation: cancel(reason) should pass through the given reason to the underlying source assert_equals: the error passed to the underlying source's cancel method should equal the one passed to the stream's cancel expected (object) object "Error: Sorry, it just wasn't meant to be." but got (undefined) undefined
+PASS ReadableStream cancellation: integration test on an infinite stream derived from a random push source 
+PASS ReadableStream cancellation: cancel(reason) should pass through the given reason to the underlying source 
 PASS ReadableStream cancellation: cancel() on a locked stream should fail and not call the underlying source cancel 
-FAIL ReadableStream cancellation: should fulfill promise when cancel callback went fine assert_true: expected true got false
+PASS ReadableStream cancellation: should fulfill promise when cancel callback went fine 
 PASS ReadableStream cancellation: returning a value from the underlying source's cancel should not affect the fulfillment value of the promise returned by the stream's cancel 
-FAIL ReadableStream cancellation: should reject promise when cancel callback raises an exception assert_unreached: cancel should reject Reached unreachable code
+PASS ReadableStream cancellation: should reject promise when cancel callback raises an exception 
 PASS ReadableStream cancellation: if the underlying source's cancel method returns a promise, the promise returned by the stream's cancel should fulfill when that one does (1) 
-FAIL ReadableStream cancellation: if the underlying source's cancel method returns a promise, the promise returned by the stream's cancel should fulfill when that one does (2) assert_true: cancel() return value should be fulfilled only after the promise returned by the underlying source's cancel expected true got false
-FAIL ReadableStream cancellation: if the underlying source's cancel method returns a promise, the promise returned by the stream's cancel should reject when that one does assert_unreached: cancel() return value should not be rejected Reached unreachable code
+PASS ReadableStream cancellation: if the underlying source's cancel method returns a promise, the promise returned by the stream's cancel should fulfill when that one does (2) 
+PASS ReadableStream cancellation: if the underlying source's cancel method returns a promise, the promise returned by the stream's cancel should reject when that one does 
 PASS ReadableStream cancellation: cancelling before start finishes should prevent pull() from being called 
 
index 7d9f59c..dd56478 100644 (file)
@@ -25,7 +25,7 @@ PASS ReadableStream pull should be able to close a stream.
 PASS ReadableStream: enqueue should throw when the stream is readable but draining 
 PASS ReadableStream: enqueue should throw when the stream is closed 
 PASS ReadableStream: enqueue should throw the stored error when the stream is errored 
-FAIL ReadableStream: should call underlying source methods as methods assert_equals: expected 1 but got 0
+PASS ReadableStream: should call underlying source methods as methods 
 FAIL ReadableStream strategies: the default strategy should give desiredSize of 1 to start, decreasing by 1 per enqueue assert_equals: expected (number) 1 but got (undefined) undefined
 FAIL ReadableStream strategies: the default strategy should continue giving desiredSize of 1 if the chunks are read immediately assert_equals: desiredSize should start at 1 expected (number) 1 but got (undefined) undefined
 PASS ReadableStream integration test: adapting a random push source 
index 1650996..d5e6d3f 100644 (file)
@@ -10,7 +10,7 @@ PASS Getting a ReadableStreamReader via getReader should fail if the stream is a
 PASS Constructing a ReadableStreamReader directly should be OK if the stream is closed 
 PASS Constructing a ReadableStreamReader directly should be OK if the stream is errored 
 PASS Reading from a reader for an empty stream will wait until a chunk is available 
-FAIL cancel() on a reader releases the reader before calling through assert_true: expected true got false
+PASS cancel() on a reader releases the reader before calling through 
 PASS closed should be fulfilled after stream is closed (.closed access before acquiring) 
 PASS closed should be fulfilled after reader releases its lock (multiple stream locks) 
 PASS Multiple readers can access the stream in sequence 
index 899e7cc..f183948 100644 (file)
@@ -1,3 +1,26 @@
+2015-06-23  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Implement ReadableStream js source "'cancel" callback
+        https://bugs.webkit.org/show_bug.cgi?id=146204
+
+        Reviewed by Darin Adler.
+
+        Calling "cancel" JS function when web app is cancelling a JS readable stream.
+        Handling of promise returned value in case of async cancel.
+
+        Covered by rebased tests.
+
+        * bindings/js/ReadableJSStream.cpp:
+        (WebCore::ReadableJSStream::invoke): Refactoring to pass cancel reason or controller to the JS function.
+        (WebCore::ReadableJSStream::doStart): Ditto.
+        (WebCore::startReadableStreamAsync): Renaming readableStream as protectedStream.
+        (WebCore::createPullResultFulfilledFunction): Ditto.
+        (WebCore::ReadableJSStream::doPull): Refactoring to pass cancel reason or controller to the JS function.
+        (WebCore::createCancelResultFulfilledFunction): Cancel promise fullfil callback.
+        (WebCore::createCancelResultRejectedFunction): Cancel promise reject callback.
+        (WebCore::ReadableJSStream::doCancel): Calling cancel JS callback and handling promise returned value.
+        * bindings/js/ReadableJSStream.h: Refactoring to pass cancel reason or controller to the JS function.
+
 2015-06-22  Ryuan Choi  <ryuan.choi@navercorp.com>
 
         [EFL] Hyphenation is not supported
index fd3c427..952679e 100644 (file)
@@ -60,7 +60,7 @@ static inline JSValue callFunction(ExecState& exec, JSValue jsFunction, JSValue
     return call(&exec, jsFunction, callType, callData, thisValue, arguments);
 }
 
-JSPromise* ReadableJSStream::invoke(ExecState& state, const char* propertyName)
+JSPromise* ReadableJSStream::invoke(ExecState& state, const char* propertyName, JSValue parameter)
 {
     JSValue function = getPropertyFromObject(state, m_source.get(), propertyName);
     if (state.hadException())
@@ -73,7 +73,7 @@ JSPromise* ReadableJSStream::invoke(ExecState& state, const char* propertyName)
     }
 
     MarkedArgumentBuffer arguments;
-    arguments.append(jsController(state, globalObject()));
+    arguments.append(parameter);
 
     JSPromise* promise = jsDynamicCast<JSPromise*>(callFunction(state, function, m_source.get(), arguments));
 
@@ -108,11 +108,11 @@ static inline JSFunction* createStartResultFulfilledFunction(ExecState& state, R
     });
 }
 
-static inline void startReadableStreamAsync(ReadableStream& readableStream)
+static inline void startReadableStreamAsync(ReadableStream& stream)
 {
-    RefPtr<ReadableStream> stream = &readableStream;
-    stream->scriptExecutionContext()->postTask([stream](ScriptExecutionContext&) {
-        stream->start();
+    RefPtr<ReadableStream> protectedStream = &stream;
+    stream.scriptExecutionContext()->postTask([protectedStream](ScriptExecutionContext&) {
+        protectedStream->start();
     });
 }
 
@@ -120,7 +120,7 @@ void ReadableJSStream::doStart(ExecState& exec)
 {
     JSLockHolder lock(&exec);
 
-    JSPromise* promise = invoke(exec, "start");
+    JSPromise* promise = invoke(exec, "start", jsController(exec, globalObject()));
 
     if (exec.hadException())
         return;
@@ -135,9 +135,9 @@ void ReadableJSStream::doStart(ExecState& exec)
 
 static inline JSFunction* createPullResultFulfilledFunction(ExecState& exec, ReadableJSStream& stream)
 {
-    RefPtr<ReadableJSStream> readableStream = &stream;
-    return JSFunction::create(exec.vm(), exec.callee()->globalObject(), 0, String(), [readableStream](ExecState*) {
-        readableStream->finishPulling();
+    RefPtr<ReadableJSStream> protectedStream = &stream;
+    return JSFunction::create(exec.vm(), exec.callee()->globalObject(), 0, String(), [protectedStream](ExecState*) {
+        protectedStream->finishPulling();
         return JSValue::encode(jsUndefined());
     });
 }
@@ -147,7 +147,7 @@ bool ReadableJSStream::doPull()
     ExecState& state = *globalObject()->globalExec();
     JSLockHolder lock(&state);
 
-    JSPromise* promise = invoke(state, "pull");
+    JSPromise* promise = invoke(state, "pull", jsController(state, globalObject()));
 
     if (promise)
         thenPromise(state, promise, createPullResultFulfilledFunction(state, *this), m_errorFunction.get());
@@ -161,6 +161,43 @@ bool ReadableJSStream::doPull()
     return !promise;
 }
 
+static JSFunction* createCancelResultFulfilledFunction(ExecState& exec, ReadableJSStream& stream)
+{
+    RefPtr<ReadableJSStream> protectedStream = &stream;
+    return JSFunction::create(exec.vm(), exec.callee()->globalObject(), 1, String(), [protectedStream](ExecState*) {
+        protectedStream->notifyCancelSucceeded();
+        return JSValue::encode(jsUndefined());
+    });
+}
+
+static JSFunction* createCancelResultRejectedFunction(ExecState& exec, ReadableJSStream& stream)
+{
+    RefPtr<ReadableJSStream> protectedStream = &stream;
+    return JSFunction::create(exec.vm(), exec.callee()->globalObject(), 1, String(), [protectedStream](ExecState* exec) {
+        protectedStream->storeError(*exec, exec->argument(0));
+        protectedStream->notifyCancelFailed();
+        return JSValue::encode(jsUndefined());
+    });
+}
+
+bool ReadableJSStream::doCancel(JSValue reason)
+{
+    ExecState& exec = *globalObject()->globalExec();
+    JSLockHolder lock(&exec);
+
+    JSPromise* promise = invoke(exec, "cancel", reason);
+
+    if (promise)
+        thenPromise(exec, promise, createCancelResultFulfilledFunction(exec, *this), createCancelResultRejectedFunction(exec, *this));
+
+    if (exec.hadException()) {
+        storeException(exec);
+        ASSERT(!exec.hadException());
+        return true;
+    }
+    return !promise;
+}
+
 RefPtr<ReadableJSStream> ReadableJSStream::create(ExecState& exec, ScriptExecutionContext& scriptExecutionContext)
 {
     JSObject* jsSource;
@@ -182,12 +219,6 @@ RefPtr<ReadableJSStream> ReadableJSStream::create(ExecState& exec, ScriptExecuti
     return readableStream;
 }
 
-bool ReadableJSStream::doCancel(JSValue)
-{
-    // FIXME: Implement it.
-    return true;
-}
-
 ReadableJSStream::ReadableJSStream(ScriptExecutionContext& scriptExecutionContext, ExecState& state, JSObject* source)
     : ReadableStream(scriptExecutionContext)
 {
index d3f7358..409afa1 100644 (file)
@@ -66,7 +66,7 @@ private:
 
     void doStart(JSC::ExecState&);
 
-    JSC::JSPromise* invoke(JSC::ExecState&, const char*);
+    JSC::JSPromise* invoke(JSC::ExecState&, const char*, JSC::JSValue parameter);
     void storeException(JSC::ExecState&);
 
     virtual bool hasValue() const override;