[Streams API] Collecting a ReadableStreamReader should not unlock its stream
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 4 Apr 2015 17:16:21 +0000 (17:16 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 4 Apr 2015 17:16:21 +0000 (17:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143333

Reviewed by Benjamin Poulain.

Source/WebCore:

This patch stores as a boolean whether the stream is locked to a reader.
In case the reader forget to unlock the stream, the reader can be collected and destructor called.
In that case, the link between reader and stream will be reset but the stream will remain in locked state.

Covered by new test in streams/readablestreamreader-constructor.html.

* Modules/streams/ReadableStream.h:
(WebCore::ReadableStream::isLocked):
(WebCore::ReadableStream::lock):
(WebCore::ReadableStream::release):
(WebCore::ReadableStream::releaseButKeepLocked): Introduced to cut the link between reader and stream but stil keeping the stream locked.
* Modules/streams/ReadableStreamReader.cpp:
(WebCore::ReadableStreamReader::~ReadableStreamReader):
* bindings/js/JSReadableStreamCustom.cpp:
(WebCore::JSReadableStream::getReader):
* bindings/js/JSReadableStreamReaderCustom.cpp:
(WebCore::constructJSReadableStreamReader):

LayoutTests:

* streams/readablestreamreader-constructor-expected.txt:
* streams/readablestreamreader-constructor.html:

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

LayoutTests/ChangeLog
LayoutTests/streams/readablestreamreader-constructor-expected.txt
LayoutTests/streams/readablestreamreader-constructor.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableStream.h
Source/WebCore/Modules/streams/ReadableStreamReader.cpp
Source/WebCore/bindings/js/JSReadableStreamCustom.cpp
Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp

index cab5043efcc65f00be96821c9ee12f392bbfd306..08d21b1fc53a265b0ba6075c121ec430ec3cbda5 100644 (file)
@@ -1,3 +1,13 @@
+2015-04-04  Xabier Rodriguez Calvar <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Collecting a ReadableStreamReader should not unlock its stream
+        https://bugs.webkit.org/show_bug.cgi?id=143333
+
+        Reviewed by Benjamin Poulain.
+
+        * streams/readablestreamreader-constructor-expected.txt:
+        * streams/readablestreamreader-constructor.html:
+
 2015-04-04  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Implement ES6 Object.getOwnPropertySymbols
index 8a976348a73803392b329f365cc7be9b4752e6e9..b50efd2ba721e62e50f6238426db0e5125a32927 100644 (file)
@@ -3,4 +3,5 @@ PASS ReadableStreamReader constructor should get a ReadableStream object as argu
 PASS ReadableStream instances should have the correct list of properties 
 PASS ReadableStreamReader closed should always return the same promise object 
 PASS ReadableStream getReader should throw if ReadableStream is locked 
+PASS Collecting a ReadableStreamReader should not unlock its stream. 
 
index 2bd073afa084f6523a76f014fef14183c1f42ad9..ea46bf3d2d3f88a16d5b45359f86ac449252105c 100644 (file)
@@ -1,6 +1,7 @@
 <!DOCTYPE html>
 <script src='../resources/testharness.js'></script>
 <script src='../resources/testharnessreport.js'></script>
+<script src='../resources/gc.js'></script>
 <script>
  
 test(function() {
@@ -82,4 +83,11 @@ test(function() {
 
 }, 'ReadableStream getReader should throw if ReadableStream is locked');
 
+test(function() {
+    rs = new ReadableStream({});
+    rs.getReader();
+    window.gc();
+    assert_throws(new TypeError(), function() { rs.getReader(); });
+}, 'Collecting a ReadableStreamReader should not unlock its stream.');
+
 </script>
index 8956ca29bb97dc6a007045ac4f9d53a5e69ecb2a..0969459ca97ff5f6249e799a69adf1cf06edc13c 100644 (file)
@@ -1,3 +1,28 @@
+2015-04-04  Xabier Rodriguez Calvar <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [Streams API] Collecting a ReadableStreamReader should not unlock its stream
+        https://bugs.webkit.org/show_bug.cgi?id=143333
+
+        Reviewed by Benjamin Poulain.
+
+        This patch stores as a boolean whether the stream is locked to a reader.
+        In case the reader forget to unlock the stream, the reader can be collected and destructor called.
+        In that case, the link between reader and stream will be reset but the stream will remain in locked state.
+
+        Covered by new test in streams/readablestreamreader-constructor.html.
+
+        * Modules/streams/ReadableStream.h:
+        (WebCore::ReadableStream::isLocked):
+        (WebCore::ReadableStream::lock):
+        (WebCore::ReadableStream::release):
+        (WebCore::ReadableStream::releaseButKeepLocked): Introduced to cut the link between reader and stream but stil keeping the stream locked.
+        * Modules/streams/ReadableStreamReader.cpp:
+        (WebCore::ReadableStreamReader::~ReadableStreamReader):
+        * bindings/js/JSReadableStreamCustom.cpp:
+        (WebCore::JSReadableStream::getReader):
+        * bindings/js/JSReadableStreamReaderCustom.cpp:
+        (WebCore::constructJSReadableStreamReader):
+
 2015-04-03  Zan Dobersek  <zdobersek@igalia.com>
 
         MediaDevices possesses a vtable despite using ImplementationLacksVTable IDL attribute
index 263247ba828b0815830ba10a8657b74dad0bb31d..a1c84ed61b3fde1ff2da4357338a1bc99cda1fd3 100644 (file)
@@ -58,10 +58,13 @@ public:
     virtual ~ReadableStream();
 
     ReadableStreamReader* reader() { return m_reader; }
-    void lock(ReadableStreamReader& reader) { m_reader = &reader; }
-    void release() { m_reader = nullptr; }
     virtual Ref<ReadableStreamReader> createReader() = 0;
 
+    bool isLocked() const { return m_isLocked; }
+    void lock(ReadableStreamReader&);
+    void release();
+    void releaseButKeepLocked();
+
     State internalState() { return m_state; }
 
 protected:
@@ -75,8 +78,29 @@ private:
     State m_state;
     Ref<ReadableStreamSource> m_source;
     ReadableStreamReader* m_reader { nullptr };
+    bool m_isLocked { false };
 };
 
+inline void ReadableStream::lock(ReadableStreamReader& reader)
+{
+    m_reader = &reader;
+    m_isLocked = true;
+}
+
+inline void ReadableStream::release()
+{
+    m_reader = nullptr;
+    m_isLocked = false;
+}
+
+inline void ReadableStream::releaseButKeepLocked()
+{
+    m_reader = nullptr;
+    m_isLocked = true;
+}
+
+
+
 }
 
 #endif
index 6f39989940aadc82186e2d555715e3d3677e2a6c..209cd5bc96c608adad81222ac2a729126cffe8af 100644 (file)
@@ -57,7 +57,7 @@ ReadableStreamReader::~ReadableStreamReader()
     readableStreamReaderCounter.decrement();
 #endif
     if (m_stream) {
-        m_stream->release();
+        m_stream->releaseButKeepLocked();
         m_stream = nullptr;
     }
 }
index 59865155a83cb03eadc0f541beca7e75f2f5b420..581e4363fd180b37ae19829699b1f2de40269a72 100644 (file)
@@ -54,7 +54,7 @@ JSValue JSReadableStream::cancel(ExecState* exec)
 
 JSValue JSReadableStream::getReader(ExecState* exec)
 {
-    if (impl().reader())
+    if (impl().isLocked())
         return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("ReadableStream is locked")));
     return toJS(exec, globalObject(), impl().createReader());
 }
index 61e6aed4731290feee7e57157e4fe6eaf0e6a96a..f06cb022b4431875df0129a09df31b9e08f9916b 100644 (file)
@@ -106,7 +106,7 @@ EncodedJSValue JSC_HOST_CALL constructJSReadableStreamReader(ExecState* exec)
     if (!stream)
         return throwVMError(exec, createTypeError(exec, ASCIILiteral("ReadableStreamReader constructor parameter is not a ReadableStream")));
 
-    if (stream->impl().reader())
+    if (stream->impl().isLocked())
         return throwVMError(exec, createTypeError(exec, ASCIILiteral("ReadableStreamReader constructor parameter is a locked ReadableStream")));
 
     return JSValue::encode(toJS(exec, stream->globalObject(), stream->impl().createReader()));