Add plumbing for stale cache information to HttpResponseInfo

BUG=308233
R=rvargas@chromium.org

Review URL: https://codereview.chromium.org/105893009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244670 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 50 insertions, 2 deletions

diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 17681a9..d186b99 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -992,6 +992,42 @@ TEST(HttpCache, SimpleGET_CacheOverride_NonOffline) {
   RemoveMockTransaction(&transaction);
 }
 
+// Tests that was_cached was set properly on a failure, even if the cached
+// response wasn't returned.
+TEST(HttpCache, SimpleGET_CacheSignal_Failure) {
+  MockHttpCache cache;
+
+  // Prime cache.
+  MockTransaction transaction(kSimpleGET_Transaction);
+  transaction.response_headers = "Cache-Control: no-cache\n";
+
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+  RemoveMockTransaction(&transaction);
+
+  // Network failure with error; should fail but have was_cached set.
+  transaction.return_code = net::ERR_FAILED;
+  AddMockTransaction(&transaction);
+
+  MockHttpRequest request(transaction);
+  net::TestCompletionCallback callback;
+  scoped_ptr<net::HttpTransaction> trans;
+  int rv = cache.http_cache()->CreateTransaction(net::DEFAULT_PRIORITY, &trans);
+  EXPECT_EQ(net::OK, rv);
+  ASSERT_TRUE(trans.get());
+  rv = trans->Start(&request, callback.callback(), net::BoundNetLog());
+  EXPECT_EQ(net::ERR_FAILED, callback.GetResult(rv));
+
+  const net::HttpResponseInfo* response_info = trans->GetResponseInfo();
+  ASSERT_TRUE(response_info);
+  EXPECT_TRUE(response_info->was_cached);
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+
+  RemoveMockTransaction(&transaction);
+}
+
 // Confirm if we have an empty cache, a read is marked as network verified.
 TEST(HttpCache, SimpleGET_NetworkAccessed_Network) {
   MockHttpCache cache;
diff --git a/net/http/http_response_info.h b/net/http/http_response_info.h
index f0908b0..9c4a781 100644
--- a/net/http/http_response_info.h
+++ b/net/http/http_response_info.h
@@ -59,12 +59,14 @@ class NET_EXPORT HttpResponseInfo {
                bool response_truncated) const;
 
   // The following is only defined if the request_time member is set.
-  // If this response was resurrected from cache, then this bool is set, and
+  // If this resource was found in the cache, then this bool is set, and
   // request_time may corresponds to a time "far" in the past.  Note that
   // stale content (perhaps un-cacheable) may be fetched from cache subject to
   // the load flags specified on the request info.  For example, this is done
   // when a user presses the back button to re-render pages, or at startup,
   // when reloading previously visited pages (without going over the network).
+  // Note also that under normal circumstances, was_cached is set to the correct
+  // value even if the request fails.
   bool was_cached;
 
   // True if the request was fetched from cache rather than the network
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 7bec1f7..a1567a9 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -853,6 +853,10 @@ void URLRequestHttpJob::OnStartCompleted(int result) {
     NotifyCertificateRequested(
         transaction_->GetResponseInfo()->cert_request_info.get());
   } else {
+    // Even on an error, there may be useful information in the response
+    // info (e.g. whether there's a cached copy).
+    if (transaction_.get())
+      response_info_ = transaction_->GetResponseInfo();
     NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, result));
   }
 }
@@ -944,9 +948,10 @@ bool URLRequestHttpJob::GetCharset(std::string* charset) {
 
 void URLRequestHttpJob::GetResponseInfo(HttpResponseInfo* info) {
   DCHECK(request_);
-  DCHECK(transaction_.get());
 
   if (response_info_) {
+    DCHECK(transaction_.get());
+
     *info = *response_info_;
     if (override_response_headers_.get())
       info->headers = override_response_headers_;
diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc
index 1b6e314..3d9e17e 100644
--- a/net/url_request/url_request_job.cc
+++ b/net/url_request/url_request_job.cc
@@ -430,8 +430,13 @@ void URLRequestJob::NotifyStartError(const URLRequestStatus &status) {
   DCHECK(!has_handled_response_);
   has_handled_response_ = true;
   if (request_) {
+    // There may be relevant information in the response info even in the
+    // error case.
+    GetResponseInfo(&request_->response_info_);
+
     request_->set_status(status);
     request_->NotifyResponseStarted();
+    // We may have been deleted.
   }
 }