Add a way to cancel deferred navigations in NavigationHandle

components/navigation_interception/intercept_navigation_throttle.cc

 // Copyright 2015 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 "components/navigation_interception/intercept_navigation_throttle.h"
 
 #include "components/navigation_interception/navigation_params.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 
+using content::BrowserThread;
+
 namespace navigation_interception {
 
+namespace {
+
+using ChecksPerformedCallback = base::Callback<void(bool)>;
+
+// This is used to run |should_ignore_callback| if it can destroy the
+// WebContents (and the InterceptNavigationThrottle along). In that case,
+// |on_checks_performed_callback| will be a no-op.
+void RunCallback(
+    content::WebContents* web_contents,
+    const NavigationParams& navigation_params,
+    InterceptNavigationThrottle::CheckCallback should_ignore_callback,
+    ChecksPerformedCallback on_checks_performed_callback) {
+  bool should_ignore_navigation =
+      should_ignore_callback.Run(web_contents, navigation_params);
+  on_checks_performed_callback.Run(should_ignore_navigation);
+}
+
+}  // namespace
+
 InterceptNavigationThrottle::InterceptNavigationThrottle(
     content::NavigationHandle* navigation_handle,
-    CheckCallback should_ignore_callback)
+    CheckCallback should_ignore_callback,
+    bool run_callback_synchronously)
     : content::NavigationThrottle(navigation_handle),
-      should_ignore_callback_(should_ignore_callback) {}
+      should_ignore_callback_(should_ignore_callback),
+      run_callback_synchronously_(run_callback_synchronously),
+      weak_factory_(this) {}
 
 InterceptNavigationThrottle::~InterceptNavigationThrottle() {}
 
 content::NavigationThrottle::ThrottleCheckResult
 InterceptNavigationThrottle::WillStartRequest() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return CheckIfShouldIgnoreNavigation(false);
 }
 
 content::NavigationThrottle::ThrottleCheckResult
 InterceptNavigationThrottle::WillRedirectRequest() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return CheckIfShouldIgnoreNavigation(true);
 }
 
 content::NavigationThrottle::ThrottleCheckResult
 InterceptNavigationThrottle::CheckIfShouldIgnoreNavigation(bool is_redirect) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   NavigationParams navigation_params(
       navigation_handle()->GetURL(), navigation_handle()->GetReferrer(),
       navigation_handle()->HasUserGesture(), navigation_handle()->IsPost(),
       navigation_handle()->GetPageTransition(), is_redirect,
       navigation_handle()->IsExternalProtocol(), true);
 
-  bool should_ignore_navigation = should_ignore_callback_.Run(
-      navigation_handle()->GetWebContents(), navigation_params);
-  return should_ignore_navigation
-             ? content::NavigationThrottle::CANCEL_AND_IGNORE
-             : content::NavigationThrottle::PROCEED;
+  if (run_callback_synchronously_) {
+    bool should_ignore_navigation = should_ignore_callback_.Run(
+        navigation_handle()->GetWebContents(), navigation_params);
+    return should_ignore_navigation
+               ? content::NavigationThrottle::CANCEL_AND_IGNORE
+               : content::NavigationThrottle::PROCEED;
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&InterceptNavigationThrottle::RunCallbackAsynchronously,
+                 weak_factory_.GetWeakPtr(), navigation_params));
+  return DEFER;
+}
+
+void InterceptNavigationThrottle::RunCallbackAsynchronously(
+    const NavigationParams& navigation_params) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Run the callback in a helper function as it may lead ot the destruction of
+  // this InterceptNavigationThrottle.
+  RunCallback(

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

If the callback can destruct this object, then it doesn't seem correct to still
run it through this object. Shouldn't the PostTask on line 74 bind it to the
helper function? This way there is no object being destroyed that we return back
into.

The way the code is structured right now, the only guard is for
InterceptNavigationThrottle being deleted between the posting of the task and
the execution. But if the callback can delete the object, we aren't guarding
this.

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

The callback needs the WebContents to run, which may go away, hence why I can't
PostTask the execution of the callback (unless I pull out the routing id +
process id for the RVH and get the WebContents back using those).

This is why I made sure we would not execute anything in the object after
running the callback. I thought that having the handling of the result in a
second callback bound on a weak_ptr to this object would ensure the second
callback did not run when this was deleted.

+      navigation_handle()->GetWebContents(), navigation_params,
+      should_ignore_callback_,
+      base::Bind(&InterceptNavigationThrottle::OnAsynchronousChecksPerformed,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void InterceptNavigationThrottle::OnAsynchronousChecksPerformed(
+    bool should_ignore_navigation) {
+  if (should_ignore_navigation)
+    navigation_handle()->CancelDeferredNavigation(true);
+  else
+    navigation_handle()->Resume();
 }
 
 }  // namespace navigation_interception