[HttpCache::Transaction] Ensure each state only sets the next state once

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 172 matching lines @@
       fail_conditionalization_for_test_(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());
 }
 
@@ skipping 504 matching lines @@
 //
 //   Read():
 //   CacheReadData*
 //
 // 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_);
 
+  in_do_loop_ = true;

[Randy Smith (Not in Mondays), 2017/03/22 15:42:12]

Thought (not even a suggestion): AutoReset?

[jkarlin, 2017/03/22 17:12:00]

I think we want to explicitly set it back to false before calling the callback
at the end of the function, else we have reentrancy issues.

[Randy Smith (Not in Mondays), 2017/03/22 17:16:06]

It could be put inside the do{ } while, but understood and that's fine.

[jkarlin, 2017/03/22 17:32:55]

Good point. It's a bit of overhead but I like the safety. Done.

[Randy Smith (Not in Mondays), 2017/03/22 17:37:05]

The AutoReset class doesn't have any volatile funkiness, so I think there's a
reasonable chance that the compiler will optimize the overhead away.

   int rv = result;
   do {
     State state = next_state_;
     next_state_ = STATE_UNSET;
     switch (state) {
       case STATE_GET_BACKEND:
         DCHECK_EQ(OK, rv);
         rv = DoGetBackend();
         break;
       case STATE_GET_BACKEND_COMPLETE:
@@ skipping 152 matching lines @@
         rv = DoCacheWriteTruncatedResponseComplete(rv);
         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);
+  in_do_loop_ = false;
 
   if (rv != ERR_IO_PENDING && !callback_.is_null()) {
     read_buf_ = NULL;  // Release the buffer before invoking the callback.
     base::ResetAndReturn(&callback_).Run(rv);
   }
 
   return rv;
 }
 
 int HttpCache::Transaction::DoGetBackend() {
   cache_pending_ = true;
-  next_state_ = STATE_GET_BACKEND_COMPLETE;
+  SetNextStateFromDoLoop(STATE_GET_BACKEND_COMPLETE);
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_GET_BACKEND);
   return cache_->GetBackendForTransaction(this);
 }
 
 int HttpCache::Transaction::DoGetBackendComplete(int result) {
   DCHECK(result == OK || result == ERR_FAILED);
   net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_GET_BACKEND,
                                     result);
   cache_pending_ = false;
 
   if (!ShouldPassThrough()) {
     cache_key_ = cache_->GenerateCacheKey(request_);
 
     // Requested cache access mode.
     if (effective_load_flags_ & LOAD_ONLY_FROM_CACHE) {
       if (effective_load_flags_ & LOAD_BYPASS_CACHE) {
         // The client has asked for nonsense.
-        next_state_ = STATE_NONE;
+        SetNextStateFromDoLoop(STATE_NONE);
         return ERR_CACHE_MISS;
       }
       mode_ = READ;
     } else if (effective_load_flags_ & LOAD_BYPASS_CACHE) {
       mode_ = WRITE;
     } else {
       mode_ = READ_WRITE;
     }
 
     // Downgrade to UPDATE if the request has been externally conditionalized.
@@ skipping 18 matching lines @@
   // transaction behaves the same for GET and HEAD requests at this point: if it
   // was not modified, the entry is updated and a response is not returned from
   // the cache. If we receive 200, it doesn't matter if there was a validation
   // header or not.
   if (request_->method == "HEAD" && mode_ == WRITE)
     mode_ = NONE;
 
   // If must use cache, then we must fail.  This can happen for back/forward
   // navigations to a page generated via a form post.
   if (!(mode_ & READ) && effective_load_flags_ & LOAD_ONLY_FROM_CACHE) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_CACHE_MISS;
   }
 
   if (mode_ == NONE) {
     if (partial_) {
       partial_->RestoreHeaders(&custom_request_->extra_headers);
       partial_.reset();
     }
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
   } else {
-    next_state_ = STATE_INIT_ENTRY;
+    SetNextStateFromDoLoop(STATE_INIT_ENTRY);
   }
 
   // This is only set if we have something to do with the response.
   range_requested_ = (partial_.get() != NULL);
 
   return OK;
 }
 
 int HttpCache::Transaction::DoInitEntry() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoInitEntry");
   DCHECK(!new_entry_);
 
   if (!cache_.get()) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_UNEXPECTED;
   }
 
   if (mode_ == WRITE) {
-    next_state_ = STATE_DOOM_ENTRY;
+    SetNextStateFromDoLoop(STATE_DOOM_ENTRY);
     return OK;
   }
 
-  next_state_ = STATE_OPEN_ENTRY;
+  SetNextStateFromDoLoop(STATE_OPEN_ENTRY);
   return OK;
 }
 
 int HttpCache::Transaction::DoOpenEntry() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoOpenEntry");
   DCHECK(!new_entry_);
-  next_state_ = STATE_OPEN_ENTRY_COMPLETE;
+  SetNextStateFromDoLoop(STATE_OPEN_ENTRY_COMPLETE);
   cache_pending_ = true;
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_OPEN_ENTRY);
   first_cache_access_since_ = TimeTicks::Now();
   return cache_->OpenEntry(cache_key_, &new_entry_, this);
 }
 
 int HttpCache::Transaction::DoOpenEntryComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoOpenEntryComplete");
   // It is important that we go to STATE_ADD_TO_ENTRY whenever the result is
   // OK, otherwise the cache will end up with an active entry without any
   // transaction attached.
   net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_OPEN_ENTRY,
                                     result);
   cache_pending_ = false;
   if (result == OK) {
-    next_state_ = STATE_ADD_TO_ENTRY;
+    SetNextStateFromDoLoop(STATE_ADD_TO_ENTRY);
     return OK;
   }
 
   if (result == ERR_CACHE_RACE) {
-    next_state_ = STATE_INIT_ENTRY;
+    SetNextStateFromDoLoop(STATE_INIT_ENTRY);
     return OK;
   }
 
   if (request_->method == "PUT" || request_->method == "DELETE" ||
       (request_->method == "HEAD" && mode_ == READ_WRITE)) {
     DCHECK(mode_ == READ_WRITE || mode_ == WRITE || request_->method == "HEAD");
     mode_ = NONE;
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
     return OK;
   }
 
   if (mode_ == READ_WRITE) {
     mode_ = WRITE;
-    next_state_ = STATE_CREATE_ENTRY;
+    SetNextStateFromDoLoop(STATE_CREATE_ENTRY);
     return OK;
   }
   if (mode_ == UPDATE) {
     // There is no cache entry to update; proceed without caching.
     mode_ = NONE;
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
     return OK;
   }
 
   // The entry does not exist, and we are not permitted to create a new entry,
   // so we must fail.
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return ERR_CACHE_MISS;
 }
 
 int HttpCache::Transaction::DoDoomEntry() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoDoomEntry");
-  next_state_ = STATE_DOOM_ENTRY_COMPLETE;
+  SetNextStateFromDoLoop(STATE_DOOM_ENTRY_COMPLETE);
   cache_pending_ = true;
   if (first_cache_access_since_.is_null())
     first_cache_access_since_ = TimeTicks::Now();
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_DOOM_ENTRY);
   return cache_->DoomEntry(cache_key_, this);
 }
 
 int HttpCache::Transaction::DoDoomEntryComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoDoomEntryComplete");
   net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_DOOM_ENTRY,
                                     result);
-  next_state_ = STATE_CREATE_ENTRY;
   cache_pending_ = false;
-  if (result == ERR_CACHE_RACE)
-    next_state_ = STATE_INIT_ENTRY;
+  SetNextStateFromDoLoop(result == ERR_CACHE_RACE ? STATE_INIT_ENTRY
+                                                  : STATE_CREATE_ENTRY);
   return OK;
 }
 
 int HttpCache::Transaction::DoCreateEntry() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCreateEntry");
   DCHECK(!new_entry_);
-  next_state_ = STATE_CREATE_ENTRY_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CREATE_ENTRY_COMPLETE);
   cache_pending_ = true;
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_CREATE_ENTRY);
   return cache_->CreateEntry(cache_key_, &new_entry_, this);
 }
 
 int HttpCache::Transaction::DoCreateEntryComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCreateEntryComplete");
   // It is important that we go to STATE_ADD_TO_ENTRY whenever the result is
   // OK, otherwise the cache will end up with an active entry without any
   // transaction attached.
   net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_CREATE_ENTRY,
                                     result);
   cache_pending_ = false;
   switch (result) {
     case OK:
-      next_state_ = STATE_ADD_TO_ENTRY;
+      SetNextStateFromDoLoop(STATE_ADD_TO_ENTRY);
       break;
 
     case ERR_CACHE_RACE:
-      next_state_ = STATE_INIT_ENTRY;
+      SetNextStateFromDoLoop(STATE_INIT_ENTRY);
       break;
 
     default:
       // We have a race here: Maybe we failed to open the entry and decided to
       // create one, but by the time we called create, another transaction
       // already created the entry. If we want to eliminate this issue, we
       // need an atomic OpenOrCreate() method exposed by the disk cache.
       DLOG(WARNING) << "Unable to create cache entry";
       mode_ = NONE;
       if (partial_)
         partial_->RestoreHeaders(&custom_request_->extra_headers);
-      next_state_ = STATE_SEND_REQUEST;
+      SetNextStateFromDoLoop(STATE_SEND_REQUEST);
   }
   return OK;
 }
 
 int HttpCache::Transaction::DoAddToEntry() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoAddToEntry");
   DCHECK(new_entry_);
   cache_pending_ = true;
-  next_state_ = STATE_ADD_TO_ENTRY_COMPLETE;
+  SetNextStateFromDoLoop(STATE_ADD_TO_ENTRY_COMPLETE);
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_ADD_TO_ENTRY);
   DCHECK(entry_lock_waiting_since_.is_null());
   entry_lock_waiting_since_ = TimeTicks::Now();
   int rv = cache_->AddTransactionToEntry(new_entry_, this);
   if (rv == ERR_IO_PENDING) {
     if (bypass_lock_for_test_) {
-      OnAddToEntryTimeout(entry_lock_waiting_since_);
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE,
+          base::Bind(&HttpCache::Transaction::OnAddToEntryTimeout,
+                     weak_factory_.GetWeakPtr(), entry_lock_waiting_since_));

[Randy Smith (Not in Mondays), 2017/03/22 15:42:12]

Why?

[jkarlin, 2017/03/22 17:12:00]

OnAddToEntryTimeout ultimately winds up calling DoLoop(). We don't want to do
that while we're in a DoLoop.

     } else {
       int timeout_milliseconds = 20 * 1000;
       if (partial_ && new_entry_->writer &&
           new_entry_->writer->range_requested_) {
         // Quickly timeout and bypass the cache if we're a range request and
         // we're blocked by the reader/writer lock. Doing so eliminates a long
         // running issue, http://crbug.com/31014, where two of the same media
         // resources could not be played back simultaneously due to one locking
         // the cache entry until the entire video was downloaded.
         //
@@ skipping 29 matching lines @@
   DCHECK(new_entry_);
   cache_pending_ = false;
 
   if (result == OK)
     entry_ = new_entry_;
 
   // If there is a failure, the cache should have taken care of new_entry_.
   new_entry_ = NULL;
 
   if (result == ERR_CACHE_RACE) {
-    next_state_ = STATE_INIT_ENTRY;
+    SetNextStateFromDoLoop(STATE_INIT_ENTRY);
     return OK;
   }
 
   if (result == ERR_CACHE_LOCK_TIMEOUT) {
     if (mode_ == READ) {
-      next_state_ = STATE_NONE;
+      SetNextStateFromDoLoop(STATE_NONE);
       return ERR_CACHE_MISS;
     }
 
     // The cache is busy, bypass it for this transaction.
     mode_ = NONE;
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
     if (partial_) {
       partial_->RestoreHeaders(&custom_request_->extra_headers);
       partial_.reset();
     }
     return OK;
   }
 
   open_entry_last_used_ = entry_->disk_entry->GetLastUsed();
 
   // TODO(jkarlin): We should either handle the case or DCHECK.
   if (result != OK) {
     NOTREACHED();
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return result;
   }
 
   if (mode_ == WRITE) {
     if (partial_)
       partial_->RestoreHeaders(&custom_request_->extra_headers);
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
   } else {
     // We have to read the headers from the cached entry.
     DCHECK(mode_ & READ_META);
-    next_state_ = STATE_CACHE_READ_RESPONSE;
+    SetNextStateFromDoLoop(STATE_CACHE_READ_RESPONSE);
   }
   return OK;
 }
 
 int HttpCache::Transaction::DoCacheReadResponse() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadResponse");
   DCHECK(entry_);
-  next_state_ = STATE_CACHE_READ_RESPONSE_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_READ_RESPONSE_COMPLETE);
 
   io_buf_len_ = entry_->disk_entry->GetDataSize(kResponseInfoIndex);
   read_buf_ = new IOBuffer(io_buf_len_);
 
   net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_READ_INFO);
   return entry_->disk_entry->ReadData(kResponseInfoIndex, 0, read_buf_.get(),
                                       io_buf_len_, io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheReadResponseComplete(int result) {
@@ skipping 20 matching lines @@
   // cache should not be storing multi gigabyte resources. See
   // http://crbug.com/89567.
   if ((truncated_ || response_.headers->response_code() == 206) &&
       !range_requested_ &&
       full_response_length > std::numeric_limits<int32_t>::max()) {
     // Does not release the cache entry. If another transaction wants to use
     // this cache entry while this transaction is active, the second transaction
     // will fall back to the network after the timeout.
     DCHECK(!partial_);
     mode_ = NONE;
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
     return OK;
   }
 
   if (response_.unused_since_prefetch !=
       !!(request_->load_flags & LOAD_PREFETCH)) {
     // Either this is the first use of an entry since it was prefetched XOR
     // this is a prefetch. The value of response.unused_since_prefetch is
     // valid for this transaction but the bit needs to be flipped in storage.
-    next_state_ = STATE_TOGGLE_UNUSED_SINCE_PREFETCH;
+    SetNextStateFromDoLoop(STATE_TOGGLE_UNUSED_SINCE_PREFETCH);
     return OK;
   }
 
-  next_state_ = STATE_CACHE_DISPATCH_VALIDATION;
+  SetNextStateFromDoLoop(STATE_CACHE_DISPATCH_VALIDATION);
   return OK;
 }
 
 int HttpCache::Transaction::DoCacheToggleUnusedSincePrefetch() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheToggleUnusedSincePrefetch");
   // Write back the toggled value for the next use of this entry.
   response_.unused_since_prefetch = !response_.unused_since_prefetch;
 
   // TODO(jkarlin): If DoUpdateCachedResponse is also called for this
   // transaction then metadata will be written to cache twice. If prefetching
   // becomes more common, consider combining the writes.
 
-  next_state_ = STATE_TOGGLE_UNUSED_SINCE_PREFETCH_COMPLETE;
+  SetNextStateFromDoLoop(STATE_TOGGLE_UNUSED_SINCE_PREFETCH_COMPLETE);
   return WriteResponseInfoToEntry(false);
 }
 
 int HttpCache::Transaction::DoCacheToggleUnusedSincePrefetchComplete(
     int result) {
   TRACE_EVENT0(
       kNetTracingCategory,
       "HttpCacheTransaction::DoCacheToggleUnusedSincePrefetchComplete");
   // Restore the original value for this transaction.
   response_.unused_since_prefetch = !response_.unused_since_prefetch;
-  next_state_ = STATE_CACHE_DISPATCH_VALIDATION;
+  SetNextStateFromDoLoop(STATE_CACHE_DISPATCH_VALIDATION);
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
 int HttpCache::Transaction::DoCacheDispatchValidation() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheDispatchValidation");
   // We now have access to the cache entry.
   //
   //  o if we are a reader for the transaction, then we can start reading the
   //    cache entry.
   //
@@ skipping 18 matching lines @@
       result = BeginExternallyConditionalizedRequest();
       break;
     case WRITE:
     default:
       NOTREACHED();
   }
   return result;
 }
 
 int HttpCache::Transaction::DoCacheQueryData() {
-  next_state_ = STATE_CACHE_QUERY_DATA_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_QUERY_DATA_COMPLETE);
   return entry_->disk_entry->ReadyForSparseIO(io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheQueryDataComplete(int result) {
   DCHECK_EQ(OK, result);
   if (!cache_.get()) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(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) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return OK;
   }
 
-  next_state_ = STATE_COMPLETE_PARTIAL_CACHE_VALIDATION;
+  SetNextStateFromDoLoop(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.
     if (mode_ & WRITE) {
       DoneWritingToEntry(true);
     } else {
       cache_->DoneReadingFromEntry(entry_, this);
       entry_ = NULL;
     }
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return result;
   }
 
   if (result < 0) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return result;
   }
 
   partial_->PrepareCacheValidation(entry_->disk_entry,
                                    &custom_request_->extra_headers);
 
   if (reading_ && partial_->IsCurrentRangeCached()) {
-    next_state_ = STATE_CACHE_READ_DATA;
+    SetNextStateFromDoLoop(STATE_CACHE_READ_DATA);
     return OK;
   }
 
   return BeginCacheValidation();
 }
 
 int HttpCache::Transaction::DoSendRequest() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoSendRequest");
   DCHECK(mode_ & WRITE || mode_ == NONE);
   DCHECK(!network_trans_.get());
 
   send_request_since_ = TimeTicks::Now();
 
   // Create a network transaction.
   int rv =
       cache_->network_layer_->CreateTransaction(priority_, &network_trans_);
   if (rv != OK) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return rv;
   }
   network_trans_->SetBeforeNetworkStartCallback(before_network_start_callback_);
   network_trans_->SetBeforeHeadersSentCallback(before_headers_sent_callback_);
 
   // Old load timing information, if any, is now obsolete.
   old_network_trans_load_timing_.reset();
   old_remote_endpoint_ = IPEndPoint();
 
   if (websocket_handshake_stream_base_create_helper_)
     network_trans_->SetWebSocketHandshakeStreamCreateHelper(
         websocket_handshake_stream_base_create_helper_);
 
-  next_state_ = STATE_SEND_REQUEST_COMPLETE;
+  SetNextStateFromDoLoop(STATE_SEND_REQUEST_COMPLETE);
   rv = network_trans_->Start(request_, io_callback_, net_log_);
   return rv;
 }
 
 int HttpCache::Transaction::DoSendRequestComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoSendRequestComplete");
   if (!cache_.get()) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_UNEXPECTED;
   }
 
   // If we tried to conditionalize the request and failed, we know
   // we won't be reading from the cache after this point.
   if (couldnt_conditionalize_request_)
     mode_ = WRITE;
 
   if (result == OK) {
-    next_state_ = STATE_SUCCESSFUL_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SUCCESSFUL_SEND_REQUEST);
     return OK;
   }
 
   const HttpResponseInfo* response = network_trans_->GetResponseInfo();
   response_.network_accessed = response->network_accessed;
 
   // Do not record requests that have network errors or restarts.
   UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_OTHER);
   if (IsCertificateError(result)) {
     // If we get a certificate error, then there is a certificate in ssl_info,
     // so GetResponseInfo() should never return NULL here.
     DCHECK(response);
     response_.ssl_info = response->ssl_info;
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     DCHECK(response);
     response_.cert_request_info = response->cert_request_info;
   } else if (response_.was_cached) {
     DoneWritingToEntry(true);
   }
 
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return result;
 }
 
 // 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_) {
-      next_state_ = STATE_NONE;
+      SetNextStateFromDoLoop(STATE_NONE);
       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());
-      next_state_ = STATE_SEND_REQUEST_COMPLETE;
+      SetNextStateFromDoLoop(STATE_SEND_REQUEST_COMPLETE);
       // In theory we should check to see if there are new cookies, but there
       // is no way to do that from here.
       return network_trans_->RestartWithAuth(AuthCredentials(), io_callback_);
     }
 
     // We have to perform cleanup at this point so that at least the next
     // request can succeed.  We do not retry at this point, because data
     // has been read and we have no way to gather credentials.  We would
     // fail again, and potentially loop.  This can happen if the credentials
     // expire while chrome is suspended.
     if (entry_)
       DoomPartialEntry(false);
     mode_ = NONE;
     partial_.reset();
     ResetNetworkTransaction();
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_CACHE_AUTH_FAILURE_AFTER_READ;
   }
 
   new_response_ = new_response;
   if (!ValidatePartialResponse() && !auth_response_.headers.get()) {
     // Something went wrong with this request and we have to restart it.
     // If we have an authentication response, we are exposed to weird things
     // hapenning if the user cancels the authentication before we receive
     // the new response.
     net_log_.AddEvent(NetLogEventType::HTTP_CACHE_RE_SEND_PARTIAL_REQUEST);
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_OTHER);
     SetResponse(HttpResponseInfo());
     ResetNetworkTransaction();
     new_response_ = NULL;
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
     return OK;
   }
 
   if (handling_206_ && mode_ == READ_WRITE && !truncated_ && !is_sparse_) {
     // We have stored the full entry, but it changed and the server is
     // sending a range. We have to delete the old entry.
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_OTHER);
     DoneWritingToEntry(false);
   }
 
@@ skipping 20 matching lines @@
       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_);
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     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);
-      next_state_ = STATE_UPDATE_CACHED_RESPONSE;
+      SetNextStateFromDoLoop(STATE_UPDATE_CACHED_RESPONSE);
       return OK;
     }
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_UPDATED);
     mode_ = WRITE;
   }
 
-  next_state_ = STATE_OVERWRITE_CACHED_RESPONSE;
+  SetNextStateFromDoLoop(STATE_OVERWRITE_CACHED_RESPONSE);
   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");
-  next_state_ = STATE_UPDATE_CACHED_RESPONSE_COMPLETE;
   int rv = OK;
   // Update the cached response based on the headers and properties of
   // new_response_.
   response_.headers->Update(*new_response_->headers.get());
   response_.response_time = new_response_->response_time;
   response_.request_time = new_response_->request_time;
   response_.network_accessed = new_response_->network_accessed;
   response_.unused_since_prefetch = new_response_->unused_since_prefetch;
   response_.ssl_info = new_response_->ssl_info;
   if (new_response_->vary_data.is_valid()) {
     response_.vary_data = new_response_->vary_data;
   } else if (response_.vary_data.is_valid()) {
     // There is a vary header in the stored response but not in the current one.
     // Update the data with the new request headers.
     HttpVaryData new_vary_data;
     new_vary_data.Init(*request_, *response_.headers.get());
     response_.vary_data = new_vary_data;
   }
 
   if (response_.headers->HasHeaderValue("cache-control", "no-store")) {
     if (!entry_->doomed) {
       int ret = cache_->DoomEntry(cache_key_, NULL);
       DCHECK_EQ(OK, ret);
     }
   } else {
     // If we are already reading, we already updated the headers for this
     // request; doing it again will change Content-Length.
     if (!reading_) {
-      next_state_ = STATE_CACHE_WRITE_UPDATED_RESPONSE;
+      SetNextStateFromDoLoop(STATE_CACHE_WRITE_UPDATED_RESPONSE);
       rv = OK;
     }
   }
+
+  if (next_state_ == STATE_UNSET)
+    SetNextStateFromDoLoop(STATE_UPDATE_CACHED_RESPONSE_COMPLETE);

[Randy Smith (Not in Mondays), 2017/03/22 15:42:12]

Slight preference for rewriting the above conditional s.t. we explicitly set
state along each path.  (The rewrite I imagine is storing the result of
HasHeaderVAlue in a boolean, then doing a (!success && !reading_) test for the
state setting, but just setting things everywhere is fine too.  As is what you
have; I'm just expressing a preference.)

[jkarlin, 2017/03/22 17:12:00]

Agree, this is lame. Fixed.

   return rv;
 }
 
 int HttpCache::Transaction::DoCacheWriteUpdatedResponse() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteUpdatedResponse");
 
-  next_state_ = STATE_CACHE_WRITE_UPDATED_RESPONSE_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_WRITE_UPDATED_RESPONSE_COMPLETE);
   return WriteResponseInfoToEntry(false);
 }
 
 int HttpCache::Transaction::DoCacheWriteUpdatedResponseComplete(int result) {
   TRACE_EVENT0("io",
                "HttpCacheTransaction::DoCacheWriteUpdatedResponseComplete");
-  next_state_ = STATE_UPDATE_CACHED_RESPONSE_COMPLETE;
+  SetNextStateFromDoLoop(STATE_UPDATE_CACHED_RESPONSE_COMPLETE);
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
 int HttpCache::Transaction::DoUpdateCachedResponseComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoUpdateCachedResponseComplete");
   if (mode_ == UPDATE) {
     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();
     new_response_ = NULL;
-    next_state_ = STATE_START_PARTIAL_CACHE_VALIDATION;
+    SetNextStateFromDoLoop(STATE_START_PARTIAL_CACHE_VALIDATION);
     partial_->SetRangeToStartDownload();
     return OK;
   }
-  next_state_ = STATE_OVERWRITE_CACHED_RESPONSE;
+  SetNextStateFromDoLoop(STATE_OVERWRITE_CACHED_RESPONSE);
   return OK;
 }
 
 int HttpCache::Transaction::DoOverwriteCachedResponse() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoOverwriteCachedResponse");
   if (mode_ & READ) {
-    next_state_ = STATE_PARTIAL_HEADERS_RECEIVED;
+    SetNextStateFromDoLoop(STATE_PARTIAL_HEADERS_RECEIVED);
     return OK;
   }
 
   // We change the value of Content-Length for partial content.
   if (handling_206_ && partial_)
     partial_->FixContentLength(new_response_->headers.get());
 
   SetResponse(*new_response_);
 
   if (request_->method == "HEAD") {
     // This response is replacing the cached one.
     DoneWritingToEntry(false);
     mode_ = NONE;
     new_response_ = NULL;
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return OK;
   }
 
   if (handling_206_ && !CanResume(false)) {
     // There is no point in storing this resource because it will never be used.
     // This may change if we support LOAD_ONLY_FROM_CACHE with sparse entries.
     DoneWritingToEntry(false);
     if (partial_)
       partial_->FixResponseHeaders(response_.headers.get(), true);
-    next_state_ = STATE_PARTIAL_HEADERS_RECEIVED;
+    SetNextStateFromDoLoop(STATE_PARTIAL_HEADERS_RECEIVED);
     return OK;
   }
 
-  next_state_ = STATE_CACHE_WRITE_RESPONSE;
+  SetNextStateFromDoLoop(STATE_CACHE_WRITE_RESPONSE);
   return OK;
 }
 
 int HttpCache::Transaction::DoCacheWriteResponse() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteResponse");
-  next_state_ = STATE_CACHE_WRITE_RESPONSE_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_WRITE_RESPONSE_COMPLETE);
   return WriteResponseInfoToEntry(truncated_);
 }
 
 int HttpCache::Transaction::DoCacheWriteResponseComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteResponseComplete");
-  next_state_ = STATE_TRUNCATE_CACHED_DATA;
+  SetNextStateFromDoLoop(STATE_TRUNCATE_CACHED_DATA);
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
 int HttpCache::Transaction::DoTruncateCachedData() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoTruncateCachedData");
-  next_state_ = STATE_TRUNCATE_CACHED_DATA_COMPLETE;
+  SetNextStateFromDoLoop(STATE_TRUNCATE_CACHED_DATA_COMPLETE);
   if (!entry_)
     return OK;
   if (net_log_.IsCapturing())
     net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_WRITE_DATA);
   // Truncate the stream.
   return WriteToEntry(kResponseContentIndex, 0, NULL, 0, io_callback_);
 }
 
 int HttpCache::Transaction::DoTruncateCachedDataComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoInitEntry");
   if (entry_) {
     if (net_log_.IsCapturing()) {
       net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_WRITE_DATA,
                                         result);
     }
   }
 
-  next_state_ = STATE_TRUNCATE_CACHED_METADATA;
+  SetNextStateFromDoLoop(STATE_TRUNCATE_CACHED_METADATA);
   return OK;
 }
 
 int HttpCache::Transaction::DoTruncateCachedMetadata() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoTruncateCachedMetadata");
-  next_state_ = STATE_TRUNCATE_CACHED_METADATA_COMPLETE;
+  SetNextStateFromDoLoop(STATE_TRUNCATE_CACHED_METADATA_COMPLETE);
   if (!entry_)
     return OK;
 
   if (net_log_.IsCapturing())
     net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_WRITE_INFO);
   return WriteToEntry(kMetadataIndex, 0, NULL, 0, io_callback_);
 }
 
 int HttpCache::Transaction::DoTruncateCachedMetadataComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoTruncateCachedMetadataComplete");
   if (entry_) {
     if (net_log_.IsCapturing()) {
       net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_WRITE_INFO,
                                         result);
     }
   }
 
-  next_state_ = STATE_PARTIAL_HEADERS_RECEIVED;
+  SetNextStateFromDoLoop(STATE_PARTIAL_HEADERS_RECEIVED);
   return OK;
 }
 
 int HttpCache::Transaction::DoPartialHeadersReceived() {
   new_response_ = NULL;
 
   if (!partial_) {
     if (entry_ && entry_->disk_entry->GetDataSize(kMetadataIndex))
-      next_state_ = STATE_CACHE_READ_METADATA;
+      SetNextStateFromDoLoop(STATE_CACHE_READ_METADATA);
     else
-      next_state_ = STATE_NONE;
+      SetNextStateFromDoLoop(STATE_NONE);
     return OK;
   }
 
   if (reading_) {
     if (network_trans_.get()) {
-      next_state_ = STATE_NETWORK_READ;
+      SetNextStateFromDoLoop(STATE_NETWORK_READ);
     } else {
-      next_state_ = STATE_CACHE_READ_DATA;
+      SetNextStateFromDoLoop(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);
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
   } else {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
   }
   return OK;
 }
 
 int HttpCache::Transaction::DoCacheReadMetadata() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadMetadata");
   DCHECK(entry_);
   DCHECK(!response_.metadata.get());
-  next_state_ = STATE_CACHE_READ_METADATA_COMPLETE;
+  SetNextStateFromDoLoop(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);
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return OK;
 }
 
 int HttpCache::Transaction::DoNetworkRead() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoNetworkRead");
-  next_state_ = STATE_NETWORK_READ_COMPLETE;
+  SetNextStateFromDoLoop(STATE_NETWORK_READ_COMPLETE);
   return network_trans_->Read(read_buf_.get(), io_buf_len_, io_callback_);
 }
 
 int HttpCache::Transaction::DoNetworkReadComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoNetworkReadComplete");
   DCHECK(mode_ & WRITE || mode_ == NONE);
 
   if (!cache_.get()) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_UNEXPECTED;
   }
 
   // If there is an error or we aren't saving the data, we are done; just wait
   // until the destructor runs to see if we can keep the data.
   if (mode_ == NONE || result < 0) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return result;
   }
 
-  next_state_ = STATE_CACHE_WRITE_DATA;
+  SetNextStateFromDoLoop(STATE_CACHE_WRITE_DATA);
   return result;
 }
 
 int HttpCache::Transaction::DoCacheReadData() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadData");
 
   if (request_->method == "HEAD") {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return 0;
   }
 
   DCHECK(entry_);
-  next_state_ = STATE_CACHE_READ_DATA_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_READ_DATA_COMPLETE);
 
   if (net_log_.IsCapturing())
     net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_READ_DATA);
   if (partial_) {
     return partial_->CacheRead(entry_->disk_entry, read_buf_.get(), io_buf_len_,
                                io_callback_);
   }
 
   return entry_->disk_entry->ReadData(kResponseContentIndex, read_offset_,
                                       read_buf_.get(), io_buf_len_,
                                       io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheReadDataComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadDataComplete");
   if (net_log_.IsCapturing()) {
     net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_READ_DATA,
                                       result);
   }
 
   if (!cache_.get()) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_UNEXPECTED;
   }
 
   if (partial_) {
     // Partial requests are confusing to report in histograms because they may
     // have multiple underlying requests.
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_OTHER);
     return DoPartialCacheReadCompleted(result);
   }
 
   if (result > 0) {
     read_offset_ += result;
   } else if (result == 0) {  // End of file.
     RecordHistograms();
     cache_->DoneReadingFromEntry(entry_, this);
     entry_ = NULL;
   } else {
     return OnCacheReadError(result, false);
   }
 
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return result;
 }
 
 int HttpCache::Transaction::DoCacheWriteData(int num_bytes) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteData");
-  next_state_ = STATE_CACHE_WRITE_DATA_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_WRITE_DATA_COMPLETE);
   write_len_ = num_bytes;
   if (entry_) {
     if (net_log_.IsCapturing())
       net_log_.BeginEvent(NetLogEventType::HTTP_CACHE_WRITE_DATA);
   }
 
   if (!entry_ || !num_bytes)
     return num_bytes;
 
   int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex);
   return WriteToEntry(kResponseContentIndex, current_size, read_buf_.get(),
                       num_bytes, io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheWriteDataComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteDataComplete");
   if (entry_) {
     if (net_log_.IsCapturing()) {
       net_log_.EndEventWithNetErrorCode(NetLogEventType::HTTP_CACHE_WRITE_DATA,
                                         result);
     }
   }
   if (!cache_.get()) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_UNEXPECTED;
   }
 
   if (result != write_len_) {
     DLOG(ERROR) << "failed to write response data to cache";
     DoneWritingToEntry(false);
 
     // We want to ignore errors writing to disk and just keep reading from
     // the network.
     result = write_len_;
@@ skipping 14 matching lines @@
 
   if (result == 0) {
     // End of file. This may be the result of a connection problem so see if we
     // have to keep the entry around to be flagged as truncated later on.
     if (done_reading_ || !entry_ || partial_ ||
         response_.headers->GetContentLength() <= 0) {
       DoneWritingToEntry(true);
     }
   }
 
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return result;
 }
 
 int HttpCache::Transaction::DoCacheWriteTruncatedResponse() {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteTruncatedResponse");
-  next_state_ = STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE;
+  SetNextStateFromDoLoop(STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE);
   return WriteResponseInfoToEntry(true);
 }
 
 int HttpCache::Transaction::DoCacheWriteTruncatedResponseComplete(int result) {
   TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheWriteTruncatedResponse");
 
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
 //-----------------------------------------------------------------------------
 
 void HttpCache::Transaction::SetRequest(const NetLogWithSource& net_log,
                                         const HttpRequestInfo* request) {
   net_log_ = net_log;
   request_ = request;
   effective_load_flags_ = request_->load_flags;
@@ skipping 116 matching lines @@
     return false;
 
   return true;
 }
 
 int HttpCache::Transaction::BeginCacheRead() {
   // We don't support any combination of LOAD_ONLY_FROM_CACHE and byte ranges.
   // TODO(jkarlin): Either handle this case or DCHECK.
   if (response_.headers->response_code() == 206 || partial_) {
     NOTREACHED();
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_CACHE_MISS;
   }
 
   // We don't have the whole resource.
   if (truncated_) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_CACHE_MISS;
   }
 
   if (RequiresValidation() != VALIDATION_NONE) {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
     return ERR_CACHE_MISS;
   }
 
   if (request_->method == "HEAD")
     FixHeadersForHead();
 
   if (entry_->disk_entry->GetDataSize(kMetadataIndex))
-    next_state_ = STATE_CACHE_READ_METADATA;
+    SetNextStateFromDoLoop(STATE_CACHE_READ_METADATA);
   else
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
 
   return OK;
 }
 
 int HttpCache::Transaction::BeginCacheValidation() {
   DCHECK_EQ(mode_, READ_WRITE);
 
   ValidationType required_validation = RequiresValidation();
 
   bool skip_validation = (required_validation == VALIDATION_NONE);
 
   if ((effective_load_flags_ & LOAD_SUPPORT_ASYNC_REVALIDATION) &&
       required_validation == VALIDATION_ASYNCHRONOUS) {
     DCHECK_EQ(request_->method, "GET");
     skip_validation = true;
     response_.async_revalidation_required = true;
   }
 
   if (request_->method == "HEAD" &&
       (truncated_ || response_.headers->response_code() == 206)) {
     DCHECK(!partial_);
     if (skip_validation)
       return SetupEntryForRead();
 
     // Bail out!
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
     mode_ = NONE;
     return OK;
   }
 
   if (truncated_) {
     // Truncated entries can cause partial gets, so we shouldn't record this
     // load in histograms.
     UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_OTHER);
     skip_validation = !partial_->initial_validation();
   }
@@ skipping 17 matching lines @@
     // know we won't be falling back to using the cache entry in the
     // LOAD_FROM_CACHE_IF_OFFLINE case.
     if (!ConditionalizeRequest()) {
       couldnt_conditionalize_request_ = true;
       UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_CANT_CONDITIONALIZE);
       if (partial_)
         return DoRestartPartialRequest();
 
       DCHECK_NE(206, response_.headers->response_code());
     }
-    next_state_ = STATE_SEND_REQUEST;
+    SetNextStateFromDoLoop(STATE_SEND_REQUEST);
   }
   return OK;
 }
 
 int HttpCache::Transaction::BeginPartialCacheValidation() {
   DCHECK_EQ(mode_, READ_WRITE);
 
   if (response_.headers->response_code() != 206 && !partial_ && !truncated_)
     return BeginCacheValidation();
 
   // Partial requests should not be recorded in histograms.
   UpdateCacheEntryStatus(CacheEntryStatus::ENTRY_OTHER);
   if (request_->method == "HEAD")
     return BeginCacheValidation();
 
   if (!range_requested_) {
     // The request is not for a range, but we have stored just ranges.
 
     partial_.reset(new PartialData());
     partial_->SetHeaders(request_->extra_headers);
     if (!custom_request_.get()) {
       custom_request_.reset(new HttpRequestInfo(*request_));
       request_ = custom_request_.get();
     }
   }
 
-  next_state_ = STATE_CACHE_QUERY_DATA;
+  SetNextStateFromDoLoop(STATE_CACHE_QUERY_DATA);
   return OK;
 }
 
 // This should only be called once per request.
 int HttpCache::Transaction::ValidateEntryHeadersAndContinue() {
   DCHECK_EQ(mode_, READ_WRITE);
 
   if (!partial_->UpdateFromStoredHeaders(
           response_.headers.get(), entry_->disk_entry, truncated_)) {
     return DoRestartPartialRequest();
   }
 
   if (response_.headers->response_code() == 206)
     is_sparse_ = true;
 
   if (!partial_->IsRequestedRangeOK()) {
     // The stored data is fine, but the request may be invalid.
     invalid_range_ = true;
   }
 
-  next_state_ = STATE_START_PARTIAL_CACHE_VALIDATION;
+  SetNextStateFromDoLoop(STATE_START_PARTIAL_CACHE_VALIDATION);
   return OK;
 }
 
 int HttpCache::Transaction::BeginExternallyConditionalizedRequest() {
   DCHECK_EQ(UPDATE, mode_);
   DCHECK(external_validation_.initialized);
 
   for (size_t i = 0;  i < arraysize(kValidationHeaders); i++) {
     if (external_validation_.values[i].empty())
       continue;
@@ skipping 22 matching lines @@
     type = EXTERNALLY_CONDITIONALIZED_MISMATCHED_VALIDATORS;
   else if (RequiresValidation() != VALIDATION_NONE)
     type = EXTERNALLY_CONDITIONALIZED_CACHE_REQUIRES_VALIDATION;
 
   // TODO(ricea): Add CACHE_USABLE_STALE once stale-while-revalidate CL landed.
   // TODO(ricea): Either remove this histogram or make it permanent by M40.
   UMA_HISTOGRAM_ENUMERATION("HttpCache.ExternallyConditionalized",
                             type,
                             EXTERNALLY_CONDITIONALIZED_MAX);
 
-  next_state_ = STATE_SEND_REQUEST;
+  SetNextStateFromDoLoop(STATE_SEND_REQUEST);
   return OK;
 }
 
 int HttpCache::Transaction::RestartNetworkRequest() {
   DCHECK(mode_ & WRITE || mode_ == NONE);
   DCHECK(network_trans_.get());
   DCHECK_EQ(STATE_NONE, next_state_);
 
   next_state_ = STATE_SEND_REQUEST_COMPLETE;
   int rv = network_trans_->RestartIgnoringLastError(io_callback_);
@@ skipping 331 matching lines @@
   }
 }
 
 int HttpCache::Transaction::SetupEntryForRead() {
   if (network_trans_)
     ResetNetworkTransaction();
   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.
-      next_state_ = STATE_PARTIAL_HEADERS_RECEIVED;
+      SetNextStateFromDoLoop(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))
-    next_state_ = STATE_CACHE_READ_METADATA;
+    SetNextStateFromDoLoop(STATE_CACHE_READ_METADATA);
   else
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
   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 85 matching lines @@
   if (cache_.get())
     cache_->DoomActiveEntry(cache_key_);
 
   if (restart) {
     DCHECK(!reading_);
     DCHECK(!network_trans_.get());
     cache_->DoneWithEntry(entry_, this, false);
     entry_ = NULL;
     is_sparse_ = false;
     partial_.reset();
-    next_state_ = STATE_GET_BACKEND;
+    SetNextStateFromDoLoop(STATE_GET_BACKEND);
     return OK;
   }
 
-  next_state_ = STATE_NONE;
+  SetNextStateFromDoLoop(STATE_NONE);
   return ERR_CACHE_READ_FAILURE;
 }
 
 void HttpCache::Transaction::OnAddToEntryTimeout(base::TimeTicks start_time) {
   if (entry_lock_waiting_since_ != start_time)
     return;
 
   DCHECK_EQ(next_state_, STATE_ADD_TO_ENTRY_COMPLETE);
 
   if (!cache_)
@@ skipping 14 matching lines @@
   if (delete_object)
     partial_.reset(NULL);
 }
 
 int HttpCache::Transaction::DoPartialNetworkReadCompleted(int result) {
   partial_->OnNetworkReadCompleted(result);
 
   if (result == 0) {
     // We need to move on to the next range.
     ResetNetworkTransaction();
-    next_state_ = STATE_START_PARTIAL_CACHE_VALIDATION;
+    SetNextStateFromDoLoop(STATE_START_PARTIAL_CACHE_VALIDATION);
   } else {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
   }
   return result;
 }
 
 int HttpCache::Transaction::DoPartialCacheReadCompleted(int result) {
   partial_->OnCacheReadCompleted(result);
 
   if (result == 0 && mode_ == READ_WRITE) {
     // We need to move on to the next range.
-    next_state_ = STATE_START_PARTIAL_CACHE_VALIDATION;
+    SetNextStateFromDoLoop(STATE_START_PARTIAL_CACHE_VALIDATION);
   } else if (result < 0) {
     return OnCacheReadError(result, false);
   } else {
-    next_state_ = STATE_NONE;
+    SetNextStateFromDoLoop(STATE_NONE);
   }
   return result;
 }
 
 int HttpCache::Transaction::DoRestartPartialRequest() {
   // The stored data cannot be used. Get rid of it and restart this request.
   net_log_.AddEvent(NetLogEventType::HTTP_CACHE_RESTART_PARTIAL_REQUEST);
 
   // WRITE + Doom + STATE_INIT_ENTRY == STATE_CREATE_ENTRY (without an attempt
   // to Doom the entry again).
   mode_ = WRITE;
   ResetPartialState(!range_requested_);
-  next_state_ = STATE_CREATE_ENTRY;
+  SetNextStateFromDoLoop(STATE_CREATE_ENTRY);
   return OK;
 }
 
 void HttpCache::Transaction::ResetPartialState(bool delete_object) {
   partial_->RestoreHeaders(&custom_request_->extra_headers);
   DoomPartialEntry(delete_object);
 
   if (!delete_object) {
     // The simplest way to re-initialize partial_ is to create a new object.
     partial_.reset(new PartialData());
@@ skipping 244 matching lines @@
     }
     default:
       NOTREACHED();
   }
 }
 
 void HttpCache::Transaction::OnIOComplete(int result) {
   DoLoop(result);
 }
 
+void HttpCache::Transaction::SetNextStateFromDoLoop(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