[net] Better StopCaching() handling for HttpCache::Transaction.

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 248 matching lines @@
       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),
+      stopped_caching_(false),
       bypass_lock_for_test_(false),
       fail_conditionalization_for_test_(false),
+      read_buf_len_(0),
       io_buf_len_(0),
       read_offset_(0),
       effective_load_flags_(0),
       write_len_(0),
       transaction_pattern_(PATTERN_UNDEFINED),
       total_received_bytes_(0),
       total_sent_bytes_(0),
       websocket_handshake_stream_base_create_helper_(NULL),
       weak_factory_(this) {
   static_assert(HttpCache::Transaction::kNumValidationHeaders ==
                     arraysize(kValidationHeaders),
                 "invalid number of validation headers");
 
   io_callback_ = base::Bind(&Transaction::OnIOComplete,
                               weak_factory_.GetWeakPtr());
 }
 
 HttpCache::Transaction::~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);
-        }
-      }
+  RecordHistograms();
 
-      cache_->DoneWithEntry(entry_, this, cancel_request);
-    } else if (cache_pending_) {
+  if (!cache_)
+    return;
+
+  if (!entry_) {
+    if (cache_pending_)
       cache_->RemovePendingTransaction(this);
-    }
+    return;
   }
+
+  bool cancel_request = reading_ && response_.headers.get() &&
+                        (response_.headers->response_code() == 200 || partial_);
+  if (cancel_request) {
+    if (partial_)
+      entry_->disk_entry->CancelSparseIO();
+
+    AbandonCacheEntry(CompletionCallback());
+    return;
+  }
+
+  ReleaseCacheEntry(EntryState::DOOM);
 }
 
 int HttpCache::Transaction::WriteMetadata(IOBuffer* buf, int buf_len,
                                           const CompletionCallback& callback) {
   DCHECK(buf);
   DCHECK_GT(buf_len, 0);
   DCHECK(!callback.is_null());
   if (!cache_.get() || !entry_)
     return ERR_UNEXPECTED;
 
   // We don't need to track this operation for anything.
   // It could be possible to check if there is something already written and
   // avoid writing again (it should be the same, right?), but let's allow the
   // caller to "update" the contents with something new.
   return entry_->disk_entry->WriteData(kMetadataIndex, 0, buf, buf_len,
                                        callback, true);
 }
 
-bool HttpCache::Transaction::AddTruncatedFlag() {
-  DCHECK(mode_ & WRITE || mode_ == NONE);
-
-  // Don't set the flag for sparse entries.
-  if (partial_ && !truncated_)
-    return true;
-
-  if (!CanResume(true))
-    return false;
-
-  // We may have received the whole resource already.
-  if (done_reading_)
-    return true;
-
-  truncated_ = true;
-  next_state_ = STATE_CACHE_WRITE_TRUNCATED_RESPONSE;
-  DoLoop(OK);
-  return true;
-}
-
 LoadState HttpCache::Transaction::GetWriterLoadState() const {
   if (network_trans_.get())
     return network_trans_->GetLoadState();
   if (entry_ || !request_)
     return LOAD_STATE_IDLE;
   return LOAD_STATE_WAITING_FOR_CACHE;
 }
 
 const BoundNetLog& HttpCache::Transaction::net_log() const {
   return net_log_;
@@ skipping 94 matching lines @@
     return false;
   return network_trans_->IsReadyToRestartForAuth();
 }
 
 int HttpCache::Transaction::Read(IOBuffer* buf, int buf_len,
                                  const CompletionCallback& callback) {
   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;
 
   // If we have an intermediate auth response at this point, then it means the
   // user wishes to read the network response (the error page).  If there is a
   // previous response in the cache then we should leave it intact.
   if (auth_response_.headers.get() && mode_ != NONE) {
     UpdateTransactionPattern(PATTERN_NOT_COVERED);
     DCHECK(mode_ & WRITE);
-    DoneWritingToEntry(mode_ == READ_WRITE);
-    mode_ = NONE;
+    ReleaseCacheEntry(mode_ == READ_WRITE ? EntryState::KEEP
+                                          : EntryState::DOOM);
   }
 
   reading_ = true;
   read_buf_ = buf;
-  io_buf_len_ = buf_len;
-  if (network_trans_) {
+  read_buf_len_ = buf_len;
+  if (stopped_caching_ && entry_) {
+    DCHECK(mode_ & WRITE);
+    next_state_ = STATE_SWITCH_TO_NETWORK;
+  } else if (network_trans_) {
     DCHECK(mode_ == WRITE || mode_ == NONE ||
            (mode_ == READ_WRITE && partial_));
     next_state_ = STATE_NETWORK_READ;
   } else {
     DCHECK(mode_ == READ || (mode_ == READ_WRITE && partial_));
     next_state_ = STATE_CACHE_READ_DATA;
   }
 
   int rv = DoLoop(OK);
 
   if (rv == ERR_IO_PENDING) {
     DCHECK(callback_.is_null());
     callback_ = callback;
   }
   return rv;
 }
 
 void HttpCache::Transaction::StopCaching() {
-  // We really don't know where we are now. Hopefully there is no operation in
-  // progress, but nothing really prevents this method to be called after we
-  // returned ERR_IO_PENDING. We cannot attempt to truncate the entry at this
-  // point because we need the state machine for that (and even if we are really
-  // free, that would be an asynchronous operation). In other words, keep the
-  // entry how it is (it will be marked as truncated at destruction), and let
-  // the next piece of code that executes know that we are now reading directly
-  // from the net.
-  // TODO(mmenke):  This doesn't release the lock on the cache entry, so a
-  //                future request for the resource will be blocked on this one.
-  //                Fix this.
-  if (cache_.get() && entry_ && (mode_ & WRITE) && network_trans_.get() &&
-      !is_sparse_ && !range_requested_) {
-    mode_ = NONE;
-  }
+  DCHECK(callback_.is_null());
+  DCHECK(request_);
+
+  // StopCaching requires being able to restart partial network requests. Doing
+  // so is only reliable if the request is idempotent. In addition, StopCaching
+  // is only effective if the transaction is writing to the cache.
+  if ((!partial_ || request_->method == "GET") && (mode_ & WRITE))
+    stopped_caching_ = true;

[rvargas (doing something else), 2015/09/21 22:06:00]

It looks like we set the flag when method != GET. I don't think this call makes
sense (atm) for any other method so it should be ignored if that's the case, no?

 }
 
 bool HttpCache::Transaction::GetFullRequestHeaders(
     HttpRequestHeaders* headers) const {
   if (network_trans_)
     return network_trans_->GetFullRequestHeaders(headers);
 
   // TODO(ttuttle): Read headers from cache.
   return false;
 }
 
 int64 HttpCache::Transaction::GetTotalReceivedBytes() const {
   int64 total_received_bytes = total_received_bytes_;
   if (network_trans_)
     total_received_bytes += network_trans_->GetTotalReceivedBytes();
   return total_received_bytes;
 }
 
 int64_t HttpCache::Transaction::GetTotalSentBytes() const {
   int64_t total_sent_bytes = total_sent_bytes_;
   if (network_trans_)
     total_sent_bytes += network_trans_->GetTotalSentBytes();
   return total_sent_bytes;
 }
 
 void HttpCache::Transaction::DoneReading() {
-  if (cache_.get() && entry_) {
-    DCHECK_NE(mode_, UPDATE);
-    if (mode_ & WRITE) {
-      DoneWritingToEntry(true);
-    } else if (mode_ & READ) {
-      // It is necessary to check mode_ & READ because it is possible
-      // for mode_ to be NONE and entry_ non-NULL with a write entry
-      // if StopCaching was called.
-      cache_->DoneReadingFromEntry(entry_, this);
-      entry_ = NULL;
-    }
-  }
+  DCHECK_NE(mode_, UPDATE);
+  if (!cache_ || !entry_)
+    return;
+  done_reading_ = true;
+  ReleaseCacheEntry(EntryState::KEEP);
 }
 
 const HttpResponseInfo* HttpCache::Transaction::GetResponseInfo() const {
   // Null headers means we encountered an error or haven't a response yet
   if (auth_response_.headers.get())
     return &auth_response_;
   return &response_;
 }
 
 LoadState HttpCache::Transaction::GetLoadState() const {
@@ skipping 341 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_SWITCH_TO_NETWORK:
+        rv = DoSwitchToNetwork();
+        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();
         break;
       case STATE_CACHE_READ_DATA_COMPLETE:
         rv = DoCacheReadDataComplete(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:
-        DCHECK_EQ(OK, rv);
-        rv = DoCacheWriteTruncatedResponse();
-        break;
-      case STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE:
-        rv = DoCacheWriteTruncatedResponseComplete(rv);
-        break;
       default:
         NOTREACHED() << "bad state";
         rv = ERR_FAILED;
         break;
     }
   } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
 
   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.
+    read_buf_len_ = 0;
     base::ResetAndReturn(&callback_).Run(rv);
   }
 
   return rv;
 }
 
 int HttpCache::Transaction::DoGetBackend() {
   cache_pending_ = true;
   next_state_ = STATE_GET_BACKEND_COMPLETE;
   net_log_.BeginEvent(NetLog::TYPE_HTTP_CACHE_GET_BACKEND);
@@ skipping 111 matching lines @@
     mode_ = NONE;
     next_state_ = STATE_SEND_REQUEST;
     return OK;
   }
 
   if (mode_ == READ_WRITE) {
     mode_ = WRITE;
     next_state_ = 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;
     return OK;
   }
 
   // The entry does not exist, and we are not permitted to create a new entry,
   // so we must fail.
   return ERR_CACHE_MISS;
@@ skipping 197 matching lines @@
   // 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.
 
   // TODO(rtenneti): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 HttpCache::Transaction::DoCacheToggleUnusedSincePrefetch"));
 
   next_state_ = STATE_TOGGLE_UNUSED_SINCE_PREFETCH_COMPLETE;
-  return WriteResponseInfoToEntry(false);
+  return WriteResponseInfoToEntry(false, io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheToggleUnusedSincePrefetchComplete(
     int result) {
   // Restore the original value for this transaction.
   response_.unused_since_prefetch = !response_.unused_since_prefetch;
   next_state_ = STATE_CACHE_DISPATCH_VALIDATION;
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
@@ skipping 47 matching lines @@
 int HttpCache::Transaction::DoStartPartialCacheValidation() {
   if (mode_ == NONE)
     return OK;
 
   next_state_ = 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;
-    }
-    return result;
+    ReleaseCacheEntry(EntryState::KEEP);
+    return 0;
   }
 
   if (result < 0)
     return result;
 
   partial_->PrepareCacheValidation(entry_->disk_entry,
                                    &custom_request_->extra_headers);
 
   if (reading_ && partial_->IsCurrentRangeCached()) {
     next_state_ = STATE_CACHE_READ_DATA;
@@ skipping 51 matching lines @@
   UpdateTransactionPattern(PATTERN_NOT_COVERED);
   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);
+    ReleaseCacheEntry(EntryState::KEEP);
   }
 
   return result;
 }
 
 // We received the response headers and there is no error.
 int HttpCache::Transaction::DoSuccessfulSendRequest() {
   DCHECK(!new_response_);
   const HttpResponseInfo* new_response = network_trans_->GetResponseInfo();
 
@@ skipping 35 matching lines @@
     // the new response.
     net_log_.AddEvent(NetLog::TYPE_HTTP_CACHE_RE_SEND_PARTIAL_REQUEST);
     UpdateTransactionPattern(PATTERN_NOT_COVERED);
     response_ = HttpResponseInfo();
     ResetNetworkTransaction();
     new_response_ = NULL;
     next_state_ = STATE_SEND_REQUEST;
     return OK;
   }
 
+  if (handling_206_ && stopped_caching_ && partial_ &&
+      partial_->IsLastRange() && !partial_->IsCurrentRangeCached() &&
+      (mode_ & WRITE) && entry_) {
+    // The current state of the transaction is:
+    // * The client has requested that no addtional data be cached.
+    // * The current network transaction spans the entire remainder of the
+    // requested range.
+    // * A cache entry exists and was being written to or updated.
+    // * The current range is not cached.
+    //
+    // At this point it is safe to abandon the cache entry and proceed with just
+    // the network transaction.
+    next_state_ = STATE_NETWORK_READ;
+    return AbandonCacheEntry(io_callback_);
+  }
+
   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.
     UpdateTransactionPattern(PATTERN_NOT_COVERED);
-    DoneWritingToEntry(false);
+    ReleaseCacheEntry(EntryState::DOOM);
   }
 
   if (mode_ == WRITE &&
       transaction_pattern_ != PATTERN_ENTRY_CANT_CONDITIONALIZE) {
     UpdateTransactionPattern(PATTERN_ENTRY_NOT_CACHED);
   }
 
   // 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);
-    entry_ = NULL;
-    mode_ = NONE;
+    ReleaseCacheEntry(EntryState::KEEP);
   }
 
   // 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);
@@ skipping 58 matching lines @@
   return rv;
 }
 
 int HttpCache::Transaction::DoCacheWriteUpdatedResponse() {
   // TODO(rtenneti): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 HttpCache::Transaction::DoCacheWriteUpdatedResponse"));
 
   next_state_ = STATE_CACHE_WRITE_UPDATED_RESPONSE_COMPLETE;
-  return WriteResponseInfoToEntry(false);
+  return WriteResponseInfoToEntry(false, io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheWriteUpdatedResponseComplete(int result) {
   next_state_ = STATE_UPDATE_CACHED_RESPONSE_COMPLETE;
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
 int HttpCache::Transaction::DoUpdateCachedResponseComplete(int result) {
   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);
+    ReleaseCacheEntry(EntryState::KEEP);
   } 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.
     final_upload_progress_ = network_trans_->GetUploadProgress();
     ResetNetworkTransaction();
   } else if (entry_ && handling_206_ && truncated_ &&
@@ skipping 18 matching lines @@
   }
 
   // We change the value of Content-Length for partial content.
   if (handling_206_ && partial_)
     partial_->FixContentLength(new_response_->headers.get());
 
   response_ = *new_response_;
 
   if (request_->method == "HEAD") {
     // This response is replacing the cached one.
-    DoneWritingToEntry(false);
-    mode_ = NONE;
-    new_response_ = NULL;
+    ReleaseCacheEntry(EntryState::DOOM);
+    new_response_ = nullptr;
     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);
+    ReleaseCacheEntry(EntryState::DOOM);
     if (partial_)
       partial_->FixResponseHeaders(response_.headers.get(), true);
     next_state_ = STATE_PARTIAL_HEADERS_RECEIVED;
     return OK;
   }
 
   next_state_ = STATE_CACHE_WRITE_RESPONSE;
   return OK;
 }
 
 int HttpCache::Transaction::DoCacheWriteResponse() {
   // TODO(rtenneti): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 HttpCache::Transaction::DoCacheWriteResponse"));
 
   next_state_ = STATE_CACHE_WRITE_RESPONSE_COMPLETE;
-  return WriteResponseInfoToEntry(truncated_);
+  return WriteResponseInfoToEntry(truncated_, io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheWriteResponseComplete(int result) {
   next_state_ = STATE_TRUNCATE_CACHED_DATA;
   return OnWriteResponseInfoToEntryComplete(result);
 }
 
 int HttpCache::Transaction::DoTruncateCachedData() {
   next_state_ = STATE_TRUNCATE_CACHED_DATA_COMPLETE;
   if (!entry_)
@@ skipping 75 matching lines @@
                                       io_callback_);
 }
 
 int HttpCache::Transaction::DoCacheReadMetadataComplete(int result) {
   net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_READ_INFO, result);
   if (result != response_.metadata->size())
     return OnCacheReadError(result, false);
   return OK;
 }
 
+int HttpCache::Transaction::DoSwitchToNetwork() {
+  DCHECK(entry_);
+  DCHECK(mode_ & WRITE);
+
+  // If the transaction is currently reading from the cache, then it should have
+  // a valid partial state. I.e. only part of the requested resource could be
+  // fulfiled via the cache. If the entire request was being fulfiled via the
+  // cache, the transaction shouldn't be trying to switch to the network at this
+  // point, and mode_ should be READ.
+  DCHECK_IMPLIES(!network_trans_, partial_);
+
+  // The network transaction can be trivially reused if it can promise to
+  // deliver all the bits necessary to fulfil the original request. If so,
+  // abandon the cache entry and continue with a network read.
+  if (network_trans_ && (!partial_ || partial_->IsLastRange())) {
+    next_state_ = STATE_NETWORK_READ;
+    return AbandonCacheEntry(io_callback_);
+  }
+
+  if (request_->method != "GET") {
+    // We have a non-GET request that the cache or network can partially fulfil.
+    // In other words, at some point the transaction is going to have to issue a
+    // partial network request to fulfil this non-GET request. Let's just ignore
+    // the StopCaching() directive for now.
+    stopped_caching_ = false;
+    next_state_ = network_trans_ ? STATE_NETWORK_READ : STATE_CACHE_READ_DATA;
+    return OK;
+  }
+
+  // The existing network transaction doesn't cover the entire range we wanted.
+  // Let's discard the network transaction and create a new one that will cover
+  // the entire range we need.
+  if (network_trans_)
+    ResetNetworkTransaction();
+
+  if (!partial_->SkipCacheForRemainder()) {
+    // PartialData only refuses to switch to network reads if the entire
+    // resource has been accounted for. There's nothing more to do.
+    return OK;
+  }
+
+  // The new network request that's going to be issued has to be validated
+  // against the existing cache entry. By extension, this also validates the new
+  // request against the portion that has already been sent to our client.
+  partial_->PrepareCacheValidation(entry_->disk_entry,
+                                   &custom_request_->extra_headers);
+
+  // Since we called SkipCacheForRemainder(), the partial_ state should now
+  // assume that the remainder of the resource is not cached.
+  DCHECK(!partial_->IsCurrentRangeCached());
+  DCHECK(partial_->IsLastRange());
+
+  // READ shouldn't reach here at all. WRITE/UPDATE require a network_trans_
+  // that spans the entire request range and is handled at the top of this
+  // function. At this point we should be left with just READ_WRITE.
+  DCHECK_EQ(mode_, READ_WRITE);
+
+  // Start validating the cache. DoSuccessfulSendRequest() will now bear the
+  // responsibility of abandoning the cache entry if the new network transaction
+  // is successfully validated.
+  return BeginCacheValidation();
+}
+
 int HttpCache::Transaction::DoNetworkRead() {
+  DCHECK_GT(read_buf_len_, 0);
+
+  io_buf_len_ = read_buf_len_;
   next_state_ = STATE_NETWORK_READ_COMPLETE;
   return network_trans_->Read(read_buf_.get(), io_buf_len_, io_callback_);
 }
 
 int HttpCache::Transaction::DoNetworkReadComplete(int result) {
   DCHECK(mode_ & WRITE || mode_ == NONE);
 
   if (!cache_.get())
     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)
     return result;
 
   next_state_ = STATE_CACHE_WRITE_DATA;
   return result;
 }
 
 int HttpCache::Transaction::DoCacheReadData() {
+  DCHECK_GT(read_buf_len_, 0);
+
   if (request_->method == "HEAD")
     return 0;
 
   DCHECK(entry_);
   next_state_ = STATE_CACHE_READ_DATA_COMPLETE;
+  io_buf_len_ = read_buf_len_;
 
   if (net_log_.IsCapturing())
     net_log_.BeginEvent(NetLog::TYPE_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) {
   if (net_log_.IsCapturing()) {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_READ_DATA,
                                       result);
   }
 
   if (!cache_.get())
     return ERR_UNEXPECTED;
 
   if (partial_) {
     // Partial requests are confusing to report in histograms because they may
     // have multiple underlying requests.
     UpdateTransactionPattern(PATTERN_NOT_COVERED);
     return DoPartialCacheReadCompleted(result);
   }
 
-  if (result > 0) {
-    read_offset_ += result;
-  } else if (result == 0) {  // End of file.
-    RecordHistograms();
-    cache_->DoneReadingFromEntry(entry_, this);
-    entry_ = NULL;
-  } else {
+  if (result < 0)
     return OnCacheReadError(result, false);
-  }
+
+  if (result == 0)
+    ReleaseCacheEntry(EntryState::KEEP);
+
+  read_offset_ += result;
   return result;
 }
 
 int HttpCache::Transaction::DoCacheWriteData(int num_bytes) {
   next_state_ = STATE_CACHE_WRITE_DATA_COMPLETE;
   write_len_ = num_bytes;
   if (entry_) {
     if (net_log_.IsCapturing())
       net_log_.BeginEvent(NetLog::TYPE_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) {
-  if (entry_) {
-    if (net_log_.IsCapturing()) {
-      net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_WRITE_DATA,
-                                        result);
-    }
+  if (entry_ && net_log_.IsCapturing()) {
+    net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_WRITE_DATA,
+                                      result);
   }
+
   if (!cache_.get())
     return ERR_UNEXPECTED;
 
   if (result != write_len_) {
     DLOG(ERROR) << "failed to write response data to cache";
-    DoneWritingToEntry(false);
+    ReleaseCacheEntry(EntryState::DOOM);
 
     // We want to ignore errors writing to disk and just keep reading from
     // the network.
     result = write_len_;
+
+    // TODO(asanka): More needs to be done to ignore the error and keep reading
+    // from the network. E.g. if the cache entry was sparse, it is possible that
+    // the current network_trans_ can't fulfil the entire requested range.
+    // Consider setting up state to perform an immediate STATE_SWITCH_TO_NETWORK
+    // at the next Read() instead of releasing the entry now.
   } else if (!done_reading_ && entry_ && (!partial_ || truncated_)) {
     int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex);
     int64 body_size = response_.headers->GetContentLength();
     if (body_size >= 0 && body_size <= current_size)
       done_reading_ = true;
   }
 
   if (partial_) {
-    // This may be the last request.
-    if (result != 0 || truncated_ ||
-        !(partial_->IsLastRange() || mode_ == WRITE)) {
+    // The transaction needs to update partial state under the following
+    // circumstances:
+    const bool need_to_update_partial_state =
+        // Still reading. The partial state needs to be updated to account for
+        // the data read so far.
+        result != 0 ||
+
+        // Reached the end of one network request, but more transactions may be
+        // necessary to fulfil the request. DoPartialNetworkReadCompleted()
+        // initiates additional network transactions as needed.
+        truncated_ ||
+
+        // If this isn't the last range, then more network transactions may be
+        // necessary. But skip this step for new cache entries (mode_ == WRITE).
+        // Range requests with new cache entries will have
+        // partial_->IsLastRange() set to false. IsLastRange() is only set to
+        // true when validating the last range of an existing cache entry.
+        (!partial_->IsLastRange() && mode_ != WRITE);
+
+    if (need_to_update_partial_state)
       return DoPartialNetworkReadCompleted(result);
-    }
   }
 
   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);
+    // If the entire response body was successfully read, then the transaction
+    // should release the cache entry.
+    //
+    // While response == 0 typically indicates EOF, it is possible for that to
+    // happen due to a connection problem. In the latter case the resulting

[rvargas (doing something else), 2015/09/21 22:06:00]

tiny nit: maybe remove "the resulting" and "call"

+    // AbandonCacheEntry() call marks the cache entry as truncated.
+    //
+    // A succesful receipt of the response body is considered to be indicated by
+    // one of:
+    //  * done_reading_ : Received response body was of expected size.
+    //  * Content-Length <= 0 : Response body size was not known.
+    //  * partial_ :
+    if (entry_ && (done_reading_ || partial_ ||
+                   response_.headers->GetContentLength() <= 0)) {
+      ReleaseCacheEntry(EntryState::KEEP);
+    } else {
+      AbandonCacheEntry(CompletionCallback());
     }
   }
 
   return result;
 }
 
-int HttpCache::Transaction::DoCacheWriteTruncatedResponse() {
-  next_state_ = STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE;
-  return WriteResponseInfoToEntry(true);
-}
-
-int HttpCache::Transaction::DoCacheWriteTruncatedResponseComplete(int result) {
-  return OnWriteResponseInfoToEntryComplete(result);
-}
-
 //-----------------------------------------------------------------------------
 
 void HttpCache::Transaction::ReadCertChain() {
   std::string key =
       GetCacheKeyForCert(response_.ssl_info.cert->os_cert_handle());
   const X509Certificate::OSCertHandles& intermediates =
       response_.ssl_info.cert->GetIntermediateCertificates();
   int dist_from_root = intermediates.size();
 
   scoped_refptr<SharedChainData> shared_chain_data(
@@ skipping 237 matching lines @@
     UpdateTransactionPattern(PATTERN_ENTRY_USED);
     return SetupEntryForRead();
   } else {
     // Make the network request conditional, to see if we may reuse our cached
     // response.  If we cannot do so, then we just resort to a normal fetch.
     // Our mode remains READ_WRITE for a conditional request.  Even if the
     // conditionalization fails, we don't switch to WRITE mode until we
     // know we won't be falling back to using the cache entry in the
     // LOAD_FROM_CACHE_IF_OFFLINE case.
     if (!ConditionalizeRequest()) {
+      // Conditionalization should only depend on the state of the cache prior
+      // to starting the request and the request method. Failure is unexpected
+      // after the client has started reading the stream.
+      DCHECK(!reading_);
+
       couldnt_conditionalize_request_ = true;
       UpdateTransactionPattern(PATTERN_ENTRY_CANT_CONDITIONALIZE);
       if (partial_)
         return DoRestartPartialRequest();
 
       DCHECK_NE(206, response_.headers->response_code());
     }
     next_state_ = STATE_SEND_REQUEST;
   }
   return OK;
@@ skipping 58 matching lines @@
     response_.headers->EnumerateHeader(
         NULL,
         kValidationHeaders[i].related_response_header_name,
         &validator);
 
     if (response_.headers->response_code() != 200 || truncated_ ||
         validator.empty() || validator != external_validation_.values[i]) {
       // The externally conditionalized request is not a validation request
       // for our existing cache entry. Proceed with caching disabled.
       UpdateTransactionPattern(PATTERN_NOT_COVERED);
-      DoneWritingToEntry(true);
+      ReleaseCacheEntry(EntryState::KEEP);
     }
   }
 
   // TODO(ricea): This calculation is expensive to perform just to collect
   // statistics. Either remove it or use the result, depending on the result of
   // the experiment.
   ExternallyConditionalizedType type =
       EXTERNALLY_CONDITIONALIZED_CACHE_USABLE;
   if (mode_ == NONE)
     type = EXTERNALLY_CONDITIONALIZED_MISMATCHED_VALIDATORS;
@@ skipping 307 matching lines @@
   IgnoreRangeRequest();
   return true;
 }
 
 void HttpCache::Transaction::IgnoreRangeRequest() {
   // We have a problem. We may or may not be reading already (in which case we
   // returned the headers), but we'll just pretend that this request is not
   // using the cache and see what happens. Most likely this is the first
   // response from the server (it's not changing its mind midway, right?).
   UpdateTransactionPattern(PATTERN_NOT_COVERED);
-  if (mode_ & WRITE)
-    DoneWritingToEntry(mode_ != WRITE);
-  else if (mode_ & READ && entry_)
-    cache_->DoneReadingFromEntry(entry_, this);
-
-  partial_.reset(NULL);
-  entry_ = NULL;
-  mode_ = NONE;
+  ReleaseCacheEntry(mode_ == WRITE ? EntryState::DOOM : EntryState::KEEP);
+  partial_.reset();
 }
 
 void HttpCache::Transaction::FixHeadersForHead() {
   if (response_.headers->response_code() == 206) {
     response_.headers->RemoveHeader("Content-Range");
     response_.headers->ReplaceStatusLine("HTTP/1.1 200 OK");
   }
 }
 
 int HttpCache::Transaction::SetupEntryForRead() {
@@ skipping 29 matching lines @@
   int rv = 0;
   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::Transaction::WriteResponseInfoToEntry(bool truncated) {
+int HttpCache::Transaction::WriteResponseInfoToEntry(
+    bool truncated,
+    const CompletionCallback& callback) {
   if (!entry_)
     return OK;
 
   if (net_log_.IsCapturing())
     net_log_.BeginEvent(NetLog::TYPE_HTTP_CACHE_WRITE_INFO);
 
   // Do not cache no-store content.  Do not cache content with cert errors
   // either.  This is to prevent not reporting net errors when loading a
   // resource from the cache.  When we load a page over HTTPS with a cert error
   // we show an SSL blocking page.  If the user clicks proceed we reload the
   // resource ignoring the errors.  The loaded resource is then cached.  If that
   // resource is subsequently loaded from the cache, no net error is reported
   // (even though the cert status contains the actual errors) and no SSL
   // blocking page is shown.  An alternative would be to reverse-map the cert
   // status to a net error and replay the net error.
   if ((response_.headers->HasHeaderValue("cache-control", "no-store")) ||
       IsCertStatusError(response_.ssl_info.cert_status)) {
-    DoneWritingToEntry(false);
+    ReleaseCacheEntry(EntryState::DOOM);
     if (net_log_.IsCapturing())
       net_log_.EndEvent(NetLog::TYPE_HTTP_CACHE_WRITE_INFO);
     return OK;
   }
 
   // cert_cache() will be null if the CertCacheTrial field trial is disabled.
   if (cache_->cert_cache() && response_.ssl_info.is_valid())
     WriteCertChain();
 
   if (truncated)
     DCHECK_EQ(200, response_.headers->response_code());
 
   // When writing headers, we normally only write the non-transient headers.
   bool skip_transient_headers = true;
   scoped_refptr<PickledIOBuffer> data(new PickledIOBuffer());
   response_.Persist(data->pickle(), skip_transient_headers, truncated);
   data->Done();
 
   io_buf_len_ = data->pickle()->size();
   return entry_->disk_entry->WriteData(kResponseInfoIndex, 0, data.get(),
-                                       io_buf_len_, io_callback_, true);
+                                       io_buf_len_, callback, true);
 }
 
 int HttpCache::Transaction::OnWriteResponseInfoToEntryComplete(int result) {
   if (!entry_)
     return OK;
   if (net_log_.IsCapturing()) {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_WRITE_INFO,
                                       result);
   }
 
   if (result != io_buf_len_) {
     DLOG(ERROR) << "failed to write response info to cache";
-    DoneWritingToEntry(false);
+    ReleaseCacheEntry(EntryState::DOOM);
   }
   return OK;
 }
 
-void HttpCache::Transaction::DoneWritingToEntry(bool success) {
-  if (!entry_)
+void HttpCache::Transaction::ReleaseCacheEntry(EntryState entry_state) {
+  if (!cache_ || !entry_)
     return;
 
-  RecordHistograms();
-
-  cache_->DoneWritingToEntry(entry_, success);
-  entry_ = NULL;
-  mode_ = NONE;  // switch to 'pass through' mode
+  cache_->DoneWithEntry(entry_, this, entry_state);
+  entry_ = nullptr;
+  mode_ = NONE;
 }
 
 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);
   } else {
     UMA_HISTOGRAM_SPARSE_SLOWLY("HttpCache.ReadErrorNonRestartable",
                                 result_for_histogram);
   }
 
   // Avoid using this entry in the future.
   if (cache_.get())
     cache_->DoomActiveEntry(cache_key_);
 
   if (restart) {
     DCHECK(!reading_);
     DCHECK(!network_trans_.get());
-    cache_->DoneWithEntry(entry_, this, false);
-    entry_ = NULL;
+    ReleaseCacheEntry(EntryState::DOOM);
     is_sparse_ = false;
     partial_.reset();
     next_state_ = STATE_GET_BACKEND;
     return OK;
   }
 
   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_)
     return;
 
   cache_->RemovePendingTransaction(this);
   OnIOComplete(ERR_CACHE_LOCK_TIMEOUT);
 }
 
 void HttpCache::Transaction::DoomPartialEntry(bool delete_object) {
   DVLOG(2) << "DoomPartialEntry";
-  int rv = cache_->DoomEntry(cache_key_, NULL);
-  DCHECK_EQ(OK, rv);
-  cache_->DoneWithEntry(entry_, this, false);
-  entry_ = NULL;
+  ReleaseCacheEntry(EntryState::DOOM);
   is_sparse_ = false;
   truncated_ = false;
   if (delete_object)
-    partial_.reset(NULL);
+    partial_.reset();
 }
 
 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;
   }
@@ skipping 11 matching lines @@
   }
   return result;
 }
 
 int HttpCache::Transaction::DoRestartPartialRequest() {
   // The stored data cannot be used. Get rid of it and restart this request.
   net_log_.AddEvent(NetLog::TYPE_HTTP_CACHE_RESTART_PARTIAL_REQUEST);
 
   // WRITE + Doom + STATE_INIT_ENTRY == STATE_CREATE_ENTRY (without an attempt
   // to Doom the entry again).
+  ResetPartialState(!range_requested_);
   mode_ = WRITE;
-  ResetPartialState(!range_requested_);
   next_state_ = 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.
@@ skipping 55 matching lines @@
 void HttpCache::Transaction::UpdateTransactionPattern(
     TransactionPattern new_transaction_pattern) {
   if (transaction_pattern_ == PATTERN_NOT_COVERED)
     return;
   DCHECK(transaction_pattern_ == PATTERN_UNDEFINED ||
          new_transaction_pattern == PATTERN_NOT_COVERED);
   transaction_pattern_ = new_transaction_pattern;
 }
 
 void HttpCache::Transaction::RecordHistograms() {
-  DCHECK_NE(PATTERN_UNDEFINED, transaction_pattern_);
-  if (!cache_.get() || !cache_->GetCurrentBackend() ||
+  if (transaction_pattern_ == PATTERN_UNDEFINED || !cache_.get() ||
+      !cache_->GetCurrentBackend() ||
       cache_->GetCurrentBackend()->GetCacheType() != DISK_CACHE ||
       cache_->mode() != NORMAL || request_->method != "GET") {
     return;
   }
-  UMA_HISTOGRAM_ENUMERATION(
-      "HttpCache.Pattern", transaction_pattern_, PATTERN_MAX);
+  UMA_HISTOGRAM_ENUMERATION("HttpCache.Pattern", transaction_pattern_,
+                            PATTERN_MAX);
   if (transaction_pattern_ == PATTERN_NOT_COVERED)
     return;
   DCHECK(!range_requested_);
   DCHECK(!first_cache_access_since_.is_null());
 
   TimeDelta total_time = base::TimeTicks::Now() - first_cache_access_since_;
 
   UMA_HISTOGRAM_TIMES("HttpCache.AccessToDone", total_time);
 
   bool did_send_request = !send_request_since_.is_null();
@@ skipping 53 matching lines @@
     }
     default:
       NOTREACHED();
   }
 }
 
 void HttpCache::Transaction::OnIOComplete(int result) {
   DoLoop(result);
 }
 
+int HttpCache::Transaction::AbandonCacheEntry(
+    const CompletionCallback& callback) {
+  DCHECK(abandon_cache_entry_callback_.is_null());
+  // Is the cache entry is sparse or if the entry is complete? Then the entry

[rvargas (doing something else), 2015/09/21 22:06:00]

needs some rewording :). I guess you were going for "is foo or is bar?"

+  // doesn't need to be marked as truncated.
+  //
+  //   * done_reading_: All pertinent response data has been written.
+  //   * !(mode_ & WRITE): This transaction wasn't writing to the entry. Leave
+  //                       it be.
+  //   * is_sparse_: Pre-existing sparse entry.
+  //   * (partial_ && !truncated_): New sparse entry.
+  if (done_reading_ || !(mode_ & WRITE) || is_sparse_ ||
+      (partial_ && !truncated_)) {
+    ReleaseCacheEntry(EntryState::KEEP);
+    return OK;
+  }
+
+  // If the caller hasn't started reading (implies that no response data was
+  // written) or if the request can't be resumed, then another transaction is
+  // unlikely to be able to use a partial cache entry for this request.
+  if (!reading_ || !CanResume(true)) {
+    ReleaseCacheEntry(EntryState::DOOM);
+    return OK;
+  }
+
+  // If we get here, the cache entry only contains part of the response and can
+  // be resumed in the future. Mark it as truncated so it'll be handled
+  // correctly.
+  // TODO(asanka): Let's get rid of a persistent truncated flag and make it
+  // implicit based on what we find in the cache the next time the entry is
+  // opened.
+  truncated_ = true;
+  int result = WriteResponseInfoToEntry(
+      true, base::Bind(&HttpCache::Transaction::OnAbandonCacheEntryIOComplete,
+                       weak_factory_.GetWeakPtr()));
+  if (result < 0 && result != ERR_IO_PENDING)
+    return result;
+
+  if (callback.is_null()) {
+    ReleaseCacheEntry(EntryState::KEEP);
+    return OK;
+  }
+
+  abandon_cache_entry_callback_ = callback;
+  return result;
+}
+
+void HttpCache::Transaction::OnAbandonCacheEntryIOComplete(int result) {
+  result = OnWriteResponseInfoToEntryComplete(result);
+  ReleaseCacheEntry(EntryState::KEEP);
+  if (!abandon_cache_entry_callback_.is_null())
+    base::ResetAndReturn(&abandon_cache_entry_callback_).Run(result);
+}
+
 }  // namespace net