[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 245 matching lines @@
       new_entry_(NULL),
       new_response_(NULL),
       mode_(NONE),
       reading_(false),
       invalid_range_(false),
       truncated_(false),
       is_sparse_(false),
       range_requested_(false),
       handling_206_(false),
       cache_pending_(false),
-      done_reading_(false),
+      done_writing_(false),
       vary_mismatch_(false),
       couldnt_conditionalize_request_(false),
+      stopped_caching_(false),
       bypass_lock_for_test_(false),
       fail_conditionalization_for_test_(false),
       io_buf_len_(0),
       read_offset_(0),
       effective_load_flags_(0),
       write_len_(0),
       transaction_pattern_(PATTERN_UNDEFINED),
       total_received_bytes_(0),
       websocket_handshake_stream_base_create_helper_(NULL),
       weak_factory_(this) {
@@ skipping 14 matching lines @@
     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);
         }
       }
 
-      cache_->DoneWithEntry(entry_, this, cancel_request);
+      DoneWithEntry(cancel_request);
     } else if (cache_pending_) {
       cache_->RemovePendingTransaction(this);
     }
   }
 }
 
 int HttpCache::Transaction::WriteMetadata(IOBuffer* buf, int buf_len,
                                           const CompletionCallback& callback) {
   DCHECK(buf);
   DCHECK_GT(buf_len, 0);
@@ skipping 13 matching lines @@
   DCHECK(mode_ & WRITE || mode_ == NONE);
 
   // Don't set the flag for sparse entries.
   if (is_sparse_)
     return true;
 
   if (!CanResume(true))
     return false;
 
   // We may have received the whole resource already.
-  if (done_reading_)
+  if (done_writing_)
     return true;
 
   truncated_ = true;
   next_state_ = STATE_CACHE_WRITE_TRUNCATED_RESPONSE;
   DoLoop(OK);
   return true;
 }
 
 LoadState HttpCache::Transaction::GetWriterLoadState() const {
   if (network_trans_.get())
@@ skipping 140 matching lines @@
   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;
-  }
+
+  stopped_caching_ = true;
+  UpdateTransactionPattern(PATTERN_NOT_COVERED);
 }
 
 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;
 }
 
 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;
-    }
-  }
+  DoneWithRequest();
 }
 
 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 790 matching lines @@
 // We may end up here multiple times for a given request.
 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;
-  }
+  // This is the end of the request.
+  if (!result)
+    return DoneWithRequest();
 
   if (result < 0)
     return result;
 
   partial_->PrepareCacheValidation(entry_->disk_entry,
                                    &custom_request_->extra_headers);
 
   if (reading_ && partial_->IsCurrentRangeCached()) {
     next_state_ = STATE_CACHE_READ_DATA;
     return OK;
@@ skipping 125 matching lines @@
     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;
+    DoneWritingToEntry(true);
   }
 
   // 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 114 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;
     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);
@@ skipping 112 matching lines @@
   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)
+  if (result < 0)
+    return result;
+
+  // Skip writing if StopCaching() has been called.
+  //
+  // For partial requests the transaction still needs to update its state. If
+  // the network transaction being read was conditionalized, the cache
+  // transaction may need to issue additional network or cache reads to fulfil
+  // the original request.
+  if (stopped_caching_ && partial_)
+    return DoPartialNetworkReadCompleted(result);
+
+  if (mode_ == NONE || stopped_caching_)
     return result;
 
   next_state_ = STATE_CACHE_WRITE_DATA;
   return result;
 }
 
 int HttpCache::Transaction::DoCacheReadData() {
   if (request_->method == "HEAD")
     return 0;
 
@@ skipping 50 matching lines @@
 
   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);
 
     // We want to ignore errors writing to disk and just keep reading from
     // the network.
     result = write_len_;
-  } else if (!done_reading_ && entry_) {
+  } else if (!done_writing_ && entry_) {
     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;
+      done_writing_ = true;
   }
 
   if (partial_) {
     // This may be the last request.
     if (result != 0 || truncated_ ||
         !(partial_->IsLastRange() || mode_ == WRITE)) {
       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) {
+    if (done_writing_ || !entry_ || partial_ ||
+        response_.headers->GetContentLength() <= 0)
       DoneWritingToEntry(true);
-    }
   }
 
   return result;
 }
 
 int HttpCache::Transaction::DoCacheWriteTruncatedResponse() {
   next_state_ = STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE;
   return WriteResponseInfoToEntry(true);
 }
 
@@ skipping 766 matching lines @@
                                       result);
   }
 
   if (result != io_buf_len_) {
     DLOG(ERROR) << "failed to write response info to cache";
     DoneWritingToEntry(false);
   }
   return OK;
 }
 
+int HttpCache::Transaction::DoneWithRequest() {
+  if (!cache_ || !entry_)
+    return OK;
+
+  DCHECK_EQ(STATE_NONE, next_state_);

[hubbe, 2015/07/16 15:56:12]

Shouldn't these DCHECKs go first?

[asanka, 2015/07/16 17:57:00]

Moved up.

+  DCHECK_NE(mode_, UPDATE);
+
+  if (!(mode_ & WRITE)) {
+    cache_->DoneReadingFromEntry(entry_, this);
+    entry_ = NULL;
+    return OK;
+  }
+
+  if (stopped_caching_ && !is_sparse_ && !done_writing_ && !entry_->doomed) {
+    if (CanResume(true)) {
+      truncated_ = true;
+      return DoCacheWriteTruncatedResponse();
+    }
+    int ret = cache_->DoomEntry(cache_key_, nullptr);
+    DCHECK_EQ(OK, ret);
+  }
+
+  DoneWritingToEntry(true);
+  return OK;
+}
+
 void HttpCache::Transaction::DoneWritingToEntry(bool success) {
   if (!entry_)
     return;
 
   RecordHistograms();
 
   cache_->DoneWritingToEntry(entry_, success);
   entry_ = NULL;
   mode_ = NONE;  // switch to 'pass through' mode
 }
 
+void HttpCache::Transaction::DoneWithEntry(bool cancel) {
+  cache_->DoneWithEntry(entry_, this, cancel);
+  entry_ = nullptr;
+  is_sparse_ = false;
+  truncated_ = false;
+}
+
 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;
-    is_sparse_ = false;
+    DoneWithEntry(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;
-  is_sparse_ = false;
-  truncated_ = false;
+  DoneWithEntry(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 169 matching lines @@
     default:
       NOTREACHED();
   }
 }
 
 void HttpCache::Transaction::OnIOComplete(int result) {
   DoLoop(result);
 }
 
 }  // namespace net