Add a way to cancel deferred navigations in NavigationHandle

components/navigation_interception/intercept_navigation_throttle.cc

Index: components/navigation_interception/intercept_navigation_throttle.cc
diff --git a/components/navigation_interception/intercept_navigation_throttle.cc b/components/navigation_interception/intercept_navigation_throttle.cc
index cc256a1273e0a0889e1df2cbabd3f999c3350bbb..989c1a9a094714d077916fc35f998dfb536794dc 100644
--- a/components/navigation_interception/intercept_navigation_throttle.cc
+++ b/components/navigation_interception/intercept_navigation_throttle.cc
@@ -8,13 +8,37 @@
 #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() {}
 
@@ -39,11 +63,41 @@ InterceptNavigationThrottle::CheckIfShouldIgnoreNavigation(bool is_redirect) {
       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(

[nasko, 2015/11/06 17:31:59]

Let's also add a comment here why there is a PostTask - mainly to ensure there
are no objects on the stack that can be deleted and unwinding through them in
deleted state.

[clamy, 2015/11/09 13:56:27]

Done.

+      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(
+      navigation_handle()->GetWebContents(), navigation_params,
+      should_ignore_callback_,
+      base::Bind(&InterceptNavigationThrottle::OnAsynchronousChecksPerformed,
+                 weak_factory_.GetWeakPtr()));
+}

[nasko, 2015/11/06 17:31:59]

As discussed, let's put a very scary warning here that no code should be added
after the RunCallback, as the object can be deleted and it will result in a
use-after-free bug.

[clamy, 2015/11/09 13:56:27]

Done.

+
+void InterceptNavigationThrottle::OnAsynchronousChecksPerformed(
+    bool should_ignore_navigation) {
+  if (should_ignore_navigation)
+    navigation_handle()->CancelDeferredNavigation(
+        content::NavigationThrottle::CANCEL_AND_IGNORE);
+  else
+    navigation_handle()->Resume();
 }
 
 }  // namespace navigation_interception