Re-enabled URLRequestTestHTTP.BasicAuth.

The problem was the test was expecting |response_time| of cached responses to be the timestamp of the original request, rather than the timestamp of the last cache validation.

Also updated the header to clarify what |request_time| and |response_time| correspond to in the cached cases.

BUG=http://crbug.com/20594

Review URL: http://codereview.chromium.org/197030

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

3 files changed, 9 insertions, 28 deletions

diff --git a/net/http/http_response_info.h b/net/http/http_response_info.h
index 7cbe13c..62ac2f4 100644
--- a/net/http/http_response_info.h
+++ b/net/http/http_response_info.h
@@ -31,11 +31,11 @@ class HttpResponseInfo {
   bool was_cached;
 
   // The time at which the request was made that resulted in this response.
-  // For cached responses, this time could be "far" in the past.
+  // For cached responses, this is the last time the cache entry was validated.
   base::Time request_time;
 
   // The time at which the response headers were received.  For cached
-  // responses, this time could be "far" in the past.
+  // this is the last time the cache entry was validated.
   base::Time response_time;
 
   // If the response headers indicate a 401 or 407 failure, then this structure
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index f45fa07..2e23d72 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -346,13 +346,13 @@ class URLRequest {
   void GetAllResponseHeaders(std::string* headers);
 
   // The time at which the returned response was requested.  For cached
-  // responses, this may be a time well in the past.
+  // responses, this is the last time the cache entry was validated.
   const base::Time& request_time() const {
     return response_info_.request_time;
   }
 
   // The time at which the returned response was generated.  For cached
-  // responses, this may be a time well in the past.
+  // responses, this is the last time the cache entry was validated.
   const base::Time& response_time() const {
     return response_info_.response_time;
   }
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 6aedaf8..3623d45 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1207,8 +1207,6 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
 
   scoped_refptr<URLRequestContext> context = new URLRequestTestContext();
 
-  Time response_time;
-
   // populate the cache
   {
     TestDelegate d;
@@ -1217,14 +1215,8 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
     req.SetExtraRequestHeaders("foo:1");
     req.Start();
     MessageLoop::current()->Run();
-
-    response_time = req.response_time();
   }
 
-  // Make sure that the response time of a future response will be in the
-  // future!
-  PlatformThread::Sleep(10);
-
   // expect a cache hit
   {
     TestDelegate d;
@@ -1234,7 +1226,7 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
     req.Start();
     MessageLoop::current()->Run();
 
-    EXPECT_TRUE(req.response_time() == response_time);
+    EXPECT_TRUE(req.was_cached());
   }
 
   // expect a cache miss
@@ -1246,18 +1238,14 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
     req.Start();
     MessageLoop::current()->Run();
 
-    EXPECT_FALSE(req.response_time() == response_time);
+    EXPECT_FALSE(req.was_cached());
   }
 }
 
-// TODO(eroman): Broke with commit of r25484 -- this is depending on
-// the response time value.
-TEST_F(URLRequestTestHTTP, DISABLED_BasicAuth) {
+TEST_F(URLRequestTestHTTP, BasicAuth) {
   scoped_refptr<URLRequestContext> context = new URLRequestTestContext();
   ASSERT_TRUE(NULL != server_.get());
 
-  Time response_time;
-
   // populate the cache
   {
     TestDelegate d;
@@ -1271,14 +1259,8 @@ TEST_F(URLRequestTestHTTP, DISABLED_BasicAuth) {
     MessageLoop::current()->Run();
 
     EXPECT_TRUE(d.data_received().find("user/secret") != std::string::npos);
-
-    response_time = r.response_time();
   }
 
-  // Let some time pass so we can ensure that a future response will have a
-  // response time value in the future.
-  PlatformThread::Sleep(10 /* milliseconds */);
-
   // repeat request with end-to-end validation.  since auth-basic results in a
   // cachable page, we expect this test to result in a 304.  in which case, the
   // response should be fetched from the cache.
@@ -1296,9 +1278,8 @@ TEST_F(URLRequestTestHTTP, DISABLED_BasicAuth) {
 
     EXPECT_TRUE(d.data_received().find("user/secret") != std::string::npos);
 
-    // Should be the same cached document, which means that the response time
-    // should not have changed.
-    EXPECT_TRUE(response_time == r.response_time());
+    // Should be the same cached document.
+    EXPECT_TRUE(r.was_cached());
   }
 }