Title: [185537] trunk
Revision
185537
Author
[email protected]
Date
2015-06-13 02:09:53 -0700 (Sat, 13 Jun 2015)

Log Message

[Streams API] ReadableJSStream should handle promises returned by JS source start callback
https://bugs.webkit.org/show_bug.cgi?id=145792

Source/_javascript_Core:

Reviewed by Darin Adler.

Added support for JSFunction implemented by std::function.

* runtime/JSFunction.cpp:
(JSC::getNativeExecutable): Refactored code to share it with the two JSFunction::create
(JSC::JSFunction::create):
(JSC::runStdFunction):
* runtime/JSFunction.h: Added std::function based JSFunction::create prototype.
* runtime.JSPromise.h:

Source/WebCore:

Reviewed by Darin Adler.

Covered by rebased tests.

When calling start callback, the returned value is checked.
If it is not a promise, we do as if it is a resolved promise.
If it is a promise, we call its then() method with two resolve/reject JS functions.

* Modules/streams/ReadableStream.cpp:
* bindings/js/ReadableJSStream.cpp:
(WebCore::ReadableJSStream::invoke): Returns a JSPromise* if any is returned by the JS source callback.
(WebCore::thenPromise): Utility method to call the promise.
(WebCore::createStartResultFulfilledFunction): The promise resolve callback.
(WebCore::ReadableJSStream::doStart): Calls thenPromise if a JSPromise* is returned by invoke.
(WebCore::ReadableJSStream::ReadableJSStream):
* bindings/js/ReadableJSStream.h:

LayoutTests:

Reviewed by Darin Adler.

Rebasing expectations, and removing timeouts for tests that no longer timeout.

* streams/reference-implementation/readable-stream-expected.txt:
* streams/reference-implementation/readable-stream.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (185536 => 185537)


--- trunk/LayoutTests/ChangeLog	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/LayoutTests/ChangeLog	2015-06-13 09:09:53 UTC (rev 185537)
@@ -1,3 +1,15 @@
+2015-06-13  Xabier Rodriguez Calvar  <[email protected]> and Youenn Fablet <[email protected]>
+
+        [Streams API] ReadableJSStream should handle promises returned by JS source start callback
+        https://bugs.webkit.org/show_bug.cgi?id=145792
+
+        Reviewed by Darin Adler.
+
+        Rebasing expectations, and removing timeouts for tests that no longer timeout.
+
+        * streams/reference-implementation/readable-stream-expected.txt:
+        * streams/reference-implementation/readable-stream.html:
+
 2015-06-13  Andres Gonzalez  <[email protected]>
 
         AX: WebKit exposes all Ruby Text as Unknown (Japanese EPUB accessibility blocker)

Modified: trunk/LayoutTests/streams/reference-implementation/readable-stream-expected.txt (185536 => 185537)


--- trunk/LayoutTests/streams/reference-implementation/readable-stream-expected.txt	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/LayoutTests/streams/reference-implementation/readable-stream-expected.txt	2015-06-13 09:09:53 UTC (rev 185537)
@@ -5,8 +5,8 @@
 PASS ReadableStream constructor can get initial garbage as cancel argument 
 PASS ReadableStream constructor can get initial garbage as pull argument 
 PASS ReadableStream constructor can get initial garbage as strategy argument 
-TIMEOUT ReadableStream start should be able to return a promise Test timed out
-TIMEOUT ReadableStream start should be able to return a promise and reject it Test timed out
+PASS ReadableStream start should be able to return a promise 
+PASS ReadableStream start should be able to return a promise and reject it 
 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
@@ -20,6 +20,6 @@
 FAIL ReadableStream: should call underlying source methods as methods releaseLock is not implemented
 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
+PASS ReadableStream integration test: adapting a random push source 
 PASS ReadableStream integration test: adapting a sync pull source 
 

Modified: trunk/LayoutTests/streams/reference-implementation/readable-stream.html (185536 => 185537)


--- trunk/LayoutTests/streams/reference-implementation/readable-stream.html	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/LayoutTests/streams/reference-implementation/readable-stream.html	2015-06-13 09:09:53 UTC (rev 185537)
@@ -53,7 +53,7 @@
     new ReadableStream({ strategy: 2 }); // Constructor should not throw when strategy is not an object.
 }, 'ReadableStream constructor can get initial garbage as strategy argument');
 
-var test1 = async_test('ReadableStream start should be able to return a promise', { timeout: 50 });
+var test1 = async_test('ReadableStream start should be able to return a promise');
 test1.step(function()
 {
     var readCalled = false;
@@ -82,7 +82,7 @@
     }));
 });
 
-var test2 = async_test('ReadableStream start should be able to return a promise and reject it', { timeout: 100 });
+var test2 = async_test('ReadableStream start should be able to return a promise and reject it');
 test2.step(function()
 {
     var theError = new Error('rejected!');
@@ -658,7 +658,7 @@
     })).catch(test17.step_func(function(e) { assert_unreached(e); } ));
 });
 
-var test18 = async_test('ReadableStream integration test: adapting a random push source', { timeout: 50 });
+var test18 = async_test('ReadableStream integration test: adapting a random push source');
 test18.step(function() {
     var pullChecked = false;
     var randomSource = new RandomPushSource(8);

Modified: trunk/Source/_javascript_Core/ChangeLog (185536 => 185537)


--- trunk/Source/_javascript_Core/ChangeLog	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-06-13 09:09:53 UTC (rev 185537)
@@ -1,3 +1,19 @@
+2015-06-13  Youenn Fablet  <[email protected]>
+
+        [Streams API] ReadableJSStream should handle promises returned by JS source start callback
+        https://bugs.webkit.org/show_bug.cgi?id=145792
+
+        Reviewed by Darin Adler.
+
+        Added support for JSFunction implemented by std::function.
+
+        * runtime/JSFunction.cpp:
+        (JSC::getNativeExecutable): Refactored code to share it with the two JSFunction::create
+        (JSC::JSFunction::create):
+        (JSC::runStdFunction):
+        * runtime/JSFunction.h: Added std::function based JSFunction::create prototype.
+        * runtime.JSPromise.h:
+
 2015-06-12  Gyuyoung Kim  <[email protected]>
 
         Purge PassRefPtr in _javascript_Core - 2

Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (185536 => 185537)


--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp	2015-06-13 09:09:53 UTC (rev 185537)
@@ -4,6 +4,7 @@
  *  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2015 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Cameron Zwarich ([email protected])
  *  Copyright (C) 2007 Maks Orlovich
+ *  Copyright (C) 2015 Canon Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
@@ -66,25 +67,53 @@
     return result;
 }
 
-JSFunction* JSFunction::create(VM& vm, JSGlobalObject* globalObject, int length, const String& name, NativeFunction nativeFunction, Intrinsic intrinsic, NativeFunction nativeConstructor)
+static inline NativeExecutable* getNativeExecutable(VM& vm, NativeFunction nativeFunction, Intrinsic intrinsic, NativeFunction nativeConstructor)
 {
-    NativeExecutable* executable;
 #if !ENABLE(JIT)
     UNUSED_PARAM(intrinsic);
 #else
     if (intrinsic != NoIntrinsic && vm.canUseJIT()) {
         ASSERT(nativeConstructor == callHostFunctionAsConstructor);
-        executable = vm.getHostFunction(nativeFunction, intrinsic);
-    } else
+        return vm.getHostFunction(nativeFunction, intrinsic);
+    }
 #endif
-        executable = vm.getHostFunction(nativeFunction, nativeConstructor);
+    return vm.getHostFunction(nativeFunction, nativeConstructor);
+}
 
+JSFunction* JSFunction::create(VM& vm, JSGlobalObject* globalObject, int length, const String& name, NativeFunction nativeFunction, Intrinsic intrinsic, NativeFunction nativeConstructor)
+{
+    NativeExecutable* executable = getNativeExecutable(vm, nativeFunction, intrinsic, nativeConstructor);
     JSFunction* function = new (NotNull, allocateCell<JSFunction>(vm.heap)) JSFunction(vm, globalObject, globalObject->functionStructure());
     // Can't do this during initialization because getHostFunction might do a GC allocation.
     function->finishCreation(vm, executable, length, name);
     return function;
 }
 
+class JSStdFunction : public JSFunction {
+public:
+    JSStdFunction(VM& vm, JSGlobalObject* object, Structure* structure, NativeStdFunction&& function)
+        : JSFunction(vm, object, structure)
+        , stdFunction(WTF::move(function)) { }
+
+    NativeStdFunction stdFunction;
+};
+
+static EncodedJSValue JSC_HOST_CALL runStdFunction(ExecState* state)
+{
+    JSStdFunction* jsFunction = jsCast<JSStdFunction*>(state->callee());
+    ASSERT(jsFunction);
+    return jsFunction->stdFunction(state);
+}
+
+JSFunction* JSFunction::create(VM& vm, JSGlobalObject* globalObject, int length, const String& name, NativeStdFunction&& nativeStdFunction, Intrinsic intrinsic, NativeFunction nativeConstructor)
+{
+    NativeExecutable* executable = getNativeExecutable(vm, runStdFunction, intrinsic, nativeConstructor);
+    JSStdFunction* function = new (NotNull, allocateCell<JSStdFunction>(vm.heap)) JSStdFunction(vm, globalObject, globalObject->functionStructure(), WTF::move(nativeStdFunction));
+    // Can't do this during initialization because getHostFunction might do a GC allocation.
+    function->finishCreation(vm, executable, length, name);
+    return function;
+}
+
 JSFunction::JSFunction(VM& vm, JSGlobalObject* globalObject, Structure* structure)
     : Base(vm, globalObject, structure)
     , m_executable()

Modified: trunk/Source/_javascript_Core/runtime/JSFunction.h (185536 => 185537)


--- trunk/Source/_javascript_Core/runtime/JSFunction.h	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.h	2015-06-13 09:09:53 UTC (rev 185537)
@@ -45,6 +45,8 @@
 class JITCompiler;
 }
 
+typedef std::function<EncodedJSValue (ExecState*)> NativeStdFunction;
+
 JS_EXPORT_PRIVATE EncodedJSValue JSC_HOST_CALL callHostFunctionAsConstructor(ExecState*);
 
 JS_EXPORT_PRIVATE String getCalculatedDisplayName(CallFrame*, JSObject*);
@@ -68,6 +70,7 @@
     JS_EXPORT_PRIVATE static JSFunction* create(VM&, JSGlobalObject*, int length, const String& name, NativeFunction, Intrinsic = NoIntrinsic, NativeFunction nativeConstructor = callHostFunctionAsConstructor);
     
     static JSFunction* createWithInvalidatedReallocationWatchpoint(VM&, FunctionExecutable*, JSScope*);
+    JS_EXPORT_PRIVATE static JSFunction* create(VM&, JSGlobalObject*, int length, const String& name, NativeStdFunction&&, Intrinsic = NoIntrinsic, NativeFunction nativeConstructor = callHostFunctionAsConstructor);
 
     static JSFunction* create(VM&, FunctionExecutable*, JSScope*);
 

Modified: trunk/Source/_javascript_Core/runtime/JSPromise.h (185536 => 185537)


--- trunk/Source/_javascript_Core/runtime/JSPromise.h	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/_javascript_Core/runtime/JSPromise.h	2015-06-13 09:09:53 UTC (rev 185537)
@@ -42,7 +42,7 @@
     static JSPromise* create(VM&, JSGlobalObject*, JSPromiseConstructor*);
     static Structure* createStructure(VM&, JSGlobalObject*, JSValue);
 
-    DECLARE_INFO;
+    DECLARE_EXPORT_INFO;
 
     enum class Status {
         Unresolved,

Modified: trunk/Source/WebCore/ChangeLog (185536 => 185537)


--- trunk/Source/WebCore/ChangeLog	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/WebCore/ChangeLog	2015-06-13 09:09:53 UTC (rev 185537)
@@ -1,3 +1,25 @@
+2015-06-13  Xabier Rodriguez Calvar  <[email protected]> and Youenn Fablet <[email protected]>
+
+        [Streams API] ReadableJSStream should handle promises returned by JS source start callback
+        https://bugs.webkit.org/show_bug.cgi?id=145792
+
+        Reviewed by Darin Adler.
+
+        Covered by rebased tests.
+
+        When calling start callback, the returned value is checked.
+        If it is not a promise, we do as if it is a resolved promise.
+        If it is a promise, we call its then() method with two resolve/reject JS functions.
+
+        * Modules/streams/ReadableStream.cpp:
+        * bindings/js/ReadableJSStream.cpp:
+        (WebCore::ReadableJSStream::invoke): Returns a JSPromise* if any is returned by the JS source callback.
+        (WebCore::thenPromise): Utility method to call the promise.
+        (WebCore::createStartResultFulfilledFunction): The promise resolve callback.
+        (WebCore::ReadableJSStream::doStart): Calls thenPromise if a JSPromise* is returned by invoke.
+        (WebCore::ReadableJSStream::ReadableJSStream):
+        * bindings/js/ReadableJSStream.h:
+
 2015-06-13  Andres Gonzalez  <[email protected]>
 
         AX: WebKit exposes all Ruby Text as Unknown (Japanese EPUB accessibility blocker)

Modified: trunk/Source/WebCore/Modules/streams/ReadableStream.cpp (185536 => 185537)


--- trunk/Source/WebCore/Modules/streams/ReadableStream.cpp	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/WebCore/Modules/streams/ReadableStream.cpp	2015-06-13 09:09:53 UTC (rev 185537)
@@ -32,7 +32,6 @@
 
 #if ENABLE(STREAMS_API)
 
-#include "NotImplemented.h"
 #include "ReadableStreamReader.h"
 #include <runtime/JSCJSValueInlines.h>
 #include <wtf/RefCountedLeakCounter.h>

Modified: trunk/Source/WebCore/bindings/js/ReadableJSStream.cpp (185536 => 185537)


--- trunk/Source/WebCore/bindings/js/ReadableJSStream.cpp	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/WebCore/bindings/js/ReadableJSStream.cpp	2015-06-13 09:09:53 UTC (rev 185537)
@@ -36,11 +36,11 @@
 #include "JSDOMPromise.h"
 #include "JSReadableStream.h"
 #include "JSReadableStreamController.h"
-#include "NotImplemented.h"
 #include "ScriptExecutionContext.h"
 #include <runtime/Error.h>
 #include <runtime/Exception.h>
 #include <runtime/JSCJSValueInlines.h>
+#include <runtime/JSPromise.h>
 #include <runtime/JSString.h>
 #include <runtime/StructureInlines.h>
 
@@ -60,31 +60,57 @@
     return call(&exec, jsFunction, callType, callData, thisValue, arguments);
 }
 
-JSValue ReadableJSStream::invoke(ExecState& exec, const char* propertyName)
+JSPromise* ReadableJSStream::invoke(ExecState& state, const char* propertyName)
 {
-    JSValue function = getPropertyFromObject(exec, m_source.get(), propertyName);
-    if (exec.hadException())
-        return jsUndefined();
+    JSValue function = getPropertyFromObject(state, m_source.get(), propertyName);
+    if (state.hadException())
+        return nullptr;
 
     if (!function.isFunction()) {
         if (!function.isUndefined())
-            throwVMError(&exec, createTypeError(&exec, ASCIILiteral("ReadableStream trying to call a property that is not callable")));
-        return jsUndefined();
+            throwVMError(&state, createTypeError(&state, ASCIILiteral("ReadableStream trying to call a property that is not callable")));
+        return nullptr;
     }
 
     MarkedArgumentBuffer arguments;
-    arguments.append(jsController(exec, globalObject()));
-    return callFunction(exec, function, m_source.get(), arguments);
+    arguments.append(jsController(state, globalObject()));
+
+    JSPromise* promise = jsDynamicCast<JSPromise*>(callFunction(state, function, m_source.get(), arguments));
+
+    ASSERT(!(promise && state.hadException()));
+    return promise;
 }
 
+static void thenPromise(ExecState& state, JSPromise* deferredPromise, JSValue fullfilFunction, JSValue rejectFunction)
+{
+    JSValue thenValue = deferredPromise->get(&state, state.vm().propertyNames->then);
+    if (state.hadException())
+        return;
+
+    MarkedArgumentBuffer arguments;
+    arguments.append(fullfilFunction);
+    arguments.append(rejectFunction);
+
+    callFunction(state, thenValue, deferredPromise, arguments);
+}
+
 JSDOMGlobalObject* ReadableJSStream::globalObject()
 {
     return jsDynamicCast<JSDOMGlobalObject*>(m_source->globalObject());
 }
 
-static void startReadableStreamAsync(ReadableStream& readableStream)
+static inline JSFunction* createStartResultFulfilledFunction(ExecState& state, ReadableStream& readableStream)
 {
     RefPtr<ReadableStream> stream = &readableStream;
+    return JSFunction::create(state.vm(), state.callee()->globalObject(), 1, String(), [stream](ExecState*) {
+        stream->start();
+        return JSValue::encode(jsUndefined());
+    });
+}
+
+static inline void startReadableStreamAsync(ReadableStream& readableStream)
+{
+    RefPtr<ReadableStream> stream = &readableStream;
     stream->scriptExecutionContext()->postTask([stream](ScriptExecutionContext&) {
         stream->start();
     });
@@ -94,13 +120,17 @@
 {
     JSLockHolder lock(&exec);
 
-    invoke(exec, "start");
+    JSPromise* promise = invoke(exec, "start");
 
     if (exec.hadException())
         return;
 
-    // FIXME: Implement handling promise as result of calling start function.
-    startReadableStreamAsync(*this);
+    if (!promise) {
+        startReadableStreamAsync(*this);
+        return;
+    }
+
+    thenPromise(exec, promise, createStartResultFulfilledFunction(exec, *this), m_errorFunction.get());
 }
 
 void ReadableJSStream::doPull()
@@ -139,10 +169,16 @@
     return readableStream;
 }
 
-ReadableJSStream::ReadableJSStream(ScriptExecutionContext& scriptExecutionContext, ExecState& exec, JSObject* source)
+ReadableJSStream::ReadableJSStream(ScriptExecutionContext& scriptExecutionContext, ExecState& state, JSObject* source)
     : ReadableStream(scriptExecutionContext)
 {
-    m_source.set(exec.vm(), source);
+    m_source.set(state.vm(), source);
+    // We do not take a Ref to the stream as this would cause a Ref cycle.
+    // The resolution callback used jointly with m_errorFunction as promise callbacks should protect the stream instead.
+    m_errorFunction.set(state.vm(), JSFunction::create(state.vm(), state.callee()->globalObject(), 1, String(), [this](ExecState* state) {
+        storeError(*state);
+        return JSValue::encode(jsUndefined());
+    }));
 }
 
 JSValue ReadableJSStream::jsController(ExecState& exec, JSDOMGlobalObject* globalObject)

Modified: trunk/Source/WebCore/bindings/js/ReadableJSStream.h (185536 => 185537)


--- trunk/Source/WebCore/bindings/js/ReadableJSStream.h	2015-06-13 08:29:23 UTC (rev 185536)
+++ trunk/Source/WebCore/bindings/js/ReadableJSStream.h	2015-06-13 09:09:53 UTC (rev 185537)
@@ -40,6 +40,11 @@
 #include <wtf/Deque.h>
 #include <wtf/Ref.h>
 
+namespace JSC {
+class JSFunction;
+class JSPromise;
+}
+    
 namespace WebCore {
 
 class JSDOMGlobalObject;
@@ -61,7 +66,7 @@
 
     void doStart(JSC::ExecState&);
 
-    JSC::JSValue invoke(JSC::ExecState&, const char*);
+    JSC::JSPromise* invoke(JSC::ExecState&, const char*);
     void storeException(JSC::ExecState&);
     void storeError(JSC::ExecState&, JSC::JSValue);
 
@@ -73,6 +78,7 @@
 
     std::unique_ptr<ReadableStreamController> m_controller;
     JSC::Strong<JSC::Unknown> m_error;
+    JSC::Strong<JSC::JSFunction> m_errorFunction;
     JSC::Strong<JSC::JSObject> m_source;
     Deque<JSC::Strong<JSC::Unknown>> m_chunkQueue;
 };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to