Reland "Allow navigations to opener's site to go back into opener SiteInstance."

This is a reland of cd5b76ea.

PrerenderBrowserTest.PrerenderWindowSize still failing after revert.

Original change's description:
> Allow navigations to opener's site to go back into opener SiteInstance.
>
> Previously, when a popup and its opener were cross-site, and neither
> site required a dedicated process, navigating the popup back to the
> opener's site did not swap processes, so the two ended up in different
> processes and couldn't script each other.  Even though
> DetermineSiteInstanceForURL contains logic to place same-site popups
> into their opener's SiteInstance (added as part of the fix for a
> similar issue 796912), this logic was never reached because we never
> attempted a process transfer in this case, thanks to
> IsRendererTransferNeededForNavigation() returning false.
>
> This CL tweaks IsRendererTransferNeededForNavigation so that a
> cross-process transfer is always attempted is such cases.  This allows
> DetermineSiteInstanceForURL to always place the popup that's going back
> to its opener site into the opener's SiteInstance.
>
> Bug: 807184
> Change-Id: Ia4d8d8b2107d3b9e3b9329dd473c06d49b7bd5ec
> Reviewed-on: https://chromium-review.googlesource.com/894369
> Reviewed-by: Charlie Reis <creis@chromium.org>
> Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533410}

TBR=alexmos@chromium.org

Bug: 807184, 807821
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I7808cef9292ff069bcfdd00fe71883d321d7f577
Reviewed-on: https://chromium-review.googlesource.com/897262
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533588}

content/browser/frame_host/render_frame_host_manager.cc

@@ -1316,6 +1316,8 @@ RenderFrameHostManager::DetermineSiteInstanceForURL(
   // isn't a strict invariant but rather a heuristic to avoid unnecessary
   // OOPIFs; see https://crbug.com/711006.  Note that this shouldn't apply to
   // TopDocumentIsolation, so do this after TDI checks above.
+  //
+  // TODO(alexmos): Remove this check after fixing https://crbug.com/787576.
   if (!frame_tree_node_->IsMainFrame()) {
     RenderFrameHostImpl* parent =
         frame_tree_node_->parent()->current_frame_host();
@@ -1382,6 +1384,20 @@ bool RenderFrameHostManager::IsRendererTransferNeededForNavigation(
     return true;
   }
 
+  // If the destination URL is not same-site with current RenderFrameHost and
+  // doesn't require a dedicated process (see above), but it is same-site with
+  // the opener RenderFrameHost, attempt a transfer so that the destination URL
+  // can go back to the opener SiteInstance.  This avoids breaking scripting in
+  // some cases when only a subset of sites is isolated
+  // (https://crbug.com/807184).
+  //
+  // TODO(alexmos): This is a temporary workaround and should be removed after
+  // fixing https://crbug.com/787576.
+  FrameTreeNode* opener = frame_tree_node_->opener();
+  if (opener && IsCurrentlySameSite(opener->current_frame_host(), dest_url) &&
+      opener->current_frame_host()->GetSiteInstance() != rfh->GetSiteInstance())
+    return true;
+
   return false;
 }
 

content/browser/isolated_origin_browsertest.cc

@@ -421,6 +421,77 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest,
   EXPECT_EQ(bar_url2.spec(), popup_location);
 }
 
+// Check that when a non-isolated-origin page opens a popup, navigates it
+// to an isolated origin, and then the popup navigates to a third non-isolated
+// origin and finally back to its opener's origin, the popup and the opener
+// iframe end up in the same process and can script each other:
+//
+//   foo.com
+//      |
+//  window.open()
+//      |
+//      V
+//  about:blank -> isolated.foo.com -> bar.com -> foo.com
+//
+// This is a variant of PopupNavigatesToIsolatedOriginAndBack where the popup
+// navigates to a third site before coming back to the opener's site. See
+// https://crbug.com/807184.
+IN_PROC_BROWSER_TEST_F(IsolatedOriginTest,
+                       PopupNavigatesToIsolatedOriginThenToAnotherSiteAndBack) {
+  // Start on www.foo.com.
+  GURL foo_url(embedded_test_server()->GetURL("www.foo.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), foo_url));
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+
+  // Open a blank popup.
+  ShellAddedObserver new_shell_observer;
+  EXPECT_TRUE(ExecuteScript(root, "window.w = window.open();"));
+  Shell* new_shell = new_shell_observer.GetShell();
+
+  // Have the opener navigate the popup to an isolated origin.
+  GURL isolated_url(
+      embedded_test_server()->GetURL("isolated.foo.com", "/title1.html"));
+  {
+    TestNavigationManager manager(new_shell->web_contents(), isolated_url);
+    EXPECT_TRUE(ExecuteScript(
+        root, "window.w.location.href = '" + isolated_url.spec() + "';"));
+    manager.WaitForNavigationFinished();
+  }
+
+  // Simulate the isolated origin in the popup navigating to bar.com.
+  GURL bar_url(embedded_test_server()->GetURL("bar.com", "/title2.html"));
+  {
+    TestNavigationManager manager(new_shell->web_contents(), bar_url);
+    EXPECT_TRUE(
+        ExecuteScript(new_shell, "location.href = '" + bar_url.spec() + "';"));
+    manager.WaitForNavigationFinished();
+  }
+
+  // At this point, the popup and the opener should still be in separate
+  // SiteInstances.
+  EXPECT_NE(new_shell->web_contents()->GetMainFrame()->GetSiteInstance(),
+            root->current_frame_host()->GetSiteInstance());
+
+  // Simulate the isolated origin in the popup navigating to www.foo.com.
+  {
+    TestNavigationManager manager(new_shell->web_contents(), foo_url);
+    EXPECT_TRUE(
+        ExecuteScript(new_shell, "location.href = '" + foo_url.spec() + "';"));
+    manager.WaitForNavigationFinished();
+  }
+
+  // The popup should now be in the same SiteInstance as its same-site opener.
+  EXPECT_EQ(new_shell->web_contents()->GetMainFrame()->GetSiteInstance(),
+            root->current_frame_host()->GetSiteInstance());
+
+  // Check that the popup can script the opener.
+  std::string opener_location;
+  EXPECT_TRUE(ExecuteScriptAndExtractString(
+      new_shell, "domAutomationController.send(window.opener.location.href);",
+      &opener_location));
+  EXPECT_EQ(foo_url.spec(), opener_location);
+}
+
 // Check that with an ABA hierarchy, where B is an isolated origin, the root
 // and grandchild frames end up in the same process and can script each other.
 // See https://crbug.com/796912.

content/browser/top_document_isolation_browsertest.cc

@@ -596,20 +596,20 @@ IN_PROC_BROWSER_TEST_F(TopDocumentIsolationTest, PopupAndRedirection) {
       DepictFrameTree(root()));
 
   // The popup redirects itself to the advertiser's website (ad.com).
+  RenderFrameDeletedObserver deleted_observer(popup_root->current_frame_host());
   RendererInitiatedNavigateToURL(popup_root, ad_url);
+  deleted_observer.WaitUntilDeleted();
 
   // This must join its same-site opener, in the default subframe SiteInstance.
   EXPECT_EQ(
-      " Site A ------------ proxies for B C\n"
-      "   +--Site B ------- proxies for A C\n"
+      " Site A ------------ proxies for B\n"
+      "   +--Site B ------- proxies for A\n"
       "Where A = http://page.com/\n"
-      "      B = default subframe process\n"
-      "      C = http://adnetwork.com/",
+      "      B = default subframe process",
       DepictFrameTree(root()));
   EXPECT_EQ(
-      " Site C ------------ proxies for B\n"
-      "Where B = default subframe process\n"
-      "      C = http://adnetwork.com/",
+      " Site B\n"
+      "Where B = default subframe process",
       DepictFrameTree(popup_root));
 }