diff options
author | creis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-12 15:37:15 +0000 |
---|---|---|
committer | creis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-12 15:37:15 +0000 |
commit | 992db4c20a063a1009ae28c2cfb5b220701ea301 (patch) | |
tree | 570549ba54be36ea4b81ef446791e1d2e546e6f9 /content/browser/renderer_host | |
parent | 08c5fe5d62ec33cd7215bcdcd532d83866076580 (diff) | |
download | chromium_src-992db4c20a063a1009ae28c2cfb5b220701ea301.zip chromium_src-992db4c20a063a1009ae28c2cfb5b220701ea301.tar.gz chromium_src-992db4c20a063a1009ae28c2cfb5b220701ea301.tar.bz2 |
Support window.opener after a process swap.
Changes:
1. Swap out RVHs instead of closing them, so we can return to the same Frame object if we later come back.
2. Filter out messages from swapped out RVHs, in case any are in-flight.
3. Remove the workaround for navigating away from an app.
4. Update tests to reflect the new sequence of events.
BUG=65953
TEST=AppApiTest.AppProcess
TEST=ResourceDispatcherTest.CrossSiteImmediateLoadOnunloadCookie
Review URL: http://codereview.chromium.org/6319001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85134 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content/browser/renderer_host')
16 files changed, 207 insertions, 100 deletions
diff --git a/content/browser/renderer_host/browser_render_process_host.cc b/content/browser/renderer_host/browser_render_process_host.cc index aa5808f..ef90cd65 100644 --- a/content/browser/renderer_host/browser_render_process_host.cc +++ b/content/browser/renderer_host/browser_render_process_host.cc @@ -412,9 +412,9 @@ void BrowserRenderProcessHost::CancelResourceRequests(int render_widget_id) { widget_helper_->CancelResourceRequests(render_widget_id); } -void BrowserRenderProcessHost::CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { - widget_helper_->CrossSiteClosePageACK(params); +void BrowserRenderProcessHost::CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { + widget_helper_->CrossSiteSwapOutACK(params); } bool BrowserRenderProcessHost::WaitForUpdateMsg( diff --git a/content/browser/renderer_host/browser_render_process_host.h b/content/browser/renderer_host/browser_render_process_host.h index 3f7a23c..ca21e1e 100644 --- a/content/browser/renderer_host/browser_render_process_host.h +++ b/content/browser/renderer_host/browser_render_process_host.h @@ -50,7 +50,7 @@ class BrowserRenderProcessHost : public RenderProcessHost, virtual bool Init(bool is_accessibility_enabled, bool is_extensions_process); virtual int GetNextRoutingID(); virtual void CancelResourceRequests(int render_widget_id); - virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + virtual void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); virtual bool WaitForUpdateMsg(int render_widget_id, const base::TimeDelta& max_delay, IPC::Message* msg); diff --git a/content/browser/renderer_host/mock_render_process_host.cc b/content/browser/renderer_host/mock_render_process_host.cc index 104395f..b4c9a9c 100644 --- a/content/browser/renderer_host/mock_render_process_host.cc +++ b/content/browser/renderer_host/mock_render_process_host.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -36,8 +36,8 @@ int MockRenderProcessHost::GetNextRoutingID() { void MockRenderProcessHost::CancelResourceRequests(int render_widget_id) { } -void MockRenderProcessHost::CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { +void MockRenderProcessHost::CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { } bool MockRenderProcessHost::WaitForUpdateMsg(int render_widget_id, diff --git a/content/browser/renderer_host/mock_render_process_host.h b/content/browser/renderer_host/mock_render_process_host.h index 0e7656c..8faabfb 100644 --- a/content/browser/renderer_host/mock_render_process_host.h +++ b/content/browser/renderer_host/mock_render_process_host.h @@ -39,7 +39,7 @@ class MockRenderProcessHost : public RenderProcessHost { virtual bool Init(bool is_accessibility_enabled, bool is_extensions_process); virtual int GetNextRoutingID(); virtual void CancelResourceRequests(int render_widget_id); - virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + virtual void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); virtual bool WaitForUpdateMsg(int render_widget_id, const base::TimeDelta& max_delay, IPC::Message* msg); diff --git a/content/browser/renderer_host/render_process_host.h b/content/browser/renderer_host/render_process_host.h index b43fbe2..66c3200 100644 --- a/content/browser/renderer_host/render_process_host.h +++ b/content/browser/renderer_host/render_process_host.h @@ -17,7 +17,7 @@ #include "ui/gfx/surface/transport_dib.h" class Profile; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; namespace base { class SharedMemory; @@ -173,12 +173,12 @@ class RenderProcessHost : public IPC::Channel::Sender, // the specified render widget. virtual void CancelResourceRequests(int render_widget_id) = 0; - // Called on the UI thread to simulate a ClosePage_ACK message to the + // Called on the UI thread to simulate a SwapOut_ACK message to the // ResourceDispatcherHost. Necessary for a cross-site request, in the case // that the original RenderViewHost is not live and thus cannot run an - // onunload handler. - virtual void CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) = 0; + // unload handler. + virtual void CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) = 0; // Called on the UI thread to wait for the next UpdateRect message for the // specified render widget. Returns true if successful, and the msg out- diff --git a/content/browser/renderer_host/render_view_host.cc b/content/browser/renderer_host/render_view_host.cc index 47e5457..a51b567 100644 --- a/content/browser/renderer_host/render_view_host.cc +++ b/content/browser/renderer_host/render_view_host.cc @@ -39,6 +39,7 @@ #include "content/common/notification_service.h" #include "content/common/notification_type.h" #include "content/common/result_codes.h" +#include "content/common/swapped_out_messages.h" #include "content/common/url_constants.h" #include "content/common/view_messages.h" #include "net/base/net_util.h" @@ -92,9 +93,10 @@ RenderViewHost::RenderViewHost(SiteInstance* instance, delegate_(delegate), waiting_for_drag_context_response_(false), enabled_bindings_(0), - pending_request_id_(0), + pending_request_id_(-1), navigations_suspended_(false), suspended_nav_message_(NULL), + is_swapped_out_(false), run_modal_reply_msg_(NULL), is_waiting_for_beforeunload_ack_(false), is_waiting_for_unload_ack_(false), @@ -258,11 +260,20 @@ void RenderViewHost::SetNavigationsSuspended(bool suspend) { navigations_suspended_ = suspend; if (!suspend && suspended_nav_message_.get()) { // There's a navigation message waiting to be sent. Now that we're not - // suspended anymore, resume navigation by sending it. + // suspended anymore, resume navigation by sending it. If we were swapped + // out, we should also stop filtering out the IPC messages now. + is_swapped_out_ = false; Send(suspended_nav_message_.release()); } } +void RenderViewHost::CancelSuspendedNavigations() { + // Clear any state if a pending navigation is canceled or pre-empted. + if (suspended_nav_message_.get()) + suspended_nav_message_.reset(); + navigations_suspended_ = false; +} + void RenderViewHost::FirePageBeforeUnload(bool for_cross_site_transition) { if (!IsRenderViewLive()) { // This RenderViewHost doesn't have a live renderer, so just skip running @@ -295,45 +306,66 @@ void RenderViewHost::FirePageBeforeUnload(bool for_cross_site_transition) { } } -void RenderViewHost::ClosePage(bool for_cross_site_transition, - int new_render_process_host_id, - int new_request_id) { - // This will be set back to false in OnClosePageACK, just before we close the - // tab or replace it with a pending RVH. There are some cases (such as 204 - // errors) where we'll continue to show this RVH. +void RenderViewHost::SwapOut(int new_render_process_host_id, + int new_request_id) { + // Start filtering IPC messages to avoid confusing the delegate. This will + // prevent any dialogs from appearing during unload handlers, but we've + // already decided to silence them in crbug.com/68780. We will set it back + // to false in SetNavigationsSuspended if we swap back in. + is_swapped_out_ = true; + + // This will be set back to false in OnSwapOutACK, just before we replace + // this RVH with the pending RVH. is_waiting_for_unload_ack_ = true; // Start the hang monitor in case the renderer hangs in the unload handler. StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS)); - ViewMsg_ClosePage_Params params; + ViewMsg_SwapOut_Params params; params.closing_process_id = process()->id(); params.closing_route_id = routing_id(); - params.for_cross_site_transition = for_cross_site_transition; params.new_render_process_host_id = new_render_process_host_id; params.new_request_id = new_request_id; if (IsRenderViewLive()) { - NotificationService::current()->Notify( - NotificationType::RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW, - Source<RenderViewHost>(this), - NotificationService::NoDetails()); - - Send(new ViewMsg_ClosePage(routing_id(), params)); + Send(new ViewMsg_SwapOut(routing_id(), params)); } else { - // This RenderViewHost doesn't have a live renderer, so just skip closing - // the page. We must notify the ResourceDispatcherHost on the IO thread, + // This RenderViewHost doesn't have a live renderer, so just skip the unload + // event. We must notify the ResourceDispatcherHost on the IO thread, // which we will do through the RenderProcessHost's widget helper. - process()->CrossSiteClosePageACK(params); + process()->CrossSiteSwapOutACK(params); } } -void RenderViewHost::OnClosePageACK(bool for_cross_site_transition) { +void RenderViewHost::OnSwapOutACK() { + // Stop the hang monitor now that the unload handler has finished. StopHangMonitorTimeout(); is_waiting_for_unload_ack_ = false; +} + +void RenderViewHost::WasSwappedOut() { + // Don't bother reporting hung state anymore. + StopHangMonitorTimeout(); - // If this ClosePageACK is not for a cross-site transition, then it is for an - // attempt to close the tab. We have now finished the unload handler and can - // proceed with closing the tab. - if (!for_cross_site_transition) { + // Inform the renderer that it can exit if no one else is using it. + Send(new ViewMsg_WasSwappedOut(routing_id())); +} + +void RenderViewHost::ClosePage() { + // Start the hang monitor in case the renderer hangs in the unload handler. + is_waiting_for_unload_ack_ = true; + StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS)); + + if (IsRenderViewLive()) { + // TODO(creis): Should this be moved to Shutdown? It may not be called for + // RenderViewHosts that have been swapped out. + NotificationService::current()->Notify( + NotificationType::RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW, + Source<RenderViewHost>(this), + NotificationService::NoDetails()); + + Send(new ViewMsg_ClosePage(routing_id())); + } else { + // This RenderViewHost doesn't have a live renderer, so just skip the unload + // event and close the page. ClosePageIgnoringUnloadEvents(); } } @@ -671,6 +703,12 @@ bool RenderViewHost::OnMessageReceived(const IPC::Message& msg) { if (!BrowserMessageFilter::CheckCanDispatchOnUI(msg, this)) return true; + // Filter out most IPC messages if this renderer is swapped out. + // We still want to certain ACKs to keep our state consistent. + if (is_swapped_out_) + if (!content::SwappedOutMessages::CanHandleWhileSwappedOut(msg)) + return true; + { // delegate_->OnMessageReceived can end up deleting |this|, in which case // the destructor for ObserverListBase::Iterator would access the deleted @@ -728,6 +766,7 @@ bool RenderViewHost::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(ViewHostMsg_TakeFocus, OnTakeFocus) IPC_MESSAGE_HANDLER(ViewHostMsg_AddMessageToConsole, OnAddMessageToConsole) IPC_MESSAGE_HANDLER(ViewHostMsg_ShouldClose_ACK, OnMsgShouldCloseACK) + IPC_MESSAGE_HANDLER(ViewHostMsg_ClosePage_ACK, OnMsgClosePageACK) IPC_MESSAGE_HANDLER(ViewHostMsg_SelectionChanged, OnMsgSelectionChanged) IPC_MESSAGE_HANDLER(ViewHostMsg_AccessibilityNotifications, OnAccessibilityNotifications) @@ -797,7 +836,8 @@ void RenderViewHost::OnMsgShowView(int route_id, bool user_gesture) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->ShowCreatedWindow(route_id, disposition, initial_pos, user_gesture); + if (!is_swapped_out_) + view->ShowCreatedWindow(route_id, disposition, initial_pos, user_gesture); Send(new ViewMsg_Move_ACK(route_id)); } } @@ -806,7 +846,8 @@ void RenderViewHost::OnMsgShowWidget(int route_id, const gfx::Rect& initial_pos) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->ShowCreatedWidget(route_id, initial_pos); + if (!is_swapped_out_) + view->ShowCreatedWidget(route_id, initial_pos); Send(new ViewMsg_Move_ACK(route_id)); } } @@ -814,7 +855,8 @@ void RenderViewHost::OnMsgShowWidget(int route_id, void RenderViewHost::OnMsgShowFullscreenWidget(int route_id) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->ShowCreatedFullscreenWidget(route_id); + if (!is_swapped_out_) + view->ShowCreatedFullscreenWidget(route_id); Send(new ViewMsg_Move_ACK(route_id)); } } @@ -936,7 +978,8 @@ void RenderViewHost::OnMsgUpdateEncoding(const std::string& encoding_name) { void RenderViewHost::OnMsgUpdateTargetURL(int32 page_id, const GURL& url) { - delegate_->UpdateTargetURL(page_id, url); + if (!is_swapped_out_) + delegate_->UpdateTargetURL(page_id, url); // Send a notification back to the renderer that we are ready to // receive more target urls. @@ -955,7 +998,8 @@ void RenderViewHost::OnMsgClose() { } void RenderViewHost::OnMsgRequestMove(const gfx::Rect& pos) { - delegate_->RequestMove(pos); + if (!is_swapped_out_) + delegate_->RequestMove(pos); Send(new ViewMsg_Move_ACK(routing_id())); } @@ -1077,8 +1121,8 @@ void RenderViewHost::OnMsgRunJavaScriptMessage( // process input events. process()->set_ignore_input_events(true); StopHangMonitorTimeout(); - delegate_->RunJavaScriptMessage(message, default_prompt, frame_url, flags, - reply_msg, + delegate_->RunJavaScriptMessage(this, message, default_prompt, frame_url, + flags, reply_msg, &are_javascript_messages_suppressed_); } @@ -1089,7 +1133,7 @@ void RenderViewHost::OnMsgRunBeforeUnloadConfirm(const GURL& frame_url, // shouldn't process input events. process()->set_ignore_input_events(true); StopHangMonitorTimeout(); - delegate_->RunBeforeUnloadConfirm(message, reply_msg); + delegate_->RunBeforeUnloadConfirm(this, message, reply_msg); } void RenderViewHost::MediaPlayerActionAt(const gfx::Point& location, @@ -1110,7 +1154,7 @@ void RenderViewHost::OnMsgStartDragging( const gfx::Point& image_offset) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) - view->StartDragging(drop_data, drag_operations_mask, image, image_offset); + view->StartDragging(drop_data, drag_operations_mask, image, image_offset); } void RenderViewHost::OnUpdateDragCursor(WebDragOperation current_op) { @@ -1167,7 +1211,7 @@ void RenderViewHost::OnMsgShouldCloseACK(bool proceed) { // If this renderer navigated while the beforeunload request was in flight, we // may have cleared this state in OnMsgNavigate, in which case we can ignore // this message. - if (!is_waiting_for_beforeunload_ack_) + if (!is_waiting_for_beforeunload_ack_ || is_swapped_out_) return; is_waiting_for_beforeunload_ack_ = false; @@ -1184,6 +1228,10 @@ void RenderViewHost::OnMsgShouldCloseACK(bool proceed) { delegate_->DidCancelLoading(); } +void RenderViewHost::OnMsgClosePageACK() { + ClosePageIgnoringUnloadEvents(); +} + void RenderViewHost::WindowMoveOrResizeStarted() { Send(new ViewMsg_MoveOrResizeStarted(routing_id())); } @@ -1326,7 +1374,7 @@ void RenderViewHost::FilterURL(ChildProcessSecurityPolicy* policy, void RenderViewHost::OnAccessibilityNotifications( const std::vector<ViewHostMsg_AccessibilityNotification_Params>& params) { - if (view()) + if (view() && !is_swapped_out_) view()->OnAccessibilityNotifications(params); if (!params.empty()) { diff --git a/content/browser/renderer_host/render_view_host.h b/content/browser/renderer_host/render_view_host.h index bfd7bf85..add44a4 100644 --- a/content/browser/renderer_host/render_view_host.h +++ b/content/browser/renderer_host/render_view_host.h @@ -163,27 +163,48 @@ class RenderViewHost : public RenderWidgetHost { // are_navigations_suspended() first. void SetNavigationsSuspended(bool suspend); + // Clears any suspended navigation state after a cross-site navigation is + // canceled or suspended. This is important if we later return to this + // RenderViewHost. + void CancelSuspendedNavigations(); + + // Whether this RenderViewHost has been swapped out to be displayed by a + // different process. + bool is_swapped_out() const { return is_swapped_out_; } + // 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. + // result will be returned via ViewMsg_ShouldClose. See also ClosePage and + // SwapOut, which fire the PageUnload event. // // 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); + // Tells the renderer that this RenderView is being swapped out for one in a + // different renderer process. It should run its unload handler and move to + // a blank document. The renderer should preserve the Frame object until it + // exits, in case we come back. The renderer can exit if it has no other + // active RenderViews, but not until WasSwappedOut is called (when it is no + // longer visible). + // + // Please see ViewMsg_SwapOut_Params in view_messages.h for a description + // of the parameters. + void SwapOut(int new_render_process_host_id, int new_request_id); + + // Called by ResourceDispatcherHost after the SwapOutACK is received. + void OnSwapOutACK(); + + // Called to notify the renderer that it has been visibly swapped out and + // replaced by another RenderViewHost, after an earlier call to SwapOut. + // It is now safe for the process to exit if there are no other active + // RenderViews. + void WasSwappedOut(); + // Causes the renderer to close the current page, including running its // onunload event handler. A ClosePage_ACK message will be sent to the // ResourceDispatcherHost when it is finished. - // - // Please see ViewMsg_ClosePage in resource_messages_internal.h for a - // description of the parameters. - void ClosePage(bool for_cross_site_transition, - int new_render_process_host_id, - int new_request_id); - - // Called by ResourceDispatcherHost after the ClosePageACK is received. - void OnClosePageACK(bool for_cross_site_transition); + void ClosePage(); // Close the page ignoring whether it has unload events registers. // This is called after the beforeunload and unload events have fired @@ -516,6 +537,7 @@ class RenderViewHost : public RenderWidgetHost { void OnUpdateInspectorSetting(const std::string& key, const std::string& value); void OnMsgShouldCloseACK(bool proceed); + void OnMsgClosePageACK(); void OnAccessibilityNotifications( const std::vector<ViewHostMsg_AccessibilityNotification_Params>& params); @@ -570,6 +592,10 @@ class RenderViewHost : public RenderWidgetHost { // navigation occurs. scoped_ptr<ViewMsg_Navigate> suspended_nav_message_; + // Whether this RenderViewHost is currently swapped out, such that the view is + // being rendered by another process. + bool is_swapped_out_; + // If we were asked to RunModal, then this will hold the reply_msg that we // must return to the renderer to unblock it. IPC::Message* run_modal_reply_msg_; diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h index 9e8cb54..cb48801 100644 --- a/content/browser/renderer_host/render_view_host_delegate.h +++ b/content/browser/renderer_host/render_view_host_delegate.h @@ -476,14 +476,16 @@ class RenderViewHostDelegate : public IPC::Channel::Listener { const std::string& target) {} // A javascript message, confirmation or prompt should be shown. - virtual void RunJavaScriptMessage(const std::wstring& message, + virtual void RunJavaScriptMessage(const RenderViewHost* rvh, + const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, const int flags, IPC::Message* reply_msg, bool* did_suppress_message) {} - virtual void RunBeforeUnloadConfirm(const std::wstring& message, + virtual void RunBeforeUnloadConfirm(const RenderViewHost* rvh, + const std::wstring& message, IPC::Message* reply_msg) {} // |url| is assigned to a server that can provide alternate error pages. If diff --git a/content/browser/renderer_host/render_view_host_unittest.cc b/content/browser/renderer_host/render_view_host_unittest.cc index 26b8e39..315cdbc 100644 --- a/content/browser/renderer_host/render_view_host_unittest.cc +++ b/content/browser/renderer_host/render_view_host_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -48,10 +48,9 @@ TEST_F(RenderViewHostTest, ResetUnloadOnReload) { NavigateAndCommit(url1); controller().LoadURL(url2, GURL(), PageTransition::LINK); // Simulate the ClosePage call which is normally sent by the net::URLRequest. - rvh()->ClosePage(true, 0, 0); - // Needed so that navigations are not suspended on the RVH. Normally handled - // by way of ViewHostMsg_ShouldClose_ACK. - contents()->render_manager()->ShouldClosePage(true, true); + rvh()->ClosePage(); + // Needed so that navigations are not suspended on the RVH. + rvh()->SendShouldCloseACK(true); contents()->Stop(); controller().Reload(false); EXPECT_FALSE(rvh()->is_waiting_for_unload_ack()); diff --git a/content/browser/renderer_host/render_widget_helper.cc b/content/browser/renderer_host/render_widget_helper.cc index 161bf88..f297110 100644 --- a/content/browser/renderer_host/render_widget_helper.cc +++ b/content/browser/renderer_host/render_widget_helper.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -85,12 +85,12 @@ void RenderWidgetHelper::CancelResourceRequests(int render_widget_id) { render_widget_id)); } -void RenderWidgetHelper::CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { +void RenderWidgetHelper::CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, NewRunnableMethod(this, - &RenderWidgetHelper::OnCrossSiteClosePageACK, + &RenderWidgetHelper::OnCrossSiteSwapOutACK, params)); } @@ -202,9 +202,9 @@ void RenderWidgetHelper::OnCancelResourceRequests( render_process_id_, render_widget_id); } -void RenderWidgetHelper::OnCrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { - resource_dispatcher_host_->OnClosePageACK(params); +void RenderWidgetHelper::OnCrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { + resource_dispatcher_host_->OnSwapOutACK(params); } void RenderWidgetHelper::CreateNewWindow( diff --git a/content/browser/renderer_host/render_widget_helper.h b/content/browser/renderer_host/render_widget_helper.h index 6576700..0c7f695 100644 --- a/content/browser/renderer_host/render_widget_helper.h +++ b/content/browser/renderer_host/render_widget_helper.h @@ -28,7 +28,7 @@ class TimeDelta; class ResourceDispatcherHost; struct ViewHostMsg_CreateWindow_Params; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; // Instantiated per RenderProcessHost to provide various optimizations on @@ -107,7 +107,7 @@ class RenderWidgetHelper // corresponding functions in RenderProcessHost. See those declarations // for documentation. void CancelResourceRequests(int render_widget_id); - void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); bool WaitForUpdateMsg(int render_widget_id, const base::TimeDelta& max_delay, IPC::Message* msg); @@ -182,7 +182,7 @@ class RenderWidgetHelper void OnCancelResourceRequests(int render_widget_id); // Called on the IO thread to resume a cross-site response. - void OnCrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + void OnCrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); #if defined(OS_MACOSX) // Called on destruction to release all allocated transport DIBs diff --git a/content/browser/renderer_host/resource_dispatcher_host.cc b/content/browser/renderer_host/resource_dispatcher_host.cc index ce775c7..a9525dd 100644 --- a/content/browser/renderer_host/resource_dispatcher_host.cc +++ b/content/browser/renderer_host/resource_dispatcher_host.cc @@ -345,7 +345,7 @@ bool ResourceDispatcherHost::OnMessageReceived(const IPC::Message& message, IPC_MESSAGE_HANDLER(ResourceHostMsg_UploadProgress_ACK, OnUploadProgressACK) IPC_MESSAGE_HANDLER(ResourceHostMsg_CancelRequest, OnCancelRequest) IPC_MESSAGE_HANDLER(ResourceHostMsg_FollowRedirect, OnFollowRedirect) - IPC_MESSAGE_HANDLER(ViewHostMsg_ClosePage_ACK, OnClosePageACK) + IPC_MESSAGE_HANDLER(ViewHostMsg_SwapOut_ACK, OnSwapOutACK) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP_EX() @@ -717,30 +717,23 @@ ResourceDispatcherHost::CreateRequestInfoForBrowserRequest( &context); } -void ResourceDispatcherHost::OnClosePageACK( - const ViewMsg_ClosePage_Params& params) { - if (params.for_cross_site_transition) { - // Closes for cross-site transitions are handled such that the cross-site - // transition continues. - GlobalRequestID global_id(params.new_render_process_host_id, - params.new_request_id); - PendingRequestList::iterator i = pending_requests_.find(global_id); - if (i != pending_requests_.end()) { - // The response we were meant to resume could have already been canceled. - ResourceDispatcherHostRequestInfo* info = InfoForRequest(i->second); - if (info->cross_site_handler()) - info->cross_site_handler()->ResumeResponse(); - } - } else { - // This is a tab close, so we will close the tab in OnClosePageACK. - DCHECK(params.new_render_process_host_id == -1); - DCHECK(params.new_request_id == -1); +void ResourceDispatcherHost::OnSwapOutACK( + const ViewMsg_SwapOut_Params& params) { + // Closes for cross-site transitions are handled such that the cross-site + // transition continues. + GlobalRequestID global_id(params.new_render_process_host_id, + params.new_request_id); + PendingRequestList::iterator i = pending_requests_.find(global_id); + if (i != pending_requests_.end()) { + // The response we were meant to resume could have already been canceled. + ResourceDispatcherHostRequestInfo* info = InfoForRequest(i->second); + if (info->cross_site_handler()) + info->cross_site_handler()->ResumeResponse(); } // Update the RenderViewHost's internal state after the ACK. CallRenderViewHost(params.closing_process_id, params.closing_route_id, - &RenderViewHost::OnClosePageACK, - params.for_cross_site_transition); + &RenderViewHost::OnSwapOutACK); } // We are explicitly forcing the download of 'url'. diff --git a/content/browser/renderer_host/resource_dispatcher_host.h b/content/browser/renderer_host/resource_dispatcher_host.h index a8b621e..2354a23 100644 --- a/content/browser/renderer_host/resource_dispatcher_host.h +++ b/content/browser/renderer_host/resource_dispatcher_host.h @@ -44,7 +44,7 @@ class WebKitThread; struct DownloadSaveInfo; struct GlobalRequestID; struct ResourceHostMsg_Request; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; namespace content { class ResourceContext; @@ -157,8 +157,8 @@ class ResourceDispatcherHost : public net::URLRequest::Delegate { return webkit_thread_.get(); } - // Called when the onunload handler for a cross-site request has finished. - void OnClosePageACK(const ViewMsg_ClosePage_Params& params); + // Called when the unload handler for a cross-site request has finished. + void OnSwapOutACK(const ViewMsg_SwapOut_Params& params); // Force cancels any pending requests for the given process. void CancelRequestsForProcess(int process_unique_id); diff --git a/content/browser/renderer_host/resource_dispatcher_host_uitest.cc b/content/browser/renderer_host/resource_dispatcher_host_uitest.cc index 5de1430..7f21171 100644 --- a/content/browser/renderer_host/resource_dispatcher_host_uitest.cc +++ b/content/browser/renderer_host/resource_dispatcher_host_uitest.cc @@ -193,6 +193,38 @@ TEST_F(ResourceDispatcherTest, CrossSiteOnunloadCookie) { ASSERT_STREQ("foo", value_result.c_str()); } +// Tests that onunload is run for cross-site requests to URLs that complete +// without network loads (e.g., about:blank, data URLs). +TEST_F(ResourceDispatcherTest, CrossSiteImmediateLoadOnunloadCookie) { + net::TestServer test_server(net::TestServer::TYPE_HTTP, + FilePath(FILE_PATH_LITERAL("chrome/test/data"))); + ASSERT_TRUE(test_server.Start()); + + scoped_refptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0)); + ASSERT_TRUE(browser_proxy.get()); + scoped_refptr<TabProxy> tab(browser_proxy->GetActiveTab()); + ASSERT_TRUE(tab.get()); + + GURL url(test_server.GetURL("files/onunload_cookie.html")); + ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab->NavigateToURL(url)); + + // Confirm that the page has loaded (since it changes its title during load). + std::wstring tab_title; + EXPECT_TRUE(tab->GetTabTitle(&tab_title)); + EXPECT_EQ(L"set cookie on unload", tab_title); + + // Navigate to a cross-site page that loads immediately without making a + // network request. The unload event should still be run. + ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, + tab->NavigateToURL(GURL("about:blank"))); + + // Check that the cookie was set. + std::string value_result; + ASSERT_TRUE(tab->GetCookieByName(url, "onunloadCookie", &value_result)); + ASSERT_FALSE(value_result.empty()); + ASSERT_STREQ("foo", value_result.c_str()); +} + // Tests that the unload handler is not run for 204 responses. TEST_F(ResourceDispatcherTest, CrossSiteNoUnloadOn204) { net::TestServer test_server(net::TestServer::TYPE_HTTP, diff --git a/content/browser/renderer_host/test_render_view_host.cc b/content/browser/renderer_host/test_render_view_host.cc index 0ff0194..fd488d2 100644 --- a/content/browser/renderer_host/test_render_view_host.cc +++ b/content/browser/renderer_host/test_render_view_host.cc @@ -108,6 +108,10 @@ void TestRenderViewHost::SendNavigateWithTransition( OnMsgNavigate(msg); } +void TestRenderViewHost::SendShouldCloseACK(bool proceed) { + OnMsgShouldCloseACK(proceed); +} + void TestRenderViewHost::set_simulate_fetch_via_proxy(bool proxy) { simulate_fetch_via_proxy_ = proxy; } diff --git a/content/browser/renderer_host/test_render_view_host.h b/content/browser/renderer_host/test_render_view_host.h index a860806..37312cc 100644 --- a/content/browser/renderer_host/test_render_view_host.h +++ b/content/browser/renderer_host/test_render_view_host.h @@ -182,6 +182,9 @@ class TestRenderViewHost : public RenderViewHost { void SendNavigateWithTransition(int page_id, const GURL& url, PageTransition::Type transition); + // Calls OnMsgShouldCloseACK on the RenderViewHost with the given parameter. + void SendShouldCloseACK(bool proceed); + // If set, *delete_counter is incremented when this object destructs. void set_delete_counter(int* delete_counter) { delete_counter_ = delete_counter; |