Issue AboutToNavigateRenderFrame notification after beforeunload handling.

content/browser/frame_host/navigator_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/navigator_impl.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 296 matching lines @@
     return false;  // Unable to create the desired RenderFrameHost.
 
   // Make sure no code called via RFHM::Navigate clears the pending entry.
   CHECK_EQ(controller_->GetPendingEntry(), &entry);
 
   // For security, we should never send non-Web-UI URLs to a Web UI renderer.
   // Double check that here.
   CheckWebUIRendererDoesNotDisplayNormalURL(dest_render_frame_host, dest_url);
 
   // Notify observers that we will navigate in this RenderFrame.
-  if (delegate_) {
+  if (delegate_ && !dest_render_frame_host->are_navigations_suspended()) {
     delegate_->AboutToNavigateRenderFrame(frame_tree_node->current_frame_host(),

[Charlie Reis, 2015/09/22 23:12:10]

What's happening in AboutToNavigateRenderFrame that must be delayed until after
the beforeunload handler?

I'm confused, because even after this change, the browser process will time out
and allow the navigation to proceed if there's a breakpoint in beforeunload. 
What's different about firing AboutToNavigateRenderFrame later?

[dgozman, 2015/09/22 23:27:55]

It's called from another place.
I didn't know about the timeout. How long is that? I didn't notice it while
testing locally.

[Charlie Reis, 2015/09/22 23:56:42]

That didn't really answer my question, though.

What is DevTools doing in AboutToNavigateRenderFrame that needs to be delayed?
RenderFrameHostImpl::DispatchBeforeUnload sets a timer for kUnloadTimeoutMS,
which is only 1 second.

That means that the navigation will likely try to proceed while the breakpoint
is hit, which seems wrong-- we'll be calling AboutToNavigateRenderFrame 1 second
later than before, but still during the breakpoint.  (I imagine the timeout is
part of the problem in today's behavior as well, though I'm not sure how that
leads to the hang.)

                                           dest_render_frame_host);
   }
 
   // Navigate in the desired RenderFrameHost.
   // We can skip this step in the rare case that this is a transfer navigation
   // which began in the chosen RenderFrameHost, since the request has already
   // been issued.  In that case, simply resume the response.
   bool is_transfer_to_same =
       entry.transferred_global_request_id().child_id != -1 &&
       entry.transferred_global_request_id().child_id ==
@@ skipping 616 matching lines @@
       entry->set_should_replace_entry(pending_entry->should_replace_entry());
       entry->SetRedirectChain(pending_entry->GetRedirectChain());
     }
     controller_->SetPendingEntry(entry.Pass());
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content