refactor: implement will-redirect using NavigationThrottle instead of…

… PostTask

This avoids a potential race condition and immediately cancels the
navigation

atom/browser/api/atom_api_web_contents.cc

@@ -27,6 +27,7 @@
 #include "atom/browser/child_web_contents_tracker.h"
 #include "atom/browser/lib/bluetooth_chooser.h"
 #include "atom/browser/native_window.h"
+#include "atom/browser/net/atom_navigation_throttle.h"
 #include "atom/browser/net/atom_network_delegate.h"
 #if defined(ENABLE_OSR)
 #include "atom/browser/osr/osr_output_device.h"
@@ -908,18 +909,9 @@ void WebContents::DidStartNavigation(
   EmitNavigationEvent("did-start-navigation", navigation_handle);
 }
 
-void OnStopSoon(WebContents* web_contents) {
-  if (web_contents && !web_contents->IsDestroyed())
-    web_contents->Stop();
-}
-
 void WebContents::DidRedirectNavigation(
     content::NavigationHandle* navigation_handle) {
-  if (EmitNavigationEvent("will-redirect", navigation_handle)) {
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner(
-        base::ThreadTaskRunnerHandle::Get());
-    task_runner->PostTask(FROM_HERE, base::BindOnce(&OnStopSoon, this));
-  }
+  EmitNavigationEvent("did-redirect-navigation", navigation_handle);
 }
 
 void WebContents::DidFinishNavigation(

atom/browser/api/atom_api_web_contents.h

@@ -257,6 +257,9 @@ class WebContents : public mate::TrackableObject<WebContents>,
     observers_.RemoveObserver(obs);
   }
 
+  bool EmitNavigationEvent(const std::string& event,
+                           content::NavigationHandle* navigation_handle);
+
  protected:
   WebContents(v8::Isolate* isolate,
               content::WebContents* web_contents,
@@ -433,8 +436,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
 
   void InitZoomController(content::WebContents* web_contents,
                           const mate::Dictionary& options);
-  bool EmitNavigationEvent(const std::string& event,
-                           content::NavigationHandle* navigation_handle);
 
   v8::Global<v8::Value> session_;
   v8::Global<v8::Value> devtools_web_contents_;

atom/browser/atom_browser_client.cc

@@ -19,6 +19,7 @@
 #include "atom/browser/child_web_contents_tracker.h"
 #include "atom/browser/login_handler.h"
 #include "atom/browser/native_window.h"
+#include "atom/browser/net/atom_navigation_throttle.h"
 #include "atom/browser/session_preferences.h"
 #include "atom/browser/web_contents_permission_helper.h"
 #include "atom/browser/web_contents_preferences.h"
@@ -561,4 +562,12 @@ void AtomBrowserClient::RenderProcessExited(content::RenderProcessHost* host,
   }
 }
 
+std::vector<std::unique_ptr<content::NavigationThrottle>>
+AtomBrowserClient::CreateThrottlesForNavigation(
+    content::NavigationHandle* handle) {
+  std::vector<std::unique_ptr<content::NavigationThrottle>> throttles;
+  throttles.push_back(base::WrapUnique(new AtomNavigationThrottle(handle)));
+  return throttles;
+}
+
 }  // namespace atom

atom/browser/atom_browser_client.h

@@ -46,6 +46,9 @@ class AtomBrowserClient : public brightray::BrowserClient,
   static void SetCustomServiceWorkerSchemes(
       const std::vector<std::string>& schemes);
 
+  std::vector<std::unique_ptr<content::NavigationThrottle>>
+  CreateThrottlesForNavigation(content::NavigationHandle* handle) override;
+
  protected:
   // content::ContentBrowserClient:
   void RenderProcessWillLaunch(

atom/browser/net/atom_navigation_throttle.cc

@@ -0,0 +1,36 @@
+// Copyright (c) 2015 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/net/atom_navigation_throttle.h"
+
+#include "atom/browser/api/atom_api_web_contents.h"
+#include "content/public/browser/navigation_handle.h"
+
+namespace atom {
+
+AtomNavigationThrottle::AtomNavigationThrottle(
+    content::NavigationHandle* navigation_handle)
+    : content::NavigationThrottle(navigation_handle) {}
+
+AtomNavigationThrottle::~AtomNavigationThrottle() {}
+
+content::NavigationThrottle::ThrottleCheckResult
+AtomNavigationThrottle::WillRedirectRequest() {
+  auto* handle = navigation_handle();
+  auto* contents = handle->GetWebContents();
+  if (!contents)
+    return PROCEED;
+
+  auto api_contents =
+      atom::api::WebContents::CreateFrom(v8::Isolate::GetCurrent(), contents);
+  if (api_contents.IsEmpty())
+    return PROCEED;
+
+  if (api_contents->EmitNavigationEvent("will-redirect", handle)) {
+    return CANCEL;
+  }
+  return PROCEED;
+}
+
+}  // namespace atom

atom/browser/net/atom_navigation_throttle.h

@@ -0,0 +1,29 @@
+// Copyright (c) 2018 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_
+#define ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_
+
+#include "content/public/browser/navigation_throttle.h"
+
+class URLPattern;
+
+namespace atom {
+
+class AtomNavigationThrottle : public content::NavigationThrottle {
+ public:
+  AtomNavigationThrottle(content::NavigationHandle* handle);
+  ~AtomNavigationThrottle() override;
+
+  AtomNavigationThrottle::ThrottleCheckResult WillRedirectRequest() override;
+
+  const char* GetNameForLogging() { return "AtomNavigationThrottle"; }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AtomNavigationThrottle);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_BROWSER_NET_ATOM_NAVIGATION_THROTTLE_H_

filenames.gypi

@@ -282,6 +282,8 @@
       'atom/browser/net/asar/url_request_asar_job.h',
       'atom/browser/net/atom_cert_verifier.cc',
       'atom/browser/net/atom_cert_verifier.h',
+      'atom/browser/net/atom_navigation_throttle.h',
+      'atom/browser/net/atom_navigation_throttle.cc',
       'atom/browser/net/atom_network_delegate.cc',
       'atom/browser/net/atom_network_delegate.h',
       'atom/browser/net/atom_url_request.cc',