Enable swapping a frame back in to its parent process

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 4c2a419d71a88ef50291c68083a0a014bb89fff5..3e6beaf3101c4f49eaada2a7ba879857db45434c 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -298,7 +298,9 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, MAYBE_CrossSiteIframe) {
             ->GetRenderWidgetHostViewsInTree();
     EXPECT_EQ(2U, views_set.size());
   }
-  EXPECT_EQ(proxy_to_parent, child->render_manager()->GetProxyToParent());
+  EXPECT_NE(proxy_to_parent, child->render_manager()->GetProxyToParent());

[nasko, 2014/10/27 22:52:54]

Hmm, why is that? The proxy object for the parent shouldn't change on the second
cross-process navigation, since the parent stays the same.

[Nate Chapin, 2014/10/28 19:59:20]

I would have thought that the child changing processes would cause it to have a
differnet proxy to its parent. Is that not the case?

[Charlie Reis, 2014/10/28 20:10:10]

I'll defer to Nasko (or Ken) here, but let's figure out what the right
expectation is before landing.

[nasko, 2014/10/28 21:58:14]

When a child frame navigates, it changes process, but the proxy for the parent's
process shouldn't change. The reason is that as far as the parent process is
concerned, if a child frame navigates from B to C (where parent is A), nothing
really changes.

I'm now curious, is the proxy changing with your CL? The only case where I can
see it is when parent in A and child in B has the child navigating back to A
(the case you want to enable), but this test doesn't do that.

[Nate Chapin, 2014/10/29 20:39:42]

Ah, I think I understand this now. The problem has to do with how the test
server implements urls like "/cross-site/bar.com/title3.html". Specifically, it
navigates to that url, which triggers a redirect to bar.com/title3.html. With
this patch in place, that means we swap in to the parent process (which
presumably clobbers the proxy_to_parent), then swap back out when the redirect
occurs.

Thoughts on that behavior?

[Charlie Reis, 2014/10/29 21:28:32]

Nasko: Actually, I think Nate found an interesting problem here.  Suppose the
main frame is localhost and the subframe is foo.com.  If the subframe then goes
to bar.com, the URL looks like localhost/cross-site/bar.com.  That means we try
to swap it into it's parent process (yuck) before the redirect sends it to a new
process.  All kinds of things break at that point.

[Nate Chapin, 2014/10/30 19:00:50]

Nasko tells me this should be resolved once
https://codereview.chromium.org/688363002/ lands, so I'll just wait for it
before committing this CL.

+  proxy_to_parent = child->render_manager()->GetProxyToParent();
+  EXPECT_TRUE(proxy_to_parent);
   EXPECT_TRUE(proxy_to_parent->cross_process_frame_connector());
   EXPECT_EQ(
       child->current_frame_host()->render_view_host()->GetView(),
@@ -364,11 +366,11 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, MAYBE_NavigateRemoteFrame) {
 
   // TODO(japhet): This currently causes an assertion in the renderer process.

[nasko, 2014/10/27 22:52:54]

The comment is no longer needed if this is indeed fixed : ).

[Nate Chapin, 2014/10/28 19:59:20]

Will update to describe what this block actually does.

   // Enable when the assertion is fixed.
-  //NavigateFrameToURL(child, http_url);
-  //EXPECT_EQ(http_url, observer.navigation_url());
-  //EXPECT_TRUE(observer.navigation_succeeded());
-  //EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
-  //          child->current_frame_host()->GetSiteInstance());
+  NavigateFrameToURL(child, http_url);
+  EXPECT_EQ(http_url, observer.navigation_url());
+  EXPECT_TRUE(observer.navigation_succeeded());
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            child->current_frame_host()->GetSiteInstance());
 }
 
 // Crash a subframe and ensures its children are cleared from the FrameTree.