diff options
| -rw-r--r-- | chrome/browser/browser.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/render_view_host.cc | 25 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/render_view_host.h | 15 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/render_view_host_delegate.h | 8 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/render_view_host_manager.cc | 41 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/render_view_host_manager.h | 2 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/tab_contents.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/tab_contents.h | 1 |
8 files changed, 62 insertions, 38 deletions
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index c400d48..3f54397 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -1563,7 +1563,7 @@ bool Browser::RunUnloadListenerBeforeClosing(TabContents* contents) { // them. Once they have fired, we'll get a message back saying whether // to proceed closing the page or not, which sends us back to this method // with the HasUnloadListener bit cleared. - contents->render_view_host()->FirePageBeforeUnload(); + contents->render_view_host()->FirePageBeforeUnload(false); return true; } return false; @@ -2476,7 +2476,7 @@ void Browser::ProcessPendingTabs() { // Null check render_view_host here as this gets called on a PostTask and // the tab's render_view_host may have been nulled out. if (tab->render_view_host()) { - tab->render_view_host()->FirePageBeforeUnload(); + tab->render_view_host()->FirePageBeforeUnload(false); } else { ClearUnloadState(tab); } diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index 070bd55..9a97eb0 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -108,6 +108,7 @@ RenderViewHost::RenderViewHost(SiteInstance* instance, suspended_nav_message_(NULL), run_modal_reply_msg_(NULL), is_waiting_for_unload_ack_(false), + unload_ack_is_for_cross_site_transition_(false), are_javascript_messages_suppressed_(false), sudden_termination_allowed_(false), in_inspect_element_mode_(false) { @@ -300,21 +301,33 @@ void RenderViewHost::SetNavigationsSuspended(bool suspend) { } } -void RenderViewHost::FirePageBeforeUnload() { +void RenderViewHost::FirePageBeforeUnload(bool for_cross_site_transition) { if (!IsRenderViewLive()) { // This RenderViewHost doesn't have a live renderer, so just skip running // the onbeforeunload handler. + is_waiting_for_unload_ack_ = true; // Prevent check in OnMsgShouldCloseACK. + unload_ack_is_for_cross_site_transition_ = for_cross_site_transition; OnMsgShouldCloseACK(true); return; } // This may be called more than once (if the user clicks the tab close button // several times, or if she clicks the tab close button then the browser close - // button), so this test makes sure we only send the message once. - if (!is_waiting_for_unload_ack_) { + // button), and we only send the message once. + if (is_waiting_for_unload_ack_) { + // Some of our close messages could be for the tab, others for cross-site + // transitions. We always want to think it's for closing the tab if any + // of the messages were, since otherwise it might be impossible to close + // (if there was a cross-site "close" request pending when the user clicked + // the close button). We want to keep the "for cross site" flag only if + // both the old and the new ones are also for cross site. + unload_ack_is_for_cross_site_transition_ = + unload_ack_is_for_cross_site_transition_ && for_cross_site_transition; + } else { // Start the hang monitor in case the renderer hangs in the beforeunload // handler. is_waiting_for_unload_ack_ = true; + unload_ack_is_for_cross_site_transition_ = for_cross_site_transition; StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS)); Send(new ViewMsg_ShouldClose(routing_id())); } @@ -1439,8 +1452,10 @@ void RenderViewHost::OnMsgShouldCloseACK(bool proceed) { RenderViewHostDelegate::RendererManagement* management_delegate = delegate_->GetRendererManagementDelegate(); - if (management_delegate) - management_delegate->ShouldClosePage(proceed); + if (management_delegate) { + management_delegate->ShouldClosePage( + unload_ack_is_for_cross_site_transition_, proceed); + } } void RenderViewHost::OnQueryFormFieldAutofill(const std::wstring& field_name, diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h index 6d196f4e..d0836bc 100644 --- a/chrome/browser/renderer_host/render_view_host.h +++ b/chrome/browser/renderer_host/render_view_host.h @@ -151,7 +151,11 @@ class RenderViewHost : public RenderWidgetHost, // Causes the renderer to invoke the onbeforeunload event handler. The // result will be returned via ViewMsg_ShouldClose. See also ClosePage which // will fire the PageUnload event. - void FirePageBeforeUnload(); + // + // Set bool for_cross_site_transition when this close is just for the current + // RenderView in the case of a cross-site transition. False means we're + // closing the entire tab. + void FirePageBeforeUnload(bool for_cross_site_transition); // Causes the renderer to close the current page, including running its // onunload event handler. A ClosePage_ACK message will be sent to the @@ -619,8 +623,17 @@ class RenderViewHost : public RenderWidgetHost, // must return to the renderer to unblock it. IPC::Message* run_modal_reply_msg_; + // Set to true when there is a pending ViewMsg_ShouldClose message pending. + // This ensures we don't spam the renderer many times to close. When true, + // the value of unload_ack_is_for_cross_site_transition_ indicates which type + // of unload this is for. bool is_waiting_for_unload_ack_; + // Valid only when is_waiting_for_unload_ack_ is true, this tells us if the + // unload request is for closing the entire tab ( = false), or only this + // RenderViewHost in the case of a cross-site transition ( = true). + bool unload_ack_is_for_cross_site_transition_; + bool are_javascript_messages_suppressed_; // True if the render view can be shut down suddenly. diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h index e33d64b..8ea6e16 100644 --- a/chrome/browser/renderer_host/render_view_host_delegate.h +++ b/chrome/browser/renderer_host/render_view_host_delegate.h @@ -145,9 +145,11 @@ class RenderViewHostDelegate { public: // Notification whether we should close the page, after an explicit call to // AttemptToClosePage. This is called before a cross-site request or before - // a tab/window is closed, to allow the appropriate renderer to approve or - // deny the request. |proceed| indicates whether the user chose to proceed. - virtual void ShouldClosePage(bool proceed) = 0; + // a tab/window is closed (as indicated by the first parameter) to allow the + // appropriate renderer to approve or deny the request. |proceed| indicates + // whether the user chose to proceed. + virtual void ShouldClosePage(bool for_cross_site_transition, + bool proceed) = 0; // Called by ResourceDispatcherHost when a response for a pending cross-site // request is received. The ResourceDispatcherHost will pause the response diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc index 4146f862..959db7f 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.cc +++ b/chrome/browser/tab_contents/render_view_host_manager.cc @@ -211,33 +211,32 @@ void RenderViewHostManager::OnJavaScriptMessageBoxWindowDestroyed() { render_view_host_->JavaScriptMessageBoxWindowDestroyed(); } -void RenderViewHostManager::ShouldClosePage(bool proceed) { - // Should only see this while we have a pending renderer. Otherwise, we - // should ignore. - if (!pending_render_view_host_) { +void RenderViewHostManager::ShouldClosePage(bool for_cross_site_transition, + bool proceed) { + if (for_cross_site_transition) { + if (proceed) { + // Ok to unload the current page, so proceed with the cross-site + // navigation. Note that if navigations are not currently suspended, it + // might be because the renderer was deemed unresponsive and this call was + // already made by ShouldCloseTabOnUnresponsiveRenderer. In that case, it + // is ok to do nothing here. + if (pending_render_view_host_->are_navigations_suspended()) + pending_render_view_host_->SetNavigationsSuspended(false); + } else { + // Current page says to cancel. + CancelPending(); + cross_navigation_pending_ = false; + } + } else { + // Non-cross site transition means closing the entire tab. bool proceed_to_fire_unload; delegate_->BeforeUnloadFiredFromRenderManager(proceed, &proceed_to_fire_unload); if (proceed_to_fire_unload) { // This is not a cross-site navigation, the tab is being closed. - render_view_host_->ClosePage(true, -1, -1); + render_view_host_->ClosePage(false, -1, -1); } - return; - } - - if (proceed) { - // Ok to unload the current page, so proceed with the cross-site - // navigation. Note that if navigations are not currently suspended, it - // might be because the renderer was deemed unresponsive and this call was - // already made by ShouldCloseTabOnUnresponsiveRenderer. In that case, it - // is ok to do nothing here. - if (pending_render_view_host_->are_navigations_suspended()) - pending_render_view_host_->SetNavigationsSuspended(false); - } else { - // Current page says to cancel. - CancelPending(); - cross_navigation_pending_ = false; } } @@ -567,7 +566,7 @@ RenderViewHost* RenderViewHostManager::UpdateRendererStateForNavigate( // Tell the old render view to run its onbeforeunload handler, since it // doesn't otherwise know that the cross-site request is happening. This // will trigger a call to ShouldClosePage with the reply. - render_view_host_->FirePageBeforeUnload(); + render_view_host_->FirePageBeforeUnload(true); return pending_render_view_host_; } else { diff --git a/chrome/browser/tab_contents/render_view_host_manager.h b/chrome/browser/tab_contents/render_view_host_manager.h index 0b2460b..8c8dde2 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.h +++ b/chrome/browser/tab_contents/render_view_host_manager.h @@ -160,7 +160,7 @@ class RenderViewHostManager } // RenderViewHostDelegate::RendererManagement implementation. - virtual void ShouldClosePage(bool proceed); + virtual void ShouldClosePage(bool for_cross_site_transition, bool proceed); virtual void OnCrossSiteResponse(int new_render_process_host_id, int new_request_id); virtual void OnCrossSiteNavigationCanceled(); diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 9b5098f..4546161 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -2204,10 +2204,6 @@ void TabContents::OnJSOutOfMemory() { this, l10n_util::GetString(IDS_JS_OUT_OF_MEMORY_PROMPT), NULL)); } -void TabContents::ShouldClosePage(bool proceed) { - render_manager_.ShouldClosePage(proceed); -} - void TabContents::OnCrossSiteResponse(int new_render_process_host_id, int new_request_id) { // Allows the TabContents to react when a cross-site response is ready to be diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index fc6f76f..5d987a2 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -850,7 +850,6 @@ class TabContents : public PageNavigator, virtual RendererPreferences GetRendererPrefs() const; virtual WebPreferences GetWebkitPrefs(); virtual void OnJSOutOfMemory(); - virtual void ShouldClosePage(bool proceed); virtual void OnCrossSiteResponse(int new_render_process_host_id, int new_request_id); virtual bool CanBlur() const; |