Implment BytesConsumer::tee

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

+// Copyright 2016 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/BytesConsumer.h"
+
+#include "core/testing/DummyPageHolder.h"
+#include "modules/fetch/BytesConsumerTestUtil.h"
+#include "platform/blob/BlobData.h"
+#include "platform/testing/UnitTestHelpers.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "wtf/RefPtr.h"
+
+namespace blink {
+
+namespace {
+
+using Command = BytesConsumerTestUtil::Command;
+using Result = BytesConsumer::Result;
+using ReplayingBytesConsumer = BytesConsumerTestUtil::ReplayingBytesConsumer;
+
+String toString(const Vector<char>& v)
+{
+    return String(v.data(), v.size());
+}
+
+class TestClient final : public GarbageCollectedFinalized<TestClient>, public BytesConsumer::Client {
+    USING_GARBAGE_COLLECTED_MIXIN(TestClient);
+public:
+    void onStateChange() override { ++m_numOnStateChangeCalled; }
+    int numOnStateChangeCalled() const { return m_numOnStateChangeCalled; }
+
+private:
+    int m_numOnStateChangeCalled = 0;
+};
+
+class BytesConsumerTeeTest : public ::testing::Test {
+public:
+    BytesConsumerTeeTest()
+        : m_page(DummyPageHolder::create())
+    {
+    }
+
+    Document* document() { return &m_page->document(); }
+
+private:
+    std::unique_ptr<DummyPageHolder> m_page;
+};
+
+class FakeBlobBytesConsumer : public BytesConsumer {
+public:
+    explicit FakeBlobBytesConsumer(PassRefPtr<BlobDataHandle> handle) : m_blobHandle(handle) {}
+    ~FakeBlobBytesConsumer() override {}
+
+    Result beginRead(const char** buffer, size_t* available) override
+    {
+        if (m_state == PublicState::Closed)
+            return Result::Done;
+        m_blobHandle = nullptr;
+        m_state = PublicState::Errored;
+        return Result::Error;
+    }
+    Result endRead(size_t readSize) override
+    {
+        if (m_state == PublicState::Closed)
+            return Result::Error;
+        m_blobHandle = nullptr;
+        m_state = PublicState::Errored;
+        return Result::Error;
+    }
+    PassRefPtr<BlobDataHandle> drainAsBlobDataHandle(BlobSizePolicy policy)
+    {
+        if (m_state != PublicState::ReadableOrWaiting)
+            return nullptr;
+        DCHECK(m_blobHandle);
+        if (policy == BlobSizePolicy::DisallowBlobWithInvalidSize && m_blobHandle->size() == UINT64_MAX)
+            return nullptr;
+        m_state = PublicState::Closed;
+        return m_blobHandle.release();
+    }
+
+    void setClient(Client*) override {}
+    void clearClient() override {}
+    void cancel() override {}
+    PublicState getPublicState() const override { return m_state; }
+    Error getError() const override { return Error(); }
+    String debugName() const override { return "FakeBlobBytesConsumer"; }
+
+private:
+    PublicState m_state = PublicState::ReadableOrWaiting;
+    RefPtr<BlobDataHandle> m_blobHandle;
+};
+
+TEST_F(BytesConsumerTeeTest, CreateDone)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+    src->add(Command(Command::Done));
+    EXPECT_FALSE(src->isCancelled());
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+
+    auto result1 = (new BytesConsumerTestUtil::Reader(dest1))->run();
+    auto result2 = (new BytesConsumerTestUtil::Reader(dest2))->run();
+
+    EXPECT_EQ(Result::Done, result1.first);
+    EXPECT_TRUE(result1.second.isEmpty());

[hiroshige, 2016/09/08 08:25:55]

Add EXPECT_EQ(Done, dest1->getPublicState());

[yhirano, 2016/09/08 09:41:35]

Done.

+    EXPECT_EQ(Result::Done, result2.first);
+    EXPECT_TRUE(result2.second.isEmpty());

[hiroshige, 2016/09/08 08:25:55]

Add EXPECT_EQ(Done, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:34]

Done.

+    EXPECT_FALSE(src->isCancelled());

[hiroshige, 2016/09/08 08:25:54]

How about adding here or somewhere:
  dest1->cancel();
  dest2->cancel();
  EXPECT_EQ(Done, dest1->getPublicState()); // not changing
  EXPECT_EQ(Done, dest2->getPublicState()); // not changing
  EXPECT_FALSE(src->isCancelled());
to confirm cancel does nothing after Done.

[yhirano, 2016/09/08 09:41:35]

Done.

+}
+
+TEST_F(BytesConsumerTeeTest, Read)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Data, "hello, "));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Data, "world"));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Done));
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+

[hiroshige, 2016/09/08 08:25:54]

Add EXPECT_EQ(ReadableOrWaiting, dest1->getPublicState());
Add EXPECT_EQ(ReadableOrWaiting, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:34]

Done.

+    auto result1 = (new BytesConsumerTestUtil::Reader(dest1))->run();
+    auto result2 = (new BytesConsumerTestUtil::Reader(dest2))->run();
+
+    EXPECT_EQ(Result::Done, result1.first);
+    EXPECT_EQ("hello, world", toString(result1.second));

[hiroshige, 2016/09/08 08:25:55]

Add EXPECT_EQ(Done, dest1->getPublicState());

[yhirano, 2016/09/08 09:41:34]

Done.

+    EXPECT_EQ(Result::Done, result2.first);
+    EXPECT_EQ("hello, world", toString(result2.second));

[hiroshige, 2016/09/08 08:25:54]

Add EXPECT_EQ(Done, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:35]

Done.

+    EXPECT_FALSE(src->isCancelled());
+}
+
+TEST_F(BytesConsumerTeeTest, TwoPhaseRead)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Data, "hello, "));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Data, "world"));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Done));
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+

[hiroshige, 2016/09/08 08:25:54]

Add EXPECT_EQ(ReadableOrWaiting, dest1->getPublicState());
Add EXPECT_EQ(ReadableOrWaiting, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:35]

Done.

+    auto result1 = (new BytesConsumerTestUtil::TwoPhaseReader(dest1))->run();
+    auto result2 = (new BytesConsumerTestUtil::TwoPhaseReader(dest2))->run();
+
+    EXPECT_EQ(Result::Done, result1.first);
+    EXPECT_EQ("hello, world", toString(result1.second));

[hiroshige, 2016/09/08 08:25:54]

Add EXPECT_EQ(Done, dest1->getPublicState());

[yhirano, 2016/09/08 09:41:34]

Done.

+    EXPECT_EQ(Result::Done, result2.first);
+    EXPECT_EQ("hello, world", toString(result2.second));

[hiroshige, 2016/09/08 08:25:54]

Add EXPECT_EQ(Done, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:35]

Done.

+    EXPECT_FALSE(src->isCancelled());
+}
+
+TEST_F(BytesConsumerTeeTest, Error)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+
+    src->add(Command(Command::Data, "hello, "));
+    src->add(Command(Command::Data, "world"));
+    src->add(Command(Command::Error));
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+

[hiroshige, 2016/09/08 08:25:55]

Add EXPECT_EQ(Errored, dest1->getPublicState());
Add EXPECT_EQ(Errored, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:34]

Done.

+    auto result1 = (new BytesConsumerTestUtil::TwoPhaseReader(dest1))->run();
+    auto result2 = (new BytesConsumerTestUtil::TwoPhaseReader(dest2))->run();
+
+    EXPECT_EQ(Result::Error, result1.first);
+    EXPECT_EQ(Result::Error, result2.first);

[hiroshige, 2016/09/08 08:25:54]

Add
EXPECT_TRUE(result1.second.isEmpty());
EXPECT_TRUE(result2.second.isEmpty());

If the public state before run() is Errored, then the result must be empty.
(If the public state before run() was not Error, the result doesn't have to be
empty though)

[yhirano, 2016/09/08 09:41:35]

Done.

+    EXPECT_FALSE(src->isCancelled());

[hiroshige, 2016/09/08 08:25:55]

How about adding
  dest1->cancel();
  dest2->cancel();
  EXPECT_EQ(Errored, dest1->getPublicState()); // not changing to Done
  EXPECT_EQ(Errored, dest2->getPublicState()); // not changing to Done
  EXPECT_FALSE(src->isCancelled());
to confirm cancel does nothing after errored.

[yhirano, 2016/09/08 09:41:34]

Done.

+}
+
+TEST_F(BytesConsumerTeeTest, Cancel)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+
+    src->add(Command(Command::Data, "hello, "));
+    src->add(Command(Command::Wait));
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+
+    EXPECT_FALSE(src->isCancelled());

[hiroshige, 2016/09/08 08:25:54]

EXPECT_EQ(ReadableOrWaiting, dest1->getPublicState());

[yhirano, 2016/09/08 09:41:35]

Done.

+    dest1->cancel();

[hiroshige, 2016/09/08 08:25:54]

EXPECT_EQ(Closed, dest1->getPublicState());
EXPECT_EQ(ReadableOrWaiting, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:34]

Done.

+    EXPECT_FALSE(src->isCancelled());
+    dest2->cancel();

[hiroshige, 2016/09/08 08:25:55]

EXPECT_EQ(Closed, dest1->getPublicState());
EXPECT_EQ(Closed, dest2->getPublicState());

[yhirano, 2016/09/08 09:41:35]

Done.

+    EXPECT_TRUE(src->isCancelled());
+}
+

[hiroshige, 2016/09/08 08:25:55]

Please add tests where:
- |dest1| is cancelled and |dest2| is not cancelled and becomes |Done|
- |dest1| is cancelled and |dest2| is not cancelled and becomes |Errored|
to confirm nothing wrong happens when only one branch of tee is cancelled?
e.g.
  dest1->cancel();
  EXPECT_EQ(Done, dest1->getPublicState());
  EXPECT_EQ(ReadableOrWaiting, dest2->getPublicState());
  (...Reader(dest2)...)->run();
  EXPECT_EQ(Done, dest1->getPublicState()); // check that dest1's status doesn't
change
  EXPECT_EQ(Done/Errored, dest2->getPublicState());
  // check read result for dest2
  EXPECT_FALSE(src->isCancelled());

[yhirano, 2016/09/08 09:41:35]

Done.

+TEST_F(BytesConsumerTeeTest, BlobHandle)
+{
+    RefPtr<BlobDataHandle> blobDataHandle = BlobDataHandle::create(BlobData::create(), 12345);
+    BytesConsumer* src = new FakeBlobBytesConsumer(blobDataHandle);
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+
+    RefPtr<BlobDataHandle> destBlobDataHandle1 = dest1->drainAsBlobDataHandle(BytesConsumer::BlobSizePolicy::AllowBlobWithInvalidSize);
+    RefPtr<BlobDataHandle> destBlobDataHandle2 = dest2->drainAsBlobDataHandle(BytesConsumer::BlobSizePolicy::DisallowBlobWithInvalidSize);
+    ASSERT_TRUE(destBlobDataHandle1);
+    ASSERT_TRUE(destBlobDataHandle2);
+    EXPECT_EQ(12345u, destBlobDataHandle1->size());
+    EXPECT_EQ(12345u, destBlobDataHandle2->size());
+}
+
+TEST_F(BytesConsumerTeeTest, BlobHandleWithInvalidSize)
+{
+    RefPtr<BlobDataHandle> blobDataHandle = BlobDataHandle::create(BlobData::create(), -1);
+    BytesConsumer* src = new FakeBlobBytesConsumer(blobDataHandle);
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+
+    RefPtr<BlobDataHandle> destBlobDataHandle1 = dest1->drainAsBlobDataHandle(BytesConsumer::BlobSizePolicy::AllowBlobWithInvalidSize);
+    RefPtr<BlobDataHandle> destBlobDataHandle2 = dest2->drainAsBlobDataHandle(BytesConsumer::BlobSizePolicy::DisallowBlobWithInvalidSize);
+    ASSERT_TRUE(destBlobDataHandle1);
+    ASSERT_FALSE(destBlobDataHandle2);
+    EXPECT_EQ(UINT64_MAX, destBlobDataHandle1->size());
+}
+
+TEST_F(BytesConsumerTeeTest, ConsumerCanBeErroredInTwoPhaseRead)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+    src->add(Command(Command::Data, "a"));
+    src->add(Command(Command::Wait));
+    src->add(Command(Command::Error));
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+    TestClient* client = new TestClient();
+    dest1->setClient(client);
+
+    {
+        const char* buffer = nullptr;
+        size_t available = 0;
+        while (true) {

[hiroshige, 2016/09/08 08:25:54]

Why do we need this while loop? Doesn't dest1->beginRead() return Ok for the
first invocation? To avoid this test depending on the current behavior of tee
too much?

[yhirano, 2016/09/08 09:41:34]

Hmm ok, deleted

+            auto result = dest1->beginRead(&buffer, &available);
+            if (result == Result::Ok)
+                break;
+            ASSERT_EQ(Result::ShouldWait, result);
+            testing::runPendingTasks();
+        }
+    }
+

[hiroshige, 2016/09/08 08:25:54]

ASSERT_EQ(BytesConsumer::PublicState::Errored, dest1->getPublicState());

[yhirano, 2016/09/08 09:41:34]

It's ReadableOrWaiting here.

[hiroshige, 2016/09/09 06:37:30]

Acknowledged.

+    int numOnStateChangeCalled = client->numOnStateChangeCalled();
+    EXPECT_EQ(Result::Error, (new BytesConsumerTestUtil::Reader(dest2))->run().first);
+    EXPECT_EQ(BytesConsumer::PublicState::Errored, dest1->getPublicState());
+    EXPECT_EQ(numOnStateChangeCalled + 1, client->numOnStateChangeCalled());
+    EXPECT_EQ(Result::Ok, dest1->endRead(0));

[hiroshige, 2016/09/08 08:25:54]

How about testing that we can read the data (i.e. EXPECT_EQ(buffer[0], 'a') and
endRead(1)) even when the state becomes Errored?

[yhirano, 2016/09/08 09:41:35]

Done.

+}
+
+TEST_F(BytesConsumerTeeTest, AsyncNotificationShouldBeDispatchedWhenAllDataIsConsumed)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+    src->add(Command(Command::Data, "a"));
+    src->add(Command(Command::Done));
+    TestClient* client = new TestClient();
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+

[hiroshige, 2016/09/08 08:25:54]

In this test, |src->getPublicState()| is Closed here.
Do we need another test where |src->getPublicState()| becomes Closed during
two-phase read?

[yhirano, 2016/09/08 09:41:35]

I think we don't need the former if we have the latter.

[hiroshige, 2016/09/09 06:37:30]

Acknowledged.

+    dest1->setClient(client);
+
+    const char* buffer = nullptr;
+    size_t available = 0;
+    ASSERT_EQ(Result::Ok, dest1->beginRead(&buffer, &available));
+    ASSERT_EQ(1u, available);
+    EXPECT_EQ('a', buffer[0]);
+
+    testing::runPendingTasks();

[hiroshige, 2016/09/08 08:25:54]

Add EXPECT_EQ('a', buffer[0]);
(this might be helpful in catching use-after-free if tee failed to retain
|*buffer| alive when src becomes Done)

[yhirano, 2016/09/08 09:41:35]

Done.

+    ASSERT_EQ(Result::Ok, dest1->endRead(1));
+
+    EXPECT_EQ(0, client->numOnStateChangeCalled());
+    EXPECT_EQ(BytesConsumer::PublicState::ReadableOrWaiting, dest1->getPublicState());
+    testing::runPendingTasks();
+    EXPECT_EQ(1, client->numOnStateChangeCalled());
+    EXPECT_EQ(BytesConsumer::PublicState::Closed, dest1->getPublicState());
+}
+
+TEST_F(BytesConsumerTeeTest, AsyncCloseNotificationShouldBeCancelledBySubsequentReadCall)
+{
+    ReplayingBytesConsumer* src = new ReplayingBytesConsumer(document());
+    src->add(Command(Command::Data, "a"));
+    src->add(Command(Command::Done));
+    TestClient* client = new TestClient();
+
+    BytesConsumer* dest1 = nullptr;
+    BytesConsumer* dest2 = nullptr;
+    BytesConsumer::tee(document(), src, &dest1, &dest2);
+
+    dest1->setClient(client);
+
+    const char* buffer = nullptr;
+    size_t available = 0;
+    ASSERT_EQ(Result::Ok, dest1->beginRead(&buffer, &available));
+    ASSERT_EQ(1u, available);
+    EXPECT_EQ('a', buffer[0]);
+
+    testing::runPendingTasks();
+    ASSERT_EQ(Result::Ok, dest1->endRead(1));
+    EXPECT_EQ(BytesConsumer::PublicState::ReadableOrWaiting, dest1->getPublicState());
+
+    EXPECT_EQ(Result::Done, dest1->beginRead(&buffer, &available));
+    EXPECT_EQ(0, client->numOnStateChangeCalled());
+    EXPECT_EQ(BytesConsumer::PublicState::Closed, dest1->getPublicState());
+    testing::runPendingTasks();
+    EXPECT_EQ(0, client->numOnStateChangeCalled());
+    EXPECT_EQ(BytesConsumer::PublicState::Closed, dest1->getPublicState());
+}
+
+} // namespace
+
+} // namespace blink