Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(789)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp

Issue 1539803002: [Fetch API] Fix a memory leak with a Response constructed with a ReadableStream (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@response-constructed-with-stream
Patch Set: Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp ('K') | « third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp ('k') | third_party/WebKit/Source/modules/fetch/Response.cpp » ('j') | third_party/WebKit/Source/modules/fetch/Response.cpp » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 aaaadddc06c0616683102acf13d28f6ac94745d8..0dd9b23968ce729f05912413aad7b4496ff0d82a 100644
--- a/third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
+++ b/third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandleTest.cpp
@@ -5,8 +5,10 @@
#include "config.h"
#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"
@@ -79,6 +81,17 @@ public:
return r;
}
+ PassOwnPtr<ReadableStreamDataConsumerHandle> createHandle(ScriptValue stream)
+ {
+ NonThrowableExceptionState es;
+ ScriptValue reader = ReadableStreamOperations::getReader(scriptState(), stream.v8Value(), es);
+ ASSERT(!reader.isEmpty());
+ ASSERT(reader.v8Value()->IsObject());
+ return ReadableStreamDataConsumerHandle::create(scriptState(), reader.v8Value().As<v8::Object>());
+ }
+
+ void gc() { V8GCController::collectAllGarbageForTesting(isolate()); }
+
private:
OwnPtr<DummyPageHolder> m_page;
};
@@ -88,7 +101,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.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -111,7 +124,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.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -142,7 +155,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.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -178,7 +191,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, Read)
"controller.close();"
"stream"));
ASSERT_FALSE(stream.isEmpty());
- OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -240,7 +253,7 @@ TEST_F(ReadableStreamDataConsumerHandleTest, TwoPhaseRead)
"controller.close();"
"stream"));
ASSERT_FALSE(stream.isEmpty());
- OwnPtr<ReadableStreamDataConsumerHandle> handle = ReadableStreamDataConsumerHandle::create(scriptState(), stream.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -302,15 +315,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.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -319,28 +334,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.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -365,17 +385,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.v8Value());
+ OwnPtr<ReadableStreamDataConsumerHandle> handle = createHandle(stream);
ASSERT_TRUE(handle);
MockClient* client = MockClient::create();
Checkpoint checkpoint;
@@ -400,38 +420,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.v8Value());
- 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));
}
« third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp ('K') | « third_party/WebKit/Source/modules/fetch/ReadableStreamDataConsumerHandle.cpp ('k') | third_party/WebKit/Source/modules/fetch/Response.cpp » ('j') | third_party/WebKit/Source/modules/fetch/Response.cpp » ('J')

Powered by Google App Engine
This is Rietveld 408576698