In --site-per-process, don't try to call beforeunload on subframes yet.

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 416b0dcdc1ad7e1fac14ed4d6a6cde7f0d3c2246..7b527c63918271f2c3c9fc8cea0416a281f1a74d 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -169,9 +169,11 @@ class SitePerProcessBrowserTest : public ContentBrowserTest {
     ASSERT_TRUE(ExecuteScript(shell()->web_contents(), data_url_script));
   }
 
-  bool NavigateIframeToURL(Shell* window,
-                           const GURL& url,
-                           std::string iframe_id) {
+  // Navigates a frame using JavaScript in the renderer process, causing a
+  // cross-process transfer navigation.
+  bool NavigateFrameToURLInRenderer(Shell* window,
+                                    const GURL& url,
+                                    std::string iframe_id) {
     // TODO(creis): This should wait for LOAD_STOP, but cross-site subframe
     // navigations generate extra DidStartLoading and DidStopLoading messages.
     // Until we replace swappedout:// with frame proxies, we need to listen for
@@ -191,6 +193,25 @@ class SitePerProcessBrowserTest : public ContentBrowserTest {
     return result;
   }
 
+  // Navigates a frame via the browser process, with no transfer.
+  void NavigateFrameToURL(WebContents* web_contents,

[Charlie Reis, 2014/03/21 23:24:33]

This should make iframe tests a bit easier to write, since we don't need to
worry about cross-origin access in the script call.

I'm also hoping to use it to write a nested iframe test, but I haven't figured
out how to get the test to wait for the right events yet.

[nasko, 2014/03/22 04:07:17]

Wouldn't WCO::DidNavigateAnyFrame help us here? We just need one of the *Watcher
type objects.

Also, wouldn't be better to have those methods in
content/test/content_browser_test_utils.*?

+                          FrameTreeNode* node,
+                          const GURL& url) {
+    // TODO(creis): This should wait for LOAD_STOP, but cross-site subframe
+    // navigations generate extra DidStartLoading and DidStopLoading messages.
+    // Until we replace swappedout:// with frame proxies, we need to listen for
+    // something else.  For now, we listen for commit.
+    WindowedNotificationObserver load_observer(
+        NOTIFICATION_NAV_ENTRY_COMMITTED,
+        Source<NavigationController>(
+            &web_contents->GetController()));
+    NavigationController::LoadURLParams params(url);
+    params.transition_type = PageTransitionFromInt(PAGE_TRANSITION_LINK);
+    params.frame_tree_node_id = node->frame_tree_node_id();
+    web_contents->GetController().LoadURLWithParams(params);
+    load_observer.Wait();
+  }
+
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     command_line->AppendSwitch(switches::kSitePerProcess);
 
@@ -212,34 +233,59 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
 
   // Load same-site page into iframe.
   GURL http_url(test_server()->GetURL("files/title1.html"));
-  EXPECT_TRUE(NavigateIframeToURL(shell(), http_url, "test"));
+  EXPECT_TRUE(NavigateFrameToURLInRenderer(shell(), http_url, "test"));
   EXPECT_EQ(http_url, observer.navigation_url());
   EXPECT_TRUE(observer.navigation_succeeded());
 
+  FrameTreeNode* root =
+      static_cast<WebContentsImpl*>(shell()->web_contents())->
+          GetFrameTree()->root();
+  ASSERT_EQ(1U, root->child_count());
+  FrameTreeNode* child = root->child_at(0);
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            child->current_frame_host()->GetSiteInstance());
+
   // These must stay in scope with replace_host.
   GURL::Replacements replace_host;
   std::string foo_com("foo.com");
+  std::string bar_com("bar.com");
 
   // Load cross-site page into iframe.
-  GURL cross_site_url(test_server()->GetURL("files/title2.html"));
+  GURL foo_url(test_server()->GetURL("files/title2.html"));
   replace_host.SetHostStr(foo_com);
-  cross_site_url = cross_site_url.ReplaceComponents(replace_host);
-  EXPECT_TRUE(NavigateIframeToURL(shell(), cross_site_url, "test"));
-  EXPECT_EQ(cross_site_url, observer.navigation_url());
+  foo_url = foo_url.ReplaceComponents(replace_host);
+  EXPECT_TRUE(NavigateFrameToURLInRenderer(shell(), foo_url, "test"));
+  EXPECT_EQ(foo_url, observer.navigation_url());
   EXPECT_TRUE(observer.navigation_succeeded());
 
   // Ensure that we have created a new process for the subframe.
-  FrameTreeNode* root =
-      static_cast<WebContentsImpl*>(shell()->web_contents())->
-          GetFrameTree()->root();
-  ASSERT_EQ(1U, root->child_count());
-  FrameTreeNode* child = root->child_at(0);
-  EXPECT_NE(shell()->web_contents()->GetRenderViewHost(),
-            child->current_frame_host()->render_view_host());
-  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
-            child->current_frame_host()->render_view_host()->GetSiteInstance());
-  EXPECT_NE(shell()->web_contents()->GetRenderProcessHost(),
-            child->current_frame_host()->GetProcess());
+  RenderFrameHost* foo_rfh = child->current_frame_host();
+  EXPECT_NE(shell()->web_contents()->GetMainFrame(), foo_rfh);
+  int foo_site_instance_id = foo_rfh->GetSiteInstance()->GetId();
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance()->GetId(),
+            foo_site_instance_id);
+  int foo_process_id = foo_rfh->GetProcess()->GetID();
+  EXPECT_NE(shell()->web_contents()->GetRenderProcessHost()->GetID(),
+            foo_process_id);
+
+  // Load another cross-site page into the same iframe.

[Charlie Reis, 2014/03/21 23:24:33]

This second navigation tests the beforeunload crash I mentioned.

+  GURL bar_url(test_server()->GetURL("files/title1.html"));
+  replace_host.SetHostStr(bar_com);
+  bar_url = bar_url.ReplaceComponents(replace_host);
+  NavigateFrameToURL(shell()->web_contents(), child, bar_url);
+  EXPECT_EQ(bar_url, observer.navigation_url());
+  EXPECT_TRUE(observer.navigation_succeeded());
+
+  // Ensure the SiteInstance and process swapped.
+  RenderFrameHost* bar_rfh = child->current_frame_host();
+  int bar_site_instance_id = bar_rfh->GetSiteInstance()->GetId();
+  EXPECT_NE(foo_site_instance_id, bar_site_instance_id);
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance()->GetId(),
+            bar_site_instance_id);
+  int bar_process_id = bar_rfh->GetProcess()->GetID();
+  EXPECT_NE(foo_process_id, bar_process_id);
+  EXPECT_NE(shell()->web_contents()->GetRenderProcessHost()->GetID(),
+            bar_process_id);
 }
 
 // Crash a subframe and ensures its children are cleared from the FrameTree.
@@ -266,7 +312,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, MAYBE_CrashSubframe) {
   GURL cross_site_url(test_server()->GetURL("files/title2.html"));
   replace_host.SetHostStr(foo_com);
   cross_site_url = cross_site_url.ReplaceComponents(replace_host);
-  EXPECT_TRUE(NavigateIframeToURL(shell(), cross_site_url, "test"));
+  EXPECT_TRUE(NavigateFrameToURLInRenderer(shell(), cross_site_url, "test"));
 
   // Check the subframe process.
   FrameTreeNode* root =
@@ -330,8 +376,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
     // Should be blocked.
     GURL client_redirect_https_url(https_server.GetURL(
         "client-redirect?files/title1.html"));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    client_redirect_https_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), client_redirect_https_url, "test"));
     // DidFailProvisionalLoad when navigating to client_redirect_https_url.
     EXPECT_EQ(observer.navigation_url(), client_redirect_https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
@@ -342,8 +388,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
     // which redirects to same-site page.
     GURL server_redirect_http_url(https_server.GetURL(
         "server-redirect?" + http_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
   }
@@ -353,8 +399,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
     // which redirects to cross-site page.
     GURL server_redirect_http_url(https_server.GetURL(
         "server-redirect?files/title1.html"));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
     // DidFailProvisionalLoad when navigating to https_url.
     EXPECT_EQ(observer.navigation_url(), https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
@@ -365,8 +411,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
     // which redirects to cross-site page.
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + https_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
 
     EXPECT_EQ(observer.navigation_url(), https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
@@ -383,8 +429,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
         Source<NavigationController>(
             &shell()->web_contents()->GetController()));
 
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    client_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), client_redirect_http_url, "test"));
 
     // Same-site Client-Redirect Page should be loaded successfully.
     EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
@@ -401,8 +447,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
     // which redirects to same-site page.
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?files/title1.html"));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
    }
@@ -417,8 +463,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
         Source<NavigationController>(
             &shell()->web_contents()->GetController()));
 
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    client_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), client_redirect_http_url, "test"));
 
     // Same-site Client-Redirect Page should be loaded successfully.
     EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
@@ -465,7 +511,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
         Source<NavigationController>(
             &shell()->web_contents()->GetController()));
 
-    EXPECT_TRUE(NavigateIframeToURL(shell(), client_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), client_redirect_http_url, "test"));
 
     // DidFailProvisionalLoad when navigating to client_redirect_https_url.
     load_observer2.Wait();
@@ -480,8 +527,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
         "server-redirect?" + http_url.spec()));
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + server_redirect_https_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
   }
@@ -493,7 +540,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
         "server-redirect?" + https_url.spec()));
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + server_redirect_https_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(), server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
 
     // DidFailProvisionalLoad when navigating to https_url.
     EXPECT_EQ(observer.navigation_url(), https_url);
@@ -507,7 +555,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
         "client-redirect?" + http_url.spec()));
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + client_redirect_http_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(), server_redirect_http_url, "test"));
+    EXPECT_TRUE(NavigateFrameToURLInRenderer(
+        shell(), server_redirect_http_url, "test"));
 
     // DidFailProvisionalLoad when navigating to client_redirect_http_url.
     EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);