Fix flakiness and enable RenderViewHostManagerTest on Linux/Mac.

content/browser/renderer_host/render_view_host_manager_browsertest.cc

Index: content/browser/renderer_host/render_view_host_manager_browsertest.cc
diff --git a/content/browser/renderer_host/render_view_host_manager_browsertest.cc b/content/browser/renderer_host/render_view_host_manager_browsertest.cc
index 351019a73fb2e4b7ce5359ec3ce285255d9f53eb..5408c747e8994c56f332779ce2a56197667d08a6 100644
--- a/content/browser/renderer_host/render_view_host_manager_browsertest.cc
+++ b/content/browser/renderer_host/render_view_host_manager_browsertest.cc
@@ -45,16 +45,16 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
                        SwapProcessWithRelNoreferrerAndTargetBlank) {
   // Start two servers with different sites.
   ASSERT_TRUE(test_server()->Start());
-  net::TestServer https_server_(
+  net::TestServer https_server(
       net::TestServer::TYPE_HTTPS,
       FilePath(FILE_PATH_LITERAL("chrome/test/data")));
-  ASSERT_TRUE(https_server_.Start());
+  ASSERT_TRUE(https_server.Start());
 
   // Load a page with links that open in a new window.
   std::string replacement_path;
   ASSERT_TRUE(GetFilePathWithHostAndPortReplacement(
       "files/click-noreferrer-links.html",
-      https_server_.host_port_pair(),
+      https_server.host_port_pair(),
       &replacement_path));
   ui_test_utils::NavigateToURL(browser(),
                                test_server()->GetURL(replacement_path));
@@ -72,14 +72,15 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
       &success));
   EXPECT_TRUE(success);
   // Wait for the cross-site transition to finish.
-  ui_test_utils::WaitForLoadStop(
-      &(browser()->GetSelectedTabContents()->controller()));
+  if (browser()->GetSelectedTabContents()->is_loading())
+    ui_test_utils::WaitForLoadStop(

[Paweł Hajdan Jr., 2011/03/24 17:22:57]

Is it possibly a more general problem with WaitForLoadStop? In other words, does
every client of it has to check is_loading to avoid intermittent problems?

I'm just wondering whether we can put the is_loading() check inside
WaitForLoadStop to make it easy to use and hard to misuse.

[Charlie Reis, 2011/03/24 17:33:55]

Yes, that sounds like a good idea-- I'll give it a try.

The try bots are also being kind today and showing all the various forms of
flakiness here.  :)  The linux try bot just failed on the last line of this
test.  It looks like we need to wait for both the current and newly opened tabs
to finish their loads.  I'll make sure it clears the try servers before I send
you the updated patch.

Thanks!

[Charlie Reis, 2011/03/24 23:15:39]

Fixed.

I had to also check whether the tab has a pending RenderViewHost, since it's
possible that we get here even before the DidStartLoading message is sent from
the renderer (in addition to getting here after the DidStopLoading message is
sent).  That's even more of an argument to put this check in WaitForLoadStop, so
that everyone else doesn't have to worry about it.

+        &(browser()->GetSelectedTabContents()->controller()));
 
   // Opens in new tab.
   EXPECT_EQ(2, browser()->tab_count());
   EXPECT_EQ(1, browser()->selected_index());
-  EXPECT_EQ(L"Title Of Awesomeness",
-      browser()->GetSelectedTabContents()->GetTitle());
+  EXPECT_EQ("/files/title2.html",
+            browser()->GetSelectedTabContents()->GetURL().path());
 
   // Should have a new SiteInstance.
   scoped_refptr<SiteInstance> noref_blank_site_instance(
@@ -89,21 +90,20 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
 
 // Test for crbug.com/24447.  Following a cross-site link with just
 // target=_blank should not create a new SiteInstance.
-// Disabled, http://crbug.com/67532.
 IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
-                       DISABLED_DontSwapProcessWithOnlyTargetBlank) {
+                       DontSwapProcessWithOnlyTargetBlank) {
   // Start two servers with different sites.
   ASSERT_TRUE(test_server()->Start());
-  net::TestServer https_server_(
+  net::TestServer https_server(
       net::TestServer::TYPE_HTTPS,
       FilePath(FILE_PATH_LITERAL("chrome/test/data")));
-  ASSERT_TRUE(https_server_.Start());
+  ASSERT_TRUE(https_server.Start());
 
   // Load a page with links that open in a new window.
   std::string replacement_path;
   ASSERT_TRUE(GetFilePathWithHostAndPortReplacement(
       "files/click-noreferrer-links.html",
-      https_server_.host_port_pair(),
+      https_server.host_port_pair(),
       &replacement_path));
   ui_test_utils::NavigateToURL(browser(),
                                test_server()->GetURL(replacement_path));
@@ -121,14 +121,15 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
       &success));
   EXPECT_TRUE(success);
   // Wait for the cross-site transition to finish.
-  ui_test_utils::WaitForLoadStop(
-      &(browser()->GetSelectedTabContents()->controller()));
+  if (browser()->GetSelectedTabContents()->is_loading())
+    ui_test_utils::WaitForLoadStop(
+        &(browser()->GetSelectedTabContents()->controller()));
 
   // Opens in new tab.
   EXPECT_EQ(2, browser()->tab_count());
   EXPECT_EQ(1, browser()->selected_index());
-  EXPECT_EQ(L"Title Of Awesomeness",
-      browser()->GetSelectedTabContents()->GetTitle());
+  EXPECT_EQ("/files/title2.html",
+            browser()->GetSelectedTabContents()->GetURL().path());
 
   // Should have the same SiteInstance.
   scoped_refptr<SiteInstance> blank_site_instance(
@@ -142,16 +143,16 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
                        DontSwapProcessWithOnlyRelNoreferrer) {
   // Start two servers with different sites.
   ASSERT_TRUE(test_server()->Start());
-  net::TestServer https_server_(
+  net::TestServer https_server(
       net::TestServer::TYPE_HTTPS,
       FilePath(FILE_PATH_LITERAL("chrome/test/data")));
-  ASSERT_TRUE(https_server_.Start());
+  ASSERT_TRUE(https_server.Start());
 
   // Load a page with links that open in a new window.
   std::string replacement_path;
   ASSERT_TRUE(GetFilePathWithHostAndPortReplacement(
       "files/click-noreferrer-links.html",
-      https_server_.host_port_pair(),
+      https_server.host_port_pair(),
       &replacement_path));
   ui_test_utils::NavigateToURL(browser(),
                                test_server()->GetURL(replacement_path));
@@ -169,14 +170,15 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
       &success));
   EXPECT_TRUE(success);
   // Wait for the cross-site transition to finish.
-  ui_test_utils::WaitForLoadStop(
-      &(browser()->GetSelectedTabContents()->controller()));
+  if (browser()->GetSelectedTabContents()->is_loading())
+    ui_test_utils::WaitForLoadStop(
+        &(browser()->GetSelectedTabContents()->controller()));
 
   // Opens in same tab.
   EXPECT_EQ(1, browser()->tab_count());
   EXPECT_EQ(0, browser()->selected_index());
-  EXPECT_EQ(L"Title Of Awesomeness",
-      browser()->GetSelectedTabContents()->GetTitle());
+  EXPECT_EQ("/files/title2.html",
+            browser()->GetSelectedTabContents()->GetURL().path());
 
   // Should have the same SiteInstance.
   scoped_refptr<SiteInstance> noref_site_instance(
@@ -235,6 +237,9 @@ class BrowserClosedObserver : public NotificationObserver {
       case NotificationType::BROWSER_CLOSED:
         MessageLoopForUI::current()->Quit();
         break;
+      default:
+        NOTREACHED();
+        break;
     }
   }