Fix flakyness in NavigateRemoteFrame browsertest.

content/browser/site_per_process_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 "content/browser/site_per_process_browsertest.h"
 
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
@@ skipping 265 matching lines @@
   EXPECT_EQ(proxy_to_parent, child->render_manager()->GetProxyToParent());
   EXPECT_TRUE(proxy_to_parent->cross_process_frame_connector());
   EXPECT_NE(
       child->current_frame_host()->render_view_host()->GetView(),
       proxy_to_parent->cross_process_frame_connector()->get_view_for_testing());
   EXPECT_TRUE(child->current_frame_host()->GetRenderWidgetHost());
 }
 
 // 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);
 
   // 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());
 
@@ skipping 31 matching lines @@
   EXPECT_NE(site_instance,
             child->current_frame_host()->GetSiteInstance());
 
   // Navigate back to the parent's origin and ensure we return to the
   // parent's process.
   NavigateFrameToURL(child, http_url);
   EXPECT_EQ(http_url, observer.last_navigation_url());
   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
 // of C from the tree.
 //
 //     1          A                  A
 //    / \        / \                / \    .
 //   2   3 ->   B   A -> Kill B -> B*  A
 //  /          /
 // 4          C
@@ skipping 692 matching lines @@
 
   // Check that the window.name seen by the frame matches the name attribute
   // specified by its parent in the iframe tag.
   std::string result;
   EXPECT_TRUE(ExecuteScriptAndExtractString(
       root->child_at(0)->current_frame_host(),
       "window.domAutomationController.send(window.name);", &result));
   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) {
   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();
@@ skipping 15 matching lines @@
   params.frame_tree_node_id = child->frame_tree_node_id();
   child->navigator()->GetController()->LoadURLWithParams(params);
   nav_observer.Wait();
 
   // Verify that the navigation succeeded and the expected URL was loaded.
   EXPECT_TRUE(observer.last_navigation_succeeded());
   EXPECT_EQ(url, observer.last_navigation_url());
 }
 
 }  // namespace content