PlzNavigate: don't reuse current_frame_host() for error pages

if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
TEST=ToolbarModelTest.ShouldDisplayURL
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2884123002
Cr-Commit-Position: refs/heads/master@{#472303}

content/browser/frame_host/navigation_handle_impl_browsertest.cc

@@ -1174,18 +1174,72 @@ IN_PROC_BROWSER_TEST_F(PlzNavigateNavigationHandleImplBrowserTest,
   scoped_refptr<SiteInstance> site_instance =
       shell()->web_contents()->GetMainFrame()->GetSiteInstance();
 
-  TestNavigationThrottleInstaller installer(
+  auto installer = base::MakeUnique<TestNavigationThrottleInstaller>(
       shell()->web_contents(), NavigationThrottle::BLOCK_REQUEST,
       NavigationThrottle::PROCEED, NavigationThrottle::PROCEED);
 
   {
+    // A blocked, renderer-initiated navigation should commit an error page
+    // in the process that originated the navigation.
     NavigationHandleObserver observer(shell()->web_contents(), blocked_url);
-    EXPECT_FALSE(NavigateToURL(shell(), blocked_url));
+    TestNavigationObserver navigation_observer(shell()->web_contents(), 1);
+    EXPECT_TRUE(
+        ExecuteScript(shell(), base::StringPrintf("location.href = '%s'",
+                                                  blocked_url.spec().c_str())));
+    navigation_observer.Wait();
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_TRUE(observer.is_error());
+    EXPECT_EQ(site_instance,
+              shell()->web_contents()->GetMainFrame()->GetSiteInstance());
+  }
+
+  {
+    // Reloading the page should not transfer processes.
+    NavigationHandleObserver observer(shell()->web_contents(), blocked_url);
+    TestNavigationObserver navigation_observer(shell()->web_contents(), 1);
+
+    shell()->Reload();
+    navigation_observer.Wait();
     EXPECT_TRUE(observer.has_committed());
     EXPECT_TRUE(observer.is_error());
     EXPECT_EQ(site_instance,
               shell()->web_contents()->GetMainFrame()->GetSiteInstance());
   }
+
+  installer.reset();
+
+  {
+    // With the throttle uninstalled, going back should return to |start_url| in
+    // the same process, and clear the error page.
+    NavigationHandleObserver observer(shell()->web_contents(), start_url);
+    TestNavigationObserver navigation_observer(shell()->web_contents(), 1);
+
+    shell()->GoBackOrForward(-1);
+    navigation_observer.Wait();
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_FALSE(observer.is_error());
+    EXPECT_EQ(site_instance,
+              shell()->web_contents()->GetMainFrame()->GetSiteInstance());
+  }
+
+  installer = base::MakeUnique<TestNavigationThrottleInstaller>(
+      shell()->web_contents(), NavigationThrottle::BLOCK_REQUEST,
+      NavigationThrottle::PROCEED, NavigationThrottle::PROCEED);
+
+  {
+    // A blocked, browser-initiated navigation should commit an error page in a
+    // different process.
+    NavigationHandleObserver observer(shell()->web_contents(), blocked_url);
+    TestNavigationObserver navigation_observer(shell()->web_contents(), 1);
+
+    EXPECT_FALSE(NavigateToURL(shell(), blocked_url));
+
+    navigation_observer.Wait();
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_TRUE(observer.is_error());
+    EXPECT_NE(site_instance,
+              shell()->web_contents()->GetMainFrame()->GetSiteInstance());
+  }
 }
 
 // Test to verify that error pages caused by network error or other

content/browser/frame_host/navigation_request.cc

@@ -634,18 +634,23 @@ void NavigationRequest::OnRequestFailed(bool has_stale_copy_in_cache,
     return;
   }
 
-  // There are two types of error pages that need to be handled differently.
-  // * Error pages resulting from blocking the request, because the original
-  //   document wasn't even allowed to make the request. In such case,
-  //   the error pages should be committed in the process of the original
-  //   document, to avoid creating a process for the destination.
-  // * Error pages resulting from either network outage (no network, DNS
-  //   error, etc) or similar cases, where the user can reasonably expect that
-  //   a reload at a later point in time can be successful. Such error pages
-  //   do belong to the process that will host the destination URL, as a
-  //   reload will end up committing in that process anyway.
+  // Decide whether to leave the error page in the original process.
+  // * If this was a renderer-initiated navigation, and the request is blocked
+  //   because the initiating document wasn't allowed to make the request,
+  //   commit the error in the existing process. This is a strategy to to avoid
+  //   creating a process for the destination, which may belong to an origin
+  //   with a higher privilege level.
+  // * Error pages resulting from errors like network outage, no network, or DNS
+  //   error can reasonably expect that a reload at a later point in time would
+  //   work. These should be allowed to transfer away from the current process:
+  //   they do belong to whichever process that will host the destination URL,
+  //   as a reload will end up committing in that process anyway.
+  // * Error pages that arise during browser-initiated navigations to blocked
+  //   URLs should be allowed to transfer away from the current process, which
+  //   didn't request the navigation and may have a higher privilege level than
+  //   the blocked destination.
   RenderFrameHostImpl* render_frame_host = nullptr;
-  if (net_error == net::ERR_BLOCKED_BY_CLIENT) {
+  if (net_error == net::ERR_BLOCKED_BY_CLIENT && !browser_initiated()) {
     render_frame_host = frame_tree_node_->current_frame_host();
   } else {
     render_frame_host =