Keep track in the browser of which frames have onunload and onbeforeunload handlers.

content/browser/frame_host/navigation_request.cc

Index: content/browser/frame_host/navigation_request.cc
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index c4ce4b8242ceb50f2b3ec01ac65ab6402ae4a4fc..778e2a5431b29a38b5db3218aa2dfdef3de42fa7 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -28,6 +28,7 @@
 #include "content/common/appcache_interfaces.h"
 #include "content/common/resource_request_body_impl.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/navigation_controller.h"
@@ -311,7 +312,8 @@ NavigationRequest::NavigationRequest(
       bindings_(NavigationEntryImpl::kInvalidBindings),
       response_should_be_rendered_(true),
       associated_site_instance_type_(AssociatedSiteInstanceType::NONE),
-      may_transfer_(may_transfer) {
+      may_transfer_(may_transfer),
+      weak_factory_(this) {
   DCHECK(!browser_initiated || (entry != nullptr && frame_entry != nullptr));
 
   // Sanitize the referrer.
@@ -663,17 +665,19 @@ void NavigationRequest::OnStartChecksComplete(
 
   // Abort the request if needed. This will destroy the NavigationRequest.
   if (result == NavigationThrottle::CANCEL_AND_IGNORE ||
-      result == NavigationThrottle::CANCEL) {
+      result == NavigationThrottle::CANCEL ||
+      result == NavigationThrottle::BLOCK_REQUEST) {
     // TODO(clamy): distinguish between CANCEL and CANCEL_AND_IGNORE.
-    OnRequestFailed(false, net::ERR_ABORTED);
-
-    // DO NOT ADD CODE after this. The previous call to OnRequestFailed has
-    // destroyed the NavigationRequest.
-    return;
-  }
-
-  if (result == NavigationThrottle::BLOCK_REQUEST) {
-    OnRequestFailed(false, net::ERR_BLOCKED_BY_CLIENT);
+    int error_code = result == NavigationThrottle::BLOCK_REQUEST
+                         ? net::ERR_BLOCKED_BY_CLIENT
+                         : net::ERR_ABORTED;
+    // If the start checks completed synchronously, which could happen if there
+    // is no onbeforeunload handler, then this could cause reentrancy into
+    // NavigationController. So use a PostTask to avoid that.
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&NavigationRequest::OnRequestFailed,
+                   weak_factory_.GetWeakPtr(), false, error_code));

[nasko, 2017/03/29 23:31:26]

Let's try to reason through this one, as it isn't obvious by looking at it. 
If we post a task to call the OnRequestFailed the NavigationRequest remains
valid and associated with the FrameTreeNode. It is possible that the event loop
already has a task to perform a renderer-initiated navigation queued up before
the task is posted. When it is executed, it will be ignored, as the
browser-initiated navigation won't be cancelled (checks inside
NavigatorImpl::OnBeginNavigation). Also an IPC will be sent to the renderer to
stop the navigation.

If the navigation has user gesture, then it will cancel the existing
NavigationRequest, but then the task won't execute because of the WeakPtr will
be invalidated.

Prior to this CL, we would send the IPC to execute any beforeunload handlers and
then finish with the task. If we had the same case as above, I guess we are in
the same situation and the same events will happen.

Seems fine :)

 
     // DO NOT ADD CODE after this. The previous call to OnRequestFailed has
     // destroyed the NavigationRequest.