[Fetch API] Fix a memory leak with a Response constructed with a ReadableStream

third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp

Index: third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp b/third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
index 347519606c5bc74d875fa2a4377fcd9699aeb977..478e01701322b3022faf2fe2d205157a654432e7 100644
--- a/third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
+++ b/third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
@@ -4,8 +4,10 @@
 
 #include "modules/fetch/ReadableStreamDataConsumerHandle.h"
 
+#include "bindings/core/v8/ReadableStreamOperations.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/V8BindingMacros.h"
+#include "bindings/core/v8/V8GCController.h"
 #include "core/dom/Document.h"
 #include "core/testing/DummyPageHolder.h"
 #include "modules/fetch/DataConsumerHandleTestUtil.h"
@@ -78,6 +80,17 @@ public:
         return r;
     }
 
+    PassOwnPtr<ReadableStreamDataConsumerHandle> createHandle(ScriptValue stream)
+    {
+        NonThrowableExceptionState es;
+        ScriptValue reader = ReadableStreamOperations::getReader(scriptState(), stream, es);
+        ASSERT(!reader.isEmpty());
+        ASSERT(reader.v8Value()->IsObject());
+        return ReadableStreamDataConsumerHandle::create(scriptState(), reader);
+    }
+
+    void gc() { V8GCController::collectAllGarbageForTesting(isolate()); }
+
 private:
     OwnPtr<DummyPageHolder> m_page;
 };
@@ -87,7 +100,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, Create)
     ScriptState::Scope scope(scriptState());
     ScriptValue stream(scriptState(), evalWithPrintingError("new ReadableStream"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -110,7 +123,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, EmptyStream)
     ScriptValue stream(scriptState(), evalWithPrintingError(
         "new ReadableStream({start: c => c.close()})"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -141,7 +154,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, ErroredStream)
     ScriptValue stream(scriptState(), evalWithPrintingError(
         "new ReadableStream({start: c => c.error()})"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -177,7 +190,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, Read)
         "controller.close();"
         "stream"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -239,7 +252,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, TwoPhaseRead)
         "controller.close();"
         "stream"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -301,15 +314,17 @@ TEST_F(ReadableStreamDataConsumerHandleTest, TwoPhaseRead)
     EXPECT_EQ(kDone, reader->beginRead(&buffer, kNone, &available));
 }
 
-TEST_F(ReadableStreamDataConsumerHandleTest, LockedStream)
+TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueUndefined)
 {
     ScriptState::Scope scope(scriptState());
     ScriptValue stream(scriptState(), evalWithPrintingError(
-        "var stream = new ReadableStream;"
-        "stream.getReader();"
+        "var controller;"
+        "var stream = new ReadableStream({start: c => controller = c});"
+        "controller.enqueue(undefined);"
+        "controller.close();"
         "stream"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -318,28 +333,33 @@ TEST_F(ReadableStreamDataConsumerHandleTest, LockedStream)
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
+    EXPECT_CALL(*client, didGetReadable());
+    EXPECT_CALL(checkpoint, Call(3));
 
-    char c;
-    size_t readBytes;
+    const void* buffer;
+    size_t available;
     OwnPtr<FetchDataConsumerHandle::Reader> reader = handle->obtainReader(client);
     ASSERT_TRUE(reader);
     checkpoint.Call(1);
     testing::runPendingTasks();
     checkpoint.Call(2);
-    EXPECT_EQ(kUnexpectedError, reader->read(&c, 1, kNone, &readBytes));
+    EXPECT_EQ(kShouldWait, reader->beginRead(&buffer, kNone, &available));
+    testing::runPendingTasks();
+    checkpoint.Call(3);
+    EXPECT_EQ(kUnexpectedError, reader->beginRead(&buffer, kNone, &available));
 }
 
-TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueUndefined)
+TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueNull)
 {
     ScriptState::Scope scope(scriptState());
     ScriptValue stream(scriptState(), evalWithPrintingError(
         "var controller;"
         "var stream = new ReadableStream({start: c => controller = c});"
-        "controller.enqueue(undefined);"
+        "controller.enqueue(null);"
         "controller.close();"
         "stream"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -364,17 +384,17 @@ TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueUndefined)
     EXPECT_EQ(kUnexpectedError, reader->beginRead(&buffer, kNone, &available));
 }
 
-TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueNull)
+TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueString)
 {
     ScriptState::Scope scope(scriptState());
     ScriptValue stream(scriptState(), evalWithPrintingError(
         "var controller;"
         "var stream = new ReadableStream({start: c => controller = c});"
-        "controller.enqueue(null);"
+        "controller.enqueue('hello');"
         "controller.close();"
         "stream"));
     ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
+    OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
     ASSERT_TRUE(handle);
     MockClient* client = MockClient::create();
     Checkpoint checkpoint;
@@ -399,38 +419,90 @@ TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueNull)
     EXPECT_EQ(kUnexpectedError, reader->beginRead(&buffer, kNone, &available));
 }
 
-TEST_F(ReadableStreamDataConsumerHandleTest, EnqueueString)
+TEST_F(ReadableStreamDataConsumerHandleTest, StreamReaderShouldBeWeak)
 {
-    ScriptState::Scope scope(scriptState());
-    ScriptValue stream(scriptState(), evalWithPrintingError(
-        "var controller;"
-        "var stream = new ReadableStream({start: c => controller = c});"
-        "controller.enqueue('hello');"
-        "controller.close();"
-        "stream"));
-    ASSERT_FALSE(stream.isEmpty());
-    OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream);
-    ASSERT_TRUE(handle);
-    MockClient* client = MockClient::create();
+    OwnPtr<FetchDataConsumerHandle::Reader> reader;
     Checkpoint checkpoint;
+    Persistent<MockClient> client = MockClient::create();
+    ScriptValue stream;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
+    EXPECT_CALL(checkpoint, Call(3));
+    EXPECT_CALL(*client, didGetReadable());
+    EXPECT_CALL(checkpoint, Call(4));
+
+    {
+        // We need this scope to collect local handles.
+        ScriptState::Scope scope(scriptState());
+        stream = ScriptValue(scriptState(), evalWithPrintingError("new ReadableStream()"));
+        ASSERT_FALSE(stream.isEmpty());
+        OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
+        ASSERT_TRUE(handle);
+
+        reader = handle->obtainReader(client);
+        ASSERT_TRUE(reader);
+    }
+
+    checkpoint.Call(1);
+    testing::runPendingTasks();
+    checkpoint.Call(2);
+    stream.clear();
+    gc();
+    checkpoint.Call(3);
+    testing::runPendingTasks();
+
+    checkpoint.Call(4);
+    const void* buffer;
+    size_t available;
+    EXPECT_EQ(kUnexpectedError, reader->beginRead(&buffer, kNone, &available));
+}
+
+TEST_F(ReadableStreamDataConsumerHandleTest, StreamReaderShouldBeWeakWhenReading)
+{
+    OwnPtr<FetchDataConsumerHandle::Reader> reader;
+    Checkpoint checkpoint;
+    Persistent<MockClient> client = MockClient::create();
+    ScriptValue stream;
+
+    InSequence s;
+    EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
+    EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(checkpoint, Call(3));
+    EXPECT_CALL(checkpoint, Call(4));
+    EXPECT_CALL(*client, didGetReadable());
+    EXPECT_CALL(checkpoint, Call(5));
+
+    {
+        // We need this scope to collect local handles.
+        ScriptState::Scope scope(scriptState());
+        stream = ScriptValue(scriptState(), evalWithPrintingError("new ReadableStream()"));
+        ASSERT_FALSE(stream.isEmpty());
+        OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
+        ASSERT_TRUE(handle);
+
+        reader = handle->obtainReader(client);
+        ASSERT_TRUE(reader);
+    }
 
     const void* buffer;
     size_t available;
-    OwnPtr<FetchDataConsumerHandle::Reader> reader = handle->obtainReader(client);
-    ASSERT_TRUE(reader);
     checkpoint.Call(1);
     testing::runPendingTasks();
     checkpoint.Call(2);
     EXPECT_EQ(kShouldWait, reader->beginRead(&buffer, kNone, &available));
+
     testing::runPendingTasks();
     checkpoint.Call(3);
+    stream.clear();
+    gc();
+    checkpoint.Call(4);
+    testing::runPendingTasks();
+
+    checkpoint.Call(5);
     EXPECT_EQ(kUnexpectedError, reader->beginRead(&buffer, kNone, &available));
 }