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

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "platform/heap/Handle.h"
 #include "platform/testing/UnitTestHelpers.h"
 #include "public/platform/WebDataConsumerHandle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include <v8.h>
 
@@ skipping 52 matching lines @@
     {
         v8::TryCatch block(isolate());
         v8::MaybeLocal<v8::Value> r = eval(s);
         if (block.HasCaught()) {
             ADD_FAILURE() << toCoreString(block.Exception()->ToString(isolate())).utf8().data();
             block.ReThrow();
         }
         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;
 };
 
 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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
 
     OwnPtr<FetchDataConsumerHandle::Reader> reader = handle->obtainReader(client);
     ASSERT_TRUE(reader);
     checkpoint.Call(1);
     testing::runPendingTasks();
     checkpoint.Call(2);
 }
 
 TEST_F(ReadableStreamDataConsumerHandleTest, EmptyStream)
 {
     ScriptState::Scope scope(scriptState());
     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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(3));
@@ skipping 10 matching lines @@
     checkpoint.Call(3);
     EXPECT_EQ(kDone, reader->read(&c, 1, kNone, &readBytes));
 }
 
 TEST_F(ReadableStreamDataConsumerHandleTest, ErroredStream)
 {
     ScriptState::Scope scope(scriptState());
     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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(3));
@@ skipping 15 matching lines @@
 {
     ScriptState::Scope scope(scriptState());
     ScriptValue stream(scriptState(), evalWithPrintingError(
         "var controller;"
         "var stream = new ReadableStream({start: c => controller = c});"
         "controller.enqueue(new Uint8Array([0x43, 0x44, 0x45, 0x46]));"
         "controller.enqueue(new Uint8Array([0x47, 0x48, 0x49, 0x4a]));"
         "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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(3));
@@ skipping 41 matching lines @@
 {
     ScriptState::Scope scope(scriptState());
     ScriptValue stream(scriptState(), evalWithPrintingError(
         "var controller;"
         "var stream = new ReadableStream({start: c => controller = c});"
         "controller.enqueue(new Uint8Array([0x43, 0x44, 0x45, 0x46]));"
         "controller.enqueue(new Uint8Array([0x47, 0x48, 0x49, 0x4a]));"
         "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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(3));
@@ skipping 41 matching lines @@
     EXPECT_EQ(0x48, static_cast<const char*>(buffer)[1]);
     EXPECT_EQ(0x49, static_cast<const char*>(buffer)[2]);
     EXPECT_EQ(0x4a, static_cast<const char*>(buffer)[3]);
     EXPECT_EQ(kOk, reader->endRead(4));
     EXPECT_EQ(kShouldWait, reader->beginRead(&buffer, kNone, &available));
     testing::runPendingTasks();
     checkpoint.Call(5);
     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;
 
     InSequence s;
     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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(3));
 
     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);
     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;
 
     InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(2));
     EXPECT_CALL(*client, didGetReadable());
     EXPECT_CALL(checkpoint, Call(3));
 
     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);
     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));
 }
 
 } // namespace
 
 } // namespace blink