Remove revalidation check from PrerenderResourceHandler.

This was initially a way to try to prevent prerendering non-idempotent GETs. A large number of pages were unable to prerender effectively this way due to short max-age's or other cache directives.

BUG=None
TEST=unit_tests --gtest_filter="*PrerenderResourceHandler*"

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

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

3 files changed, 7 insertions, 102 deletions

diff --git a/chrome/browser/prerender/prerender_resource_handler.cc b/chrome/browser/prerender/prerender_resource_handler.cc
index 652e89b..64b5184 100644
--- a/chrome/browser/prerender/prerender_resource_handler.cc
+++ b/chrome/browser/prerender/prerender_resource_handler.cc
@@ -12,14 +12,7 @@
 
 namespace {
 
-base::Time DefaultGetCurrentTime() {
-  return base::Time::Now();
-}
-
-bool ShouldPrerender(const GURL& url,
-                     const ResourceResponse* response,
-                     PrerenderResourceHandler::GetCurrentTimeFunction get_time,
-                     base::TimeDelta prerender_duration) {
+bool ShouldPrerender(const GURL& url, const ResourceResponse* response) {
   if (!response)
     return false;
   const ResourceResponseHead& rrh = response->response_head;
@@ -33,11 +26,6 @@ bool ShouldPrerender(const GURL& url,
     return false;
   if (rrh.headers->response_code() != 200)
     return false;
-  if (rrh.headers->RequiresValidation(
-          rrh.request_time,
-          rrh.response_time,
-          get_time() + prerender_duration))
-    return false;
   return true;
 }
 
@@ -64,10 +52,7 @@ PrerenderResourceHandler::PrerenderResourceHandler(
       prerender_manager_(prerender_manager),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           prerender_callback_(NewCallback(
-              this, &PrerenderResourceHandler::StartPrerender))),
-      prerender_duration_(
-          base::TimeDelta::FromSeconds(kDefaultPrerenderDurationSeconds)),
-      get_current_time_(&DefaultGetCurrentTime) {
+              this, &PrerenderResourceHandler::StartPrerender))) {
   DCHECK(next_handler);
   DCHECK(prerender_manager);
 }
@@ -77,10 +62,7 @@ PrerenderResourceHandler::PrerenderResourceHandler(
     ResourceHandler* next_handler,
     PrerenderCallback* callback)
     : next_handler_(next_handler),
-      prerender_callback_(callback),
-      prerender_duration_(
-          base::TimeDelta::FromSeconds(kDefaultPrerenderDurationSeconds)),
-      get_current_time_(&DefaultGetCurrentTime) {
+      prerender_callback_(callback) {
   DCHECK(next_handler);
   DCHECK(callback);
 }
@@ -109,10 +91,7 @@ bool PrerenderResourceHandler::OnRequestRedirected(int request_id,
 
 bool PrerenderResourceHandler::OnResponseStarted(int request_id,
                                                  ResourceResponse* response) {
-  if (ShouldPrerender(url_,
-                      response,
-                      get_current_time_,
-                      prerender_duration_)) {
+  if (ShouldPrerender(url_, response)) {
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
@@ -172,17 +151,3 @@ void PrerenderResourceHandler::StartPrerender(const GURL& url,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   prerender_manager_->AddPreload(url, alias_urls);
 }
-
-void PrerenderResourceHandler::set_prerender_duration(base::TimeDelta dt) {
-  prerender_duration_ = dt;
-}
-
-void PrerenderResourceHandler::set_get_current_time_function(
-    GetCurrentTimeFunction get_current_time) {
-  DCHECK(get_current_time);
-  get_current_time_ = get_current_time;
-}
-
-// Note: this should stay in line with prerendermanager
-// static
-const int PrerenderResourceHandler::kDefaultPrerenderDurationSeconds = 20;
diff --git a/chrome/browser/prerender/prerender_resource_handler.h b/chrome/browser/prerender/prerender_resource_handler.h
index b7a36f6..b4f191e 100644
--- a/chrome/browser/prerender/prerender_resource_handler.h
+++ b/chrome/browser/prerender/prerender_resource_handler.h
@@ -24,12 +24,8 @@ class URLRequest;
 //   - The final URL (after redirects) has a scheme of http or https.
 //   - The response status code is a 200.
 //   - The MIME type of the response (sniffed or explicit) is text/html.
-//   - The resource will not need to revalidate within 30 seconds. This prevents
-//     no-cache or very quickly refreshing resources from being prerendered.
 class PrerenderResourceHandler : public ResourceHandler {
  public:
-  typedef base::Time (*GetCurrentTimeFunction)();
-
   // Creates a new PrerenderResourceHandler if appropriate for the
   // given |request| and |context|, otherwise NULL is returned. The
   // caller is resposible for deleting the returned handler.
@@ -74,8 +70,6 @@ class PrerenderResourceHandler : public ResourceHandler {
   typedef Callback2<const GURL&, const std::vector<GURL>&>::Type
       PrerenderCallback;
 
-  static const int kDefaultPrerenderDurationSeconds;
-
   PrerenderResourceHandler(ResourceHandler* next_handler,
                            PrerenderManager* prerender_manager);
   PrerenderResourceHandler(ResourceHandler* next_handler,
@@ -86,8 +80,6 @@ class PrerenderResourceHandler : public ResourceHandler {
                                const std::vector<GURL>& alias_urls);
   void StartPrerender(const GURL& url,
                       const std::vector<GURL>& alias_urls);
-  void set_prerender_duration(base::TimeDelta prerender_duration);
-  void set_get_current_time_function(GetCurrentTimeFunction get_current_time);
 
   // The set of URLs that are aliases to the URL to be prerendered,
   // as a result of redirects.
@@ -96,8 +88,6 @@ class PrerenderResourceHandler : public ResourceHandler {
   scoped_refptr<ResourceHandler> next_handler_;
   scoped_refptr<PrerenderManager> prerender_manager_;
   scoped_ptr<PrerenderCallback> prerender_callback_;
-  base::TimeDelta prerender_duration_;
-  GetCurrentTimeFunction get_current_time_;
 
   DISALLOW_COPY_AND_ASSIGN(PrerenderResourceHandler);
 };
diff --git a/chrome/browser/prerender/prerender_resource_handler_unittest.cc b/chrome/browser/prerender/prerender_resource_handler_unittest.cc
index 5a35b98..6bc4c13 100644
--- a/chrome/browser/prerender/prerender_resource_handler_unittest.cc
+++ b/chrome/browser/prerender/prerender_resource_handler_unittest.cc
@@ -59,10 +59,6 @@ class MockResourceHandler : public ResourceHandler {
   virtual void OnDataDownloaded(int request_id, int bytes_downloaded) {}
 };
 
-base::Time FixedGetCurrentTime() {
-  return base::Time();
-}
-
 // HttpResponseHeaders expects the raw input for it's constructor
 // to be a NUL ('\0') separated string for each line. This is a little
 // difficult to do for string literals, so this helper function accepts
@@ -86,8 +82,7 @@ net::HttpResponseHeaders* CreateResponseHeaders(
 class PrerenderResourceHandlerTest : public testing::Test {
  protected:
   PrerenderResourceHandlerTest()
-      : prerender_duration_(base::TimeDelta::FromSeconds(10)),
-        ALLOW_THIS_IN_INITIALIZER_LIST(
+      : ALLOW_THIS_IN_INITIALIZER_LIST(
             pre_handler_(new PrerenderResourceHandler(
                 new MockResourceHandler(),
                 NewCallback(
@@ -96,8 +91,6 @@ class PrerenderResourceHandlerTest : public testing::Test {
         ui_thread_(BrowserThread::UI, &loop_),
         io_thread_(BrowserThread::IO, &loop_),
         default_url_("http://www.prerender.com") {
-    pre_handler_->set_prerender_duration(prerender_duration_);
-    pre_handler_->set_get_current_time_function(&FixedGetCurrentTime);
   }
 
   virtual ~PrerenderResourceHandlerTest() {
@@ -123,8 +116,6 @@ class PrerenderResourceHandlerTest : public testing::Test {
     EXPECT_TRUE(pre_handler_->OnWillStart(request_id, default_url_, &defer));
     EXPECT_FALSE(defer);
     scoped_refptr<ResourceResponse> response(new ResourceResponse);
-    response->response_head.request_time = FixedGetCurrentTime();
-    response->response_head.response_time = FixedGetCurrentTime();
     response->response_head.mime_type = mime_type;
     response->response_head.headers = CreateResponseHeaders(headers);
     EXPECT_TRUE(last_handled_url_.is_empty());
@@ -141,7 +132,6 @@ class PrerenderResourceHandlerTest : public testing::Test {
         != alias_urls_.end();
   }
 
-  base::TimeDelta prerender_duration_;
   scoped_refptr<PrerenderResourceHandler> pre_handler_;
   MessageLoop loop_;
   BrowserThread ui_thread_;
@@ -160,47 +150,10 @@ TEST_F(PrerenderResourceHandlerTest, NoOp) {
 // to the PrerenderManager.
 TEST_F(PrerenderResourceHandlerTest, Prerender) {
   StartPrerendering("text/html",
-                    "HTTP/1.1 200 OK\n"
-                    "cache-control: max-age=86400\n");
+                    "HTTP/1.1 200 OK\n");
   EXPECT_EQ(default_url_, last_handled_url_);
 }
 
-// Tests that a no-cache HTML resource will not get diverted
-// to the PrerenderManager.
-TEST_F(PrerenderResourceHandlerTest, PrerenderNoCache) {
-  StartPrerendering("text/html",
-                    "HTTP/1.1 200 OK\n"
-                    "cache-control: no-cache\n");
-  EXPECT_TRUE(last_handled_url_.is_empty());
-}
-
-// Tests that a cacheable HTML resource which needs to be revalidated
-// shortly will not be prerendered.
-TEST_F(PrerenderResourceHandlerTest, PrerenderShortMaxAge) {
-  StartPrerendering("text/html",
-                    "HTTP/1.1 200 OK\n"
-                    "cache-control: max-age=5\n");
-  EXPECT_TRUE(last_handled_url_.is_empty());
-}
-
-// Tests that a resource with the wrong MIME type (a GIF in this example)
-// will not be diverted to the PrerenderManager.
-TEST_F(PrerenderResourceHandlerTest, PrerenderWrongMimeType) {
-  StartPrerendering("image/gif",
-                    "HTTP/1.1 200 OK\n"
-                    "cache-control: max-age=86400\n");
-  EXPECT_TRUE(last_handled_url_.is_empty());
-}
-
-// Tests that a resource with a non-200 response will not be diverted
-// to the PrerenderManager
-TEST_F(PrerenderResourceHandlerTest, PrerenderBadResponseCode) {
-  StartPrerendering("text/html",
-                    "HTTP/1.1 403 Forbidden\n"
-                    "cache-control: max-age=86400\n");
-  EXPECT_TRUE(last_handled_url_.is_empty());
-}
-
 // Tests that the final request in a redirect chain will
 // get diverted to the PrerenderManager.
 TEST_F(PrerenderResourceHandlerTest, PrerenderRedirect) {
@@ -216,11 +169,8 @@ TEST_F(PrerenderResourceHandlerTest, PrerenderRedirect) {
   EXPECT_FALSE(defer);
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   response->response_head.mime_type = "text/html";
-  response->response_head.request_time = FixedGetCurrentTime();
-  response->response_head.response_time = FixedGetCurrentTime();
   response->response_head.headers = CreateResponseHeaders(
-      "HTTP/1.1 200 OK\n"
-      "cache-control: max-age=86400\n");
+      "HTTP/1.1 200 OK\n");
   EXPECT_TRUE(pre_handler_->OnResponseStarted(request_id, response));
   EXPECT_TRUE(last_handled_url_.is_empty());
   loop_.RunAllPending();