[Fetch API] Remove DrainingBuffer.

Source/modules/fetch/BodyStreamBufferTest.cpp

Index: Source/modules/fetch/BodyStreamBufferTest.cpp
diff --git a/Source/modules/fetch/BodyStreamBufferTest.cpp b/Source/modules/fetch/BodyStreamBufferTest.cpp
index c95cf0d496055300327b75ef113c93b3d396a98d..a3e6f504873ecff30f0662fe647e18c5ef77ceac 100644
--- a/Source/modules/fetch/BodyStreamBufferTest.cpp
+++ b/Source/modules/fetch/BodyStreamBufferTest.cpp
@@ -5,8 +5,10 @@
 #include "config.h"
 #include "modules/fetch/BodyStreamBuffer.h"
 
+#include "core/testing/DummyPageHolder.h"
 #include "modules/fetch/DataConsumerHandleTestUtil.h"
 #include "platform/testing/UnitTestHelpers.h"
+#include "wtf/OwnPtr.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -17,154 +19,295 @@ namespace {
 
 using ::testing::InSequence;
 using ::testing::_;
-using ::testing::Return;
+using ::testing::SaveArg;
 using Checkpoint = ::testing::StrictMock<::testing::MockFunction<void(int)>>;
 using Command = DataConsumerHandleTestUtil::Command;
 using ReplayingHandle = DataConsumerHandleTestUtil::ReplayingHandle;
-using MockFetchDataConsumerHandle = DataConsumerHandleTestUtil::MockFetchDataConsumerHandle;
-using MockFetchDataConsumerReader = DataConsumerHandleTestUtil::MockFetchDataConsumerReader;
 using MockFetchDataLoaderClient = DataConsumerHandleTestUtil::MockFetchDataLoaderClient;
 
-const FetchDataConsumerHandle::Reader::BlobSizePolicy kAllowBlobWithInvalidSize = FetchDataConsumerHandle::Reader::AllowBlobWithInvalidSize;
+using Result = WebDataConsumerHandle::Result;
+const WebDataConsumerHandle::Flags kNone = WebDataConsumerHandle::FlagNone;
+const Result kDone = WebDataConsumerHandle::Done;
 
-class MockDrainingStreamNotificationClient : public GarbageCollectedFinalized<MockDrainingStreamNotificationClient>, public BodyStreamBuffer::DrainingStreamNotificationClient {
-    USING_GARBAGE_COLLECTED_MIXIN(MockDrainingStreamNotificationClient);
+class BodyStreamBufferTest : public ::testing::Test {
 public:
-    static ::testing::StrictMock<MockDrainingStreamNotificationClient>* create() { return new ::testing::StrictMock<MockDrainingStreamNotificationClient>; }
-
-    DEFINE_INLINE_VIRTUAL_TRACE()
+    BodyStreamBufferTest()
     {
-        DrainingStreamNotificationClient::trace(visitor);
+        m_page = DummyPageHolder::create(IntSize(1, 1));
     }
+    ~BodyStreamBufferTest() override {}
+
+protected:
+    ExecutionContext* executionContext() { return &m_page->document(); }
 
-    MOCK_METHOD0(didFetchDataLoadFinishedFromDrainingStream, void());
+    OwnPtr<DummyPageHolder> m_page;
 };
 
-TEST(DrainingBodyStreamBufferTest, NotifyOnDestruction)
+TEST_F(BodyStreamBufferTest, CreateNullBodyStreamBuffer)
 {
-    Checkpoint checkpoint;
-    InSequence s;
+    BodyStreamBuffer* buffer = new BodyStreamBuffer();
+    EXPECT_FALSE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
+    EXPECT_TRUE(buffer->stream());
+}
 
-    MockDrainingStreamNotificationClient* client = MockDrainingStreamNotificationClient::create();
+TEST_F(BodyStreamBufferTest, LockNullBodyStreamBuffer)
+{
+    BodyStreamBuffer* buffer = new BodyStreamBuffer();
+    size_t size;
+
+    OwnPtr<FetchDataConsumerHandle> handle = buffer->lock(executionContext());
+
+    EXPECT_FALSE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+    ASSERT_TRUE(handle);
+
+    OwnPtr<FetchDataConsumerHandle::Reader> reader = handle->obtainReader(nullptr);
+    ASSERT_TRUE(reader);
+    Result r =reader->read(nullptr, 0, kNone, &size);
+    EXPECT_EQ(kDone, r);
+    reader = nullptr;
+}
 
+TEST_F(BodyStreamBufferTest, LoadNullBodyStreamBuffer)
+{
+    Checkpoint checkpoint;
+    MockFetchDataLoaderClient* client = MockFetchDataLoaderClient::create();
+
+    InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
-    EXPECT_CALL(*client, didFetchDataLoadFinishedFromDrainingStream());
+    EXPECT_CALL(*client, didFetchDataLoadedString(String("")));
     EXPECT_CALL(checkpoint, Call(2));
 
-    BodyStreamBuffer* buffer = BodyStreamBuffer::createEmpty();
-    OwnPtr<DrainingBodyStreamBuffer> drainingBuffer = DrainingBodyStreamBuffer::create(buffer, client);
+    BodyStreamBuffer* buffer = new BodyStreamBuffer();
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsString(), client);
+
+    EXPECT_FALSE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+
     checkpoint.Call(1);
-    drainingBuffer.clear();
+    testing::runPendingTasks();
     checkpoint.Call(2);
+
+    EXPECT_FALSE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
 }
 
-TEST(DrainingBodyStreamBufferTest, NotifyWhenBlobDataHandleIsDrained)
+TEST_F(BodyStreamBufferTest, LockBodyStreamBuffer)
 {
-    Checkpoint checkpoint;
-    InSequence s;
+    OwnPtr<FetchDataConsumerHandle> handle = createFetchDataConsumerHandleFromWebHandle(createWaitingDataConsumerHandle());
+    FetchDataConsumerHandle* rawHandle = handle.get();
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(handle.release());
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
 
-    OwnPtr<MockFetchDataConsumerHandle> src(MockFetchDataConsumerHandle::create());
-    OwnPtr<MockFetchDataConsumerReader> reader(MockFetchDataConsumerReader::create());
-    RefPtr<BlobDataHandle> blobDataHandle = BlobDataHandle::create();
+    handle = buffer->lock(executionContext());

[hiroshige, 2015/07/25 05:27:35]

nit: Reusing |handle| here is somehow confusing.

[yhirano, 2015/07/29 08:45:02]

Done.

 
-    MockDrainingStreamNotificationClient* client = MockDrainingStreamNotificationClient::create();
+    ASSERT_EQ(rawHandle, handle.get());
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_TRUE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+}
+
+TEST_F(BodyStreamBufferTest, LoadBodyStreamBufferAsArrayBuffer)

[hiroshige, 2015/07/25 05:27:35]

optional: do we need LoadBodyStreamBufferAsArrayBuffer/AsBlob tests? I think
these are covered by FetchDataLoader unit tests and we need only one test for
loading data type here.

[yhirano, 2015/07/29 08:45:02]

I don't have a strong opinion, but what I wanted to ensure was
BodyStreamBuffer::LoaderHolder works for each type.

+{
+    Checkpoint checkpoint;
+    MockFetchDataLoaderClient* client = MockFetchDataLoaderClient::create();
+    RefPtr<DOMArrayBuffer> arrayBuffer;
 
+    InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
+    EXPECT_CALL(*client, didFetchDataLoadedArrayBufferMock(_)).WillOnce(SaveArg<0>(&arrayBuffer));
     EXPECT_CALL(checkpoint, Call(2));
-    EXPECT_CALL(*src, obtainReaderInternal(_)).WillOnce(Return(reader.get()));
-    EXPECT_CALL(*reader, drainAsBlobDataHandle(kAllowBlobWithInvalidSize)).WillOnce(Return(blobDataHandle));
-    EXPECT_CALL(*client, didFetchDataLoadFinishedFromDrainingStream());
-    EXPECT_CALL(checkpoint, Call(3));
-    EXPECT_CALL(checkpoint, Call(4));
 
-    // |reader| is adopted by |obtainReader|.
-    ASSERT_TRUE(reader.leakPtr());
+    OwnPtr<ReplayingHandle> handle = ReplayingHandle::create();
+    handle->add(Command(Command::Data, "hello"));
+    handle->add(Command(Command::Done));
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(handle.release()));
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsArrayBuffer(), client);
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_TRUE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
 
-    BodyStreamBuffer* buffer = BodyStreamBuffer::create(src.release());
     checkpoint.Call(1);
-    OwnPtr<DrainingBodyStreamBuffer> drainingBuffer = DrainingBodyStreamBuffer::create(buffer, client);
+    testing::runPendingTasks();
     checkpoint.Call(2);
-    EXPECT_EQ(blobDataHandle, drainingBuffer->drainAsBlobDataHandle(kAllowBlobWithInvalidSize));
-    checkpoint.Call(3);
-    drainingBuffer.clear();
-    checkpoint.Call(4);
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
+    EXPECT_EQ("hello", String(static_cast<const char*>(arrayBuffer->data()), arrayBuffer->byteLength()));
 }
 
-TEST(DrainingBodyStreamBufferTest, DoNotNotifyWhenNullBlobDataHandleIsDrained)
+TEST_F(BodyStreamBufferTest, LoadBodyStreamBufferAsBlob)
 {
     Checkpoint checkpoint;
-    InSequence s;
-
-    MockDrainingStreamNotificationClient* client = MockDrainingStreamNotificationClient::create();
+    MockFetchDataLoaderClient* client = MockFetchDataLoaderClient::create();
+    RefPtr<BlobDataHandle> blobDataHandle;
 
+    InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
+    EXPECT_CALL(*client, didFetchDataLoadedBlobHandleMock(_)).WillOnce(SaveArg<0>(&blobDataHandle));
     EXPECT_CALL(checkpoint, Call(2));
-    EXPECT_CALL(*client, didFetchDataLoadFinishedFromDrainingStream());
-    EXPECT_CALL(checkpoint, Call(3));
 
-    BodyStreamBuffer* buffer = BodyStreamBuffer::createEmpty();
-    OwnPtr<DrainingBodyStreamBuffer> drainingBuffer = DrainingBodyStreamBuffer::create(buffer, client);
+    OwnPtr<ReplayingHandle> handle = ReplayingHandle::create();
+    handle->add(Command(Command::Data, "hello"));
+    handle->add(Command(Command::Done));
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(handle.release()));
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsBlobHandle("text/plain"), client);
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_TRUE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+
     checkpoint.Call(1);
-    EXPECT_FALSE(drainingBuffer->drainAsBlobDataHandle(kAllowBlobWithInvalidSize));
+    testing::runPendingTasks();
     checkpoint.Call(2);
-    drainingBuffer.clear();
-    checkpoint.Call(3);
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
+    EXPECT_EQ(5u, blobDataHandle->size());
 }
 
-TEST(DrainingBodyStreamBufferTest, DoNotNotifyWhenBufferIsLeaked)
+TEST_F(BodyStreamBufferTest, LoadBodyStreamBufferAsString)
 {
     Checkpoint checkpoint;
-    InSequence s;
-
-    MockDrainingStreamNotificationClient* client = MockDrainingStreamNotificationClient::create();
+    MockFetchDataLoaderClient* client = MockFetchDataLoaderClient::create();
 
+    InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
+    EXPECT_CALL(*client, didFetchDataLoadedString(String("hello")));
     EXPECT_CALL(checkpoint, Call(2));
-    EXPECT_CALL(checkpoint, Call(3));
 
-    BodyStreamBuffer* buffer = BodyStreamBuffer::createEmpty();
+    OwnPtr<ReplayingHandle> handle = ReplayingHandle::create();
+    handle->add(Command(Command::Data, "hello"));
+    handle->add(Command(Command::Done));
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(handle.release()));
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsString(), client);
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_TRUE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
 
-    OwnPtr<DrainingBodyStreamBuffer> drainingBuffer = DrainingBodyStreamBuffer::create(buffer, client);
     checkpoint.Call(1);
-    EXPECT_EQ(buffer, drainingBuffer->leakBuffer());
+    testing::runPendingTasks();
     checkpoint.Call(2);
-    drainingBuffer.clear();
-    checkpoint.Call(3);
+
+    EXPECT_TRUE(buffer->hasBody());
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
+}
+
+TEST_F(BodyStreamBufferTest, LockClosedHandle)
+{
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(createDoneDataConsumerHandle()));
+
+    EXPECT_EQ(ReadableStream::Readable, buffer->stream()->stateInternal());
+    testing::runPendingTasks();
+    EXPECT_EQ(ReadableStream::Closed, buffer->stream()->stateInternal());
+
+    EXPECT_FALSE(buffer->isLocked());
+    OwnPtr<FetchDataConsumerHandle> handle = buffer->lock(executionContext());
+    EXPECT_FALSE(buffer->isLocked());
+    OwnPtr<FetchDataConsumerHandle> handle2 = buffer->lock(executionContext());
+
+    EXPECT_TRUE(handle);

[hiroshige, 2015/07/25 05:27:35]

nit optional: looks a little nicer if this is moved after L218.

[yhirano, 2015/07/29 08:45:02]

Done.

+    EXPECT_TRUE(handle2);
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+    EXPECT_TRUE(buffer->hasBody());
 }
 
-TEST(DrainingBodyStreamBufferTest, NotifyOnStartLoading)
+TEST_F(BodyStreamBufferTest, LoadClosedHandle)
 {
     Checkpoint checkpoint;
+    MockFetchDataLoaderClient* client1 = MockFetchDataLoaderClient::create();
+    MockFetchDataLoaderClient* client2 = MockFetchDataLoaderClient::create();
+
     InSequence s;
+    EXPECT_CALL(checkpoint, Call(1));
+    EXPECT_CALL(*client1, didFetchDataLoadedString(String("")));
+    EXPECT_CALL(*client2, didFetchDataLoadedString(String("")));
+    EXPECT_CALL(checkpoint, Call(2));
 
-    OwnPtr<ReplayingHandle> src(ReplayingHandle::create());
-    src->add(Command(Command::Data, "hello, world."));
-    src->add(Command(Command::Done));
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(createDoneDataConsumerHandle()));
 
-    MockDrainingStreamNotificationClient* client = MockDrainingStreamNotificationClient::create();
-    MockFetchDataLoaderClient* fetchDataLoaderClient = MockFetchDataLoaderClient::create();
-    FetchDataLoader* fetchDataLoader = FetchDataLoader::createLoaderAsString();
+    EXPECT_EQ(ReadableStream::Readable, buffer->stream()->stateInternal());
+    testing::runPendingTasks();
+    EXPECT_EQ(ReadableStream::Closed, buffer->stream()->stateInternal());
 
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsString(), client1);
+    EXPECT_FALSE(buffer->isLocked());
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsString(), client2);
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+
+    checkpoint.Call(1);
+    testing::runPendingTasks();
+    checkpoint.Call(2);
+
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
+}
+
+TEST_F(BodyStreamBufferTest, LockErroredHandle)
+{
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(createUnexpectedErrorDataConsumerHandle()));
+
+    EXPECT_EQ(ReadableStream::Readable, buffer->stream()->stateInternal());
+    testing::runPendingTasks();
+    EXPECT_EQ(ReadableStream::Errored, buffer->stream()->stateInternal());
+
+    EXPECT_FALSE(buffer->isLocked());
+    OwnPtr<FetchDataConsumerHandle> handle = buffer->lock(executionContext());
+    EXPECT_FALSE(buffer->isLocked());
+    OwnPtr<FetchDataConsumerHandle> handle2 = buffer->lock(executionContext());
+
+    EXPECT_TRUE(handle);

[hiroshige, 2015/07/25 05:27:35]

ditto.

[yhirano, 2015/07/29 08:45:02]

Done.

+    EXPECT_TRUE(handle2);
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+    EXPECT_TRUE(buffer->hasBody());
+}
+
+TEST_F(BodyStreamBufferTest, LoadErroredHandle)
+{
+    Checkpoint checkpoint;
+    MockFetchDataLoaderClient* client1 = MockFetchDataLoaderClient::create();
+    MockFetchDataLoaderClient* client2 = MockFetchDataLoaderClient::create();
+
+    InSequence s;
     EXPECT_CALL(checkpoint, Call(1));
+    EXPECT_CALL(*client1, didFetchDataLoadFailed());
+    EXPECT_CALL(*client2, didFetchDataLoadFailed());
     EXPECT_CALL(checkpoint, Call(2));
-    EXPECT_CALL(checkpoint, Call(3));
-    EXPECT_CALL(checkpoint, Call(4));
-    EXPECT_CALL(*fetchDataLoaderClient, didFetchDataLoadedString(String("hello, world.")));
-    EXPECT_CALL(*client, didFetchDataLoadFinishedFromDrainingStream());
-    EXPECT_CALL(checkpoint, Call(5));
-
-    Persistent<BodyStreamBuffer> buffer = BodyStreamBuffer::create(createFetchDataConsumerHandleFromWebHandle(src.release()));
-    OwnPtr<DrainingBodyStreamBuffer> drainingBuffer = DrainingBodyStreamBuffer::create(buffer, client);
+
+    BodyStreamBuffer* buffer = new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(createUnexpectedErrorDataConsumerHandle()));
+
+    EXPECT_EQ(ReadableStream::Readable, buffer->stream()->stateInternal());
+    testing::runPendingTasks();
+    EXPECT_EQ(ReadableStream::Errored, buffer->stream()->stateInternal());
+
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsString(), client1);
+    EXPECT_FALSE(buffer->isLocked());
+    buffer->startLoading(executionContext(), FetchDataLoader::createLoaderAsString(), client2);
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_TRUE(buffer->hasPendingActivity());
+

[hiroshige, 2015/07/25 05:27:35]

If we call collectGarbase() here, can we test that multiple FetchDataLoaders are
kept alive?
(If so, please apply the same to other tests with startLoading() above)

[yhirano, 2015/07/29 08:45:02]

Yes, but I'm not sure if it should be tested. Generally we don't test x is held
by y.
Anyway, I think even if it should be tested, it should be tested separately. I
added LoaderShouldBeKeptAliveByBodyStreamBuffer.

     checkpoint.Call(1);
-    drainingBuffer->startLoading(fetchDataLoader, fetchDataLoaderClient);
-    checkpoint.Call(2);
-    drainingBuffer.clear();
-    checkpoint.Call(3);
-    // Loading continues as long as |buffer| is alive.
-    Heap::collectAllGarbage();
-    checkpoint.Call(4);
     testing::runPendingTasks();
-    checkpoint.Call(5);
+    checkpoint.Call(2);
+
+    EXPECT_FALSE(buffer->isLocked());
+    EXPECT_FALSE(buffer->hasPendingActivity());
 }
 
 } // namespace