Add pending preloads indexed by routing id. Start preloading once we navigate.

chrome/browser/prerender/prerender_browsertest.cc

Index: chrome/browser/prerender/prerender_browsertest.cc
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc
index 5ea44d6a20d03433c022f35b49f47c30b49098fa..c96de0b54a19b7f658c12f837668f0d018055c87 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -85,16 +85,31 @@ class WaitForLoadPrerenderContentsFactory : public PrerenderContents::Factory {
       : expected_final_status_(expected_final_status) {
   }
 
+  void set_expected_final_status(FinalStatus expected_final_status) {
+    expected_final_status_ = expected_final_status;
+  }
+
+  void set_expected_final_status_for_url(const GURL& url,
+                                         FinalStatus expected_final_status) {
+    expected_final_status_map_[url] = expected_final_status;

[cbentzel, 2011/03/15 18:42:24]

Should this DCHECK that it doesn't already exist? 

[dominich, 2011/03/15 20:41:14]

Done.

+  }
+
   virtual PrerenderContents* CreatePrerenderContents(
       PrerenderManager* prerender_manager, Profile* profile, const GURL& url,
       const std::vector<GURL>& alias_urls, const GURL& referrer) {
+    FinalStatus expected_final_status = expected_final_status_;
+    std::map<GURL, FinalStatus>::iterator it =
+        expected_final_status_map_.find(url);
+    if (it != expected_final_status_map_.end())

[cbentzel, 2011/03/15 18:42:24]

Does this need to be erased if found, or can it stick around? 

[dominich, 2011/03/15 20:41:14]

Done.

+      expected_final_status = it->second;
     return new TestPrerenderContents(prerender_manager, profile, url,
                                      alias_urls, referrer,
-                                     expected_final_status_);
+                                     expected_final_status);
   }
 
  private:
   FinalStatus expected_final_status_;
+  std::map<GURL, FinalStatus> expected_final_status_map_;
 };
 
 }  // namespace
@@ -123,9 +138,7 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
                         FinalStatus expected_final_status,
                         int total_navigations) {
     ASSERT_TRUE(test_server()->Start());
-    std::string dest_path = "files/prerender/";
-    dest_path.append(html_file);
-    dest_url_ = test_server()->GetURL(dest_path);
+    dest_url_ = UrlForHtmlFile(html_file);
 
     std::vector<net::TestServer::StringPair> replacement_text;
     replacement_text.push_back(
@@ -147,13 +160,10 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
     }
     GURL src_url = src_server->GetURL(replacement_path);
 
-    Profile* profile = browser()->GetSelectedTabContents()->profile();
-    PrerenderManager* prerender_manager = profile->GetPrerenderManager();
-    ASSERT_TRUE(prerender_manager);
-
     // This is needed to exit the event loop once the prerendered page has
     // stopped loading or was cancelled.
-    prerender_manager->SetPrerenderContentsFactory(
+    ASSERT_TRUE(prerender_manager());
+    prerender_manager()->SetPrerenderContentsFactory(
         new WaitForLoadPrerenderContentsFactory(expected_final_status));
 
     // ui_test_utils::NavigateToURL uses its own observer and message loop.
@@ -170,7 +180,7 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
 
       prerender_contents =
           static_cast<TestPrerenderContents*>(
-              prerender_manager->FindEntry(dest_url_));
+              prerender_manager()->FindEntry(dest_url_));
       if (prerender_contents == NULL ||
           !prerender_contents->did_finish_loading() ||
           navigations >= total_navigations) {
@@ -205,12 +215,8 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
   void NavigateToDestURL() const {
     ui_test_utils::NavigateToURL(browser(), dest_url_);
 
-    Profile* profile = browser()->GetSelectedTabContents()->profile();
-    PrerenderManager* prerender_manager = profile->GetPrerenderManager();
-    ASSERT_TRUE(prerender_manager);
-
     // Make sure the PrerenderContents found earlier was used or removed
-    EXPECT_TRUE(prerender_manager->FindEntry(dest_url_) == NULL);
+    EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) == NULL);
 
     // Check if page behaved as expected when actually displayed.
     bool display_test_result = false;
@@ -221,10 +227,61 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
     EXPECT_TRUE(display_test_result);
   }
 
+  bool UrlIsInPrerenderManager(const std::string& url) {

[cbentzel, 2011/03/15 18:42:24]

const

also, the name is misleading - the parameter is not actually a full URL. 

+    GURL dest_url = UrlForHtmlFile(url);
+
+    return (prerender_manager()->FindEntry(dest_url) != NULL);
+  }
+
+  bool UrlIsPendingInPrerenderManager(const std::string& url) {
+    GURL dest_url = UrlForHtmlFile(url);
+
+    for (PrerenderManager::PendingPrerenderList::iterator map_it =

[cbentzel, 2011/03/15 18:42:24]

Add a FindPendingEntry() to PrerenderManager? Would make more sense to have the
logic there. 

[dominich, 2011/03/15 20:41:14]

Done.

+            prerender_manager()->pending_prerender_list_.begin();
+         map_it != prerender_manager()->pending_prerender_list_.end();
+         ++map_it) {
+      for (std::vector<PrerenderManager::PendingContentsData>::iterator
+              content_it = map_it->second.begin();
+           content_it != map_it->second.end(); ++content_it) {
+        if (content_it->url_ == dest_url)
+          return true;
+      }
+    }
+
+    return false;
+  }
+
+  GURL UrlForHtmlFile(const std::string& html_file) {

[cbentzel, 2011/03/15 18:42:24]

const, I think GetURL is const.

[dominich, 2011/03/15 20:41:14]

Sadly not.

+    std::string dest_path = "files/prerender/";
+    dest_path.append(html_file);
+    return test_server()->GetURL(dest_path);
+  }
+
+  PrerenderManager* prerender_manager() const {
+    Profile* profile = browser()->GetSelectedTabContents()->profile();
+    PrerenderManager* prerender_manager = profile->GetPrerenderManager();
+    return prerender_manager;
+  }
+
   void set_use_https_src(bool use_https_src_server) {
     use_https_src_server_ = use_https_src_server;
   }
 
+  void SetExpectedFinalStatus(FinalStatus expected_final_status) {
+    WaitForLoadPrerenderContentsFactory* prc_factory =

[cbentzel, 2011/03/15 18:42:24]

I would just set a pointer to prc_factory in PrerenderTestURL, or even create it
in the constructor - this is a bit gross. 

[dominich, 2011/03/15 20:41:14]

Done.

+        static_cast<WaitForLoadPrerenderContentsFactory*>(
+            prerender_manager()->prerender_contents_factory_.get());
+    prc_factory->set_expected_final_status(expected_final_status);
+  }
+
+  void SetExpectedFinalStatusForUrl(const GURL& url,

[cbentzel, 2011/03/15 18:42:24]

I would change this to take a const std::string& like
UrlIsPendingInPrerenderManager does, call UrlForHtmlFile internally, and make
UrlForHtmlFile private. I think prerender_manager() could also be private.

[dominich, 2011/03/15 20:41:14]

Done.

+                                    FinalStatus expected_final_status) {
+    WaitForLoadPrerenderContentsFactory* prc_factory =
+        static_cast<WaitForLoadPrerenderContentsFactory*>(
+            prerender_manager()->prerender_contents_factory_.get());
+    prc_factory->set_expected_final_status_for_url(url, expected_final_status);
+  }
+
  private:
   GURL dest_url_;
   bool use_https_src_server_;
@@ -242,17 +299,15 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPage) {
 // Checks that the prerendering of a page is canceled correctly when a
 // Javascript alert is called.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderAlertBeforeOnload) {
-  PrerenderTestURL(
-      "prerender_alert_before_onload.html",
-      FINAL_STATUS_JAVASCRIPT_ALERT, 1);
+  PrerenderTestURL("prerender_alert_before_onload.html",
+                   FINAL_STATUS_JAVASCRIPT_ALERT, 1);
 }
 
 // Checks that the prerendering of a page is canceled correctly when a
 // Javascript alert is called.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderAlertAfterOnload) {
-  PrerenderTestURL(
-      "prerender_alert_after_onload.html",
-      FINAL_STATUS_JAVASCRIPT_ALERT, 1);
+  PrerenderTestURL("prerender_alert_after_onload.html",
+                   FINAL_STATUS_JAVASCRIPT_ALERT, 1);
 }
 
 // Checks that plugins are not loaded while a page is being preloaded, but
@@ -353,4 +408,59 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderExcessiveMemory) {
                    FINAL_STATUS_MEMORY_LIMIT_EXCEEDED, 1);
 }
 
+// Checks that we don't prerender in an infinite loop.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderInfiniteLoop) {
+  const char* url_a = "prerender_infinite_a.html";

[cbentzel, 2011/03/15 18:42:24]

Nit: const char* const kPathA

[dominich, 2011/03/15 20:41:14]

Done.

+  const char* url_b = "prerender_infinite_b.html";
+
+  PrerenderTestURL(url_a, FINAL_STATUS_USED, 1);
+
+  // Next url should be in pending list but not an active entry.
+  EXPECT_FALSE(UrlIsInPrerenderManager(url_b));
+  EXPECT_TRUE(UrlIsPendingInPrerenderManager(url_b));
+
+  // We are not going to navigate back to url_a but we will start the preload so
+  // we need to set the final status to expect here before navigating.
+  SetExpectedFinalStatus(FINAL_STATUS_APP_TERMINATING);
+
+  NavigateToDestURL();
+
+  // Make sure the PrerenderContents for the next url is now in the manager
+  // and not pending.
+  EXPECT_TRUE(UrlIsInPrerenderManager(url_b));
+  EXPECT_FALSE(UrlIsPendingInPrerenderManager(url_b));
+}
+
+// Checks that we don't prerender in an infinite loop and multiple links are
+// handled correctly.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderInfiniteLoopMultiple) {
+  const char* url_a = "prerender_infinite_a_multiple.html";
+  const char* url_b = "prerender_infinite_b_multiple.html";
+  const char* url_c = "prerender_infinite_c_multiple.html";
+
+  PrerenderTestURL(url_a, FINAL_STATUS_USED, 1);
+
+  // Next url should be in pending list but not an active entry.
+  EXPECT_FALSE(UrlIsInPrerenderManager(url_b));
+  EXPECT_FALSE(UrlIsInPrerenderManager(url_c));
+  EXPECT_TRUE(UrlIsPendingInPrerenderManager(url_b));
+  EXPECT_TRUE(UrlIsPendingInPrerenderManager(url_c));
+
+  // We are not going to navigate back to url_a but we will start the preload so
+  // we need to set the final status to expect here before navigating.
+  GURL gurl_b = UrlForHtmlFile(url_b);
+  GURL gurl_c = UrlForHtmlFile(url_c);
+  SetExpectedFinalStatusForUrl(gurl_b, FINAL_STATUS_EVICTED);
+  SetExpectedFinalStatusForUrl(gurl_c, FINAL_STATUS_APP_TERMINATING);
+
+  NavigateToDestURL();
+
+  // Make sure the PrerenderContents for the next urls are now in the manager
+  // and not pending. url_c was the last seen so should be the active
+  // entry.
+  EXPECT_FALSE(UrlIsInPrerenderManager(url_b));
+  EXPECT_TRUE(UrlIsInPrerenderManager(url_c));
+  EXPECT_FALSE(UrlIsPendingInPrerenderManager(url_b));
+  EXPECT_FALSE(UrlIsPendingInPrerenderManager(url_c));
+}

[cbentzel, 2011/03/15 18:42:24]

Nit: insert an extra newline here.

[dominich, 2011/03/15 20:41:14]

Done.

 }  // namespace prerender