Add a way to cancel deferred navigations in NavigationHandle

content/browser/frame_host/navigation_handle_impl_unittest.cc

Index: content/browser/frame_host/navigation_handle_impl_unittest.cc
diff --git a/content/browser/frame_host/navigation_handle_impl_unittest.cc b/content/browser/frame_host/navigation_handle_impl_unittest.cc
index af6aced5b310e942202093109dbfcbde51280eef..1980ace722049d765db33e293c10ed6ec6500931 100644
--- a/content/browser/frame_host/navigation_handle_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_handle_impl_unittest.cc
@@ -186,6 +186,93 @@ TEST_F(NavigationHandleImplTest, ResumeDeferred) {
   EXPECT_EQ(1, test_throttle->will_redirect_calls());
 }
 
+// Checks that a navigation deferred during WillStartRequest can be properly
+// cancelled.
+TEST_F(NavigationHandleImplTest, CancelDeferredWillStart) {
+  TestNavigationThrottle* test_throttle =
+      CreateTestNavigationThrottle(NavigationThrottle::DEFER);
+  EXPECT_FALSE(IsDeferringStart());
+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_EQ(0, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+
+  // Simulate WillStartRequest. The request should be deferred. The callback
+  // should not have been called.
+  SimulateWillStartRequest();
+  EXPECT_TRUE(IsDeferringStart());
+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_FALSE(was_callback_called());
+  EXPECT_EQ(1, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+
+  // Cancel the request. The callback should have been called.
+  test_handle()->CancelDeferredNavigation(true);
+  EXPECT_TRUE(IsDeferringStart());

[nasko, 2015/11/04 22:44:27]

If the navigation is cancelled, should the throttle be in a DeferringStart
state?

[clamy, 2015/11/05 15:52:23]

I have introduced a CANCELING state to deal with the amount of time between the
moment the throttle wants to cancel the navigation and the moment we get the
DidStopLoading IPC in the current architecture (in PlzNavigate it's immediate).

+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_TRUE(was_callback_called());
+  EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result());
+  EXPECT_EQ(1, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+}
+
+// Checks that a navigation deferred during WillRedirectRequest can be properly
+// cancelled.
+TEST_F(NavigationHandleImplTest, CancelDeferredWillRedirect) {
+  TestNavigationThrottle* test_throttle =
+      CreateTestNavigationThrottle(NavigationThrottle::DEFER);
+  EXPECT_FALSE(IsDeferringStart());
+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_EQ(0, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+
+  // Simulate WillRedirectRequest. The request should be deferred. The callback
+  // should not have been called.
+  SimulateWillRedirectRequest();
+  EXPECT_FALSE(IsDeferringStart());
+  EXPECT_TRUE(IsDeferringRedirect());
+  EXPECT_FALSE(was_callback_called());
+  EXPECT_EQ(0, test_throttle->will_start_calls());

[nasko, 2015/11/04 22:44:27]

It shouldn't be possible to have 0 calls to WillStart and 1 call to WillRedirect
in real code, should it? It will be better to match the semantics of real world,
so maybe in a separate patch change it such that simluating WillRedirect ensures
that WillSend has been called?

[clamy, 2015/11/05 15:52:23]

Added a TODO for this (this require a few modifications to the
TestNavigationThrottle to work, so I'll do that in another patch.

+  EXPECT_EQ(1, test_throttle->will_redirect_calls());
+
+  // Cancel the request. The callback should have been called.
+  test_handle()->CancelDeferredNavigation(true);
+  EXPECT_FALSE(IsDeferringStart());
+  EXPECT_TRUE(IsDeferringRedirect());
+  EXPECT_TRUE(was_callback_called());
+  EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result());
+  EXPECT_EQ(0, test_throttle->will_start_calls());
+  EXPECT_EQ(1, test_throttle->will_redirect_calls());
+}
+
+// Checks that a navigation deferred can be canceled and not ignored.
+TEST_F(NavigationHandleImplTest, CancelDeferredNoIgnore) {
+  TestNavigationThrottle* test_throttle =
+      CreateTestNavigationThrottle(NavigationThrottle::DEFER);
+  EXPECT_FALSE(IsDeferringStart());
+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_EQ(0, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+
+  // Simulate WillRedirectRequest. The request should be deferred. The callback
+  // should not have been called.
+  SimulateWillStartRequest();
+  EXPECT_TRUE(IsDeferringStart());
+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_FALSE(was_callback_called());
+  EXPECT_EQ(1, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+
+  // Cancel the request. The callback should have been called with CANCEL, and
+  // not CANCEL_AND_IGNORE.
+  test_handle()->CancelDeferredNavigation(false);
+  EXPECT_TRUE(IsDeferringStart());
+  EXPECT_FALSE(IsDeferringRedirect());
+  EXPECT_TRUE(was_callback_called());
+  EXPECT_EQ(NavigationThrottle::CANCEL, callback_result());
+  EXPECT_EQ(1, test_throttle->will_start_calls());
+  EXPECT_EQ(0, test_throttle->will_redirect_calls());
+}
+
 // Checks that a NavigationThrottle asking to defer followed by a
 // NavigationThrottle asking to proceed behave correctly.
 TEST_F(NavigationHandleImplTest, DeferThenProceed) {