Handle interface to prerenders.

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 1ae6186b057fe64be8f98e1478cd04549bf35d26..a47fcbf57b4ac709dcf03cdd0024020383fc022e 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prerender/prerender_contents.h"
+#include "chrome/browser/prerender/prerender_handle.h"
 #include "chrome/browser/prerender/prerender_link_manager.h"
 #include "chrome/browser/prerender/prerender_link_manager_factory.h"
 #include "chrome/browser/prerender/prerender_manager.h"
@@ -30,6 +31,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
@@ -49,6 +51,7 @@
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "content/public/test/test_navigation_observer.h"
 
 using content::BrowserThread;
 using content::DevToolsAgentHost;
@@ -111,13 +114,11 @@ bool ShouldRenderPrerenderedPageCorrectly(FinalStatus status) {
     case FINAL_STATUS_USED:
     case FINAL_STATUS_WINDOW_OPENER:
     case FINAL_STATUS_APP_TERMINATING:
-    case FINAL_STATUS_FRAGMENT_MISMATCH:
     case FINAL_STATUS_CACHE_OR_HISTORY_CLEARED:
     // We'll crash the renderer after it's loaded.
     case FINAL_STATUS_RENDERER_CRASHED:
     case FINAL_STATUS_CANCELLED:
     case FINAL_STATUS_DEVTOOLS_ATTACHED:
-    case FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH:
       return true;
     default:
       return false;
@@ -127,6 +128,10 @@ bool ShouldRenderPrerenderedPageCorrectly(FinalStatus status) {
 // PrerenderContents that stops the UI message loop on DidStopLoading().
 class TestPrerenderContents : public PrerenderContents {
  public:
+  using PrerenderContents::PendingPrerenderInfo;

[dominich, 2012/07/02 20:43:03]

Should these just be made protected in PrerenderContents?

[gavinp, 2012/07/03 16:41:02]

They are already protected, this would not work otherwise. However,
PrerenderBrowserTest::URLIsPendingInPrerenderManager depended on these. I moved
that method into TestPrerenderContents, and now the using statements are gone.

+  using PrerenderContents::PendingPrerenderList;
+  using PrerenderContents::pending_prerender_list;
+
   TestPrerenderContents(
       PrerenderManager* prerender_manager,
       PrerenderTracker* prerender_tracker,
@@ -227,10 +232,13 @@ class TestPrerenderContents : public PrerenderContents {
     }
   }
 
-  virtual void AddPendingPrerender(const GURL& url,
-                                   const content::Referrer& referrer,
-                                   const gfx::Size& size) OVERRIDE {
-    PrerenderContents::AddPendingPrerender(url, referrer, size);
+  virtual void AddPendingPrerender(
+      const base::WeakPtr<PrerenderHandle> weak_prerender_handle,
+      const GURL& url,
+      const content::Referrer& referrer,
+      const gfx::Size& size) OVERRIDE {
+    PrerenderContents::AddPendingPrerender(
+        weak_prerender_handle, url, referrer, size);
     if (expected_pending_prerenders_ > 0 &&
         pending_prerender_list()->size() == expected_pending_prerenders_) {
       MessageLoop::current()->Quit();
@@ -274,6 +282,8 @@ class TestPrerenderContents : public PrerenderContents {
 
   int number_of_loads() const { return number_of_loads_; }
 
+  FinalStatus expected_final_status() const { return expected_final_status_; }
+
  private:
   virtual void OnRenderViewHostCreated(
       RenderViewHost* new_render_view_host) OVERRIDE {
@@ -490,6 +500,11 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
 
   virtual ~PrerenderBrowserTest() {}
 
+  content::SessionStorageNamespace* GetSessionStorageNamespace() const {
+    return current_browser()->tab_strip_model()->GetActiveTabContents()->

[dominich, 2012/07/02 20:43:03]

Is this fragile on, say, OSX where we can have a browser but no active tab
contents? Do you need if(.. != NULL) checks? 

[gavinp, 2012/07/03 16:41:02]

Done.

+        web_contents()-> GetRenderViewHost()->GetSessionStorageNamespace();
+  }
+
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
 #if defined(ENABLE_SAFE_BROWSING)
     SafeBrowsingService::RegisterFactory(safe_browsing_factory_.get());
@@ -703,17 +718,34 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
   }
 
   bool UrlIsInPrerenderManager(const std::string& html_file) const {
-    GURL dest_url = test_server()->GetURL(html_file);
-    return (GetPrerenderManager()->FindEntry(dest_url) != NULL);
+    return UrlIsInPrerenderManager(test_server()->GetURL(html_file));
   }
 
-  bool UrlIsInPrerenderManager(const GURL& url) {
-    return (GetPrerenderManager()->FindEntry(url) != NULL);
+  bool UrlIsInPrerenderManager(const GURL& url) const {
+    return GetPrerenderManager()->FindPrerenderData(
+        url, GetSessionStorageNamespace()) != NULL;
   }
 
   bool UrlIsPendingInPrerenderManager(const std::string& html_file) const {
+    TestPrerenderContents* test_prerender_contents = GetPrerenderContents();
+    if (!test_prerender_contents)
+      return false;
     GURL dest_url = test_server()->GetURL(html_file);
-    return GetPrerenderManager()->IsPendingEntry(dest_url);
+    for (TestPrerenderContents::PendingPrerenderList::const_iterator
+             it = test_prerender_contents->pending_prerender_list()->begin(),
+             end = test_prerender_contents->pending_prerender_list()->end();
+         it != end;
+         ++it) {
+      if (it->url == dest_url && it->weak_prerender_handle) {
+        EXPECT_FALSE(GetPrerenderManager()->FindPrerenderData(
+            dest_url, GetSessionStorageNamespace()));
+        EXPECT_TRUE(
+            test_prerender_contents->IsPendingEntry(it->weak_prerender_handle));
+        EXPECT_TRUE(it->weak_prerender_handle->IsPending());
+        return true;
+      }
+    }
+    return false;
   }
 
   void set_use_https_src(bool use_https_src_server) {
@@ -764,8 +796,11 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
 #endif
 
   TestPrerenderContents* GetPrerenderContents() const {
+    PrerenderHandle::PrerenderData* prerender_data =
+        GetPrerenderManager()->FindPrerenderData(
+            dest_url_, GetSessionStorageNamespace());
     return static_cast<TestPrerenderContents*>(
-        GetPrerenderManager()->FindEntry(dest_url_));
+        prerender_data ? prerender_data->contents : NULL);
   }
 
   void set_loader_path(const std::string& path) {
@@ -882,9 +917,12 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
     } else {
       // In the failure case, we should have removed |dest_url_| from the
       // prerender_manager.  We ignore dummy PrerenderContents (as indicated
-      // by not having started).
+      // by not having started), and PrerenderContents that are expected to
+      // be left in the manager until the test finishes.
       EXPECT_TRUE(prerender_contents == NULL ||
-                  !prerender_contents->prerendering_has_started());
+                  !prerender_contents->prerendering_has_started() ||
+                  prerender_contents->expected_final_status() ==
+                      FINAL_STATUS_MANAGER_SHUTDOWN);
     }
   }
 
@@ -953,8 +991,21 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
         string16(),
         ASCIIToUTF16(javascript_function_name));
 
-    // Run message loop until the prerender contents is destroyed.
-    ui_test_utils::RunMessageLoop();
+    if (prerender_contents->expected_final_status() !=
+        FINAL_STATUS_APP_TERMINATING) {
+      // Run message loop until the prerender contents is destroyed.
+      ui_test_utils::RunMessageLoop();
+    } else {
+      // We don't expect to pick up a running prerender, so instead
+      // something else.
+      content::TestNavigationObserver observer(
+          content::NotificationService::AllSources(), NULL, 1);
+      base::RunLoop run_loop;
+      observer.WaitForObservation(
+          base::Bind(&ui_test_utils::RunThisRunLoop,
+                     base::Unretained(&run_loop)),
+          ui_test_utils::GetQuitTaskForRunLoop(&run_loop));
+    }
   }
 
   WaitForLoadPrerenderContentsFactory* prerender_contents_factory_;
@@ -1638,7 +1689,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderRendererCrash) {
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        DISABLED_PrerenderPageNavigateFragment) {
   PrerenderTestURL("files/prerender/prerender_fragment.html",
-                   FINAL_STATUS_FRAGMENT_MISMATCH,
+                   FINAL_STATUS_APP_TERMINATING,

[dominich, 2012/07/02 20:43:03]

Why is this not USED?

also below a few times.

[gavinp, 2012/07/03 16:41:02]

The URLs do not compare as equal.

[dominich, 2012/07/03 17:08:39]

So this would be TIMED_OUT in production? Maybe we need to bring back
FRAGMENT_MISMATCH to see how often we are prerendering something with a
different ref to the navigation. WDYT?

                    1);
   NavigateToURL("files/prerender/prerender_fragment.html#fragment");
 }
@@ -1649,7 +1700,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        DISABLED_PrerenderFragmentNavigatePage) {
   PrerenderTestURL("files/prerender/prerender_fragment.html#fragment",
-                   FINAL_STATUS_FRAGMENT_MISMATCH,
+                   FINAL_STATUS_APP_TERMINATING,
                    1);
   NavigateToURL("files/prerender/prerender_fragment.html");
 }
@@ -1660,7 +1711,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        DISABLED_PrerenderFragmentNavigateFragment) {
   PrerenderTestURL("files/prerender/prerender_fragment.html#other_fragment",
-                   FINAL_STATUS_FRAGMENT_MISMATCH,
+                   FINAL_STATUS_APP_TERMINATING,
                    1);
   NavigateToURL("files/prerender/prerender_fragment.html#fragment");
 }
@@ -1672,7 +1723,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        DISABLED_PrerenderClientRedirectFromFragment) {
   PrerenderTestURL(
       CreateClientRedirect("files/prerender/prerender_fragment.html#fragment"),
-      FINAL_STATUS_FRAGMENT_MISMATCH,
+      FINAL_STATUS_APP_TERMINATING,
       2);
   NavigateToURL("files/prerender/prerender_fragment.html");
 }
@@ -1684,7 +1735,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        DISABLED_PrerenderClientRedirectToFragment) {
   PrerenderTestURL(
       CreateClientRedirect("files/prerender/prerender_fragment.html"),
-      FINAL_STATUS_FRAGMENT_MISMATCH,
+      FINAL_STATUS_APP_TERMINATING,
       2);
   NavigateToURL("files/prerender/prerender_fragment.html#fragment");
 }
@@ -1699,7 +1750,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
   NavigateToURL("files/prerender/prerender_fragment_location_hash.html");
 }
 
-// Checks that prerendering a PNG works correctly.
+// Checks that preren a PNG works correctly.

[dominich, 2012/07/02 20:43:03]

typo

[gavinp, 2012/07/03 16:41:02]

Done.

 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderImagePng) {
   DisableJavascriptCalls();
   PrerenderTestURL("files/prerender/image.png", FINAL_STATUS_USED, 1);
@@ -1846,7 +1897,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPrint) {
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderSameDomainWindowOpenerWindowOpen) {
   PrerenderTestURL("files/prerender/prerender_page.html",
-                   FINAL_STATUS_WINDOW_OPENER,
+                   FINAL_STATUS_APP_TERMINATING,
                    1);
   OpenDestURLViaWindowOpen();
 }
@@ -1856,7 +1907,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderSameDomainWindowOpenerClickTarget) {
   PrerenderTestURL("files/prerender/prerender_page.html",
-                   FINAL_STATUS_WINDOW_OPENER,
+                   FINAL_STATUS_APP_TERMINATING,
                    1);
   OpenDestURLViaClickTarget();
 }
@@ -2139,21 +2190,21 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderClickNewWindow) {
   PrerenderTestURL("files/prerender/prerender_page_with_link.html",
-                   FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH,
+                   FINAL_STATUS_APP_TERMINATING,

[dominich, 2012/07/03 17:08:39]

If this isn't used because the Session Storage Namespace doesn't match, then we
should still track it as such.

[gavinp, 2012/07/03 18:45:40]

I disagree; I'm happy to throw out that result code. It's a special case that
infects all of our matching logic to find it, and to gather a stat that is
basically about how often users middle click to search results?

We have a good estimate of the frequency of that now, and we can just carry it
forward, and if we ever add more liberal session storage namespace handling,
we'll see the benefit in A/B testing...

                    1);
   OpenDestURLViaClickNewWindow();
 }
 
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderClickNewForegroundTab) {
   PrerenderTestURL("files/prerender/prerender_page_with_link.html",
-                   FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH,
+                   FINAL_STATUS_APP_TERMINATING,
                    1);
   OpenDestURLViaClickNewForegroundTab();
 }
 
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderClickNewBackgroundTab) {
   PrerenderTestURL("files/prerender/prerender_page_with_link.html",
-                   FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH,
+                   FINAL_STATUS_APP_TERMINATING,
                    1);
   OpenDestURLViaClickNewBackgroundTab();
 }