Adds a new class HttpCache::Writers for multiple cache transactions reading from the network.

net/http/http_cache_writers_unittest.cc

Index: net/http/http_cache_writers_unittest.cc
diff --git a/net/http/http_cache_writers_unittest.cc b/net/http/http_cache_writers_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fe0f33e16201a03f501c0a43658799df747f566b
--- /dev/null
+++ b/net/http/http_cache_writers_unittest.cc
@@ -0,0 +1,587 @@
+// Copyright (c) 2017 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 "net/http/http_cache_writers.h"
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/run_loop.h"
+#include "net/http/http_cache.h"
+#include "net/http/http_cache_transaction.h"
+#include "net/http/http_transaction.h"
+#include "net/http/http_transaction_test_util.h"
+#include "net/http/mock_http_cache.h"
+#include "net/test/gtest_util.h"
+
+using net::test::IsError;
+using net::test::IsOk;
+
+namespace net {
+
+class MockHttpCacheTransaction : public HttpCache::Transaction {

[jkarlin, 2017/07/12 15:05:26]

What's the purpose of this class? Generally Mocks don't actually implement
anything, but this is a copy of HttpCache::Transaction with no changes.

[shivanisha, 2017/07/13 18:35:50]

Adding the overridden empty definitions.

Its more helpful in the integration CLs for the unit tests to work correctly but
I found it worth adding in this CL to get rid of the static casts from
HttpTransaction to HttpCache::Transaction.

[jkarlin, 2017/07/14 18:30:48]

Okay, let's rename it then as it's not actually a mock. Perhaps
TestHttpCacheTransaction?

[shivanisha, 2017/07/14 20:50:27]

As discussed f2f, lets keep the name as Mock since the latest patch overrides
the empty definitions. A subsequent CL will make sure that it only inherits an
abstract class rather than HttpCache::Transaction.

+ public:
+  MockHttpCacheTransaction(RequestPriority priority, HttpCache* cache)
+      : HttpCache::Transaction(priority, cache){};
+  ~MockHttpCacheTransaction() override{};
+};
+
+class WritersTest : public testing::Test {
+ public:
+  enum class DeleteTransactionType { NONE, ACTIVE, WAITING, IDLE };
+
+  WritersTest() : disk_entry_(nullptr), request_(kSimpleGET_Transaction) {}
+
+  ~WritersTest() override {
+    if (disk_entry_)
+      disk_entry_->Close();
+  }
+
+  void CreateWriters(const std::string& url) {
+    cache_.CreateBackendEntry(kSimpleGET_Transaction.url, &disk_entry_,
+                              nullptr);
+    writers_ = base::MakeUnique<HttpCache::Writers>(disk_entry_);
+  }
+
+  std::unique_ptr<HttpTransaction> CreateNetworkTransaction() {
+    std::unique_ptr<HttpTransaction> transaction;
+    MockNetworkLayer* network_layer = cache_.network_layer();
+    network_layer->CreateTransaction(DEFAULT_PRIORITY, &transaction);
+    return transaction;
+  }
+
+  void CreateWritersAddTransaction(bool is_exclusive = false) {
+    TestCompletionCallback callback;
+
+    // Create and Start a mock network transaction.
+    std::unique_ptr<HttpTransaction> network_transaction;
+    network_transaction = CreateNetworkTransaction();
+    network_transaction->Start(&request_, callback.callback(),
+                               NetLogWithSource());
+    base::RunLoop().RunUntilIdle();
+
+    // Create a mock cache transaction.
+    std::unique_ptr<MockHttpCacheTransaction> transaction =
+        base::MakeUnique<MockHttpCacheTransaction>(DEFAULT_PRIORITY,
+                                                   cache_.http_cache());
+
+    CreateWriters(kSimpleGET_Transaction.url);
+    EXPECT_TRUE(writers_->IsEmpty());
+    writers_->AddTransaction(transaction.get(), std::move(network_transaction),
+                             is_exclusive);
+    EXPECT_TRUE(writers_->HasTransaction(transaction.get()));
+    transactions_.push_back(std::move(transaction));
+  }
+
+  void CreateWritersAddTransactionPriority(net::RequestPriority priority,
+                                           bool is_exclusive = false) {
+    CreateWritersAddTransaction(is_exclusive);
+    MockHttpCacheTransaction* transaction = transactions_.begin()->get();
+    transaction->SetPriority(priority);
+  }
+
+  void AddTransactionToExistingWriters() {
+    EXPECT_TRUE(writers_);
+
+    // Create a mock cache transaction.
+    std::unique_ptr<MockHttpCacheTransaction> transaction =
+        base::MakeUnique<MockHttpCacheTransaction>(DEFAULT_PRIORITY,
+                                                   cache_.http_cache());
+
+    writers_->AddTransaction(transaction.get(), nullptr, false);
+    transactions_.push_back(std::move(transaction));
+  }
+
+  int Read(std::string* result) {
+    EXPECT_TRUE(transactions_.size() >= (size_t)1);
+    MockHttpCacheTransaction* transaction = transactions_.begin()->get();
+    TestCompletionCallback callback;
+
+    std::string content;
+    int rv = 0;
+    do {
+      scoped_refptr<IOBuffer> buf(new IOBuffer(256));

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

nit, suggestion: 256->constant.  

[shivanisha, 2017/07/13 18:35:51]

added static const int kDefaultBufferSize = 256

+      rv = writers_->Read(buf.get(), 256, callback.callback(), transaction);
+      if (rv == ERR_IO_PENDING) {
+        rv = callback.WaitForResult();
+        base::RunLoop().RunUntilIdle();
+      }
+
+      if (rv > 0)
+        content.append(buf->data(), rv);
+      else if (rv < 0)
+        return rv;
+    } while (rv > 0);
+
+    result->swap(content);
+    return OK;
+  }
+
+  int ReadAll() {

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

nit, suggestion: I'd find this more readable if ReadAll() was defined
immediately after ReadAllDeleteTransaction().

[shivanisha, 2017/07/13 18:35:50]

Moved

+    return ReadAllDeleteTransaction(DeleteTransactionType::NONE);
+  }
+
+  void ReadVerifyTwoDifferentBufferLengths(
+      const std::vector<int>& buffer_lengths) {
+    EXPECT_EQ(2u, buffer_lengths.size());
+    EXPECT_EQ(2u, transactions_.size());
+
+    std::vector<std::string> results(buffer_lengths.size());
+
+    // Check only the 1st Read and not the complete response because the smaller
+    // buffer transaction will need to read the remaining response from the
+    // cache which will be tested when integrated with MockHttpCacheTransaction
+    // layer.
+
+    int rv = 0;
+
+    std::vector<scoped_refptr<IOBuffer>> bufs;
+    for (auto buffer_length : buffer_lengths)
+      bufs.push_back(new IOBuffer(buffer_length));
+
+    std::vector<TestCompletionCallback> callbacks(buffer_lengths.size());
+
+    // Multiple transactions should be able to read with different sized
+    // buffers.
+    for (size_t i = 0; i < transactions_.size(); i++) {
+      rv = writers_->Read(bufs[i].get(), buffer_lengths[i],
+                          callbacks[i].callback(), transactions_[i].get());
+      EXPECT_EQ(ERR_IO_PENDING, rv);  // Since the default is asynchronous.
+    }
+
+    // If first buffer is smaller, then the second one will only read the
+    // smaller length as well.
+    std::vector<size_t> expected_lengths = {

[jkarlin, 2017/07/12 15:05:26]

Why size_t? buffer_lengths is int, and you need it for the EXPECT_EQ below.

[shivanisha, 2017/07/13 18:35:50]

Changed to int.

+        buffer_lengths[0], buffer_lengths[0] < buffer_lengths[1]
+                               ? buffer_lengths[0]
+                               : buffer_lengths[1]};
+
+    for (size_t i = 0; i < callbacks.size(); i++) {
+      rv = callbacks[i].WaitForResult();
+      EXPECT_EQ(expected_lengths[i], (size_t)rv);

[jkarlin, 2017/07/12 15:05:26]

static_cast, but you shouldn't need to cat if you make expected_lengths an int
vector.

[shivanisha, 2017/07/13 18:35:50]

Done

+      results[i].append(bufs[i]->data(), expected_lengths[i]);
+    }
+
+    if (expected_lengths[0] == expected_lengths[1]) {

[jkarlin, 2017/07/12 15:05:26]

Why these checks? They're redundant with expected_lengths checks above.

[shivanisha, 2017/07/13 18:35:50]

Replaced with just one check:
EXPECT_EQ(results[0].substr(0, expected_lengths[1]), results[1]);

+      EXPECT_EQ(results[0], results[1]);
+    } else {
+      EXPECT_EQ(results[0].substr(0, expected_lengths[1]), results[1]);

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

So it took me a moment to recognize that this was depending on
expected_lengths[1] <= expected_lengths[0] in all cases.  Given that that's true
(i.e. that the code reader needs to puzzle over the code), why bother with the
conditional?  I believe this EXPECT_EQ is equivalent to the one above on line
168 if expected_lengths[0] == expected_lengths[0].

[shivanisha, 2017/07/13 18:35:50]

Replaced with just one check:
EXPECT_EQ(results[0].substr(0, expected_lengths[1]), results[1]);

+    }

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

Since we're doing copying (i.e. touching the data) can we also check that the
data read is accurate?

[shivanisha, 2017/07/13 18:35:51]

done

+  }
+
+  int ReadAllDeleteTransaction(DeleteTransactionType deleteType) {

[jkarlin, 2017/07/12 15:05:26]

This method could use a comment

[shivanisha, 2017/07/13 18:35:50]

done

+    EXPECT_TRUE(transactions_.size() >= 3u);
+
+    unsigned int delete_index = 0;
+    switch (deleteType) {
+      case DeleteTransactionType::NONE:
+        break;
+      case DeleteTransactionType::ACTIVE:
+        delete_index = 0;

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

Why set it explicitly for this value and not for NONE?  (Given that it's being
set to the default here?)

[shivanisha, 2017/07/13 18:35:50]

Changed the default value to std::numeric_limits<int>::max() for NONE to
differentiate it from 0 for ACTIVE.

+        break;
+      case DeleteTransactionType::WAITING:
+        delete_index = 1;
+        break;
+      case DeleteTransactionType::IDLE:
+        delete_index = 2;
+        break;
+    }
+
+    std::vector<std::string> results(transactions_.size());
+    int rv = 0;
+    bool first_iter = true;
+    do {
+      std::vector<scoped_refptr<IOBuffer>> bufs;
+      std::vector<TestCompletionCallback> callbacks(transactions_.size());
+
+      for (size_t i = 0; i < transactions_.size(); i++) {
+        bufs.push_back(new IOBuffer(256));
+        if (!first_iter && deleteType != DeleteTransactionType::NONE &&

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

nit, suggestion: Explanatory comment?

[shivanisha, 2017/07/13 18:35:51]

done.

+            i == delete_index)
+          continue;
+
+        // For it to be an idle transaction, do not invoke Read.
+        if (deleteType == DeleteTransactionType::IDLE && i == delete_index)
+          continue;
+
+        rv = writers_->Read(bufs[i].get(), 256, callbacks[i].callback(),
+                            transactions_[i].get());
+        EXPECT_EQ(ERR_IO_PENDING, rv);  // Since the default is asynchronous.
+      }
+
+      if (first_iter && deleteType != DeleteTransactionType::NONE) {
+        writers_->RemoveTransaction(transactions_.at(delete_index).get());
+      }
+
+      std::vector<int> rvs;
+      for (size_t i = 0; i < callbacks.size(); i++) {
+        if (i == delete_index && deleteType != DeleteTransactionType::NONE)
+          continue;
+        rv = callbacks[i].WaitForResult();

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

nit, suggestion: I personally wouldn't bother with the intermediate assignment
to rv.  (Dependent on some nits & suggestions below.)

+        rvs.push_back(rv);
+      }
+
+      // Verify all transactions should read the same length buffer.
+      for (size_t i = 1; i < rvs.size(); i++) {
+        EXPECT_EQ(rvs[i - 1], rvs[i]);

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

Given that the code below is pretty clearly assuming that all the rvs will be
the same, I'm tempted to suggest this be an ASSERT rather than an EXPECT, but up
to you.  

[shivanisha, 2017/07/13 18:35:51]

Changed to an ASSERT and also had to change the return type of the function to
void for using an ASSERT since ASSERT returns from the function and the value of
rv is now verified to be OK at the end of the function instead of returning.

+      }
+
+      if (rv > 0) {

[Randy Smith (Not in Mondays), 2017/07/11 18:51:33]

Given that the above is an EXPECT_EQ rather than an ASSERT_EQ, the code here
can't make the assumption that all of the rvs actually were the same.  Given
that, I'd suggest explicitly referring to rv[rvs.size()-1] just so the code
reader can see directly what "rv" is being used.

[shivanisha, 2017/07/13 18:35:50]

Changed to assert above

+        for (size_t i = 0; i < results.size(); i++) {
+          if (i == delete_index && deleteType != DeleteTransactionType::NONE &&
+              deleteType != DeleteTransactionType::ACTIVE) {
+            continue;
+          }
+          results.at(i).append(bufs[i]->data(), rv);
+        }
+      }
+      first_iter = false;
+    } while (rv > 0);
+
+    for (size_t i = 0; i < results.size(); i++) {
+      if (i == delete_index && deleteType != DeleteTransactionType::NONE &&
+          deleteType != DeleteTransactionType::ACTIVE) {
+        continue;
+      }
+      EXPECT_EQ(kSimpleGET_Transaction.data, results[i]);
+    }
+
+    return rv;
+  }
+
+  int ReadCacheWriteFailure(std::vector<std::string>* results) {
+    int rv = 0;
+    int active_transaction_rv = 0;
+    bool first_iter = true;
+    do {
+      std::vector<scoped_refptr<IOBuffer>> bufs;
+      std::vector<TestCompletionCallback> callbacks(results->size());
+
+      // Fail the request.
+      cache_.disk_cache()->set_soft_failures(true);
+
+      // We have to open the entry again to propagate the failure flag.
+      disk_cache::Entry* en;
+      cache_.OpenBackendEntry(kSimpleGET_Transaction.url, &en);
+      en->Close();
+
+      for (size_t i = 0; i < transactions_.size(); i++) {
+        bufs.push_back(new IOBuffer(30));
+
+        if (!first_iter && i > 0)
+          break;
+        rv = writers_->Read(bufs[i].get(), 30, callbacks[i].callback(),
+                            transactions_[i].get());
+        EXPECT_EQ(ERR_IO_PENDING, rv);  // Since the default is asynchronous.
+      }
+
+      for (size_t i = 0; i < callbacks.size(); i++) {
+        // Only active transaction should succeed.
+        if (i == 0) {
+          active_transaction_rv = callbacks[i].WaitForResult();
+          EXPECT_TRUE(active_transaction_rv >= 0);
+          results->at(0).append(bufs[i]->data(), active_transaction_rv);
+        } else if (first_iter) {
+          rv = callbacks[i].WaitForResult();
+          EXPECT_EQ(ERR_CACHE_WRITE_FAILURE, rv);
+        }
+      }
+
+      first_iter = false;
+    } while (active_transaction_rv > 0);
+
+    return active_transaction_rv;
+  }
+
+  int ReadNetworkFailure(std::vector<std::string>* results, Error error) {
+    int rv = 0;
+    std::vector<scoped_refptr<IOBuffer>> bufs;
+    std::vector<TestCompletionCallback> callbacks(results->size());
+
+    for (size_t i = 0; i < transactions_.size(); i++) {
+      bufs.push_back(new IOBuffer(30));
+
+      rv = writers_->Read(bufs[i].get(), 30, callbacks[i].callback(),
+                          transactions_[i].get());
+      EXPECT_EQ(ERR_IO_PENDING, rv);  // Since the default is asynchronous.
+    }
+
+    for (auto& callback : callbacks) {
+      rv = callback.WaitForResult();
+      EXPECT_EQ(error, rv);
+    }
+
+    return error;
+  }
+
+  bool StopCaching() {
+    MockHttpCacheTransaction* transaction = transactions_.begin()->get();
+    EXPECT_TRUE(transaction);
+    return writers_->StopCaching(transaction);
+  }
+
+  void RemoveFirstTransaction() {
+    MockHttpCacheTransaction* transaction = transactions_.begin()->get();
+    EXPECT_TRUE(transaction);
+    writers_->RemoveTransaction(transaction);
+  }
+
+  void UpdateAndVerifyPriority(RequestPriority priority) {
+    writers_->UpdatePriority();
+    EXPECT_EQ(priority, writers_->priority_);
+  }
+
+  MockHttpCache cache_;
+  std::unique_ptr<HttpCache::Writers> writers_;
+  disk_cache::Entry* disk_entry_;
+
+  // Should be before transactions_ since it is accessed in the network
+  // transaction's destructor.
+  MockHttpRequest request_;
+
+  std::vector<std::unique_ptr<MockHttpCacheTransaction>> transactions_;
+};
+
+// Tests successful addition of a transaction.
+TEST_F(WritersTest, AddTransaction) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+}
+
+// Tests successful addition of multiple transactions.
+TEST_F(WritersTest, AddManyTransactions) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  for (int i = 0; i < 5; i++)
+    AddTransactionToExistingWriters();
+
+  EXPECT_EQ(6, writers_->CountTransactionsForTesting());
+}
+
+// Tests that CanAddWriters should return false if it is writing exclusively.
+TEST_F(WritersTest, AddTransactionsExclusive) {
+  CreateWritersAddTransaction(true /* is_exclusive */);
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_FALSE(writers_->CanAddWriters());
+}
+
+// Tests StopCaching should not stop caching if there are multiple writers.
+TEST_F(WritersTest, StopCachingMultipleWriters) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+
+  EXPECT_FALSE(StopCaching());
+  EXPECT_TRUE(writers_->CanAddWriters());
+}
+
+// Tests StopCaching should stop caching if there is a single writer.
+TEST_F(WritersTest, StopCaching) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(StopCaching());
+  EXPECT_FALSE(writers_->CanAddWriters());
+}
+
+// Tests removing of an idle transaction and change in priority.
+TEST_F(WritersTest, RemoveIdleTransaction) {
+  CreateWritersAddTransactionPriority(HIGHEST);
+  UpdateAndVerifyPriority(HIGHEST);
+
+  AddTransactionToExistingWriters();
+  UpdateAndVerifyPriority(HIGHEST);
+
+  EXPECT_FALSE(writers_->IsEmpty());
+  EXPECT_EQ(2, writers_->CountTransactionsForTesting());
+
+  RemoveFirstTransaction();
+  EXPECT_EQ(1, writers_->CountTransactionsForTesting());
+
+  UpdateAndVerifyPriority(DEFAULT_PRIORITY);
+}
+
+// Tests that Read is successful.
+TEST_F(WritersTest, Read) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  std::string content;
+  int rv = Read(&content);
+
+  EXPECT_THAT(rv, IsOk());
+  std::string expected(kSimpleGET_Transaction.data);
+  EXPECT_EQ(expected, content);
+}
+
+// Tests that multiple transactions can read the same data simultaneously.
+TEST_F(WritersTest, ReadMultiple) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  int rv = ReadAll();
+  EXPECT_THAT(rv, IsOk());
+}
+
+// Tests that multiple transactions can read the same data simultaneously.
+TEST_F(WritersTest, ReadMultipleDifferentBufferSizes) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+
+  std::vector<int> buffer_lengths{20, 10};
+  ReadVerifyTwoDifferentBufferLengths(buffer_lengths);
+}
+
+// Same as above but tests the first transaction having smaller buffer size
+// than the next.
+TEST_F(WritersTest, ReadMultipleDifferentBufferSizes1) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+
+  std::vector<int> buffer_lengths{10, 20};
+  ReadVerifyTwoDifferentBufferLengths(buffer_lengths);
+}
+
+// Tests that ongoing Read completes even when active transaction is deleted
+// mid-read. Any transactions waiting should be able to get the read buffer.
+TEST_F(WritersTest, ReadMultipleDeleteActiveTransaction) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  int rv = ReadAllDeleteTransaction(DeleteTransactionType::ACTIVE);
+  EXPECT_THAT(rv, IsOk());
+}
+
+// Tests that removing a waiting for read transaction does not impact other
+// transactions.
+TEST_F(WritersTest, ReadMultipleDeleteWaitingTransaction) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  std::vector<std::string> contents(4);
+  int rv = ReadAllDeleteTransaction(DeleteTransactionType::WAITING);
+  EXPECT_THAT(rv, IsOk());
+}
+
+// Tests that removing an idle transaction does not impact other transactions.
+TEST_F(WritersTest, ReadMultipleDeleteIdleTransaction) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  std::vector<std::string> contents(3);
+  int rv = ReadAllDeleteTransaction(DeleteTransactionType::IDLE);
+  EXPECT_THAT(rv, IsOk());
+}
+
+// Tests cache write failure.
+TEST_F(WritersTest, ReadMultipleCacheWriteFailed) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  std::vector<std::string> contents(3);
+  int rv = ReadCacheWriteFailure(&contents);
+
+  EXPECT_THAT(rv, IsOk());
+  std::string expected(kSimpleGET_Transaction.data);
+
+  // Only active_transaction_ should succeed.
+  EXPECT_EQ(expected, contents.at(0));
+
+  // No new transactions should now be added.
+  EXPECT_FALSE(writers_->CanAddWriters());
+}
+
+// Tests that network read failure fails all transactions: active, waiting and
+// idle.
+TEST_F(WritersTest, ReadMultipleNetworkReadFailed) {
+  ScopedMockTransaction transaction(kSimpleGET_Transaction);
+  transaction.read_return_code = ERR_INTERNET_DISCONNECTED;
+  MockHttpRequest request(transaction);
+  request_ = request;
+
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  std::vector<std::string> contents(3);
+  int rv = ReadNetworkFailure(&contents, ERR_INTERNET_DISCONNECTED);
+
+  EXPECT_EQ(ERR_INTERNET_DISCONNECTED, rv);
+}
+
+// Tests moving idle writers to readers.
+TEST_F(WritersTest, MoveIdleWritersToReaders) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_TRUE(writers_->CanAddWriters());
+  AddTransactionToExistingWriters();
+  AddTransactionToExistingWriters();
+
+  EXPECT_FALSE(writers_->IsEmpty());
+  writers_->RemoveAllIdleWriters();
+  EXPECT_TRUE(writers_->IsEmpty());
+}
+
+// Tests GetWriterLoadState.
+TEST_F(WritersTest, GetWriterLoadState) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  EXPECT_EQ(LOAD_STATE_IDLE, writers_->GetWriterLoadState());
+}
+
+// Tests truncating the entry via Writers.
+TEST_F(WritersTest, TruncateEntry) {
+  CreateWritersAddTransaction();
+  EXPECT_FALSE(writers_->IsEmpty());
+
+  std::string content;
+  int rv = Read(&content);
+
+  EXPECT_THAT(rv, IsOk());
+  std::string expected(kSimpleGET_Transaction.data);
+  EXPECT_EQ(expected, content);
+
+  writers_->TruncateEntry();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(writers_->IsTruncatedForTesting());
+}
+
+}  // namespace net