Add a way to cancel deferred navigations in NavigationHandle

content/browser/frame_host/navigation_handle_impl.cc

Index: content/browser/frame_host/navigation_handle_impl.cc
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index b1aace3597c5ecd773339efa3b2472beac3db8a7..3185088cda2b1294ce2d51afe28307c59b950c82 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -80,6 +80,14 @@ NavigationHandleImpl::NavigationHandleImpl(const GURL& url,
 
 NavigationHandleImpl::~NavigationHandleImpl() {
   GetDelegate()->DidFinishNavigation(this);
+
+  // Cancel the navigation on the IO thread if the NavigationHandle was
+  // destroyed in the middle of the NavigationThrottles checks.

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

nit: s/was destroyed/is being destroyed/, unless I'm misunderstanding the
context that this code will run only in the case of deleting WebContents in the
observer.

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

Done.

+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kEnableBrowserSideNavigation) &&
+      !complete_callback_.is_null()) {
+    RunCompleteCallback(NavigationThrottle::CANCEL_AND_IGNORE);
+  }
 }
 
 NavigatorDelegate* NavigationHandleImpl::GetDelegate() const {
@@ -160,7 +168,13 @@ void NavigationHandleImpl::Resume() {
   }
 
   if (result != NavigationThrottle::DEFER)
-    complete_callback_.Run(result);
+    RunCompleteCallback(result);
+}
+
+void NavigationHandleImpl::CancelDeferredNavigation(bool ignore) {

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

Why not use the enum as the parameter type? In general, boolean arguments are
somewhat hard to read at the time of the method call. This will be a bit more
verbose in characters, but will be clear which intention is meant when reading
the code.

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

Done.

+  CHECK(state_ == DEFERRING_START || state_ == DEFERRING_REDIRECT);

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

DCHECK?

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

Done.

+  RunCompleteCallback(ignore ? NavigationThrottle::CANCEL_AND_IGNORE
+                             : NavigationThrottle::CANCEL);
 }
 
 void NavigationHandleImpl::RegisterThrottleForTesting(
@@ -235,7 +249,7 @@ void NavigationHandleImpl::WillStartRequest(
 
   // If the navigation is not deferred, run the callback.
   if (result != NavigationThrottle::DEFER)
-    callback.Run(result);
+    RunCompleteCallback(result);
 }
 
 void NavigationHandleImpl::WillRedirectRequest(
@@ -259,7 +273,7 @@ void NavigationHandleImpl::WillRedirectRequest(
 
   // If the navigation is not deferred, run the callback.
   if (result != NavigationThrottle::DEFER)
-    callback.Run(result);
+    RunCompleteCallback(result);
 }
 
 void NavigationHandleImpl::DidRedirectNavigation(const GURL& new_url) {
@@ -296,6 +310,7 @@ NavigationHandleImpl::CheckWillStartRequest() {
       case NavigationThrottle::PROCEED:
         continue;
 
+      case NavigationThrottle::CANCEL:
       case NavigationThrottle::CANCEL_AND_IGNORE:
         return result;
 
@@ -325,6 +340,7 @@ NavigationHandleImpl::CheckWillRedirectRequest() {
       case NavigationThrottle::PROCEED:
         continue;
 
+      case NavigationThrottle::CANCEL:
       case NavigationThrottle::CANCEL_AND_IGNORE:
         return result;
 
@@ -342,4 +358,12 @@ NavigationHandleImpl::CheckWillRedirectRequest() {
   return NavigationThrottle::PROCEED;
 }
 
+void NavigationHandleImpl::RunCompleteCallback(
+    NavigationThrottle::ThrottleCheckResult result) {
+  DCHECK(result != NavigationThrottle::DEFER);
+  if (!complete_callback_.is_null())
+    complete_callback_.Run(result);
+  complete_callback_.Reset();

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

nit: Empty line between the if statement and this call.

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

Done.

+}
+
 }  // namespace content