Prevent a browser crash if dialogs are shown when navigating away.

content/browser/web_contents/render_view_host_manager.cc

 // Copyright (c) 2012 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/web_contents/render_view_host_manager.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 407 matching lines @@
 
   // Run the unload handler of the current page.
   SwapOutOldPage();
 }
 
 void RenderViewHostManager::SwapOutOldPage() {
   // Should only see this while we have a pending renderer or transfer.
   CHECK(cross_navigation_pending_ || pending_nav_params_.get());
 
   // First close any modal dialogs that would prevent us from swapping out.
+  // TODO(creis): This is not a guarantee.  The renderer could immediately
+  // create another dialog in a loop, potentially causing a renderer crash when
+  // we tell it to swap out with a nested message loop and PageGroupLoadDeferrer
+  // on the stack.  We should prevent the renderer from showing more dialogs
+  // until the SwapOut.  See http://crbug.com/312490.
   delegate_->CancelModalDialogsForRenderManager();
 
   // Tell the old renderer it is being swapped out.  This will fire the unload
   // handler (without firing the beforeunload handler a second time).  When the
   // unload handler finishes and the navigation completes, we will send a
   // message to the ResourceDispatcherHost, allowing the pending RVH's response
   // to resume.
   render_view_host_->SwapOut();
 
   // ResourceDispatcherHost has told us to run the onunload handler, which
@@ skipping 310 matching lines @@
                                                      opener_route_id);
 }
 
 void RenderViewHostManager::CommitPending() {
   // First check whether we're going to want to focus the location bar after
   // this commit.  We do this now because the navigation hasn't formally
   // committed yet, so if we've already cleared |pending_web_ui_| the call chain
   // this triggers won't be able to figure out what's going on.
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
+  // We currently can't guarantee that the renderer isn't showing a new modal
+  // dialog, even though we canceled them in SwapOutOldPage.  (It may have
+  // created another in the mean time.)  Make sure we run and reset the callback

[darin (slow to review), 2013/10/29 16:57:23]

nit: "mean time" -> "meantime"

[Charlie Reis, 2013/10/29 17:13:17]

Done.

+  // now before we delete its RVH below.
+  // TODO(creis): Remove this if we can guarantee that no new dialogs will be
+  // shown after SwapOutOldPage.  See http://crbug.com/312490.
+  delegate_->CancelModalDialogsForRenderManager();
+
   // Next commit the Web UI, if any. Either replace |web_ui_| with
   // |pending_web_ui_|, or clear |web_ui_| if there is no pending WebUI, or
   // leave |web_ui_| as is if reusing it.
   DCHECK(!(pending_web_ui_.get() && pending_and_current_web_ui_.get()));
   if (pending_web_ui_)
     web_ui_.reset(pending_web_ui_.release());
   else if (!pending_and_current_web_ui_.get())
     web_ui_.reset();
 
   // It's possible for the pending_render_view_host_ to be NULL when we aren't
@@ skipping 316 matching lines @@
 RenderViewHostImpl* RenderViewHostManager::GetSwappedOutRenderViewHost(
     SiteInstance* instance) {
   RenderViewHostMap::iterator iter = swapped_out_hosts_.find(instance->GetId());
   if (iter != swapped_out_hosts_.end())
     return iter->second;
 
   return NULL;
 }
 
 }  // namespace content