Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(407)

Side by Side Diff: net/http/http_cache_transaction.cc

Issue 23710059: Release the cache entry on deferred redirect. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Disable new tests on Chrome Frame Created 7 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/http/http_cache_transaction.h" 5 #include "net/http/http_cache_transaction.h"
6 6
7 #include "build/build_config.h" 7 #include "build/build_config.h"
8 8
9 #if defined(OS_POSIX) 9 #if defined(OS_POSIX)
10 #include <unistd.h> 10 #include <unistd.h>
(...skipping 431 matching lines...) Expand 10 before | Expand all | Expand 10 after
442 HttpRequestHeaders* headers) const { 442 HttpRequestHeaders* headers) const {
443 if (network_trans_) 443 if (network_trans_)
444 return network_trans_->GetFullRequestHeaders(headers); 444 return network_trans_->GetFullRequestHeaders(headers);
445 445
446 // TODO(ttuttle): Read headers from cache. 446 // TODO(ttuttle): Read headers from cache.
447 return false; 447 return false;
448 } 448 }
449 449
450 void HttpCache::Transaction::DoneReading() { 450 void HttpCache::Transaction::DoneReading() {
451 if (cache_.get() && entry_) { 451 if (cache_.get() && entry_) {
452 DCHECK(reading_); 452 if (mode_ & WRITE) {
453 DCHECK_NE(mode_, UPDATE);
rvargas (doing something else) 2013/09/17 19:53:25 Is this line failing? UPDATE means that the caller
davidben 2013/09/17 22:16:04 Hrm. Yeah, I'm not sure why I removed it... I thin
454 if (mode_ & WRITE)
455 DoneWritingToEntry(true); 453 DoneWritingToEntry(true);
454 } else {
455 cache_->DoneReadingFromEntry(entry_, this);
456 entry_ = NULL;
457 }
456 } 458 }
457 } 459 }
458 460
459 const HttpResponseInfo* HttpCache::Transaction::GetResponseInfo() const { 461 const HttpResponseInfo* HttpCache::Transaction::GetResponseInfo() const {
460 // Null headers means we encountered an error or haven't a response yet 462 // Null headers means we encountered an error or haven't a response yet
461 if (auth_response_.headers.get()) 463 if (auth_response_.headers.get())
462 return &auth_response_; 464 return &auth_response_;
463 return (response_.headers.get() || response_.ssl_info.cert.get() || 465 return (response_.headers.get() || response_.ssl_info.cert.get() ||
464 response_.cert_request_info.get()) 466 response_.cert_request_info.get())
465 ? &response_ 467 ? &response_
(...skipping 867 matching lines...) Expand 10 before | Expand all | Expand 10 after
1333 1335
1334 int HttpCache::Transaction::DoTruncateCachedMetadataComplete(int result) { 1336 int HttpCache::Transaction::DoTruncateCachedMetadataComplete(int result) {
1335 if (entry_) { 1337 if (entry_) {
1336 ReportCacheActionFinish(); 1338 ReportCacheActionFinish();
1337 if (net_log_.IsLoggingAllEvents()) { 1339 if (net_log_.IsLoggingAllEvents()) {
1338 net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_WRITE_INFO, 1340 net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_WRITE_INFO,
1339 result); 1341 result);
1340 } 1342 }
1341 } 1343 }
1342 1344
1343 // If this response is a redirect, then we can stop writing now. (We don't
1344 // need to cache the response body of a redirect.)
1345 if (response_.headers->IsRedirect(NULL))
1346 DoneWritingToEntry(true);
rvargas (doing something else) 2013/09/17 19:53:25 Even though it is redundant in the sense that the
davidben 2013/09/17 22:16:04 My reasoning for removing it was that this way we
rvargas (doing something else) 2013/09/18 18:37:10 I see your point. On the other hand, having to rel
mmenke 2013/09/18 18:40:39 We don't have to have the caller do anything but r
rvargas (doing something else) 2013/09/18 19:09:12 Here we are doing two things: If we know this is a
davidben 2013/09/18 19:35:43 Well, it's a requirement to release the cache entr
mmenke 2013/09/18 19:38:31 The old contract is effectively, "After receiving
rvargas (doing something else) 2013/09/18 21:41:32 sure, but what the cache keeps or drops is not par
rvargas (doing something else) 2013/09/18 21:41:32 Sort of. The contract for releasing the entry is t
mmenke 2013/09/18 22:14:31 I think falsely claiming an HTTP redirect response
1347 next_state_ = STATE_PARTIAL_HEADERS_RECEIVED; 1345 next_state_ = STATE_PARTIAL_HEADERS_RECEIVED;
1348 return OK; 1346 return OK;
1349 } 1347 }
1350 1348
1351 int HttpCache::Transaction::DoPartialHeadersReceived() { 1349 int HttpCache::Transaction::DoPartialHeadersReceived() {
1352 new_response_ = NULL; 1350 new_response_ = NULL;
1353 if (entry_ && !partial_.get() && 1351 if (entry_ && !partial_.get() &&
1354 entry_->disk_entry->GetDataSize(kMetadataIndex)) 1352 entry_->disk_entry->GetDataSize(kMetadataIndex))
1355 next_state_ = STATE_CACHE_READ_METADATA; 1353 next_state_ = STATE_CACHE_READ_METADATA;
1356 1354
(...skipping 1142 matching lines...) Expand 10 before | Expand all | Expand 10 after
2499 UMA_HISTOGRAM_PERCENTAGE("HttpCache.PercentBeforeSend.Updated", 2497 UMA_HISTOGRAM_PERCENTAGE("HttpCache.PercentBeforeSend.Updated",
2500 before_send_percent); 2498 before_send_percent);
2501 break; 2499 break;
2502 } 2500 }
2503 default: 2501 default:
2504 NOTREACHED(); 2502 NOTREACHED();
2505 } 2503 }
2506 } 2504 }
2507 2505
2508 } // namespace net 2506 } // namespace net
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698