Changeset 182140 in webkit

Timestamp:

Mar 30, 2015 8:23:24 AM (9 years ago)

Author:

youenn.fablet@crf.canon.fr

Message:

[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.

Location:

trunk

Files:

• LayoutTests/streams/readablestream-constructor-expected.txt (modified) (1 diff)
• LayoutTests/streams/readablestream-constructor.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/streams/ReadableStreamSource.h (modified) (1 diff)
• Source/WebCore/bindings/js/JSReadableStreamCustom.cpp (modified) (2 diffs)
• Source/WebCore/bindings/js/ReadableStreamJSSource.cpp (modified) (2 diffs)
• Source/WebCore/bindings/js/ReadableStreamJSSource.h (modified) (1 diff)

trunk/LayoutTests/streams/readablestream-constructor-expected.txt

@@ -3 +3 @@
 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 
 

trunk/LayoutTests/streams/readablestream-constructor.html

@@ -46 +46 @@
 }, 'ReadableStream instances should have the correct list of properties');
 
+test(function() {
+    new ReadableStream({ });
+}, 'ReadableStream can be constructed with an empty object as argument');
+
 </script>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/Modules/streams/ReadableStreamSource.h

@@ -56 +56 @@
         return true;
     }
-
-    virtual bool isErrored() { return false; }
 };
 

trunk/Source/WebCore/bindings/js/JSReadableStreamCustom.cpp

@@ -69 +69 @@
 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());
-
     RefPtr<ReadableStream> readableStream = ReadableStream::create(*scriptExecutionContext, Ref<ReadableStreamSource>(source.get()));
 
@@ -83 +84 @@
     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);

trunk/Source/WebCore/bindings/js/ReadableStreamJSSource.cpp

@@ -35 +35 @@
 #include "JSDOMPromise.h"
 #include "JSReadableStream.h"
+#include "NotImplemented.h"
 #include <runtime/Error.h>
 #include <runtime/JSCJSValueInlines.h>
@@ -69 +70 @@
 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();
 }
 

trunk/Source/WebCore/bindings/js/ReadableStreamJSSource.h

@@ -49 +49 @@
     ~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;
 };