Fix flakyness in NavigateRemoteFrame browsertest.

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index 61b8111386321b9fe06b10687d49fa32797a9ff4..e69d924954ceeae2ab247ce3d3f6731c9e61ec96 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -283,7 +283,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
 
 // Disabled for flaky crashing: crbug.com/446575
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
-                       DISABLED_NavigateRemoteFrame) {
+                       NavigateRemoteFrame) {
   GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
   NavigateToURL(shell(), main_url);
 
@@ -335,6 +335,48 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   EXPECT_TRUE(observer.last_navigation_succeeded());
   EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
             child->current_frame_host()->GetSiteInstance());
+
+  // Load cross-site page into iframe.

[nasko, 2015/02/11 14:44:06]

Why do we need this here? These tests belongs logically in a separate test
cases, as you had them before. In addition, splitting it decreases the
probability the test case will be flaky, as it does less things, therefore on
slow bots it will complete faster (less likely to timeout).

[lfg, 2015/02/11 18:23:52]

The reason I did it is because they basically test similar things (navigating a
remote frame) and having a single test is overall faster than having 3 separate
tests.

I can split it again if you think this is the best approach.

+  url = embedded_test_server()->GetURL("baz.com", "/title2.html");
+  NavigateFrameToURL(root->child_at(0), url);
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+  EXPECT_EQ(url, observer.last_navigation_url());
+  ASSERT_EQ(2U, root->child_count());
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+
+  // Navigate iframe to a data URL. The navigation happens from a script in the
+  // parent frame, so the data URL should be committed in the same SiteInstance
+  // as the parent frame.
+  GURL data_url("data:text/html,dataurl");
+  NavigateIframeToURL(shell()->web_contents(), "test", data_url);
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+  EXPECT_EQ(data_url, observer.last_navigation_url());
+
+  // Ensure that we have navigated using the top level process.
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+
+  // Load cross-site page into iframe.
+  url = embedded_test_server()->GetURL("qux.com", "/title2.html");
+  NavigateFrameToURL(root->child_at(0), url);
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+  EXPECT_EQ(url, observer.last_navigation_url());
+  ASSERT_EQ(2U, root->child_count());
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+
+  // Navigate iframe to about:blank. The navigation happens from a script in the
+  // parent frame, so it should be committed in the same SiteInstance as the
+  // parent frame.
+  GURL about_blank_url("about:blank");
+  NavigateIframeToURL(shell()->web_contents(), "test", about_blank_url);
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+  EXPECT_EQ(about_blank_url, observer.last_navigation_url());
+
+  // Ensure that we have navigated using the top level process.
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
 }
 
 // In A-embed-B-embed-C scenario, verify that killing process B clears proxies
@@ -1047,105 +1089,6 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, WindowNameReplication) {
   EXPECT_EQ(result, "3-1-name");
 }
 
-// TODO(lfg): Merge the test below with NavigateRemoteFrame test.
-// TODO(lfg): Disabled because this triggers http://crbug.com/433012, and since
-// the renderer process crashes, it causes the title watcher to never return.
-// Alternatively, this could also be fixed if we could use NavigateIframeToURL
-// and classified the navigation as MANUAL_SUBFRAME (http://crbug.com/441863) or
-// if we waited for DidStopLoading (currently broken -- see comment in
-// NavigateIframeToURL).
-IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
-                       DISABLED_NavigateRemoteToDataURL) {
-  GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
-  NavigateToURL(shell(), main_url);
-
-  // It is safe to obtain the root frame tree node here, as it doesn't change.
-  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
-                            ->GetFrameTree()
-                            ->root();
-
-  TestNavigationObserver observer(shell()->web_contents());
-
-  // Load cross-site page into iframe.
-  GURL url = embedded_test_server()->GetURL("foo.com", "/title1.html");
-  NavigateFrameToURL(root->child_at(0), url);
-  EXPECT_TRUE(observer.last_navigation_succeeded());
-  EXPECT_EQ(url, observer.last_navigation_url());
-
-  // Ensure that we have created a new process for the subframe.
-  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
-            root->child_at(0)->current_frame_host()->GetSiteInstance());
-
-  // Navigate iframe to a data URL. The navigation happens from a script in the
-  // parent frame, so the data URL should be committed in the same SiteInstance
-  // as the parent frame.
-  GURL data_url("data:text/html,dataurl");
-  std::string script = base::StringPrintf(
-      "setTimeout(function() {"
-      "var iframe = document.getElementById('test');"
-      "iframe.onload = function() { document.title = 'LOADED'; };"
-      "iframe.src=\"%s\";"
-      "},0);",
-      data_url.spec().c_str());
-  base::string16 passed_string(base::UTF8ToUTF16("LOADED"));
-  TitleWatcher title_watcher(shell()->web_contents(), passed_string);
-  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
-  EXPECT_EQ(title_watcher.WaitAndGetTitle(), passed_string);
-  EXPECT_TRUE(observer.last_navigation_succeeded());
-  EXPECT_EQ(data_url, observer.last_navigation_url());
-
-  // Ensure that we have navigated using the top level process.
-  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
-            root->child_at(0)->current_frame_host()->GetSiteInstance());
-}
-
-// TODO(lfg): Merge the test below with NavigateRemoteFrame test.
-// Disabled due to the same reason as NavigateRemoteToDataURL.
-IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
-                       DISABLED_NavigateRemoteToBlankURL) {
-  GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
-  NavigateToURL(shell(), main_url);
-
-  // It is safe to obtain the root frame tree node here, as it doesn't change.
-  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
-                            ->GetFrameTree()
-                            ->root();
-
-  TestNavigationObserver observer(shell()->web_contents());
-
-  // Load cross-site page into iframe.
-  GURL url = embedded_test_server()->GetURL("foo.com", "/title1.html");
-  NavigateFrameToURL(root->child_at(0), url);
-  EXPECT_TRUE(observer.last_navigation_succeeded());
-  EXPECT_EQ(url, observer.last_navigation_url());
-
-  // Ensure that we have created a new process for the subframe.
-  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
-            root->child_at(0)->current_frame_host()->GetSiteInstance());
-
-  // Navigate iframe to about:blank. The navigation happens from a script in the
-  // parent frame, so it should be committed in the same SiteInstance as the
-  // parent frame.
-  GURL about_blank_url("about:blank");
-  std::string script = base::StringPrintf(
-      "setTimeout(function() {"
-      "var iframe = document.getElementById('test');"
-      "iframe.onload = function() { document.title = 'LOADED'; };"
-      "iframe.src=\"%s\";"
-      "},0);",
-      about_blank_url.spec().c_str());
-  base::string16 passed_string(base::UTF8ToUTF16("LOADED"));
-  TitleWatcher title_watcher(shell()->web_contents(), passed_string);
-  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
-  EXPECT_EQ(title_watcher.WaitAndGetTitle(), passed_string);
-  EXPECT_TRUE(observer.last_navigation_succeeded());
-  EXPECT_EQ(about_blank_url, observer.last_navigation_url());
-
-  // Ensure that we have navigated using the top level process.
-  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
-            root->child_at(0)->current_frame_host()->GetSiteInstance());
-}
-
 // Ensure that navigating subframes in --site-per-process mode properly fires
 // the DidStopLoading event on WebContentsObserver.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteDidStopLoading) {