Implement error page commit policy in PlzNavigate.

content/browser/frame_host/navigation_handle_impl_browsertest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/memory/weak_ptr.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/request_context_type.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test_utils_internal.h"
 #include "net/dns/mock_host_resolver.h"
+#include "net/test/url_request/url_request_failed_job.h"
 #include "ui/base/page_transition_types.h"
 #include "url/url_constants.h"
 
 namespace content {
 
 namespace {
 
 // Gathers data from the NavigationHandle assigned to navigations that start
 // with the expected URL.
 class NavigationHandleObserver : public WebContentsObserver {
@@ skipping 1021 matching lines @@
         shell()->web_contents(), NavigationThrottle::PROCEED,
         NavigationThrottle::PROCEED, NavigationThrottle::PROCEED);
     NavigationHandleObserver observer(shell()->web_contents(), url);
     EXPECT_TRUE(NavigateToURL(shell(), url));
     EXPECT_EQ(1, installer.will_start_called());
     EXPECT_EQ(1, installer.will_process_called());
     EXPECT_FALSE(observer.is_same_page());
   }
 }
 
+// This class allows running tests with PlzNavigate enabled, regardless of
+// default test configuration.
+class PlzNavigateNavigationHandleImplBrowserTest : public ContentBrowserTest {
+ public:
+  PlzNavigateNavigationHandleImplBrowserTest() {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kEnableBrowserSideNavigation);
+  }
+};
+
+// Test to verify that error pages caused by NavigationThrottle blocking a
+// request from being made are properly committed in the original process
+// that requested the navigation.
+IN_PROC_BROWSER_TEST_F(PlzNavigateNavigationHandleImplBrowserTest,
+                       ErrorPageBlockedNavigation) {

[nasko, 2017/03/07 03:03:16]

I put these tests here, since it has the TestNavigationThrottleInstaller helper
objects. It belongs probably more to browser_side_navigation_browsertest.cc, but
that requires a bit of moving code around. I want to keep this CL small, so I'm
leaving it here for now and will move it later.

+  host_resolver()->AddRule("*", "127.0.0.1");
+  SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL start_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  GURL blocked_url(embedded_test_server()->GetURL("bar.com", "/title2.html"));
+
+  {
+    NavigationHandleObserver observer(shell()->web_contents(), start_url);
+    EXPECT_TRUE(NavigateToURL(shell(), start_url));
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_FALSE(observer.is_error());
+  }
+
+  SiteInstance* site_instance =

[Charlie Reis, 2017/03/09 00:43:34]

scoped_refptr would be safer (since in theory it could go away)

[nasko, 2017/03/09 07:24:09]

Done.

+      shell()->web_contents()->GetMainFrame()->GetSiteInstance();
+
+  TestNavigationThrottleInstaller installer(
+      shell()->web_contents(), NavigationThrottle::BLOCK_REQUEST,

[nasko, 2017/03/07 03:03:16]

This tests only WillStartRequest, as blocking during redirect is done in
Arthur's CL. I will add this test once his CL lands as part of moving this to
browser_side_navigation_browsertest.cc.

+      NavigationThrottle::PROCEED, NavigationThrottle::PROCEED);
+
+  {
+    NavigationHandleObserver observer(shell()->web_contents(), blocked_url);
+    EXPECT_FALSE(NavigateToURL(shell(), blocked_url));
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_TRUE(observer.is_error());
+    EXPECT_EQ(site_instance,
+              shell()->web_contents()->GetMainFrame()->GetSiteInstance());
+  }
+}
+
+// Test to verify that error pages caused by network error or other
+// recoverable error are properly committed in the process for the
+// destination URL.
+IN_PROC_BROWSER_TEST_F(PlzNavigateNavigationHandleImplBrowserTest,
+                       ErrorPageNetworkError) {
+  host_resolver()->AddRule("*", "127.0.0.1");
+  SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL start_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  GURL error_url(
+      net::URLRequestFailedJob::GetMockHttpUrl(net::ERR_CONNECTION_RESET));
+  EXPECT_NE(start_url.host(), error_url.host());
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&net::URLRequestFailedJob::AddUrlHandler));
+
+  {
+    NavigationHandleObserver observer(shell()->web_contents(), start_url);
+    EXPECT_TRUE(NavigateToURL(shell(), start_url));
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_FALSE(observer.is_error());
+  }
+
+  SiteInstance* site_instance =

[Charlie Reis, 2017/03/09 00:43:34]

Same here.

[nasko, 2017/03/09 07:24:09]

Done.

+      shell()->web_contents()->GetMainFrame()->GetSiteInstance();
+  {
+    NavigationHandleObserver observer(shell()->web_contents(), error_url);
+    EXPECT_FALSE(NavigateToURL(shell(), error_url));
+    EXPECT_TRUE(observer.has_committed());
+    EXPECT_TRUE(observer.is_error());
+    EXPECT_NE(site_instance,
+              shell()->web_contents()->GetMainFrame()->GetSiteInstance());
+  }
+}
+
 }  // namespace content