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

content/browser/frame_host/navigation_request.cc

 // Copyright 2014 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 "content/browser/frame_host/navigation_request.h"
 
 #include <utility>
 
 #include "base/memory/ptr_util.h"
 #include "content/browser/appcache/appcache_navigation_handle.h"
@@ skipping 10 matching lines @@
 #include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/loader/navigation_url_loader.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_navigation_handle.h"
 #include "content/browser/site_instance_impl.h"
 #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"
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_ui_data.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/stream_handle.h"
 #include "content/public/common/appcache_info.h"
 #include "content/public/common/content_client.h"
@@ skipping 263 matching lines @@
       common_params_(common_params),
       begin_params_(begin_params),
       request_params_(request_params),
       browser_initiated_(browser_initiated),
       state_(NOT_STARTED),
       restore_type_(RestoreType::NONE),
       is_view_source_(false),
       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.
   common_params_.referrer =
       Referrer::SanitizeForRequest(common_params_.url, common_params_.referrer);
 
   if (may_transfer) {
     FrameNavigationEntry* frame_entry = entry->GetFrameEntry(frame_tree_node);
     if (frame_entry) {
       source_site_instance_ = frame_entry->source_site_instance();
@@ skipping 331 matching lines @@
 void NavigationRequest::OnStartChecksComplete(
     NavigationThrottle::ThrottleCheckResult result) {
   DCHECK(result != NavigationThrottle::DEFER);
   DCHECK(result != NavigationThrottle::BLOCK_RESPONSE);
 
   if (on_start_checks_complete_closure_)
     on_start_checks_complete_closure_.Run();
 
   // 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);
+    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.
-    return;
-  }
-
-  if (result == NavigationThrottle::BLOCK_REQUEST) {
-    OnRequestFailed(false, net::ERR_BLOCKED_BY_CLIENT);
-
-    // DO NOT ADD CODE after this. The previous call to OnRequestFailed has
-    // destroyed the NavigationRequest.
     return;
   }
 
   // Use the SiteInstance of the navigating RenderFrameHost to get access to
   // the StoragePartition. Using the url of the navigation will result in a
   // wrong StoragePartition being picked when a WebView is navigating.
   DCHECK_NE(AssociatedSiteInstanceType::NONE, associated_site_instance_type_);
   RenderFrameHostImpl* navigating_frame_host =
       associated_site_instance_type_ == AssociatedSiteInstanceType::SPECULATIVE
           ? frame_tree_node_->render_manager()->speculative_frame_host()
@@ skipping 147 matching lines @@
   DCHECK_EQ(request_params_.has_user_gesture, begin_params_.has_user_gesture);
 
   render_frame_host->CommitNavigation(response_.get(), std::move(body_),
                                       common_params_, request_params_,
                                       is_view_source_);
 
   frame_tree_node_->ResetNavigationRequest(true, true);
 }
 
 }  // namespace content