Change NavigationEntry to use virtual URL in error pages for blocked navigations

content/browser/frame_host/navigation_controller_impl_browsertest.cc

Index: content/browser/frame_host/navigation_controller_impl_browsertest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index 4c3db0e05b6a0a231a3d72f9f9ba4944ed799685..162116bd8da8b35072037639aa8f6ca0335d3c93 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -6827,4 +6827,79 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   EXPECT_TRUE(back_observer.last_navigation_succeeded());
 }
 
+// Test to verify that navigating to a blocked URL does not result in a
+// NavigationEntry that allows the navigation to succeed when using a history
+// navigation or a reload. See https://crbug.com/723796.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       VerifyBlockedErrorPageURL_History) {

[Charlie Reis, 2017/05/18 22:41:46]

Just curious, how is this different from the earlier test?

[nasko, 2017/05/18 22:55:11]

The goal is to use a navigation different than the redirect-to-data-url one, so
we get more generic coverage of "blocked navigations" behavior. I will look
around for other such cases once we have better idea of which error codes will
be considered "blocked".

[Charlie Reis, 2017/05/18 23:15:51]

Acknowledged.

+  NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
+      shell()->web_contents()->GetController());
+
+  GURL start_url(embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), start_url));
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+
+  // Navigate to an URL, which redirects to a data: URL, since it is an
+  // unsafe redirect and will result in a blocked navigation and error page.
+  GURL redirect_to_blank_url(
+      embedded_test_server()->GetURL("/server-redirect?data:foo"));
+  EXPECT_FALSE(NavigateToURL(shell(), redirect_to_blank_url));
+  EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
+  EXPECT_EQ(PAGE_TYPE_ERROR, controller.GetLastCommittedEntry()->GetPageType());
+
+  // Navigate to a new document, then go back in history trying to load the
+  // blocked URL.
+  EXPECT_TRUE(
+      NavigateToURL(shell(), embedded_test_server()->GetURL("/title1.html")));
+  EXPECT_EQ(2, controller.GetLastCommittedEntryIndex());
+
+  TestNavigationObserver back_load_observer(shell()->web_contents());
+  controller.GoBack();
+  back_load_observer.Wait();
+
+  // The expectation is that about:blank was loaded and the virtual URL is set
+  // to the URL that was blocked.
+  EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
+  EXPECT_FALSE(
+      controller.GetLastCommittedEntry()->GetURL().SchemeIs(url::kDataScheme));
+  EXPECT_TRUE(controller.GetLastCommittedEntry()->GetVirtualURL().SchemeIs(
+      url::kDataScheme));
+  EXPECT_EQ(url::kAboutBlankURL,
+            controller.GetLastCommittedEntry()->GetURL().spec());
+}
+
+// The same as the test above, but uses reload instead of history navigation.
+// See https://crbug.com/723796.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       VerifyBlockedErrorPageURL_Reload) {
+  NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
+      shell()->web_contents()->GetController());
+
+  GURL start_url(embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), start_url));
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+
+  // Navigate to an URL, which redirects to a data: URL, since it is an
+  // unsafe redirect and will result in a blocked navigation and error page.
+  GURL redirect_to_blank_url(
+      embedded_test_server()->GetURL("/server-redirect?data:foo"));
+  EXPECT_FALSE(NavigateToURL(shell(), redirect_to_blank_url));
+  EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
+  EXPECT_EQ(PAGE_TYPE_ERROR, controller.GetLastCommittedEntry()->GetPageType());
+
+  TestNavigationObserver reload_observer(shell()->web_contents());
+  controller.Reload(ReloadType::NORMAL, false);
+  reload_observer.Wait();
+
+  // The expectation is that about:blank was loaded and the virtual URL is set
+  // to the URL that was blocked.
+  EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
+  EXPECT_FALSE(
+      controller.GetLastCommittedEntry()->GetURL().SchemeIs(url::kDataScheme));
+  EXPECT_TRUE(controller.GetLastCommittedEntry()->GetVirtualURL().SchemeIs(
+      url::kDataScheme));
+  EXPECT_EQ(url::kAboutBlankURL,
+            controller.GetLastCommittedEntry()->GetURL().spec());
+}
+
 }  // namespace content