PlzNavigate: add support for BLOCK_REQUEST during redirects

content/browser/frame_host/navigation_handle_impl_browsertest.cc

Index: content/browser/frame_host/navigation_handle_impl_browsertest.cc
diff --git a/content/browser/frame_host/navigation_handle_impl_browsertest.cc b/content/browser/frame_host/navigation_handle_impl_browsertest.cc
index dd09a06c1dc6b613086f17ff0d216e39edba0827..fd37935e8f7ed0b672a54b8654b7881cfdc78308 100644
--- a/content/browser/frame_host/navigation_handle_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_handle_impl_browsertest.cc
@@ -7,6 +7,7 @@
 #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/browser_side_navigation_policy.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"
@@ -38,13 +39,15 @@ class NavigationHandleObserver : public WebContentsObserver {
         is_renderer_initiated_(true),
         is_same_page_(false),
         was_redirected_(false),
+        has_finished_(false),
         frame_tree_node_id_(-1),
         page_transition_(ui::PAGE_TRANSITION_LINK),
         expected_start_url_(expected_start_url),
         net_error_code_(net::OK) {}
 
   void DidStartNavigation(NavigationHandle* navigation_handle) override {
-    if (handle_ || navigation_handle->GetURL() != expected_start_url_)
+    if (handle_ || has_finished_ ||
+        navigation_handle->GetURL() != expected_start_url_)
       return;
 
     handle_ = navigation_handle;
@@ -88,6 +91,7 @@ class NavigationHandleObserver : public WebContentsObserver {
     }
 
     handle_ = nullptr;
+    has_finished_ = true;
   }
 
   bool has_committed() { return has_committed_; }
@@ -106,9 +110,9 @@ class NavigationHandleObserver : public WebContentsObserver {
   net::Error net_error_code() { return net_error_code_; }
 
  private:
-  // A reference to the NavigationHandle so this class will track only
-  // one navigation at a time. It is set at DidStartNavigation and cleared
-  // at DidFinishNavigation before the NavigationHandle is destroyed.
+  // A reference to the NavigationHandle so this class will track only one
+  // navigation. It is set at DidStartNavigation and cleared at
+  // DidFinishNavigation before the NavigationHandle is destroyed.
   NavigationHandle* handle_;
   bool has_committed_;
   bool is_error_;
@@ -117,6 +121,7 @@ class NavigationHandleObserver : public WebContentsObserver {
   bool is_renderer_initiated_;
   bool is_same_page_;
   bool was_redirected_;
+  bool has_finished_;
   int frame_tree_node_id_;
   ui::PageTransition page_transition_;
   GURL expected_start_url_;
@@ -923,6 +928,20 @@ IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
     EXPECT_FALSE(NavigateToURL(shell(), kUrl));
     EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, observer.net_error_code());
   }
+
+  {
+    // Set up a NavigationThrottle that will block the navigation in
+    // WillRedirectRequest.
+    TestNavigationThrottleInstaller installer(
+        shell()->web_contents(), NavigationThrottle::PROCEED,
+        NavigationThrottle::BLOCK_REQUEST, NavigationThrottle::PROCEED);
+    NavigationHandleObserver observer(shell()->web_contents(), kRedirectingUrl);
+
+    // Try to navigate to the url. The navigation should be canceled and the
+    // NavigationHandle should have the right error code.
+    EXPECT_FALSE(NavigateToURL(shell(), kRedirectingUrl));
+    EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, observer.net_error_code());
+  }
 }
 
 // Specialized test that verifies the NavigationHandle gets the HTTPS upgraded
@@ -1057,4 +1076,67 @@ IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
   }
 }
 
+// Record and list the navigations that are started and finished.
+class NavigationLogger : public WebContentsObserver {
+ public:
+  NavigationLogger(WebContents* web_contents)
+      : WebContentsObserver(web_contents) {}
+
+  void DidStartNavigation(NavigationHandle* navigation_handle) override {
+    started_navigation_urls_.push_back(navigation_handle->GetURL());
+  }
+
+  void DidFinishNavigation(NavigationHandle* navigation_handle) override {
+    finished_navigation_urls_.push_back(navigation_handle->GetURL());
+  }
+
+  const std::vector<GURL>& GetStartedNavigationURLS() {

[nasko, 2017/03/01 18:12:43]

nit: hacker_case

[clamy, 2017/03/02 12:43:17]

Done.

+    return started_navigation_urls_;
+  }
+  const std::vector<GURL>& GetFinishedNavigationURLS() {
+    return finished_navigation_urls_;
+  }
+
+ private:
+  std::vector<GURL> started_navigation_urls_;
+  std::vector<GURL> finished_navigation_urls_;
+};
+
+// There was a bug without PlzNavigate that happened when a navigation was
+// blocked after a redirect. Blink didn't know about the redirect and tried
+// to commit an error page to the post-redirect URL. The result was that the
+// NavigationHandle was not found on the browser-side and a new NavigationHandle
+// created for committing the error page. This test makes sure that only one
+// NavigationHandle is used for committing the error page.
+// See crbug.com/695421
+IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest, BlockedOnRedirect) {
+  const GURL kUrl = embedded_test_server()->GetURL("/title1.html");
+  const GURL kRedirectingUrl =
+      embedded_test_server()->GetURL("/server-redirect?" + kUrl.spec());
+
+  // Set up a NavigationThrottle that will block the navigation in
+  // WillRedirectRequest.
+  TestNavigationThrottleInstaller installer(
+      shell()->web_contents(), NavigationThrottle::PROCEED,
+      NavigationThrottle::BLOCK_REQUEST, NavigationThrottle::PROCEED);
+  NavigationHandleObserver observer(shell()->web_contents(), kRedirectingUrl);
+  NavigationLogger logger(shell()->web_contents());
+
+  // Try to navigate to the url. The navigation should be canceled and the
+  // NavigationHandle should have the right error code.
+  EXPECT_FALSE(NavigateToURL(shell(), kRedirectingUrl));
+  EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, observer.net_error_code());
+
+  // Only one navigation is expected to happen.
+  std::vector<GURL> started_navigation = {kRedirectingUrl};
+  EXPECT_EQ(started_navigation, logger.GetStartedNavigationURLS());
+
+  std::vector<GURL> finished_navigation;
+  if (IsBrowserSideNavigationEnabled())
+    finished_navigation = {kUrl};
+  else
+    finished_navigation = {kRedirectingUrl};
+  EXPECT_EQ(finished_navigation, logger.GetFinishedNavigationURLS());
+}
+
 }  // namespace content