Remove hosted app reload logic from ShouldFork.

This CL removes the logic that triggered process swaps in
ChromeExtensionsRendererClient::ShouldFork() for the case when the
main frame reloads a URL for which a hosted app was just installed or
uninstalled.  This logic isn't needed with --site-per-process, as that
mode will already force a process transfer.  This is covered by
AppApiTest.ReloadIntoAppProcess and
AppApiTest.ReloadIntoAppProcessWithJavaScript, which still pass with
that logic removed.

This CL also makes a fix on the browser side to keep this logic
working even without --site-per-process, by unconditionally checking
HasWrongProcessForURL() and forcing a process transfer if needed in
IsRendererTransferNeededForNavigation().  This is possible because
with PlzNavigate we will always go to the browser process and check
for transfers, even without --site-per-process.  With that fix, the
above two tests also pass without --site-per-process.

The above tweak also has a side effect of swapping processes when an
app opens a cross-site, non-app popup (also made possible by work on
issue 794315) in non-site-per-process mode, which seems acceptable and
desirable - no reason for the cross-site popup to be in a process with
app permissions.

Bug: 883550, 883549, 718516
Test: AppApiTest.ReloadIntoAppProcess and AppApiTest.ReloadIntoAppProcessWithJavaScript keep working, with and without --site-per-process.
Change-Id: Ibac63fff3a36318fabceb593ca7ae7967eefad89
Reviewed-on: https://chromium-review.googlesource.com/1226075Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591521}

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -1803,12 +1803,11 @@ IN_PROC_BROWSER_TEST_P(HostedAppProcessModelTest, PopupsInsideHostedApp) {
     SCOPED_TRACE("... for isolated_url popup");
     TestPopupProcess(app, isolated_url_, false, false);
   }
-  // For cross-site, the resulting process is only in the app process if we
-  // don't swap processes.
+  // For cross-site, the resulting popup should swap processes and not be in
+  // the app process.
   {
     SCOPED_TRACE("... for cross_site popup");
-    TestPopupProcess(app, cross_site_url_, !should_swap_for_cross_site_,
-                     !should_swap_for_cross_site_);
+    TestPopupProcess(app, cross_site_url_, false, false);
   }
 
   // If the iframes open popups that are same-origin with themselves, the popups
@@ -2365,12 +2364,9 @@ IN_PROC_BROWSER_TEST_P(HostedAppProcessModelTest,
       foo_contents->GetMainFrame()->GetSiteInstance()->IsRelatedSiteInstance(
           foo_contents2->GetMainFrame()->GetSiteInstance()));
 
-  // The bar.com tab should be in the same process as the foo.com tabs only if
-  // we are not in --site-per-process mode.  --site-per-process should override
-  // the process-per-site behavior.
+  // The bar.com tab should be in a different process from the foo.com tabs.
   auto* bar_process = bar_contents->GetMainFrame()->GetProcess();
-  EXPECT_EQ(foo_process == bar_process,
-            !content::AreAllSitesIsolatedForTesting());
+  EXPECT_NE(foo_process, bar_process);
 
   // Ensure all tabs are in app processes.
   auto* process_map = extensions::ProcessMap::Get(browser()->profile());

chrome/renderer/extensions/chrome_extensions_renderer_client.cc

@@ -300,17 +300,6 @@ bool ChromeExtensionsRendererClient::ShouldFork(blink::WebLocalFrame* frame,
     return true;
   }
 
-  // If this is a reload, check whether it has the wrong process type.  We
-  // should send it to the browser if it's an extension URL (e.g., hosted app)
-  // in a normal process, or if it's a process for an extension that has been
-  // uninstalled.  Without --site-per-process mode, we never fork processes for
-  // subframes, so this check only makes sense for top-level frames.
-  // TODO(alexmos,nasko): Figure out how this check should work when reloading
-  // subframes in --site-per-process mode.
-  if (!frame->Parent() && GURL(frame->GetDocument().Url()) == url) {
-    if (is_extension_url != IsStandaloneExtensionProcess())
-      return true;
-  }
   return false;
 }
 

content/browser/frame_host/render_frame_host_manager.cc

@@ -1436,21 +1436,23 @@ bool RenderFrameHostManager::IsBrowsingInstanceSwapAllowedForPageTransition(
 bool RenderFrameHostManager::IsRendererTransferNeededForNavigation(
     RenderFrameHostImpl* rfh,
     const GURL& dest_url) {
-  // A transfer is not needed if the current SiteInstance doesn't yet have a
-  // site.  This is the case for tests that use NavigateToURL.
-  //
-  // One exception is that a siteless SiteInstance may still have a process,
-  // which might be unsuitable for |dest_url|.  For example, another navigation
-  // could share that process (e.g., when over process limit) and lock it to a
-  // different origin before this SiteInstance sets its site.  Hence, we also
-  // check for cases like this.  See https://crbug.com/773809.
+  // Always attempt a process transfer if the SiteInstance has a process that's
+  // unsuitable for |dest_url|.  For example, this might happen when reloading
+  // a URL for which a hosted app was just installed or uninstalled.
   //
-  // TODO(alexmos): We should always check HasWrongProcessForURL regardless of
-  // HasSite, but currently we cannot do that because of hosted app workarounds
-  // (see https://crbug.com/92669).  Revisit this once hosted apps swap
-  // processes for cross-site web iframes and popups.
+  // This might also happen for a siteless SiteInstance which may have a
+  // process that's unsuitable for |dest_url|.  For example, another navigation
+  // could share that process when over process limit and lock it to a
+  // different site before this SiteInstance sets its site.  See
+  // https://crbug.com/773809.
+  if (rfh->GetSiteInstance()->HasWrongProcessForURL(dest_url))
+    return true;
+
+  // A transfer is not needed if the current SiteInstance doesn't yet have a
+  // site.  For example, this happens when tests use NavigateToURL or when
+  // navigating a blank window in some cases.
   if (!rfh->GetSiteInstance()->HasSite())
-    return rfh->GetSiteInstance()->HasWrongProcessForURL(dest_url);
+    return false;
 
   // We do not currently swap processes for navigations in webview tag guests.
   if (rfh->GetSiteInstance()->GetSiteURL().SchemeIs(kGuestScheme))