HttpCache::Transaction layer allowing parallel validation

net/http/http_cache_transaction.cc

 // Copyright (c) 2012 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_transaction.h"
 
 #include "build/build_config.h"  // For OS_POSIX
 
 #if defined(OS_POSIX)
 #include <unistd.h>
@@ skipping 152 matching lines @@
 
 HttpCache::Transaction::Transaction(RequestPriority priority, HttpCache* cache)
     : next_state_(STATE_NONE),
       request_(NULL),
       priority_(priority),
       cache_(cache->GetWeakPtr()),
       entry_(NULL),
       new_entry_(NULL),
       new_response_(NULL),
       mode_(NONE),
+      original_mode_(NONE),
       reading_(false),
       invalid_range_(false),
       truncated_(false),
       is_sparse_(false),
       range_requested_(false),
       handling_206_(false),
       cache_pending_(false),
       done_reading_(false),
       vary_mismatch_(false),
       couldnt_conditionalize_request_(false),
       bypass_lock_for_test_(false),
       fail_conditionalization_for_test_(false),
+      validating_cannot_proceed_(false),
       io_buf_len_(0),
       read_offset_(0),
       effective_load_flags_(0),
       write_len_(0),
       cache_entry_status_(CacheEntryStatus::ENTRY_UNDEFINED),
       validation_cause_(VALIDATION_CAUSE_UNDEFINED),
       total_received_bytes_(0),
       total_sent_bytes_(0),
       websocket_handshake_stream_base_create_helper_(NULL),
       in_do_loop_(false),
       weak_factory_(this) {
   TRACE_EVENT0("io", "HttpCacheTransaction::Transaction");
   static_assert(HttpCache::Transaction::kNumValidationHeaders ==
                     arraysize(kValidationHeaders),
                 "invalid number of validation headers");
 
   io_callback_ = base::Bind(&Transaction::OnIOComplete,
                               weak_factory_.GetWeakPtr());
 }
 
 HttpCache::Transaction::~Transaction() {
   TRACE_EVENT0("io", "HttpCacheTransaction::~Transaction");
   // We may have to issue another IO, but we should never invoke the callback_
   // after this point.
   callback_.Reset();
 
   if (cache_) {
     if (entry_) {
-      bool cancel_request = reading_ && response_.headers.get();
-      if (cancel_request) {
-        if (partial_) {
-          entry_->disk_entry->CancelSparseIO();
-        } else {
-          cancel_request &= (response_.headers->response_code() == 200);
-        }
-      }
+      bool cancel_response =
+          cache_->IsCancelResponseBody(entry_, this, request_->method);
+      if (cancel_response && partial_)
+        entry_->disk_entry->CancelSparseIO();
 
-      cache_->DoneWithEntry(entry_, this, cancel_request);
+      cache_->DoneWithEntry(entry_, this, cancel_response);
     } else if (cache_pending_) {
       cache_->RemovePendingTransaction(this);
     }
   }
 }
 
 int HttpCache::Transaction::WriteMetadata(IOBuffer* buf, int buf_len,
                                           const CompletionCallback& callback) {
   DCHECK(buf);
   DCHECK_GT(buf_len, 0);
@@ skipping 134 matching lines @@
 }
 
 bool HttpCache::Transaction::IsReadyToRestartForAuth() {
   if (!network_trans_.get())
     return false;
   return network_trans_->IsReadyToRestartForAuth();
 }
 
 int HttpCache::Transaction::Read(IOBuffer* buf, int buf_len,
                                  const CompletionCallback& callback) {
+  // TODO(shivanisha): A lot of tests invoke Read on a HEAD request which
+  // should not be allowed in the Read() method. Fix those tests.
   DCHECK_EQ(next_state_, STATE_NONE);
   DCHECK(buf);
   DCHECK_GT(buf_len, 0);
   DCHECK(!callback.is_null());
 
   DCHECK(callback_.is_null());
 
   if (!cache_.get())
     return ERR_UNEXPECTED;
 
@@ skipping 185 matching lines @@
     ConnectionAttempts* out) const {
   ConnectionAttempts new_connection_attempts;
   if (network_trans_)
     network_trans_->GetConnectionAttempts(&new_connection_attempts);
 
   out->swap(new_connection_attempts);
   out->insert(out->begin(), old_connection_attempts_.begin(),
               old_connection_attempts_.end());
 }
 
+void HttpCache::Transaction::SetValidatingCannotProceed() {
+  validating_cannot_proceed_ = true;
+  entry_ = nullptr;
+}
+
 size_t HttpCache::Transaction::EstimateMemoryUsage() const {
   // TODO(xunjieli): Consider improving the coverage. crbug.com/669108.
   return 0;
 }
 
 //-----------------------------------------------------------------------------
 
 // A few common patterns: (Foo* means Foo -> FooComplete)
 //
 // 1. Not-cached entry:
 //   Start():
 //   GetBackend* -> InitEntry -> OpenEntry* -> CreateEntry* -> AddToEntry* ->
 //   SendRequest* -> SuccessfulSendRequest -> OverwriteCachedResponse ->
 //   CacheWriteResponse* -> TruncateCachedData* -> TruncateCachedMetadata* ->
 //   PartialHeadersReceived
 //
 //   Read():
 //   NetworkRead* -> CacheWriteData*
 //
 // 2. Cached entry, no validation:
 //   Start():
 //   GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse*
 //   -> CacheDispatchValidation -> BeginPartialCacheValidation() ->
-//   BeginCacheValidation() -> SetupEntryForRead()
+//   BeginCacheValidation() -> SetupEntryForRead() -> WaitBeforeRead*
 //
 //   Read():
 //   CacheReadData*
 //
 // 3. Cached entry, validation (304):
 //   Start():
 //   GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse*
 //   -> CacheDispatchValidation -> BeginPartialCacheValidation() ->
 //   BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest ->
 //   UpdateCachedResponse -> CacheWriteUpdatedResponse* ->
 //   UpdateCachedResponseComplete -> OverwriteCachedResponse ->
-//   PartialHeadersReceived
+//   PartialHeadersReceived -> WaitBeforeRead*
 //
 //   Read():
 //   CacheReadData*
 //
 // 4. Cached entry, validation and replace (200):
 //   Start():
 //   GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse*
 //   -> CacheDispatchValidation -> BeginPartialCacheValidation() ->
 //   BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest ->
 //   OverwriteCachedResponse -> CacheWriteResponse* -> DoTruncateCachedData* ->
@@ skipping 84 matching lines @@
 //
 // 14. Cached entry more than 5 minutes old, unused_since_prefetch is true:
 //   Like examples 2-4, only CacheToggleUnusedSincePrefetch* is inserted between
 //   CacheReadResponse* and CacheDispatchValidation.
 int HttpCache::Transaction::DoLoop(int result) {
   DCHECK_NE(STATE_UNSET, next_state_);
   DCHECK_NE(STATE_NONE, next_state_);
   DCHECK(!in_do_loop_);
 
   int rv = result;
+  State state = next_state_;
   do {
-    State state = next_state_;
+    state = next_state_;
     next_state_ = STATE_UNSET;
     base::AutoReset<bool> scoped_in_do_loop(&in_do_loop_, true);
 
     switch (state) {
       case STATE_GET_BACKEND:
         DCHECK_EQ(OK, rv);
         rv = DoGetBackend();
         break;
       case STATE_GET_BACKEND_COMPLETE:
         rv = DoGetBackendComplete(rv);
@@ skipping 116 matching lines @@
         DCHECK_EQ(OK, rv);
         rv = DoPartialHeadersReceived();
         break;
       case STATE_CACHE_READ_METADATA:
         DCHECK_EQ(OK, rv);
         rv = DoCacheReadMetadata();
         break;
       case STATE_CACHE_READ_METADATA_COMPLETE:
         rv = DoCacheReadMetadataComplete(rv);
         break;
+      case STATE_WAIT_BEFORE_READ:
+        DCHECK_EQ(OK, rv);
+        rv = DoWaitBeforeRead();
+        break;
+      case STATE_WAIT_BEFORE_READ_COMPLETE:
+        rv = DoWaitBeforeReadComplete(rv);
+        break;
       case STATE_NETWORK_READ:
         DCHECK_EQ(OK, rv);
         rv = DoNetworkRead();
         break;
       case STATE_NETWORK_READ_COMPLETE:
         rv = DoNetworkReadComplete(rv);
         break;
       case STATE_CACHE_READ_DATA:
         DCHECK_EQ(OK, rv);
         rv = DoCacheReadData();
@@ skipping 16 matching lines @@
         break;
       default:
         NOTREACHED() << "bad state " << state;
         rv = ERR_FAILED;
         break;
     }
     DCHECK(next_state_ != STATE_UNSET) << "Previous state was " << state;
 
   } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
 
+  // Assert Start() state machine's allowed last state in successful cases when
+  // caching is happening.
+  DCHECK(reading_ || rv != OK || !entry_ ||
+         state == STATE_WAIT_BEFORE_READ_COMPLETE);
+
+  if (!reading_ && rv == OK) {
+    DCHECK(!entry_ || state == STATE_WAIT_BEFORE_READ_COMPLETE);
+  }
+
   if (rv != ERR_IO_PENDING && !callback_.is_null()) {
-    read_buf_ = NULL;  // Release the buffer before invoking the callback.
+    read_buf_ = nullptr;  // Release the buffer before invoking the callback.
     base::ResetAndReturn(&callback_).Run(rv);
   }
 
   return rv;
 }
 
 int HttpCache::Transaction::DoGetBackend() {
   cache_pending_ = true;
   TransitionToState(STATE_GET_BACKEND_COMPLETE);
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_GET_BACKEND);
@@ skipping 61 matching lines @@
       partial_->RestoreHeaders(&custom_request_->extra_headers);
       partial_.reset();
     }
     TransitionToState(STATE_SEND_REQUEST);
   } else {
     TransitionToState(STATE_INIT_ENTRY);
   }
 
   // This is only set if we have something to do with the response.
   range_requested_ = (partial_.get() != NULL);
-
+  original_mode_ = mode_;

[jkarlin, 2017/03/30 14:25:26]

There should be a newline above this as it doesn't have to do with the above
comment.

[shivanisha, 2017/03/30 16:38:41]

Done. Also added a comment on why we need original_mode_ to be set here.

   return OK;
 }
 
 int HttpCache::Transaction::DoInitEntry() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoInitEntry");
   DCHECK(!new_entry_);
 
   if (!cache_.get()) {
     TransitionToState(STATE_NONE);
     return ERR_UNEXPECTED;
@@ skipping 357 matching lines @@
     TransitionToState(STATE_NONE);
     return ERR_UNEXPECTED;
   }
 
   return ValidateEntryHeadersAndContinue();
 }
 
 // We may end up here multiple times for a given request.
 int HttpCache::Transaction::DoStartPartialCacheValidation() {
   if (mode_ == NONE) {
-    TransitionToState(STATE_NONE);
+    TransitionToState(reading_ ? STATE_NONE : STATE_WAIT_BEFORE_READ);

[jkarlin, 2017/03/30 14:25:26]

Is this use of reading_ really a check to see if this transaction is actually
the ActiveEntry's writer? If so, then let's just go to STATE_WAIT_BEFORE_READ
and let the ActiveEntry synchronously tell the Transaction that it can read.

[shivanisha, 2017/03/30 16:38:41]

Done.

     return OK;
   }
 
   TransitionToState(STATE_COMPLETE_PARTIAL_CACHE_VALIDATION);
   return partial_->ShouldValidateCache(entry_->disk_entry, io_callback_);
 }
 
 int HttpCache::Transaction::DoCompletePartialCacheValidation(int result) {
   if (!result) {
     // This is the end of the request.
@@ skipping 94 matching lines @@
 // We received the response headers and there is no error.
 int HttpCache::Transaction::DoSuccessfulSendRequest() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoSuccessfulSendRequest");
   DCHECK(!new_response_);
   const HttpResponseInfo* new_response = network_trans_->GetResponseInfo();
 
   if (new_response->headers->response_code() == 401 ||
       new_response->headers->response_code() == 407) {
     SetAuthResponse(*new_response);
     if (!reading_) {
-      TransitionToState(STATE_NONE);
+      TransitionToState(STATE_WAIT_BEFORE_READ);
       return OK;
     }
 
     // We initiated a second request the caller doesn't know about. We should be
     // able to authenticate this request because we should have authenticated
     // this URL moments ago.
     if (IsReadyToRestartForAuth()) {
       DCHECK(!response_.auth_challenge.get());
       TransitionToState(STATE_SEND_REQUEST_COMPLETE);
       // In theory we should check to see if there are new cookies, but there
@@ skipping 42 matching lines @@
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_NOT_IN_CACHE);
   }
 
   // Invalidate any cached GET with a successful PUT or DELETE.
   if (mode_ == WRITE &&
       (request_->method == "PUT" || request_->method == "DELETE")) {
     if (NonErrorResponse(new_response->headers->response_code())) {
       int ret = cache_->DoomEntry(cache_key_, NULL);
       DCHECK_EQ(OK, ret);
     }
-    cache_->DoneWritingToEntry(entry_, true);
+    cache_->DoneWritingToEntry(entry_, true, this);
     entry_ = NULL;
     mode_ = NONE;
   }
 
   // Invalidate any cached GET with a successful POST.
   if (!(effective_load_flags_ & LOAD_DISABLE_CACHE) &&
       request_->method == "POST" &&
       NonErrorResponse(new_response->headers->response_code())) {
     cache_->DoomMainEntryForUrl(request_->url);
   }
 
   RecordNoStoreHeaderHistogram(request_->load_flags, new_response);
 
   if (new_response_->headers->response_code() == 416 &&
       (request_->method == "GET" || request_->method == "POST")) {
     // If there is an active entry it may be destroyed with this transaction.
     SetResponse(*new_response_);
-    TransitionToState(STATE_NONE);
+    TransitionToState(reading_ ? STATE_NONE : STATE_WAIT_BEFORE_READ);
     return OK;
   }
 
   // Are we expecting a response to a conditional query?
   if (mode_ == READ_WRITE || mode_ == UPDATE) {
     if (new_response->headers->response_code() == 304 || handling_206_) {
       UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_VALIDATED);
       TransitionToState(STATE_UPDATE_CACHED_RESPONSE);
       return OK;
     }
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_UPDATED);
     mode_ = WRITE;
   }
 
   TransitionToState(STATE_OVERWRITE_CACHED_RESPONSE);
+
+  if (!entry_)
+    return OK;
+
+  // Invalidate any current entry with a successful response if this transaction
+  // cannot write to this entry.
+  if (new_response->headers->response_code() != 304 &&
+      (entry_->writer || !entry_->readers.empty())) {
+    DCHECK_EQ(entry_->headers_transaction, this);
+    cache_->DoneResponseHeaders(entry_, this, false);
+    entry_ = nullptr;
+    mode_ = NONE;
+    return OK;
+  }
+
   return OK;
 }
 
 // We received 304 or 206 and we want to update the cached response headers.
 int HttpCache::Transaction::DoUpdateCachedResponse() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoUpdateCachedResponse");
   int rv = OK;
   // Update the cached response based on the headers and properties of
   // new_response_.
   response_.headers->Update(*new_response_->headers.get());
@@ skipping 52 matching lines @@
     DCHECK(!handling_206_);
     // We got a "not modified" response and already updated the corresponding
     // cache entry above.
     //
     // By closing the cached entry now, we make sure that the 304 rather than
     // the cached 200 response, is what will be returned to the user.
     DoneWritingToEntry(true);
   } else if (entry_ && !handling_206_) {
     DCHECK_EQ(READ_WRITE, mode_);
     if (!partial_ || partial_->IsLastRange()) {
-      cache_->ConvertWriterToReader(entry_);
       mode_ = READ;
     }
     // We no longer need the network transaction, so destroy it.
     ResetNetworkTransaction();
   } else if (entry_ && handling_206_ && truncated_ &&
              partial_->initial_validation()) {
     // We just finished the validation of a truncated entry, and the server
     // is willing to resume the operation. Now we go back and start serving
     // the first part to the user.
     ResetNetworkTransaction();
@@ skipping 99 matching lines @@
   }
 
   TransitionToState(STATE_PARTIAL_HEADERS_RECEIVED);
   return OK;
 }
 
 int HttpCache::Transaction::DoPartialHeadersReceived() {
   new_response_ = NULL;
 
   if (!partial_) {
-    if (entry_ && entry_->disk_entry->GetDataSize(kMetadataIndex))
+    if (entry_ && entry_->disk_entry->GetDataSize(kMetadataIndex)) {
       TransitionToState(STATE_CACHE_READ_METADATA);
-    else
-      TransitionToState(STATE_NONE);
+    } else {
+      DCHECK(!reading_);
+      TransitionToState(STATE_WAIT_BEFORE_READ);
+    }
     return OK;
   }
 
   if (reading_) {
     if (network_trans_.get()) {
       TransitionToState(STATE_NETWORK_READ);
     } else {
       TransitionToState(STATE_CACHE_READ_DATA);
     }
   } else if (mode_ != NONE) {
     // We are about to return the headers for a byte-range request to the user,
     // so let's fix them.
     partial_->FixResponseHeaders(response_.headers.get(), true);
-    TransitionToState(STATE_NONE);
+    TransitionToState(STATE_WAIT_BEFORE_READ);
   } else {
-    TransitionToState(STATE_NONE);
+    TransitionToState(STATE_WAIT_BEFORE_READ);
   }
   return OK;
 }
 
+int HttpCache::Transaction::DoWaitBeforeRead() {
+  if (!entry_) {
+    TransitionToState(STATE_NONE);
+    return OK;
+  }
+
+  // A transaction comes here between the completion of headers phase and the
+  // start of response body phase, thus it should never be the active writer at
+  // this point.
+  DCHECK_NE(entry_->writer, this);

[jkarlin, 2017/03/30 14:25:26]

The Transaction should be unaware of its role in the ActiveEntry. Nix this
DCHECK.

[shivanisha, 2017/03/30 16:38:41]

Removed , also because now we are allowing writers to be here and it will be
treated as a no-op in DoneResponseHeaders.

+
+  TransitionToState(STATE_WAIT_BEFORE_READ_COMPLETE);
+
+  // If it was an auth failure or 416, this transaction should continue to be
+  // headers_transaction till consumer takes an action, so no need to do
+  // anything now.
+  if (auth_response_.headers.get() ||
+      (new_response_ && new_response_->headers &&
+       new_response_->headers->response_code() == 416))
+    return OK;
+
+  // If there is no response body to be written or read, it does not need to
+  // wait.
+  if (request_->method == "HEAD")
+    return OK;
+
+  // Proceed only if |this| completed the headers phase. For read-only
+  // transactions since they do not have a headers phase, they would have
+  // already been added to entry_->readers, so do nothing.
+  if (entry_->headers_transaction == this)

[jkarlin, 2017/03/30 14:25:26]

The Transaction should be unaware of its role in the ActiveEntry. Just check if
this Transaction is read-only.

[shivanisha, 2017/03/30 16:38:41]

Done

[shivanisha, 2017/03/30 16:45:59]

We cannot check mode to be read-only to imply that it is currently a reader
since mode is also set to read when we want it to be a future reader. Invoked
DoneResponseHeaders for readers as well and checked it there.

+    return cache_->DoneResponseHeaders(entry_, this, true);
+
+  return OK;
+}
+
+int HttpCache::Transaction::DoWaitBeforeReadComplete(int rv) {
+  if (rv == ERR_CACHE_RACE) {
+    RestartAfterValidationStarted();
+    return OK;
+  }
+
+  TransitionToState(STATE_NONE);
+  if (rv != OK)
+    return rv;
+

[jkarlin, 2017/03/30 14:25:26]

It looks like at this point all paths lead to return OK. One path leads to mode_
= READ and resetting the network transaction. Perhaps instead the HttpCache
should force the Transaction into READ mode so that it doesn't have to infer its
place in the ActiveEntry?

[shivanisha, 2017/03/30 16:45:59]

Done. Added a function ConvertToReadMode to be invoked from HttpCache when
inserting the transaction in readers.

+  // If successful then this must be either be the writer or a reader as the
+  // response must have completely been written to the cache.
+  if (entry_->writer == this)
+    return OK;
+
+  // If it was an auth failure or 416, this transaction should continue to be
+  // headers_transaction, so no need to do anything now.
+  if (auth_response_.headers.get() ||
+      (new_response_ && new_response_->headers &&
+       new_response_->headers->response_code() == 416)) {
+    DCHECK_EQ(entry_->headers_transaction, this);
+    return OK;
+  }
+
+  mode_ = READ;
+  if (network_trans_)
+    ResetNetworkTransaction();
+  return OK;
+}
+
+void HttpCache::Transaction::RestartAfterValidationStarted() {
+  DCHECK(!reading_);
+  next_state_ = STATE_INIT_ENTRY;
+  cache_entry_status_ = CacheEntryStatus::ENTRY_UNDEFINED;
+  entry_ = nullptr;
+  mode_ = original_mode_;
+  if (network_trans_)
+    network_trans_.reset();
+}
+
 int HttpCache::Transaction::DoCacheReadMetadata() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadMetadata");
   DCHECK(entry_);
   DCHECK(!response_.metadata.get());
   TransitionToState(STATE_CACHE_READ_METADATA_COMPLETE);
 
   response_.metadata =
       new IOBufferWithSize(entry_->disk_entry->GetDataSize(kMetadataIndex));
 
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_READ_INFO);
   return entry_->disk_entry->ReadData(kMetadataIndex, 0,
                                       response_.metadata.get(),
                                       response_.metadata->size(),
                                       io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheReadMetadataComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadMetadataComplete");
   net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_READ_INFO,
                                     result);
   if (result != response_.metadata->size())
     return OnCacheReadError(result, false);
-  TransitionToState(STATE_NONE);
+
+  TransitionToState(reading_ ? STATE_NONE : STATE_WAIT_BEFORE_READ);
   return OK;
 }
 
 int HttpCache::Transaction::DoNetworkRead() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoNetworkRead");
   TransitionToState(STATE_NETWORK_READ_COMPLETE);
   return network_trans_->Read(read_buf_.get(), io_buf_len_, io_callback_);
 }
 
 int HttpCache::Transaction::DoNetworkReadComplete(int result) {
@@ skipping 297 matching lines @@
     TransitionToState(STATE_NONE);
     return ERR_CACHE_MISS;
   }
 
   if (request_->method == "HEAD")
     FixHeadersForHead();
 
   if (entry_->disk_entry->GetDataSize(kMetadataIndex))
     TransitionToState(STATE_CACHE_READ_METADATA);
   else
-    TransitionToState(STATE_NONE);
+    TransitionToState(reading_ ? STATE_NONE : STATE_WAIT_BEFORE_READ);
 
   return OK;
 }
 
 int HttpCache::Transaction::BeginCacheValidation() {
   DCHECK_EQ(mode_, READ_WRITE);
 
   ValidationType required_validation = RequiresValidation();
 
   bool skip_validation = (required_validation == VALIDATION_NONE);
@@ skipping 492 matching lines @@
   if (partial_) {
     if (truncated_ || is_sparse_ || !invalid_range_) {
       // We are going to return the saved response headers to the caller, so
       // we may need to adjust them first.
       TransitionToState(STATE_PARTIAL_HEADERS_RECEIVED);
       return OK;
     } else {
       partial_.reset();
     }
   }
-  cache_->ConvertWriterToReader(entry_);
+
   mode_ = READ;
 
   if (request_->method == "HEAD")
     FixHeadersForHead();
 
   if (entry_->disk_entry->GetDataSize(kMetadataIndex))
     TransitionToState(STATE_CACHE_READ_METADATA);
   else
-    TransitionToState(STATE_NONE);
+    TransitionToState(reading_ ? STATE_NONE : STATE_WAIT_BEFORE_READ);
   return OK;
 }
 
 int HttpCache::Transaction::WriteToEntry(int index, int offset,
                                          IOBuffer* data, int data_len,
                                          const CompletionCallback& callback) {
   if (!entry_)
     return data_len;
 
   int rv = 0;
@@ skipping 58 matching lines @@
   }
   return OK;
 }
 
 void HttpCache::Transaction::DoneWritingToEntry(bool success) {
   if (!entry_)
     return;
 
   RecordHistograms();
 
-  cache_->DoneWritingToEntry(entry_, success);
+  cache_->DoneWritingToEntry(entry_, success, this);
   entry_ = NULL;
   mode_ = NONE;  // switch to 'pass through' mode
 }
 
 int HttpCache::Transaction::OnCacheReadError(int result, bool restart) {
   DLOG(ERROR) << "ReadData failed: " << result;
   const int result_for_histogram = std::max(0, -result);
   if (restart) {
     UMA_HISTOGRAM_SPARSE_SLOWLY("HttpCache.ReadErrorRestartable",
                                 result_for_histogram);
@@ skipping 336 matching lines @@
       UMA_HISTOGRAM_PERCENTAGE("HttpCache.PercentBeforeSend.Updated",
                                before_send_sample);
       break;
     }
     default:
       NOTREACHED();
   }
 }
 
 void HttpCache::Transaction::OnIOComplete(int result) {
+  // If its the Start state machine and it cannot proceed due to failure by
+  // another transaction in writing the response, restart this transaction.
+  if (!reading_ && validating_cannot_proceed_) {

[jkarlin, 2017/03/30 14:25:26]

Same as above, let's not use reading_ to infer our state in the ActiveEntry.

I think you can DCHECK(!reading_) however.

+    validating_cannot_proceed_ = false;
+    RestartAfterValidationStarted();
+  }
+
   DoLoop(result);
 }
 
 void HttpCache::Transaction::TransitionToState(State state) {
   // Ensure that the state is only set once per Do* state.
   DCHECK(in_do_loop_);
   DCHECK_EQ(STATE_UNSET, next_state_) << "Next state is " << state;
   next_state_ = state;
 }
 
 }  // namespace net