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..18287d7619116ef15438ed6721ee4b032a3309de 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"
@@ -44,6 +46,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/url_constants.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "grit/generated_resources.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/url_request/url_request_context.h"
@@ -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;
@@ -227,12 +228,15 @@ 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(
+      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_) {
+        pending_prerenders().size() == expected_pending_prerenders_) {
       MessageLoop::current()->Quit();
     }
   }
@@ -258,13 +262,28 @@ class TestPrerenderContents : public PrerenderContents {
   // Waits until the prerender has |expected_pending_prerenders| pending
   // prerenders.
   void WaitForPendingPrerenders(size_t expected_pending_prerenders) {
-    if (pending_prerender_list()->size() < expected_pending_prerenders) {
+    if (pending_prerenders().size() < expected_pending_prerenders) {
       expected_pending_prerenders_ = expected_pending_prerenders;
       ui_test_utils::RunMessageLoop();
       expected_pending_prerenders_ = 0;
     }
 
-    EXPECT_EQ(expected_pending_prerenders, pending_prerender_list()->size());
+    EXPECT_EQ(expected_pending_prerenders, pending_prerenders().size());
+  }
+
+  bool UrlIsPending(const GURL& url) const {
+    for (std::vector<PendingPrerenderInfo>::const_iterator
+             it = pending_prerenders().begin(),
+             end = pending_prerenders().end();
+         it != end;
+         ++it) {
+      if (it->url == url && it->weak_prerender_handle) {
+        EXPECT_TRUE(IsPendingEntry(*it->weak_prerender_handle));
+        EXPECT_TRUE(it->weak_prerender_handle->IsPending());
+        return true;
+      }
+    }
+    return false;
   }
 
   // For tests that open the prerender in a new background tab, the RenderView
@@ -274,6 +293,12 @@ class TestPrerenderContents : public PrerenderContents {
 
   int number_of_loads() const { return number_of_loads_; }
 
+  FinalStatus expected_final_status() const { return expected_final_status_; }
+
+  bool WillQuitMessageLoopOnDestruction() const {

[mmenke, 2012/07/13 14:49:10]

nit:  quit_message_loop_on_destruction()

[gavinp, 2012/07/13 16:42:43]

Done, though I avoided that as I felt the Will helped documentation.

+    return quit_message_loop_on_destruction_;
+  }
+
  private:
   virtual void OnRenderViewHostCreated(
       RenderViewHost* new_render_view_host) OVERRIDE {
@@ -490,6 +515,15 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
 
   virtual ~PrerenderBrowserTest() {}
 
+  content::SessionStorageNamespace* GetSessionStorageNamespace() const {
+    TabContents* tab_contents =
+        current_browser()->tab_strip_model()->GetActiveTabContents();
+    if (!tab_contents)
+      return NULL;
+    return tab_contents->web_contents()->GetRenderViewHost()->
+        GetSessionStorageNamespace();
+  }
+
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
 #if defined(ENABLE_SAFE_BROWSING)
     SafeBrowsingService::RegisterFactory(safe_browsing_factory_.get());
@@ -703,17 +737,21 @@ 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 {
+  // This only checks to see if the URL is pending in our TestPrerenderContents.
+  bool UrlIsPending(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);
+    return test_prerender_contents->UrlIsPending(dest_url);
   }
 
   void set_use_https_src(bool use_https_src_server) {
@@ -764,8 +802,11 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
 #endif
 
   TestPrerenderContents* GetPrerenderContents() const {
+    PrerenderManager::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) {
@@ -850,6 +891,15 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
         prerender_contents_factory_);
     FinalStatus expected_final_status = expected_final_status_queue.front();
 
+    // We construct this launch_nav_observer so that we can be certain our

[mmenke, 2012/07/13 14:49:10]

nit:  "this WindowedNotificationObserver" or "launch_nav_observer".

[gavinp, 2012/07/13 16:42:43]

Done.

+    // loader page has finished loading before continuing. This prevents
+    // ambiguous NOTIFICATION_LOAD_STOP events from making tests flaky.
+    WebContents* web_contents = chrome::GetActiveWebContents(current_browser());
+    ui_test_utils::WindowedNotificationObserver loader_nav_observer(
+        content::NOTIFICATION_LOAD_STOP,
+        content::Source<NavigationController>(
+            &web_contents->GetController()));
+
     // ui_test_utils::NavigateToURL uses its own observer and message loop.
     // Since the test needs to wait until the prerendered page has stopped
     // loading, rather than the page directly navigated to, need to
@@ -859,6 +909,7 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
         false));
 
     ui_test_utils::RunMessageLoop();
+    loader_nav_observer.Wait();

[mmenke, 2012/07/13 14:49:10]

Suggest a comment that this must be after the ui_test_utils::RunMessageLoop();
line, since otherwise the prerender's DidStopLoading event could end up exiting
the loader_nav_observer's message loop.

[gavinp, 2012/07/13 16:42:43]

Added. But is that really true? The prerender is on a different web contents, so
we're not going to get notifications from it, are we? I think what we're
guarding against here is the prerender finishing and exiting the message loop
redundantly? 

[mmenke, 2012/07/13 17:01:29]

The problem isn't that we could get its message, it's that its exit call would
mess things up.  Consider this:

load_nav_observer starts waiting.
PrerenderContents is destroyed, tries to exit message loop.
First load completes, tries to exit message loop.  (This can happen since it
uses ::Quit instead of ::QuitNow.  Order could also be reversed).
We exit message loop.
We call RunMessageLoop.
We hang forever.

Alternatively:

load_nav_observer starts waiting.
PrerenderContents is destroyed, tries to exit message loop.
We exit message loop.
loader_nav_observer notices it didn't exit the message loop.  It is sad.  :(

 
     TestPrerenderContents* prerender_contents = GetPrerenderContents();
 
@@ -882,7 +933,8 @@ 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());
     }
@@ -949,12 +1001,25 @@ class PrerenderBrowserTest : public InProcessBrowserTest {
 
     RenderViewHost* render_view_host =
         chrome::GetActiveWebContents(current_browser())->GetRenderViewHost();
-    render_view_host->ExecuteJavascriptInWebFrame(
-        string16(),
-        ASCIIToUTF16(javascript_function_name));
 
-    // Run message loop until the prerender contents is destroyed.
-    ui_test_utils::RunMessageLoop();
+    if (prerender_contents->WillQuitMessageLoopOnDestruction()) {
+      render_view_host->ExecuteJavascriptInWebFrame(
+          string16(), ASCIIToUTF16(javascript_function_name));
+      // 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
+      // observe one navigation.
+      content::TestNavigationObserver observer(
+          content::NotificationService::AllSources(), NULL, 1);
+      render_view_host->ExecuteJavascriptInWebFrame(
+          string16(), ASCIIToUTF16(javascript_function_name));

[mmenke, 2012/07/13 14:49:10]

nit:  I was wrong about creating the observer first, was thinking of the
Windowed observer.  TestNavigationObserver isn't that smart.  I'd suggest just
moving this line back up outside the loop, so you only need one copy of the
line.

[gavinp, 2012/07/13 16:42:43]

Done.

+      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_;
@@ -1437,14 +1502,14 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderInfiniteLoop) {
 
   // Next url should be in pending list but not an active entry.
   EXPECT_FALSE(UrlIsInPrerenderManager(kHtmlFileB));
-  EXPECT_TRUE(UrlIsPendingInPrerenderManager(kHtmlFileB));
+  EXPECT_TRUE(UrlIsPending(kHtmlFileB));
 
   NavigateToDestURL();
 
   // Make sure the PrerenderContents for the next url is now in the manager
   // and not pending.
   EXPECT_TRUE(UrlIsInPrerenderManager(kHtmlFileB));
-  EXPECT_FALSE(UrlIsPendingInPrerenderManager(kHtmlFileB));
+  EXPECT_FALSE(UrlIsPending(kHtmlFileB));
 }
 
 // Checks that we don't prerender in an infinite loop and multiple links are
@@ -1472,8 +1537,8 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderInfiniteLoopMultiple) {
   // Next url should be in pending list but not an active entry.
   EXPECT_FALSE(UrlIsInPrerenderManager(kHtmlFileB));
   EXPECT_FALSE(UrlIsInPrerenderManager(kHtmlFileC));
-  EXPECT_TRUE(UrlIsPendingInPrerenderManager(kHtmlFileB));
-  EXPECT_TRUE(UrlIsPendingInPrerenderManager(kHtmlFileC));
+  EXPECT_TRUE(UrlIsPending(kHtmlFileB));
+  EXPECT_TRUE(UrlIsPending(kHtmlFileC));
 
   NavigateToDestURL();
 
@@ -1484,8 +1549,8 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderInfiniteLoopMultiple) {
   bool url_c_is_active_prerender = UrlIsInPrerenderManager(kHtmlFileC);
   EXPECT_TRUE((url_b_is_active_prerender || url_c_is_active_prerender) &&
               !(url_b_is_active_prerender && url_c_is_active_prerender));
-  EXPECT_FALSE(UrlIsPendingInPrerenderManager(kHtmlFileB));
-  EXPECT_FALSE(UrlIsPendingInPrerenderManager(kHtmlFileC));
+  EXPECT_FALSE(UrlIsPending(kHtmlFileB));
+  EXPECT_FALSE(UrlIsPending(kHtmlFileC));
 }
 
 // See crbug.com/131836.
@@ -1638,7 +1703,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,
                    1);
   NavigateToURL("files/prerender/prerender_fragment.html#fragment");
 }
@@ -1649,7 +1714,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 +1725,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 +1737,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 +1749,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");
 }
@@ -1846,7 +1911,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 +1921,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 +2204,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,
                    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();
 }