Do not create NavigationHandles for inactive RenderFrameHosts

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 1020 matching lines @@
                                base::TimeDelta::FromMinutes(10), 100);
   }
   // This message is only sent for top-level frames. TODO(avi): when frame tree
   // mirroring works correctly, add a check here to enforce it.
   delegate_->DocumentOnLoadCompleted(this);
 }
 
 void RenderFrameHostImpl::OnDidStartProvisionalLoad(
     const GURL& url,
     const base::TimeTicks& navigation_start) {
+  if (!is_active())

[nasko, 2016/10/10 20:41:11]

Do we want to add the same to all navigation related methods in RFH? For
example, DidFailProvisionalLoad/DidFailLoad/etc.

[clamy, 2016/10/11 10:34:27]

Added a TODO.

+    return;
   frame_tree_node_->navigator()->DidStartProvisionalLoad(this, url,
                                                          navigation_start);
 }
 
 void RenderFrameHostImpl::OnDidFailProvisionalLoadWithError(
     const FrameHostMsg_DidFailProvisionalLoadWithError_Params& params) {
   // TODO(clamy): Kill the renderer with RFH_FAIL_PROVISIONAL_LOAD_NO_HANDLE and
   // return early if navigation_handle_ is null, once we prevent that case from
   // happening in practice.
 
@@ skipping 240 matching lines @@
 GlobalFrameRoutingId RenderFrameHostImpl::GetGlobalFrameRoutingId() {
   return GlobalFrameRoutingId(GetProcess()->GetID(), GetRoutingID());
 }
 
 int RenderFrameHostImpl::GetEnabledBindings() {
   return render_view_host_->GetEnabledBindings();
 }
 
 void RenderFrameHostImpl::SetNavigationHandle(
     std::unique_ptr<NavigationHandleImpl> navigation_handle) {
+  CHECK(!navigation_handle || is_active());
   navigation_handle_ = std::move(navigation_handle);
-
-  // TODO(clamy): Remove this debug code once we understand better how we get to
-  // the point of attempting to transfer a navigation from a RFH that is no
-  // longer active.
-  if (navigation_handle_ && !is_active())
-    base::debug::DumpWithoutCrashing();

[nasko, 2016/10/10 20:41:10]

Why not keep this around instead of the CHECK, in case there are other cases we
might have missed? We can convert it to a check after few days of bake time, as
crashing the browser process is disruptive.

[clamy, 2016/10/11 10:34:27]

Done.

 }
 
 std::unique_ptr<NavigationHandleImpl>
 RenderFrameHostImpl::PassNavigationHandleOwnership() {
   DCHECK(!IsBrowserSideNavigationEnabled());
   if (navigation_handle_)
     navigation_handle_->set_is_transferring(true);
   return std::move(navigation_handle_);
 }
 
@@ skipping 467 matching lines @@
 void RenderFrameHostImpl::OnUpdateEncoding(const std::string& encoding_name) {
   // This message is only sent for top-level frames. TODO(avi): when frame tree
   // mirroring works correctly, add a check here to enforce it.
   delegate_->UpdateEncoding(this, encoding_name);
 }
 
 void RenderFrameHostImpl::OnBeginNavigation(
     const CommonNavigationParams& common_params,
     const BeginNavigationParams& begin_params) {
   CHECK(IsBrowserSideNavigationEnabled());
+  if (!is_active())
+    return;
   CommonNavigationParams validated_params = common_params;
   GetProcess()->FilterURL(false, &validated_params.url);
   frame_tree_node()->navigator()->OnBeginNavigation(
       frame_tree_node(), validated_params, begin_params);
 }
 
 void RenderFrameHostImpl::OnDispatchLoad() {
   CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
 
   // Don't forward the load event if this RFH is pending deletion.  This can
@@ skipping 1302 matching lines @@
   web_bluetooth_service_->SetClientConnectionErrorHandler(base::Bind(
       &RenderFrameHostImpl::DeleteWebBluetoothService, base::Unretained(this)));
   return web_bluetooth_service_.get();
 }
 
 void RenderFrameHostImpl::DeleteWebBluetoothService() {
   web_bluetooth_service_.reset();
 }
 
 }  // namespace content