prerender: Add NavigationOrSwapObserver in browsertest.

chrome/browser/prerender/prerender_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <deque>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 25 matching lines @@
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_message_filter.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_client_host.h"
 #include "content/public/browser/devtools_manager.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/test/net/url_request_mock_http_job.h"
 #include "extensions/common/switches.h"
 #include "grit/generated_resources.h"
 #include "net/base/escape.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_job.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 using content::DevToolsAgentHost;
 using content::DevToolsClientHost;
 using content::DevToolsManager;
 using content::NavigationController;
 using content::OpenURLParams;
 using content::Referrer;
 using content::RenderViewHost;
 using content::RenderWidgetHost;
 using content::WebContents;
+using content::WebContentsObserver;
 
 // Prerender tests work as follows:
 //
 // A page with a prefetch link to the test page is loaded.  Once prerendered,
 // its Javascript function DidPrerenderPass() is called, which returns true if
 // the page behaves as expected when prerendered.
 //
 // The prerendered page is then displayed on a tab.  The Javascript function
 // DidDisplayPass() is called, and returns true if the page behaved as it
 // should while being displayed.
@@ skipping 102 matching lines @@
     channel_destroyed_ = true;
     run_loop_.Quit();
   }
 
   bool channel_destroyed_;
   base::RunLoop run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(ChannelDestructionWatcher);
 };
 
+// A simple observer to wait on either a load or a swap of a WebContents.
+class NavigationOrSwapObserver : public WebContentsObserver,
+                                 public TabStripModelObserver {
+ public:
+  // Waits for either a load or a swap of |tab_strip_model|'s active
+  // WebContents.
+  explicit NavigationOrSwapObserver(TabStripModel* tab_strip_model)
+      : WebContentsObserver(tab_strip_model->GetActiveWebContents()),
+        tab_strip_model_(tab_strip_model) {
+    tab_strip_model_->AddObserver(this);
+  }
+
+  ~NavigationOrSwapObserver() {
+    tab_strip_model_->RemoveObserver(this);
+  }
+
+  void Wait() {
+    loop_.Run();
+  }
+
+  // WebContentsObserver implementation:
+  virtual void DidStopLoading(RenderViewHost* render_view_host) OVERRIDE {
+    loop_.Quit();
+  }
+
+  // TabStripModelObserver implementation:
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             WebContents* old_contents,
+                             WebContents* new_contents,
+                             int index) OVERRIDE {
+    if (old_contents != web_contents())

[mmenke, 2014/01/07 15:09:17]

Does this ever happen?  If not, could use an EXPECT_EQ

[davidben, 2014/01/07 17:31:34]

Hrm. Doesn't currently happen, but I was thinking this would just apply to a
particular WebContents. Perhaps we want to test two simultaneous prerenders or
something. I've tweaked it to pass both TabStripModel and WebContents as
parameter and added a comment to make it clear it only works for CURRENT_TAB.

I'm not sure why we even have NavigateToURLWithDisposition; nothing ever passes
a different disposition and we don't even prerender in that case. Anyway, I've
added a TODO for now. We can make it use OpenURL's return value when all swaps
become async. Though, even that's not enough for NEW_WINDOW. We'd either need to
static_cast the delegate to a Browser* or call chrome::Navigate instead with a
NavigateParams and look at params.browser. I think chrome::Navigate will fill
that in.

[mmenke, 2014/01/07 17:44:50]

NavigateToDestURLInNewTab()?  There used to be a test that did a background tab
that way, too.

[davidben, 2014/01/07 18:57:14]

Well, NavigateToDestURLInNewTab creates a new tab and then uses CURRENT_TAB. The
background tab one is gone now I guess. git log -S comes up with
http://codereview.chromium.org/8392041.

Right now MaybeUsePrerenderedPage doesn't run on non-CURRENT_TAB navigations at
all.

+      return;
+    loop_.Quit();
+  }
+
+ private:
+  TabStripModel* tab_strip_model_;
+  base::RunLoop loop_;
+};
+
 // PrerenderContents that stops the UI message loop on DidStopLoading().
 class TestPrerenderContents : public PrerenderContents {
  public:
   TestPrerenderContents(
       PrerenderManager* prerender_manager,
       Profile* profile,
       const GURL& url,
       const content::Referrer& referrer,
       Origin origin,
       int expected_number_of_loads,
@@ skipping 376 matching lines @@
  public:
   RestorePrerenderMode() : prev_mode_(PrerenderManager::GetMode()) {
   }
 
   ~RestorePrerenderMode() { PrerenderManager::SetMode(prev_mode_); }
  private:
   PrerenderManager::PrerenderManagerMode prev_mode_;
 };
 
 // URLRequestJob (and associated handler) which never starts.
 class NeverStartURLRequestJob : public net::URLRequestJob {

[mmenke, 2014/01/07 15:09:17]

While you're here, could you rename this?  Maybe NeverCompleteURLRequestJob or
just HangingURLRequestJob?  "NeverStart()" sounds to me like Start() should
never be called.

[davidben, 2014/01/07 17:31:34]

Done.

  public:
   NeverStartURLRequestJob(net::URLRequest* request,
                           net::NetworkDelegate* network_delegate)
       : net::URLRequestJob(request, network_delegate) {
   }
 
   virtual void Start() OVERRIDE {}
 
  private:
   virtual ~NeverStartURLRequestJob() {}
 };
 
 class NeverStartProtocolHandler
     : public net::URLRequestJobFactory::ProtocolHandler {
  public:
-  NeverStartProtocolHandler() {}
+  explicit NeverStartProtocolHandler(const base::FilePath& file)
+      : file_(file),
+        first_run_(true) {
+  }
   virtual ~NeverStartProtocolHandler() {}
 
   virtual net::URLRequestJob* MaybeCreateJob(
       net::URLRequest* request,
       net::NetworkDelegate* network_delegate) const OVERRIDE {
-    return new NeverStartURLRequestJob(request, network_delegate);
+    if (first_run_) {

[mmenke, 2014/01/07 15:09:17]

While this isn't the most complicated class, this first run behavior isn't clear
from the class name.  Suggest either a short comment, or making a name that
encompasses this behavior (NeverStartFirstRequestProtocolHandler?)

[davidben, 2014/01/07 17:31:34]

Done.

+      first_run_ = false;
+      return new NeverStartURLRequestJob(request, network_delegate);
+    }
+    return new content::URLRequestMockHTTPJob(request, network_delegate, file_);
   }
+ private:

[mmenke, 2014/01/07 15:09:17]

nit:  Blank line before private.

[davidben, 2014/01/07 17:31:34]

Done.

+  base::FilePath file_;
+  mutable bool first_run_;
 };
 
-void CreateNeverStartProtocolHandlerOnIO(const GURL& url) {
+// Makes |url| never respond on the first load, and then with the contents of
+// |file| afterwards.
+void CreateNeverStartProtocolHandlerOnIO(const GURL& url,
+                                         const base::FilePath& file) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   scoped_ptr<net::URLRequestJobFactory::ProtocolHandler> never_respond_handler(
-      new NeverStartProtocolHandler());
+      new NeverStartProtocolHandler(file));
   net::URLRequestFilter::GetInstance()->AddUrlProtocolHandler(
       url, never_respond_handler.Pass());
 }
 
 // Makes |url| respond to requests with the contents of |file|.
 void CreateMockProtocolHandlerOnIO(const GURL& url,
                                    const base::FilePath& file) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   net::URLRequestFilter::GetInstance()->AddUrlProtocolHandler(
       url, content::URLRequestMockHTTPJob::CreateProtocolHandlerForSingleFile(
@@ skipping 183 matching lines @@
   void NavigateToDestURLInNewTab() const {
     // First, open a new tab.
     ui_test_utils::NavigateToURLWithDisposition(
         current_browser(), GURL(content::kAboutBlankURL),
         NEW_FOREGROUND_TAB,
         ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
     // Next, navigate to the destination URL. The swap-in will not succeed,
     // due to session storage namespace mismatch. The merge is only kicked off
     // asynchronously.
     NavigateToDestURLWithDisposition(CURRENT_TAB, false);
-    // Run the message loop, waiting for the merge to complete, the swapin to
-    // be reattempted, and to eventually succeed.
-    content::RunMessageLoop();
   }
 
   // Opens the url in a new tab, with no opener.
   void NavigateToDestURLWithDisposition(
       WindowOpenDisposition disposition,
       bool expect_swap_to_succeed) const {
     NavigateToURLWithParams(
         content::OpenURLParams(dest_url_, Referrer(), disposition,
                                content::PAGE_TRANSITION_TYPED, false),
         expect_swap_to_succeed);
@@ skipping 291 matching lines @@
   }
 
   void IncreasePrerenderMemory() {
     // Increase the memory allowed in a prerendered page above normal settings.
     // Debug build bots occasionally run against the default limit, and tests
     // were failing because the prerender was canceled due to memory exhaustion.
     // http://crbug.com/93076
     GetPrerenderManager()->mutable_config().max_bytes = 1000 * 1024 * 1024;
   }
 
+  bool DidPrerenderPass(WebContents* web_contents) const {
+    bool prerender_test_result = false;
+    CHECK(content::ExecuteScriptAndExtractBool(
+        web_contents,
+        "window.domAutomationController.send(DidPrerenderPass())",
+        &prerender_test_result));
+    return prerender_test_result;
+  }
+
+  bool DidDisplayPass(WebContents* web_contents) const {
+    bool display_test_result = false;
+    CHECK(content::ExecuteScriptAndExtractBool(
+        web_contents,
+        "window.domAutomationController.send(DidDisplayPass())",
+        &display_test_result));

[mmenke, 2014/01/07 15:09:17]

Do we need these CHECKs?  Seems like we can just return false.

[davidben, 2014/01/07 17:31:34]

Done. I think it's worthwhile to be able to distinguish "DidDisplayPass()
returned false" from "something went horribly wrong and we couldn't run JS at
all", but there are DLOG(ERROR)s, so we're probably good?

[mmenke, 2014/01/07 17:44:50]

You could return a testing::AssertionResult, with different text in the two
cases, if you really wanted.  Or could just stick with the CHECK, if you're that
concerned about it.

+    return display_test_result;
+  }
+
  protected:
   bool autostart_test_server_;
 
  private:
   void PrerenderTestURLImpl(
       const GURL& prerender_url,
       const GURL& destination_url,
       const std::deque<FinalStatus>& expected_final_status_queue,
       int expected_number_of_loads,
       bool prerender_should_wait_for_ready_title) {
@@ skipping 66 matching lines @@
     if (ShouldRenderPrerenderedPageCorrectly(expected_final_status)) {
       ASSERT_NE(static_cast<PrerenderContents*>(NULL), prerender_contents);
       EXPECT_EQ(FINAL_STATUS_MAX, prerender_contents->final_status());
 
       if (call_javascript_) {
         // Wait for the prerendered page to change title to signal it is ready
         // if required.
         prerender_contents->WaitForPrerenderToHaveReadyTitleIfRequired();
 
         // Check if page behaves as expected while in prerendered state.
-        bool prerender_test_result = false;
-        ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-            prerender_contents->GetRenderViewHostMutable(),
-            "window.domAutomationController.send(DidPrerenderPass())",
-            &prerender_test_result));
-        EXPECT_TRUE(prerender_test_result);
+        EXPECT_TRUE(DidPrerenderPass(prerender_contents->prerender_contents()));
       }
 
       // Test that the referring page received events.
       EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(0));
       if (check_load_events_) {
         EXPECT_EQ(expected_number_of_loads,
                   GetPrerenderLoadEventCountForLinkNumber(0));
       }
       EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(0));
     } else {
@@ skipping 40 matching lines @@
         page_load_observer.reset(
             new content::WindowedNotificationObserver(
                 content::NOTIFICATION_LOAD_STOP,
                 content::Source<NavigationController>(
                     &web_contents->GetController())));
       }
     }
 
     // Navigate to the prerendered URL, but don't run the message loop. Browser
     // issued navigations to prerendered pages will synchronously swap in the
     // prerendered page.

[mmenke, 2014/01/07 15:09:17]

This comment is no longer correct.

[davidben, 2014/01/07 17:31:34]

Done.

+    NavigationOrSwapObserver swap_observer(
+        current_browser()->tab_strip_model());
     WebContents* target_web_contents = current_browser()->OpenURL(params);
+    swap_observer.Wait();
 
     if (web_contents && expect_swap_to_succeed) {
       EXPECT_EQ(web_contents, target_web_contents);
       if (call_javascript_) {
         if (page_load_observer.get())
           page_load_observer->Wait();
 
-        bool display_test_result = false;
-        ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-            web_contents,
-            "window.domAutomationController.send(DidDisplayPass())",
-            &display_test_result));
-        EXPECT_TRUE(display_test_result);
+        EXPECT_TRUE(DidDisplayPass(web_contents));
       }
     }
   }
 
   // Opens the prerendered page using javascript functions in the
   // loader page. |javascript_function_name| should be a 0 argument function
   // which is invoked.
   void OpenDestURLWithJSImpl(const std::string& javascript_function_name)
       const {
     TestPrerenderContents* prerender_contents = GetPrerenderContents();
@@ skipping 213 matching lines @@
                    FINAL_STATUS_USED,
                    1);
   NavigateToDestURL();
 }
 
 // Checks that the prerendering of a page is canceled correctly if we try to
 // swap it in before it commits.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap) {
   // Navigate to a page that triggers a prerender for a URL that never commits.
   const GURL kNoCommitUrl("http://never-respond.example.com");
+  base::FilePath file(FILE_PATH_LITERAL(
+      "chrome/test/data/prerender/prerender_page.html"));
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&CreateNeverStartProtocolHandlerOnIO, kNoCommitUrl));
+      base::Bind(&CreateNeverStartProtocolHandlerOnIO, kNoCommitUrl, file));
   DisableJavascriptCalls();
   PrerenderTestURL(kNoCommitUrl,
                    FINAL_STATUS_NAVIGATION_UNCOMMITTED,
                    0);
 
   // Navigate to the URL, but assume the contents won't be swapped in.
   NavigateToDestURLWithDisposition(CURRENT_TAB, false);
 }
 
 // Checks that client redirects don't add alias URLs until after they commit.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap2) {
   // Navigate to a page that then navigates to a URL that never commits.
   const GURL kNoCommitUrl("http://never-respond.example.com");
+  base::FilePath file(FILE_PATH_LITERAL(
+      "chrome/test/data/prerender/prerender_page.html"));
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&CreateNeverStartProtocolHandlerOnIO, kNoCommitUrl));
+      base::Bind(&CreateNeverStartProtocolHandlerOnIO, kNoCommitUrl, file));
   DisableJavascriptCalls();
   PrerenderTestURL(CreateClientRedirect(kNoCommitUrl.spec()),
                    FINAL_STATUS_APP_TERMINATING, 1);
 
   // Navigating to the second URL should not swap.
   NavigateToURLWithDisposition(kNoCommitUrl, CURRENT_TAB, false);
 }
 
 // Checks that the prerendering of a page is canceled correctly when a
 // Javascript alert is called.
@@ skipping 61 matching lines @@
 
   // Run this check again.  When we try to load aa ppapi plugin, the
   // "loadstart" event is asynchronously posted to a message loop.
   // It's possible that earlier call could have been run before the
   // the "loadstart" event was posted.
   // TODO(mmenke):  While this should reliably fail on regressions, the
   //                reliability depends on the specifics of ppapi plugin
   //                loading.  It would be great if we could avoid that.
   WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
-  bool display_test_result = false;
-  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-      web_contents,
-      "window.domAutomationController.send(DidDisplayPass())",
-      &display_test_result));
-  EXPECT_TRUE(display_test_result);
+  EXPECT_TRUE(DidDisplayPass(web_contents));
 }
 
 // Checks that plugins in an iframe are not loaded while a page is
 // being preloaded, but are loaded when the page is displayed.
 #if defined(USE_AURA) && !defined(OS_WIN)
 // http://crbug.com/103496
 #define MAYBE_PrerenderIframeDelayLoadPlugin \
         DISABLED_PrerenderIframeDelayLoadPlugin
 #elif defined(OS_MACOSX)
 // http://crbug.com/100514
@@ skipping 1689 matching lines @@
   const NavigationController& controller = web_contents->GetController();
   // First entry is about:blank, second is prerender_page.html.
   EXPECT_TRUE(controller.GetPendingEntry() == NULL);
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(GURL(content::kAboutBlankURL),
             controller.GetEntryAtIndex(0)->GetURL());
   EXPECT_EQ(dest_url(), controller.GetEntryAtIndex(1)->GetURL());
 }
 
 }  // namespace prerender