[Streams API] Error storage should be moved from source to stream/reader
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Mar 2015 15:23:24 +0000 (15:23 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Mar 2015 15:23:24 +0000 (15:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143048

Reviewed by Benjamin Poulain.

This patch removes error storage from the source as it should be stored at the stream level as error access goes through the reader.
It removes abstract ReadableStreamSource::isErrored and the storage of JavaScript errors from ReadableStreamJSSource.

Existing tests cover most of the changes.
Added test case for creating readable stream from empty JS object.

* Modules/streams/ReadableStreamSource.h: Removing isErrored().
* bindings/js/JSReadableStreamCustom.cpp:
(WebCore::constructJSReadableStream): Added JS stream constructor parameters checking.
* bindings/js/ReadableStreamJSSource.cpp:
(WebCore::ReadableStreamJSSource::ReadableStreamJSSource): Removed JS stream constructor parameters checking.
(WebCore::ReadableStreamJSSource::start): Changed prototype of start so that start can throw errors directly.
(WebCore::ReadableStreamJSSource::setInternalError): Deleted.
* bindings/js/ReadableStreamJSSource.h: Removed m_error, setInternalError and updated start declaration.

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

LayoutTests/streams/readablestream-constructor-expected.txt
LayoutTests/streams/readablestream-constructor.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableStreamSource.h
Source/WebCore/bindings/js/JSReadableStreamCustom.cpp
Source/WebCore/bindings/js/ReadableStreamJSSource.cpp
Source/WebCore/bindings/js/ReadableStreamJSSource.h

index 8b17ee0d174c6c18c58c4d86f74d754f881c268e..12c8c335952309a5d1d4fa43867ca4b34ba0382a 100644 (file)
@@ -2,4 +2,5 @@
 PASS ReadableStream constructor should get an object as argument 
 PASS ReadableStream can be constructed with no arguments 
 PASS ReadableStream instances should have the correct list of properties 
+PASS ReadableStream can be constructed with an empty object as argument 
 
index be1ea5817e588eeb71488cffa634957b63945291..44a5096d466615fc329e0431e53006da7a32cc97 100644 (file)
@@ -45,4 +45,8 @@ test(function() {
 
 }, 'ReadableStream instances should have the correct list of properties');
 
+test(function() {
+    new ReadableStream({ });
+}, 'ReadableStream can be constructed with an empty object as argument');
+
 </script>
index 67e2870ff69c3b9feaa96fec207552b7c75dbb1d..b6ab8ea0cf7d1e9546c50eea0c3903cb7258db7e 100644 (file)
@@ -1,3 +1,25 @@
+2015-03-25 Xabier Rodriguez Calvar <calvaris@igalia.com> and Youenn Fablet <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Error storage should be moved from source to stream/reader
+        https://bugs.webkit.org/show_bug.cgi?id=143048
+
+        Reviewed by Benjamin Poulain.
+
+        This patch removes error storage from the source as it should be stored at the stream level as error access goes through the reader.
+        It removes abstract ReadableStreamSource::isErrored and the storage of JavaScript errors from ReadableStreamJSSource.
+
+        Existing tests cover most of the changes.
+        Added test case for creating readable stream from empty JS object.
+
+        * Modules/streams/ReadableStreamSource.h: Removing isErrored().
+        * bindings/js/JSReadableStreamCustom.cpp:
+        (WebCore::constructJSReadableStream): Added JS stream constructor parameters checking.
+        * bindings/js/ReadableStreamJSSource.cpp:
+        (WebCore::ReadableStreamJSSource::ReadableStreamJSSource): Removed JS stream constructor parameters checking.
+        (WebCore::ReadableStreamJSSource::start): Changed prototype of start so that start can throw errors directly.
+        (WebCore::ReadableStreamJSSource::setInternalError): Deleted.
+        * bindings/js/ReadableStreamJSSource.h: Removed m_error, setInternalError and updated start declaration.
+
 2015-03-30  Philippe Normand  <pnormand@igalia.com>
 
         [GStreamer] More GL shared context fixes
index 3eaf9ac37ced23a8a517f69e9de7f2b99b7e84f8..a8fcf21ad99c7110f1024c24d6cc78287fa2bca0 100644 (file)
@@ -55,8 +55,6 @@ public:
     {
         return true;
     }
-
-    virtual bool isErrored() { return false; }
 };
 
 }
index 3d2e8dcb07769ae1775fbfc24d3683c4892227b6..dfd33f00e4ab3bcf7f0d8cd750279074324cc77c 100644 (file)
@@ -68,22 +68,22 @@ JSValue JSReadableStream::pipeThrough(ExecState* exec)
 
 EncodedJSValue JSC_HOST_CALL constructJSReadableStream(ExecState* exec)
 {
+    if (exec->argumentCount() && !exec->argument(0).isObject())
+        return throwVMError(exec, createTypeError(exec, ASCIILiteral("ReadableStream constructor should get an object as argument.")));
+
     DOMConstructorObject* jsConstructor = jsCast<DOMConstructorObject*>(exec->callee());
     ASSERT(jsConstructor);
     ScriptExecutionContext* scriptExecutionContext = jsConstructor->scriptExecutionContext();
 
-    Ref<ReadableStreamJSSource> source = ReadableStreamJSSource::create(exec);
-    if (source->isErrored())
-        return throwVMError(exec, source->error());
 
+    Ref<ReadableStreamJSSource> source = ReadableStreamJSSource::create(exec);
     RefPtr<ReadableStream> readableStream = ReadableStream::create(*scriptExecutionContext, Ref<ReadableStreamSource>(source.get()));
 
     VM& vm = exec->vm();
     JSGlobalObject* globalObject = exec->callee()->globalObject();
     JSReadableStream* jsReadableStream = JSReadableStream::create(JSReadableStream::createStructure(vm, globalObject, JSReadableStream::createPrototype(vm, globalObject)), jsCast<JSDOMGlobalObject*>(globalObject), readableStream.releaseNonNull());
 
-    if (!source->start())
-        return throwVMError(exec, source->error());
+    source->start(exec);
 
     return JSValue::encode(jsReadableStream);
 }
index a88c038d9750e83e1430e89e71a66d4500717b5b..d8f7fb4eeff43ba6f1dcf4ea6bf41da92207eb36 100644 (file)
@@ -34,6 +34,7 @@
 
 #include "JSDOMPromise.h"
 #include "JSReadableStream.h"
+#include "NotImplemented.h"
 #include <runtime/Error.h>
 #include <runtime/JSCJSValueInlines.h>
 #include <runtime/JSString.h>
@@ -68,21 +69,15 @@ Ref<ReadableStreamJSSource> ReadableStreamJSSource::create(JSC::ExecState* exec)
 
 ReadableStreamJSSource::ReadableStreamJSSource(JSC::ExecState* exec)
 {
-    if (!exec->argumentCount())
-        return;
-
-    if (!exec->argument(0).isObject()) {
-        setInternalError(exec, ASCIILiteral("ReadableStream constructor should get an object as argument."));
-        return;
+    if (exec->argumentCount()) {
+        ASSERT(exec->argument(0).isObject());
+        // FIXME: Implement parameters support;
     }
-
-    // FIXME: Implement parameters support
-    setInternalError(exec, ASCIILiteral("ReadableStream constructor does not support parameter yet."));
 }
 
-void ReadableStreamJSSource::setInternalError(JSC::ExecState* exec, const String& message)
+void ReadableStreamJSSource::start(JSC::ExecState*)
 {
-    m_error.set(exec->vm(), createTypeError(exec, message));
+    notImplemented();
 }
 
 } // namespace WebCore
index 17e585d69567a7c7019526c8ae6456664d72df9b..a3e437d556a97c067f6fb51e6ded2d33f152f7b3 100644 (file)
@@ -48,18 +48,10 @@ public:
     static Ref<ReadableStreamJSSource> create(JSC::ExecState*);
     ~ReadableStreamJSSource() { }
 
-    JSC::JSValue error() { return m_error.get(); }
-    bool start() { return true; }
-
-    // ReadableStreamSource API.
-    virtual bool isErrored() { return !!m_error; }
+    void start(JSC::ExecState*);
 
 private:
-    void setInternalError(JSC::ExecState*, const String&);
-
     ReadableStreamJSSource(JSC::ExecState*);
-    // m_error may be an error generated from ReadableStreamJSSource or from JS callbacks.
-    JSC::Strong<JSC::Unknown> m_error;
 };
 
 void setInternalSlotToObject(JSC::ExecState*, JSC::JSValue, JSC::PrivateName&, JSC::JSValue);