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

net/http/http_cache_writers.cc

+// 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 <algorithm>
+#include <utility>
+#include "net/disk_cache/disk_cache.h"
+#include "net/http/http_cache_transaction.h"
+
+namespace net {
+
+HttpCache::Writers::Writers(HttpCache* cache, ActiveEntry* entry)
+    : cache_(cache), entry_(entry), weak_factory_(this) {
+  io_callback_ =
+      base::Bind(&HttpCache::Writers::OnIOComplete, weak_factory_.GetWeakPtr());
+}
+
+HttpCache::Writers::~Writers() {}
+
+int HttpCache::Writers::Read(scoped_refptr<IOBuffer> buf,
+                             int buf_len,
+                             const CompletionCallback& callback,
+                             Transaction* transaction) {
+  DCHECK(buf);
+  DCHECK_GT(buf_len, 0);
+  DCHECK(!callback.is_null());
+  DCHECK(transaction);
+
+  // If another transaction invoked a Read which is currently ongoing , then
+  // this transaction waits for the read to complete and gets its buffer filled
+  // with the data returned from that read.
+  if (next_state_ != State::NONE) {
+    WaitingForRead waiting_transaction(transaction, buf, buf_len, callback);
+    waiting_for_read_.push_back(waiting_transaction);
+    return ERR_IO_PENDING;
+  }
+
+  DCHECK_EQ(next_state_, State::NONE);
+  DCHECK(callback_.is_null());
+  active_transaction_ = transaction;

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

DCHECK_EQ(nullptr, active_transaction_.get())?  (Suggestion)

[shivanisha, 2017/06/07 15:19:19]

done

+
+  read_buf_ = std::move(buf);
+  io_buf_len_ = buf_len;
+  next_state_ = State::NETWORK_READ;
+
+  int rv = DoLoop(OK);
+  if (rv == ERR_IO_PENDING) {
+    callback_ = callback;
+  }
+
+  return rv;
+}
+
+bool HttpCache::Writers::StopCaching(Transaction* transaction) {
+  // If this is the only transaction in Writers, then stopping will be
+  // successful. If not, then we will not stop caching since there are
+  // other consumers waiting to read from the cache.
+  if (all_writers_.size() == 1) {
+    DCHECK(all_writers_.count(transaction));
+    network_read_only_ = true;
+    return true;
+  }
+  return false;
+}
+
+void HttpCache::Writers::AddTransaction(
+    Transaction* transaction,
+    std::unique_ptr<HttpTransaction> network_transaction) {
+  DCHECK(transaction);
+  DCHECK(CanAddWriters());
+
+  std::pair<TransactionSet::iterator, bool> return_val =
+      all_writers_.insert(transaction);
+  DCHECK_EQ(return_val.second, true);
+
+  if (network_transaction) {
+    DCHECK(!network_transaction_);
+    network_transaction_ = std::move(network_transaction);
+  }
+
+  DCHECK(network_transaction_);
+
+  priority_ = std::max(transaction->priority(), priority_);
+  network_transaction_->SetPriority(priority_);
+}
+
+void HttpCache::Writers::SetExclusive() {
+  DCHECK(all_writers_.size() <= 1);
+  is_exclusive_ = true;
+}
+
+void HttpCache::Writers::RemoveTransaction(Transaction* transaction) {

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

Suggestion: DCHECK that the transaction was found in one of the queues?  (I.e.
that we weren't passed a random transaction.)

[shivanisha, 2017/06/07 15:19:18]

DCHECK(it != all_writers_.end()); below takes care of that.

+  if (!transaction)

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

What's the use case for calling RemoveTransaction(nullptr)?  It seems a bit
weird to have the responsibility of catching this case be in the Writers class.

[shivanisha, 2017/06/07 15:19:18]

Let's say active_transaction_ is null as it has been destroyed.
Writers invokes cache_->DoneWritingToEntry with transaction as nullptr =>
RemoveTransaction(nullptr)

[Randy Smith (Not in Mondays), 2017/06/08 17:10:00]

I guess I'll put this off until the integration CL--it's hard to judge an
interface contract without seeing it being used.  But I am still confused;
specifically, DoneWritingToEntry() doesn't currently take a transaction, so the
cache can't call back into RemoveTransaction() with it.

[shivanisha, 2017/06/08 17:55:56]

DoneWritingToEntry() takes the transaction in the parallel validation CL
https://codereview.chromium.org/2721933002/

+    return;
+
+  // The transaction should be part of all_writers.
+  auto it = all_writers_.find(transaction);
+  DCHECK(it != all_writers_.end());
+  all_writers_.erase(transaction);
+  if (all_writers_.empty())
+    ResetStateForEmptyWriters();
+  PriorityChanged();
+
+  if (active_transaction_ == transaction) {
+    active_transaction_ = nullptr;
+    callback_.Reset();
+    return;
+  }
+
+  auto waiting_it = waiting_for_read_.begin();
+  for (; waiting_it != waiting_for_read_.end(); waiting_it++) {
+    if (transaction == waiting_it->transaction) {
+      waiting_for_read_.erase(waiting_it);
+      // If a waiting transaction existed, there should have been an
+      // active_transaction_.
+      DCHECK(active_transaction_);
+      return;
+    }
+  }
+}
+
+void HttpCache::Writers::PriorityChanged() {
+  // Get the current highest priority.
+  RequestPriority current_highest = MINIMUM_PRIORITY;
+  for (auto* transaction : all_writers_)
+    current_highest = std::max(transaction->priority(), current_highest);
+
+  if (priority_ != current_highest) {
+    network_transaction_->SetPriority(current_highest);
+    priority_ = current_highest;
+  }
+}
+
+bool HttpCache::Writers::ContainsOnlyIdleWriters() const {
+  return waiting_for_read_.empty() && !active_transaction_;
+}
+
+void HttpCache::Writers::MoveIdleWritersToReaders() {
+  // Should be invoked after |waiting_for_read_| transactions  and
+  // |active_transaction_| are processed so that all_writers_ only contains idle
+  // writers.
+  DCHECK(ContainsOnlyIdleWriters());
+  for (auto* idle_writer : all_writers_) {
+    entry_->readers.insert(idle_writer);
+  }
+  all_writers_.clear();
+  ResetStateForEmptyWriters();
+}
+
+bool HttpCache::Writers::CanAddWriters() {
+  if (all_writers_.empty())
+    return true;
+
+  return !is_exclusive_ && !network_read_only_;

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

What's the use case for adding writers when |all_writers_.empty() &&
network_read_only_|?

[shivanisha, 2017/06/07 15:19:18]

That shouldn't happen since network_read_only_ is false to start with and reset
when writers is empty.

+}
+
+void HttpCache::Writers::ProcessFailure(Transaction* transaction, int error) {
+  DCHECK(!transaction || transaction == active_transaction_);
+
+  // Notify waiting_for_read_ of the failure. Tasks will be posted for all the
+  // transactions.
+  ProcessWaitingForReadTransactions(error);
+
+  // Idle readers should know to fail when Read is invoked by their consumers.
+  SetIdleWritersFailState(error);
+  DCHECK(all_writers_.empty());
+  ResetStateForEmptyWriters();

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

Shouldn't this already have happened if all_writers_ is empty?  Seems like it
should be a DCHECK(), not a call.

[shivanisha, 2017/06/07 15:19:18]

This basically resets some member variables and should be invoked when the all
the transactions have been removed which is what is happening in the statements
above.

[Randy Smith (Not in Mondays), 2017/06/08 17:10:00]

This makes the interface contract for ProcessWaitingForReadTransaction() a bit
messy, in that it's now implicitly the responsibility of the caller of
ProcessWaitingForReadTransactions() to call ResetStateForEmptyWriters().  Would
you be willing to move the state reset to the end of PWFRT() outside the loop? 
That's an extra check in the failure case, but beyond that matches the
performance of the current implementation, and leaves the responsibilities
cleaner.

[shivanisha, 2017/06/08 17:55:56]

ResetStateForEmptyWriters() is not really related to
ProcessWaitingForReadTransaction(). ProcessWaitingForReadTransaction takes care
of processing the IO callbacks of waiting transactions and
ResetStateForEmptyWriters is responsible for resetting state in other cases as
well and not just when waiting_for_read is processed. So I see them as separate
responsibilities and here ProcessFailure() takes care of both responsibilities. 

[Randy Smith (Not in Mondays), 2017/06/13 17:45:05]

TL;DR: I want to make on more try at explaining my perspective, then I'll accept
whatever decision you make. 

The flip side of this argument: You want to call ResetStateForEmptyWriters()
whenever all_writers_ transitions to empty(), and you never want it to be called
in any other case.  You could imagine (not recommending or requesting this!)
creating a wrapper class for all_writers_ and it's related state, and having it
automatically track  the state of all_writers_ and execute the logic in
ResetStateForEmptyWriters() whenever all_writers_ transitions to empty.  

Given that (IMO, and I suspect yours) the right choice in terms of code clarity
is not to do the wrapping, I'd argue that the next best thing is to keep the
reset as near as possible to the code clearing all_writers_.  This has two
advantages: Reviewers can more easily validate the invariant that
ResetStateForEmptyWriters() if-and-only-if all_writers_ has transitioned to
empty, and future modifiers if this code, if they copy a section that includes
modifying all_writers_, have a higher chance of copying the call to
ResetStateForEmptyWriters(). 

Pull up a level to address your conceptual argument: I agree that
ResetStateForEmptyWriters() isn't related to ProcessWaitingForReadTransaction(),
but it's related to the lower-level concept that
ProcessWaitingForReadTransaction() also affects, which is the state of
all_writers_.  Given that, I'd put it near the state change for all_writers_.

And, having said all that, your call--this isn't a big deal.  I just wanted to
try and make clear why my preference was the other direction.

[shivanisha, 2017/06/14 02:33:16]

I understand the concern here that it's not obvious when ResetState.. should be
invoked . But there is a change in the latest patch which is that instead of
asserting that all_writers is empty , it checks if all_writers_ is empty and
calls ResetState.. if it is. This is because in cache write failure case we do
not remove the active_transaction_ from all_writers_ but in network failure
case, we do.
So conceptually ResetState... is called in all the ending scenarios where
all_writers_ "may" become empty:
RemoveTransaction()
OnDataReceived() // success
ProcessFailure() // failure

Also commenting this requirement in headers where all_writers_ is declared.

+}
+
+void HttpCache::Writers::TruncateEntry() {
+  // TODO(shivanisha) On integration, see if the entry really needs to be
+  // truncated on the lines of Transaction::AddTruncatedFlag and then proceed.
+  DCHECK_EQ(next_state_, State::NONE);
+  next_state_ = State::CACHE_WRITE_TRUNCATED_RESPONSE;
+  DoLoop(OK);
+}
+
+LoadState HttpCache::Writers::GetWriterLoadState() {
+  return network_transaction_->GetLoadState();
+}
+
+HttpCache::Writers::WaitingForRead::WaitingForRead(
+    Transaction* cache_transaction,
+    scoped_refptr<IOBuffer> buf,
+    int len,
+    const CompletionCallback& consumer_callback)
+    : transaction(cache_transaction),
+      read_buf(std::move(buf)),
+      read_buf_len(len),
+      write_len(0),
+      callback(consumer_callback) {
+  DCHECK(cache_transaction);
+  DCHECK(buf);
+  DCHECK_GT(len, 0);
+  DCHECK(!consumer_callback.is_null());
+}
+
+HttpCache::Writers::WaitingForRead::~WaitingForRead() {}
+HttpCache::Writers::WaitingForRead::WaitingForRead(const WaitingForRead&) =
+    default;
+
+int HttpCache::Writers::DoLoop(int result) {
+  DCHECK(next_state_ != State::NONE);
+  int rv = result;
+  do {
+    State state = next_state_;
+    next_state_ = State::NONE;
+    switch (state) {
+      case State::NETWORK_READ:
+        DCHECK_EQ(OK, rv);
+        rv = DoNetworkRead();
+        break;
+      case State::NETWORK_READ_COMPLETE:
+        rv = DoNetworkReadComplete(rv);
+        break;
+      case State::CACHE_WRITE_DATA:
+        rv = DoCacheWriteData(rv);
+        break;
+      case State::CACHE_WRITE_DATA_COMPLETE:
+        rv = DoCacheWriteDataComplete(rv);
+        break;
+      case State::CACHE_WRITE_TRUNCATED_RESPONSE:
+        rv = DoCacheWriteTruncatedResponse();
+        break;
+      case State::CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE:
+        rv = DoCacheWriteTruncatedResponseComplete(rv);
+        break;
+      default:
+        NOTREACHED() << "bad state";
+        rv = ERR_FAILED;
+        break;
+    }
+  } while (next_state_ != State::NONE && rv != ERR_IO_PENDING);
+
+  if (rv != ERR_IO_PENDING && !callback_.is_null()) {
+    read_buf_ = NULL;  // Release the buffer before invoking the callback.

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

Does it matter whether we release the buffer before or after invoking the
callback?  (Alternative phrasing: Suggestion: Remove the comment?)

[shivanisha, 2017/06/07 15:19:19]

From what I understand its important to release before since its a scoped_refptr
and can only be used by one layer at a time and now we want the consumer layer
to use it.

[Randy Smith (Not in Mondays), 2017/06/08 17:10:00]

It's true that it can be only used by one layer at a time, but the reference is
for keeping it alive, not for using it.  For instance, there are multiple
references to the buffer when it's passed to the network transaction, but the
network transaction is still the only layer that's allowed to use it.  

I believe the only effect of releasing the buffer after the callback instead of
before would be that the buffer deletion would occur at that line, rather than
in the callback.  Which, who knows, might make some future bug a bit harder to
debug, so I certainly don't object to releasing the buffer before the callback
call.  I just don't think that there's any requirement to do so for code
correctness, performance, or cleanliness, so I think the comment is misleading
(in its implication that it's important that it be released first).

But up to you--I'm just explaining my reasoning for clarity and possibly
interest, not because I feel strongly.

[shivanisha, 2017/06/12 18:51:14]

Removed the comment.

+    base::ResetAndReturn(&callback_).Run(rv);
+  }
+  return rv;
+}
+
+int HttpCache::Writers::DoNetworkRead() {
+  next_state_ = State::NETWORK_READ_COMPLETE;
+  io_callback_ =
+      base::Bind(&HttpCache::Writers::OnIOComplete, weak_factory_.GetWeakPtr());
+  return network_transaction_->Read(read_buf_.get(), io_buf_len_, io_callback_);
+}
+
+int HttpCache::Writers::DoNetworkReadComplete(int result) {
+  if (result < 0) {
+    OnNetworkReadFailure(result);
+    return result;
+  }
+
+  next_state_ = State::CACHE_WRITE_DATA;
+  return result;
+}
+
+void HttpCache::Writers::OnNetworkReadFailure(int result) {

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

Why does this function take a |result| argument?

[shivanisha, 2017/06/07 15:19:19]

A follow up CL will use result to invoke ProcessFailure. Since I am still
working on whether to invoke it here directly from here or through
cache_->DoneWithEntry, I am not adding it yet in this CL. Added a TODO for the
same.

+  // At this point active_transaction_ may or may not be alive.
+  // If no consumer, invoke entry processing here itself.

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

This comment doesn't seem easily relatable to the code; specifically, I don't
see any conditional that I can match to "if no consumer".  Could you clarify it?

[shivanisha, 2017/06/07 15:19:19]

Yes, removed it.

+  cache_->DoneWithEntry(entry_, active_transaction_, true);
+}
+
+int HttpCache::Writers::DoCacheWriteData(int num_bytes) {
+  next_state_ = State::CACHE_WRITE_DATA_COMPLETE;
+  write_len_ = num_bytes;
+  if (!num_bytes || network_read_only_)
+    return num_bytes;
+
+  int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex);
+  io_callback_ =
+      base::Bind(&HttpCache::Writers::OnIOComplete, weak_factory_.GetWeakPtr());
+  return WriteToEntry(kResponseContentIndex, current_size, read_buf_.get(),
+                      num_bytes, io_callback_);
+}
+
+int HttpCache::Writers::WriteToEntry(int index,

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

Given that this is a private function only called above, I'd suggest hardcoding
the kResponseContentIndex inside the function rather than taking it as an
argument.  (Unless you have an anticipated use case in your next couple of CLs
that would use the argument functionality.)

[shivanisha, 2017/06/07 15:19:18]

done

+                                     int offset,
+                                     IOBuffer* data,
+                                     int data_len,
+                                     const CompletionCallback& callback) {
+  int rv = 0;
+
+  PartialData* partial = nullptr;
+  // The active transaction must be alive if this is a partial request, as
+  // partial requests are exclusive and hence will always be the active
+  // transaction.
+  // Todo(shivanisha): When partial requests support parallel writing, this
+  // assumption will not be true.
+  if (active_transaction_)
+    partial = active_transaction_->partial();
+
+  if (!partial || !data_len) {
+    rv = entry_->disk_entry->WriteData(index, offset, data, data_len, callback,
+                                       true);
+  } else {
+    rv = partial->CacheWrite(entry_->disk_entry, data, data_len, callback);
+  }
+  return rv;
+}
+
+int HttpCache::Writers::DoCacheWriteDataComplete(int result) {
+  if (result != write_len_) {
+    OnCacheWriteFailure();
+
+    // |active_transaction_| can continue reading from the network.
+    result = write_len_;
+  } else {
+    OnDataReceived(result);
+  }
+  return result;
+}
+
+int HttpCache::Writers::DoCacheWriteTruncatedResponse() {
+  next_state_ = State::CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE;
+  const HttpResponseInfo* response = network_transaction_->GetResponseInfo();
+  scoped_refptr<PickledIOBuffer> data(new PickledIOBuffer());
+  response->Persist(data->pickle(), true /* skip_transient_headers*/, true);
+  data->Done();
+  io_buf_len_ = data->pickle()->size();
+  io_callback_ =
+      base::Bind(&HttpCache::Writers::OnIOComplete, weak_factory_.GetWeakPtr());
+  return entry_->disk_entry->WriteData(kResponseInfoIndex, 0, data.get(),
+                                       io_buf_len_, io_callback_, true);
+}
+
+int HttpCache::Writers::DoCacheWriteTruncatedResponseComplete(int result) {
+  if (result != io_buf_len_) {
+    DLOG(ERROR) << "failed to write response info to cache";

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

What state does this leave the cache in?  If it's not a consistent state (e.g.
and earlier or later write of headers would contradict the amount of data on
disk), I'm inclined to think we should doom the entry.  Of course, that's assume
the disk isn't just refusing operations :-J.

[shivanisha, 2017/06/07 15:19:19]

done

+  }
+  truncated_ = true;
+  return OK;
+}
+
+void HttpCache::Writers::OnDataReceived(int result) {
+  if (result == 0) {
+    // Check if the response is actually completed or if not, attempt to mark
+    // the entry as truncated.

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

I don't see (here or in OnNetworkReadFailure()) an attempt to mark the entry as
truncated?

[shivanisha, 2017/06/07 15:19:19]

OnNetworkReadFailure() => DoneWithEntry() => Invokes truncation code. I am
leaving the decision to do it directly here or through DoneWithEntry() for the
follow up CL which integrates it with HttpCache.

[Randy Smith (Not in Mondays), 2017/06/08 17:10:00]

Gotchat, and that's fine.  But I find the comment confusing as written; if you
leave the code like this could you add something about where the truncation
happens (e.g. "... truncated (in OnNetworkReadFailure => DoneWithEntry())")?

[shivanisha, 2017/06/12 18:51:14]

Added in the comment that the truncation attempt will be in OnNetworkReadFailure

+    bool completed = false;
+    int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex);
+    const HttpResponseInfo* response_info =
+        network_transaction_->GetResponseInfo();
+    int64_t content_length = response_info->headers->GetContentLength();
+    if ((content_length >= 0 && content_length <= current_size) ||
+        content_length < 0) {
+      completed = true;
+    }
+    if (!completed) {
+      OnNetworkReadFailure(result);
+      return;
+    }

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

Why not combine these two conditionals?  |completed| doesn't look like it's used
anywhere else.

[shivanisha, 2017/06/07 15:19:19]

done

+  }
+
+  // Notify waiting_for_read_. Tasks will be posted for all the
+  // transactions.
+  ProcessWaitingForReadTransactions(write_len_);
+
+  active_transaction_ = nullptr;
+
+  // TODO(shivanisha) Invoke cache_->DoneWritingToEntry if result is 0 implying
+  // response completion.

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

What gets in the way of doing this now?

[shivanisha, 2017/06/07 15:19:18]

Messes up with the assertions in HttpCache layer so can add it only after
integration with both HC::T and HC.

+}
+
+void HttpCache::Writers::OnCacheWriteFailure() {
+  network_read_only_ = true;
+
+  // Call the cache_ function here even if active_transaction_ is alive because
+  // it wouldn't know if this was an error case, since it gets a positive result
+  // back. This will in turn ProcessFailure.
+  // TODO(shivanisha) : Also pass active_transaction_ and
+  // ERR_CACHE_WRITE_FAILURE to DoneWritingToEntry on integration so that it can
+  // be passed back in ProcessFailure.
+  cache_->DoneWritingToEntry(entry_, false);
+}
+
+void HttpCache::Writers::ProcessWaitingForReadTransactions(int result) {
+  for (auto it = waiting_for_read_.begin(); it != waiting_for_read_.end();
+       it++) {

[Randy Smith (Not in Mondays), 2017/06/06 12:54:57]

nit, suggestion: Why not use the new C++ range syntax?  (for (auto&
waiting_transaction: waiting_for_read_))?

[shivanisha, 2017/06/07 15:19:19]

done

+    Transaction* transaction = it->transaction;
+    int callback_result = result;
+
+    if (result >= 0) {  // success
+      // Save the data in all the waiting transactions' read buffers.
+      for (auto it = waiting_for_read_.begin(); it != waiting_for_read_.end();

[Randy Smith (Not in Mondays), 2017/06/06 12:54:58]

I've read over this code a couple of times, and I'm failing to understand the
need for the two nested for loops.  Isn't the algorithm you want:
* For each waiting transaction:
** local_result = result
** If success:
*** local_result = min(result, read_buf_len) 
*** Copy local_result bytes into the read buffer.
** If failure, erase the transaction from the list.
** Post the callback?

[shivanisha, 2017/06/07 15:19:18]

Oops, corrected.

+           it++) {
+        it->write_len = std::min(it->read_buf_len, result);
+        memcpy(it->read_buf->data(), read_buf_->data(), it->write_len);
+      }
+      callback_result = it->write_len;
+    }
+
+    // If its response completion or failure, this transaction needs to be
+    // removed.
+    if (result <= 0)
+      all_writers_.erase(transaction);
+
+    // Post task to notify transaction.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(it->callback, callback_result));
+  }
+
+  waiting_for_read_.clear();
+}
+
+void HttpCache::Writers::SetIdleWritersFailState(int result) {
+  // Since this is only for idle transactions, all waiting_for_read_ and
+  // active_transaction_ should be empty.
+  DCHECK(waiting_for_read_.empty());
+  DCHECK(!active_transaction_);
+  for (auto* transaction : all_writers_)
+    transaction->SetSharedWritingFailState(result);
+  all_writers_.clear();
+  ResetStateForEmptyWriters();
+}
+
+void HttpCache::Writers::ResetStateForEmptyWriters() {
+  DCHECK(all_writers_.empty());
+  network_read_only_ = false;
+  network_transaction_.reset();
+}
+
+void HttpCache::Writers::OnIOComplete(int result) {
+  DoLoop(result);
+}
+
+}  // namespace net