Index: net/http/http_cache_transaction.cc |
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc |
index 5f870c6cdd56503b606817e410f4c0b9205bbb2c..224496f44d013e1180f222bf6220de6539571e60 100644 |
--- a/net/http/http_cache_transaction.cc |
+++ b/net/http/http_cache_transaction.cc |
@@ -170,6 +170,7 @@ HttpCache::Transaction::Transaction(RequestPriority priority, HttpCache* cache) |
new_entry_(NULL), |
new_response_(NULL), |
mode_(NONE), |
+ original_mode_(NONE), |
reading_(false), |
invalid_range_(false), |
truncated_(false), |
@@ -182,6 +183,7 @@ HttpCache::Transaction::Transaction(RequestPriority priority, HttpCache* cache) |
couldnt_conditionalize_request_(false), |
bypass_lock_for_test_(false), |
fail_conditionalization_for_test_(false), |
+ validating_cannot_proceed_(false), |
io_buf_len_(0), |
read_offset_(0), |
effective_load_flags_(0), |
@@ -210,16 +212,12 @@ HttpCache::Transaction::~Transaction() { |
if (cache_) { |
if (entry_) { |
- bool cancel_request = reading_ && response_.headers.get(); |
- if (cancel_request) { |
- if (partial_) { |
- entry_->disk_entry->CancelSparseIO(); |
- } else { |
- cancel_request &= (response_.headers->response_code() == 200); |
- } |
- } |
+ bool cancel_response = cache_->IsTransactionCurrentOrFutureWriter( |
+ entry_, this, request_->method); |
+ if (cancel_response && partial_) |
+ entry_->disk_entry->CancelSparseIO(); |
- cache_->DoneWithEntry(entry_, this, cancel_request); |
+ cache_->DoneWithEntry(entry_, this, cancel_response); |
} else if (cache_pending_) { |
cache_->RemovePendingTransaction(this); |
} |
@@ -374,6 +372,8 @@ bool HttpCache::Transaction::IsReadyToRestartForAuth() { |
int HttpCache::Transaction::Read(IOBuffer* buf, int buf_len, |
const CompletionCallback& callback) { |
+ // TODO(shivanisha): A lot of tests invoke Read on a HEAD request which |
+ // should not be allowed in the Read() method. Fix those tests. |
jkarlin
2017/04/03 14:32:48
I assume that the tests are defensive due to the h
shivanisha
2017/04/03 20:16:49
That might be the case, removing the todo from her
|
DCHECK_EQ(next_state_, STATE_NONE); |
DCHECK(buf); |
DCHECK_GT(buf_len, 0); |
@@ -579,6 +579,11 @@ void HttpCache::Transaction::GetConnectionAttempts( |
old_connection_attempts_.end()); |
} |
+void HttpCache::Transaction::SetValidatingCannotProceed() { |
Randy Smith (Not in Mondays)
2017/03/31 19:26:12
Suggestion: As I understand it, this is only expec
shivanisha
2017/04/03 20:16:49
Done. Added DCHECK(!reading_)
|
+ validating_cannot_proceed_ = true; |
+ entry_ = nullptr; |
+} |
+ |
size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// TODO(xunjieli): Consider improving the coverage. crbug.com/669108. |
return 0; |
@@ -593,7 +598,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// GetBackend* -> InitEntry -> OpenEntry* -> CreateEntry* -> AddToEntry* -> |
// SendRequest* -> SuccessfulSendRequest -> OverwriteCachedResponse -> |
// CacheWriteResponse* -> TruncateCachedData* -> TruncateCachedMetadata* -> |
-// PartialHeadersReceived |
+// PartialHeadersReceived -> FinishHeaders* |
// |
// Read(): |
// NetworkRead* -> CacheWriteData* |
@@ -602,7 +607,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// Start(): |
// GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse* |
// -> CacheDispatchValidation -> BeginPartialCacheValidation() -> |
-// BeginCacheValidation() -> SetupEntryForRead() |
+// BeginCacheValidation() -> SetupEntryForRead() -> FinishHeaders* |
// |
// Read(): |
// CacheReadData* |
@@ -614,7 +619,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest -> |
// UpdateCachedResponse -> CacheWriteUpdatedResponse* -> |
// UpdateCachedResponseComplete -> OverwriteCachedResponse -> |
-// PartialHeadersReceived |
+// PartialHeadersReceived -> FinishHeaders* |
// |
// Read(): |
// CacheReadData* |
@@ -625,7 +630,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// -> CacheDispatchValidation -> BeginPartialCacheValidation() -> |
// BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest -> |
// OverwriteCachedResponse -> CacheWriteResponse* -> DoTruncateCachedData* -> |
-// TruncateCachedMetadata* -> PartialHeadersReceived |
+// TruncateCachedMetadata* -> PartialHeadersReceived -> FinishHeaders* |
// |
// Read(): |
// NetworkRead* -> CacheWriteData* |
@@ -639,7 +644,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest -> |
// UpdateCachedResponse -> CacheWriteUpdatedResponse* -> |
// UpdateCachedResponseComplete -> OverwriteCachedResponse -> |
-// PartialHeadersReceived |
+// PartialHeadersReceived -> FinishHeaders* |
// |
// Read() 1: |
// NetworkRead* -> CacheWriteData* |
@@ -680,7 +685,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse* |
// -> CacheDispatchValidation -> BeginPartialCacheValidation() -> |
// BeginCacheValidation() -> SendRequest* -> SuccessfulSendRequest -> |
-// OverwriteCachedResponse |
+// OverwriteCachedResponse -> FinishHeaders* |
// |
// 10. HEAD. Sparse entry, partially cached: |
// Serve the request from the cache, as long as it doesn't require |
@@ -705,7 +710,7 @@ size_t HttpCache::Transaction::EstimateMemoryUsage() const { |
// GetBackend* -> InitEntry -> OpenEntry* -> AddToEntry* -> CacheReadResponse* |
// -> CacheToggleUnusedSincePrefetch* -> CacheDispatchValidation -> |
// BeginPartialCacheValidation() -> BeginCacheValidation() -> |
-// SetupEntryForRead() |
+// SetupEntryForRead() -> FinishHeaders* |
// |
// Read(): |
// CacheReadData* |
@@ -719,8 +724,9 @@ int HttpCache::Transaction::DoLoop(int result) { |
DCHECK(!in_do_loop_); |
int rv = result; |
+ State state = next_state_; |
do { |
- State state = next_state_; |
+ state = next_state_; |
next_state_ = STATE_UNSET; |
base::AutoReset<bool> scoped_in_do_loop(&in_do_loop_, true); |
@@ -857,6 +863,12 @@ int HttpCache::Transaction::DoLoop(int result) { |
case STATE_CACHE_READ_METADATA_COMPLETE: |
rv = DoCacheReadMetadataComplete(rv); |
break; |
+ case STATE_FINISH_HEADERS: |
+ rv = DoFinishHeaders(rv); |
+ break; |
+ case STATE_FINISH_HEADERS_COMPLETE: |
+ rv = DoFinishHeadersComplete(rv); |
+ break; |
case STATE_NETWORK_READ: |
DCHECK_EQ(OK, rv); |
rv = DoNetworkRead(); |
@@ -893,8 +905,13 @@ int HttpCache::Transaction::DoLoop(int result) { |
} while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE); |
+ // Assert Start() state machine's allowed last state in successful cases when |
+ // caching is happening. |
+ DCHECK(reading_ || rv != OK || !entry_ || |
+ state == STATE_FINISH_HEADERS_COMPLETE); |
+ |
if (rv != ERR_IO_PENDING && !callback_.is_null()) { |
- read_buf_ = NULL; // Release the buffer before invoking the callback. |
+ read_buf_ = nullptr; // Release the buffer before invoking the callback. |
base::ResetAndReturn(&callback_).Run(rv); |
} |
@@ -921,7 +938,7 @@ int HttpCache::Transaction::DoGetBackendComplete(int result) { |
if (effective_load_flags_ & LOAD_ONLY_FROM_CACHE) { |
if (effective_load_flags_ & LOAD_BYPASS_CACHE) { |
// The client has asked for nonsense. |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
mode_ = READ; |
@@ -960,7 +977,7 @@ int HttpCache::Transaction::DoGetBackendComplete(int result) { |
// 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) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
@@ -977,6 +994,9 @@ int HttpCache::Transaction::DoGetBackendComplete(int result) { |
// This is only set if we have something to do with the response. |
range_requested_ = (partial_.get() != NULL); |
+ // mode_ may change later, save the initial mode in case we need to restart |
+ // this request. |
+ original_mode_ = mode_; |
return OK; |
} |
@@ -985,7 +1005,7 @@ int HttpCache::Transaction::DoInitEntry() { |
DCHECK(!new_entry_); |
if (!cache_.get()) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_UNEXPECTED; |
} |
@@ -1048,7 +1068,7 @@ int HttpCache::Transaction::DoOpenEntryComplete(int result) { |
// The entry does not exist, and we are not permitted to create a new entry, |
// so we must fail. |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
@@ -1182,7 +1202,7 @@ int HttpCache::Transaction::DoAddToEntryComplete(int result) { |
if (result == ERR_CACHE_LOCK_TIMEOUT) { |
if (mode_ == READ) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
@@ -1201,7 +1221,7 @@ int HttpCache::Transaction::DoAddToEntryComplete(int result) { |
// TODO(jkarlin): We should either handle the case or DCHECK. |
if (result != OK) { |
NOTREACHED(); |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return result; |
} |
@@ -1344,7 +1364,7 @@ int HttpCache::Transaction::DoCacheQueryData() { |
int HttpCache::Transaction::DoCacheQueryDataComplete(int result) { |
DCHECK_EQ(OK, result); |
if (!cache_.get()) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_UNEXPECTED; |
} |
@@ -1354,7 +1374,7 @@ int HttpCache::Transaction::DoCacheQueryDataComplete(int result) { |
// We may end up here multiple times for a given request. |
int HttpCache::Transaction::DoStartPartialCacheValidation() { |
if (mode_ == NONE) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -1371,12 +1391,12 @@ int HttpCache::Transaction::DoCompletePartialCacheValidation(int result) { |
cache_->DoneReadingFromEntry(entry_, this); |
entry_ = NULL; |
} |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return result; |
} |
if (result < 0) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return result; |
} |
@@ -1402,7 +1422,7 @@ int HttpCache::Transaction::DoSendRequest() { |
int rv = |
cache_->network_layer_->CreateTransaction(priority_, &network_trans_); |
if (rv != OK) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return rv; |
} |
network_trans_->SetBeforeNetworkStartCallback(before_network_start_callback_); |
@@ -1424,7 +1444,7 @@ int HttpCache::Transaction::DoSendRequest() { |
int HttpCache::Transaction::DoSendRequestComplete(int result) { |
TRACE_EVENT0("io", "HttpCacheTransaction::DoSendRequestComplete"); |
if (!cache_.get()) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_UNEXPECTED; |
} |
@@ -1455,7 +1475,7 @@ int HttpCache::Transaction::DoSendRequestComplete(int result) { |
DoneWritingToEntry(true); |
} |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return result; |
} |
@@ -1469,7 +1489,7 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
new_response->headers->response_code() == 407) { |
SetAuthResponse(*new_response); |
if (!reading_) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -1494,7 +1514,7 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
mode_ = NONE; |
partial_.reset(); |
ResetNetworkTransaction(); |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_AUTH_FAILURE_AFTER_READ; |
} |
@@ -1532,7 +1552,7 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
int ret = cache_->DoomEntry(cache_key_, NULL); |
DCHECK_EQ(OK, ret); |
} |
- cache_->DoneWritingToEntry(entry_, true); |
+ cache_->DoneWritingToEntry(entry_, true, this); |
entry_ = NULL; |
mode_ = NONE; |
} |
@@ -1550,7 +1570,7 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
(request_->method == "GET" || request_->method == "POST")) { |
// If there is an active entry it may be destroyed with this transaction. |
SetResponse(*new_response_); |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -1566,6 +1586,21 @@ int HttpCache::Transaction::DoSuccessfulSendRequest() { |
} |
TransitionToState(STATE_OVERWRITE_CACHED_RESPONSE); |
+ |
+ if (!entry_) |
+ return OK; |
+ |
+ // Invalidate any current entry with a successful response if this transaction |
+ // cannot write to this entry. |
+ if (new_response->headers->response_code() != 304 && |
+ (entry_->writer || !entry_->readers.empty())) { |
jkarlin
2017/04/03 14:32:48
In regards to: entry_->writer || !entry_->readers.
shivanisha
2017/04/03 20:16:49
Good call, invoked IsCurrentOrFutureWriter for thi
|
+ DCHECK_EQ(entry_->headers_transaction, this); |
jkarlin
2017/04/03 14:32:48
Why not call DoneWritingToEntry(false); here like
shivanisha
2017/04/03 20:16:49
Done and removed is_match argument from DoneWithRe
|
+ cache_->DoneWithResponseHeaders(entry_, this, false); |
+ entry_ = nullptr; |
+ mode_ = NONE; |
+ return OK; |
+ } |
+ |
return OK; |
} |
@@ -1638,7 +1673,6 @@ int HttpCache::Transaction::DoUpdateCachedResponseComplete(int result) { |
} 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. |
@@ -1676,7 +1710,7 @@ int HttpCache::Transaction::DoOverwriteCachedResponse() { |
DoneWritingToEntry(false); |
mode_ = NONE; |
new_response_ = NULL; |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -1758,10 +1792,12 @@ int HttpCache::Transaction::DoPartialHeadersReceived() { |
new_response_ = NULL; |
if (!partial_) { |
- if (entry_ && entry_->disk_entry->GetDataSize(kMetadataIndex)) |
+ if (entry_ && entry_->disk_entry->GetDataSize(kMetadataIndex)) { |
TransitionToState(STATE_CACHE_READ_METADATA); |
- else |
- TransitionToState(STATE_NONE); |
+ } else { |
+ DCHECK(!reading_); |
+ TransitionToState(STATE_FINISH_HEADERS); |
+ } |
return OK; |
} |
@@ -1775,13 +1811,64 @@ int HttpCache::Transaction::DoPartialHeadersReceived() { |
// We are about to return the headers for a byte-range request to the user, |
// so let's fix them. |
partial_->FixResponseHeaders(response_.headers.get(), true); |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
} else { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
} |
return OK; |
} |
+int HttpCache::Transaction::DoFinishHeaders(int result) { |
+ if (!entry_ || result != OK) { |
+ TransitionToState(STATE_NONE); |
+ return result; |
+ } |
+ |
+ TransitionToState(STATE_FINISH_HEADERS_COMPLETE); |
+ |
+ // If it was an auth failure or 416, this transaction should continue to be |
+ // headers_transaction till consumer takes an action, so no need to do |
+ // anything now. |
+ if (auth_response_.headers.get() || |
+ (new_response_ && new_response_->headers && |
+ new_response_->headers->response_code() == 416)) |
+ return OK; |
+ |
+ // If there is no response body to be written or read, it does not need to |
+ // wait. |
+ if (request_->method == "HEAD") |
+ return OK; |
+ |
+ return cache_->DoneWithResponseHeaders(entry_, this, true); |
+} |
+ |
+int HttpCache::Transaction::DoFinishHeadersComplete(int rv) { |
+ if (rv == ERR_CACHE_RACE) { |
+ RestartAfterValidationStarted(); |
+ return OK; |
+ } |
+ |
+ TransitionToState(STATE_NONE); |
+ return rv; |
+} |
+ |
+void HttpCache::Transaction::RestartAfterValidationStarted() { |
+ DCHECK(!reading_); |
+ next_state_ = STATE_INIT_ENTRY; |
+ cache_entry_status_ = CacheEntryStatus::ENTRY_UNDEFINED; |
+ entry_ = nullptr; |
+ mode_ = original_mode_; |
+ if (network_trans_) |
+ network_trans_.reset(); |
+} |
+ |
+void HttpCache::Transaction::ConvertToReadMode() { |
+ DCHECK(!reading_); |
+ mode_ = READ; |
+ if (network_trans_) |
+ ResetNetworkTransaction(); |
+} |
+ |
int HttpCache::Transaction::DoCacheReadMetadata() { |
TRACE_EVENT0("io", "HttpCacheTransaction::DoCacheReadMetadata"); |
DCHECK(entry_); |
@@ -1804,7 +1891,8 @@ int HttpCache::Transaction::DoCacheReadMetadataComplete(int result) { |
result); |
if (result != response_.metadata->size()) |
return OnCacheReadError(result, false); |
- TransitionToState(STATE_NONE); |
+ |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -2101,18 +2189,18 @@ int HttpCache::Transaction::BeginCacheRead() { |
// TODO(jkarlin): Either handle this case or DCHECK. |
if (response_.headers->response_code() == 206 || partial_) { |
NOTREACHED(); |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
// We don't have the whole resource. |
if (truncated_) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
if (RequiresValidation() != VALIDATION_NONE) { |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return ERR_CACHE_MISS; |
} |
@@ -2122,7 +2210,7 @@ int HttpCache::Transaction::BeginCacheRead() { |
if (entry_->disk_entry->GetDataSize(kMetadataIndex)) |
TransitionToState(STATE_CACHE_READ_METADATA); |
else |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -2635,7 +2723,7 @@ int HttpCache::Transaction::SetupEntryForRead() { |
partial_.reset(); |
} |
} |
- cache_->ConvertWriterToReader(entry_); |
+ |
mode_ = READ; |
if (request_->method == "HEAD") |
@@ -2644,7 +2732,7 @@ int HttpCache::Transaction::SetupEntryForRead() { |
if (entry_->disk_entry->GetDataSize(kMetadataIndex)) |
TransitionToState(STATE_CACHE_READ_METADATA); |
else |
- TransitionToState(STATE_NONE); |
+ TransitionToState(STATE_FINISH_HEADERS); |
return OK; |
} |
@@ -2723,7 +2811,7 @@ void HttpCache::Transaction::DoneWritingToEntry(bool success) { |
RecordHistograms(); |
- cache_->DoneWritingToEntry(entry_, success); |
+ cache_->DoneWritingToEntry(entry_, success, this); |
entry_ = NULL; |
mode_ = NONE; // switch to 'pass through' mode |
} |
@@ -3080,6 +3168,14 @@ void HttpCache::Transaction::RecordHistograms() { |
} |
void HttpCache::Transaction::OnIOComplete(int result) { |
+ // If its the Start state machine and it cannot proceed due to failure by |
+ // another transaction in writing the response, restart this transaction. |
+ if (validating_cannot_proceed_) { |
+ DCHECK(!reading_); |
+ validating_cannot_proceed_ = false; |
+ RestartAfterValidationStarted(); |
+ } |
+ |
DoLoop(result); |
} |