diff options
Diffstat (limited to 'content/browser')
19 files changed, 540 insertions, 606 deletions
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc index 3415e46..077cf08 100644 --- a/content/browser/frame_host/frame_tree.cc +++ b/content/browser/frame_host/frame_tree.cc @@ -237,11 +237,12 @@ RenderViewHostImpl* FrameTree::CreateRenderViewHost(SiteInstance* site_instance, RenderViewHostMap::iterator iter = render_view_host_map_.find(site_instance->GetId()); if (iter != render_view_host_map_.end()) { - // If a RenderViewHost is pending shutdown for this |site_instance|, put it - // in the map of RenderViewHosts pending shutdown. Otherwise return the - // existing RenderViewHost for the SiteInstance. - if (iter->second->rvh_state() == - RenderViewHostImpl::STATE_PENDING_SHUTDOWN) { + // If a RenderViewHost's main frame is pending shutdown for this + // |site_instance|, put it in the map of RenderViewHosts pending shutdown. + // Otherwise return the existing RenderViewHost for the SiteInstance. + RenderFrameHost* main_frame = iter->second->GetMainFrame(); + if (static_cast<RenderFrameHostImpl*>(main_frame)->rfh_state() == + RenderFrameHostImpl::STATE_PENDING_SHUTDOWN) { render_view_host_pending_shutdown_map_.insert( std::pair<int, RenderViewHostImpl*>(site_instance->GetId(), iter->second)); diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc index 8961f43..ec5b5f2 100644 --- a/content/browser/frame_host/navigation_controller_impl_unittest.cc +++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc @@ -418,7 +418,7 @@ TEST_F(NavigationControllerTest, LoadURL) { // Simulate the beforeunload ack for the cross-site transition, and then the // commit. - test_rvh()->SendBeforeUnloadACK(true); + main_test_rfh()->SendBeforeUnloadACK(true); contents()->GetPendingMainFrame()->SendNavigate(1, url2); EXPECT_EQ(1U, navigation_entry_committed_counter_); navigation_entry_committed_counter_ = 0; @@ -470,7 +470,7 @@ TEST_F(NavigationControllerTest, LoadURLSameTime) { // Simulate the beforeunload ack for the cross-site transition, and then the // commit. - test_rvh()->SendBeforeUnloadACK(true); + main_test_rfh()->SendBeforeUnloadACK(true); main_test_rfh()->SendNavigate(1, url2); EXPECT_EQ(1U, navigation_entry_committed_counter_); navigation_entry_committed_counter_ = 0; @@ -756,7 +756,7 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { EXPECT_EQ(0U, notifications.size()); // After the beforeunload but before it commits, do a new navigation. - test_rvh()->SendBeforeUnloadACK(true); + main_test_rfh()->SendBeforeUnloadACK(true); const GURL kNewURL("http://see"); contents()->GetPendingMainFrame()->SendNavigate(3, kNewURL); @@ -835,7 +835,7 @@ TEST_F(NavigationControllerTest, LoadURL_PrivilegedPending) { const GURL kExistingURL2("http://foo/eh"); controller.LoadURL( kExistingURL2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - test_rvh()->SendBeforeUnloadACK(true); + main_test_rfh()->SendBeforeUnloadACK(true); TestRenderFrameHost* foo_rfh = contents()->GetPendingMainFrame(); foo_rfh->SendNavigate(1, kExistingURL2); EXPECT_EQ(1U, navigation_entry_committed_counter_); @@ -844,7 +844,7 @@ TEST_F(NavigationControllerTest, LoadURL_PrivilegedPending) { // Now make a pending back/forward navigation to a privileged entry. // The zeroth entry should be pending. controller.GoBack(); - foo_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + foo_rfh->SendBeforeUnloadACK(true); EXPECT_EQ(0U, notifications.size()); EXPECT_EQ(0, controller.GetPendingEntryIndex()); EXPECT_EQ(1, controller.GetLastCommittedEntryIndex()); @@ -1095,19 +1095,18 @@ TEST_F(NavigationControllerTest, LoadURL_WithBindings) { EXPECT_EQ(0, NavigationEntryImpl::FromNavigationEntry( controller.GetLastCommittedEntry())->bindings()); - // Manually increase the number of active views in the SiteInstance + // Manually increase the number of active frames in the SiteInstance // that orig_rfh belongs to, to prevent it from being destroyed when // it gets swapped out, so that we can reuse orig_rfh when the // controller goes back. - static_cast<SiteInstanceImpl*>(orig_rfh->GetSiteInstance())-> - increment_active_view_count(); + orig_rfh->GetSiteInstance()->increment_active_frame_count(); // Navigate to a second URL, simulate the beforeunload ack for the cross-site // transition, and set bindings on the pending RenderViewHost to simulate a // privileged url. controller.LoadURL( url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - orig_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + orig_rfh->SendBeforeUnloadACK(true); TestRenderFrameHost* new_rfh = contents()->GetPendingMainFrame(); new_rfh->GetRenderViewHost()->AllowBindings(1); new_rfh->SendNavigate(1, url2); @@ -1121,7 +1120,7 @@ TEST_F(NavigationControllerTest, LoadURL_WithBindings) { // Going back, the first entry should still appear unprivileged. controller.GoBack(); - new_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + new_rfh->SendBeforeUnloadACK(true); orig_rfh->SendNavigate(0, url1); EXPECT_EQ(0, controller.GetLastCommittedEntryIndex()); EXPECT_EQ(0, NavigationEntryImpl::FromNavigationEntry( diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc index fc0bdeb..11b322e 100644 --- a/content/browser/frame_host/navigator_impl.cc +++ b/content/browser/frame_host/navigator_impl.cc @@ -551,9 +551,10 @@ void NavigatorImpl::RequestOpenURL( SiteInstance* current_site_instance = GetRenderManager(render_frame_host)->current_frame_host()-> GetSiteInstance(); - // If this came from a swapped out RenderViewHost, we only allow the request + // If this came from a swapped out RenderFrameHost, we only allow the request // if we are still in the same BrowsingInstance. - if (render_frame_host->render_view_host()->IsSwappedOut() && + // TODO(creis): Move this to RenderFrameProxyHost::OpenURL. + if (render_frame_host->is_swapped_out() && !render_frame_host->GetSiteInstance()->IsRelatedSiteInstance( current_site_instance)) { return; diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index a614c7e..7be414e 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -149,6 +149,12 @@ base::i18n::TextDirection WebTextDirectionToChromeTextDirection( } // namespace +// static +bool RenderFrameHostImpl::IsRFHStateActive(RenderFrameHostImplState rfh_state) { + return rfh_state == STATE_DEFAULT; +} + +// static RenderFrameHost* RenderFrameHost::FromID(int render_process_id, int render_frame_id) { return RenderFrameHostImpl::FromID(render_process_id, render_frame_id); @@ -177,9 +183,10 @@ RenderFrameHostImpl::RenderFrameHostImpl(RenderViewHostImpl* render_view_host, frame_tree_(frame_tree), frame_tree_node_(frame_tree_node), routing_id_(routing_id), - is_swapped_out_(is_swapped_out), render_frame_created_(false), navigations_suspended_(false), + is_waiting_for_beforeunload_ack_(false), + unload_ack_is_for_cross_site_transition_(false), weak_ptr_factory_(this) { frame_tree_->RegisterRenderFrameHost(this); GetProcess()->AddRoute(routing_id_, this); @@ -187,6 +194,13 @@ RenderFrameHostImpl::RenderFrameHostImpl(RenderViewHostImpl* render_view_host, RenderFrameHostID(GetProcess()->GetID(), routing_id_), this)); + if (is_swapped_out) { + rfh_state_ = STATE_SWAPPED_OUT; + } else { + rfh_state_ = STATE_DEFAULT; + GetSiteInstance()->increment_active_frame_count(); + } + if (GetProcess()->GetServiceRegistry()) { RenderFrameSetupPtr setup; GetProcess()->GetServiceRegistry()->ConnectToRemoteService(&setup); @@ -196,6 +210,9 @@ RenderFrameHostImpl::RenderFrameHostImpl(RenderViewHostImpl* render_view_host, service_registry_.BindRemoteServiceProvider( service_provider.PassMessagePipe()); } + + swapout_event_monitor_timeout_.reset(new TimeoutMonitor(base::Bind( + &RenderFrameHostImpl::OnSwappedOut, weak_ptr_factory_.GetWeakPtr()))); } RenderFrameHostImpl::~RenderFrameHostImpl() { @@ -208,6 +225,11 @@ RenderFrameHostImpl::~RenderFrameHostImpl() { FrameAccessibility::GetInstance()->OnRenderFrameHostDestroyed(this); + // If this was swapped out, it already decremented the active frame count of + // the SiteInstance it belongs to. + if (IsRFHStateActive(rfh_state_)) + GetSiteInstance()->decrement_active_frame_count(); + // Notify the FrameTree that this RFH is going away, allowing it to shut down // the corresponding RenderViewHost if it is no longer needed. frame_tree_->UnregisterRenderFrameHost(this); @@ -217,7 +239,7 @@ int RenderFrameHostImpl::GetRoutingID() { return routing_id_; } -SiteInstance* RenderFrameHostImpl::GetSiteInstance() { +SiteInstanceImpl* RenderFrameHostImpl::GetSiteInstance() { return render_view_host_->GetSiteInstance(); } @@ -300,7 +322,7 @@ bool RenderFrameHostImpl::Send(IPC::Message* message) { // Route IPCs through the RenderFrameProxyHost when in swapped out state. // Note: For subframes in --site-per-process mode, we don't use swapped out // RenderFrameHosts. - if (frame_tree_node_->IsMainFrame() && render_view_host_->IsSwappedOut()) { + if (frame_tree_node_->IsMainFrame() && is_swapped_out()) { DCHECK(render_frame_proxy_host_); return render_frame_proxy_host_->Send(message); } @@ -309,12 +331,9 @@ bool RenderFrameHostImpl::Send(IPC::Message* message) { } bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) { - // Filter out most IPC messages if this renderer is swapped out. + // Filter out most IPC messages if this frame is swapped out. // We still want to handle certain ACKs to keep our state consistent. - // TODO(nasko): Only check RenderViewHost state, as this object's own state - // isn't yet properly updated. Transition this check once the swapped out - // state is correct in RenderFrameHost itself. - if (render_view_host_->IsSwappedOut()) { + if (is_swapped_out()) { if (!SwappedOutMessages::CanHandleWhileSwappedOut(msg)) { // If this is a synchronous message and we decided not to handle it, // we must send an error reply, or else the renderer will be stuck @@ -669,8 +688,8 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) { // We do not want to cancel the pending navigation in this case, since the // old page will soon be stopped. Instead, treat this as a beforeunload ack // to allow the pending navigation to continue. - if (render_view_host_->is_waiting_for_beforeunload_ack_ && - render_view_host_->unload_ack_is_for_cross_site_transition_ && + if (is_waiting_for_beforeunload_ack_ && + unload_ack_is_for_cross_site_transition_ && ui::PageTransitionIsMainFrame(validated_params.transition)) { OnBeforeUnloadACK(true, send_before_unload_start_time_, base::TimeTicks::Now()); @@ -682,7 +701,7 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) { // unload request. It will either respond to the unload request soon or our // timer will expire. Either way, we should ignore this message, because we // have already committed to closing this renderer. - if (render_view_host_->IsWaitingForUnloadACK()) + if (IsWaitingForUnloadACK()) return; RenderProcessHost* process = GetProcess(); @@ -764,20 +783,14 @@ void RenderFrameHostImpl::SwapOut(RenderFrameProxyHost* proxy) { // to be fixed when RenderViewHostImpl::OnSwapOut moves to RenderFrameHost. TRACE_EVENT_ASYNC_BEGIN0("navigation", "RenderFrameHostImpl::SwapOut", this); - // TODO(creis): Move swapped out state to RFH. Until then, only update it - // when swapping out the main frame. - if (!GetParent()) { - // If this RenderViewHost is not in the default state, it must have already - // gone through this, therefore just return. - if (render_view_host_->rvh_state_ != RenderViewHostImpl::STATE_DEFAULT) - return; + // If this RenderFrameHost is not in the default state, it must have already + // gone through this, therefore just return. + if (rfh_state_ != RenderFrameHostImpl::STATE_DEFAULT) + return; - render_view_host_->SetState( - RenderViewHostImpl::STATE_PENDING_SWAP_OUT); - render_view_host_->unload_event_monitor_timeout_->Start( - base::TimeDelta::FromMilliseconds( - RenderViewHostImpl::kUnloadTimeoutMS)); - } + SetState(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT); + swapout_event_monitor_timeout_->Start( + base::TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS)); set_render_frame_proxy_host(proxy); @@ -786,8 +799,6 @@ void RenderFrameHostImpl::SwapOut(RenderFrameProxyHost* proxy) { if (!GetParent()) delegate_->SwappedOut(this); - else - set_swapped_out(true); } void RenderFrameHostImpl::OnBeforeUnloadACK( @@ -796,12 +807,12 @@ void RenderFrameHostImpl::OnBeforeUnloadACK( const base::TimeTicks& renderer_before_unload_end_time) { TRACE_EVENT_ASYNC_END0( "navigation", "RenderFrameHostImpl::BeforeUnload", this); - // TODO(creis): Support properly beforeunload on subframes. For now just - // pretend that the handler ran and allowed the navigation to proceed. + // TODO(creis): Support beforeunload on subframes. For now just pretend that + // the handler ran and allowed the navigation to proceed. if (GetParent()) { - render_view_host_->is_waiting_for_beforeunload_ack_ = false; + is_waiting_for_beforeunload_ack_ = false; frame_tree_node_->render_manager()->OnBeforeUnloadACK( - render_view_host_->unload_ack_is_for_cross_site_transition_, proceed, + unload_ack_is_for_cross_site_transition_, proceed, renderer_before_unload_end_time); return; } @@ -816,11 +827,11 @@ void RenderFrameHostImpl::OnBeforeUnloadACK( // happen when pending cross-site navigation is canceled by a second one just // before OnDidCommitProvisionalLoad while current RVH is waiting for commit // but second navigation is started from the beginning. - if (!render_view_host_->is_waiting_for_beforeunload_ack_) { + if (!is_waiting_for_beforeunload_ack_) { return; } - render_view_host_->is_waiting_for_beforeunload_ack_ = false; + is_waiting_for_beforeunload_ack_ = false; base::TimeTicks before_unload_end_time; if (!send_before_unload_start_time_.is_null() && @@ -858,7 +869,7 @@ void RenderFrameHostImpl::OnBeforeUnloadACK( is_skew_additive); } frame_tree_node_->render_manager()->OnBeforeUnloadACK( - render_view_host_->unload_ack_is_for_cross_site_transition_, proceed, + unload_ack_is_for_cross_site_transition_, proceed, before_unload_end_time); // If canceled, notify the delegate to cancel its pending navigation entry. @@ -866,15 +877,36 @@ void RenderFrameHostImpl::OnBeforeUnloadACK( render_view_host_->GetDelegate()->DidCancelLoading(); } +bool RenderFrameHostImpl::IsWaitingForUnloadACK() const { + return render_view_host_->is_waiting_for_close_ack_ || + rfh_state_ == STATE_PENDING_SHUTDOWN || + rfh_state_ == STATE_PENDING_SWAP_OUT; +} + void RenderFrameHostImpl::OnSwapOutACK() { - OnSwappedOut(false); - TRACE_EVENT_ASYNC_END0("navigation", "RenderFrameHostImpl::SwapOut", this); + OnSwappedOut(); } -void RenderFrameHostImpl::OnSwappedOut(bool timed_out) { - // For now, we only need to update the RVH state machine for top-level swaps. - if (!GetParent()) - render_view_host_->OnSwappedOut(timed_out); +void RenderFrameHostImpl::OnSwappedOut() { + // Ignore spurious swap out ack. + if (rfh_state_ != STATE_PENDING_SWAP_OUT && + rfh_state_ != STATE_PENDING_SHUTDOWN) + return; + + TRACE_EVENT_ASYNC_END0("navigation", "RenderFrameHostImpl::SwapOut", this); + swapout_event_monitor_timeout_->Stop(); + + switch (rfh_state_) { + case STATE_PENDING_SWAP_OUT: + SetState(STATE_SWAPPED_OUT); + break; + case STATE_PENDING_SHUTDOWN: + DCHECK(!pending_shutdown_on_swap_out_.is_null()); + pending_shutdown_on_swap_out_.Run(); + break; + default: + NOTREACHED(); + } } void RenderFrameHostImpl::OnContextMenu(const ContextMenuParams& params) { @@ -931,7 +963,7 @@ void RenderFrameHostImpl::OnRunBeforeUnloadConfirm( const base::string16& message, bool is_reload, IPC::Message* reply_msg) { - // While a JS before unload dialog is showing, tabs in the same process + // While a JS beforeunload dialog is showing, tabs in the same process // shouldn't process input events. GetProcess()->SetIgnoreInputEvents(true); render_view_host_->StopHangMonitorTimeout(); @@ -1035,7 +1067,7 @@ void RenderFrameHostImpl::OnAccessibilityEvents( AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode(); if ((accessibility_mode != AccessibilityModeOff) && view && - RenderViewHostImpl::IsRVHStateActive(render_view_host_->rvh_state())) { + RenderFrameHostImpl::IsRFHStateActive(rfh_state())) { if (accessibility_mode & AccessibilityModeFlagPlatform) { GetOrCreateBrowserAccessibilityManager(); if (browser_accessibility_manager_) @@ -1106,8 +1138,7 @@ void RenderFrameHostImpl::OnAccessibilityLocationChanges( const std::vector<AccessibilityHostMsg_LocationChangeParams>& params) { RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>( render_view_host_->GetView()); - if (view && - RenderViewHostImpl::IsRVHStateActive(render_view_host_->rvh_state())) { + if (view && RenderFrameHostImpl::IsRFHStateActive(rfh_state())) { AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode(); if (accessibility_mode & AccessibilityModeFlagPlatform) { if (!browser_accessibility_manager_) { @@ -1146,8 +1177,39 @@ void RenderFrameHostImpl::OnHidePopup() { } #endif +void RenderFrameHostImpl::SetState(RenderFrameHostImplState rfh_state) { + // We update the number of RenderFrameHosts in a SiteInstance when the swapped + // out status of a RenderFrameHost gets flipped to/from active. + if (!IsRFHStateActive(rfh_state_) && IsRFHStateActive(rfh_state)) + GetSiteInstance()->increment_active_frame_count(); + else if (IsRFHStateActive(rfh_state_) && !IsRFHStateActive(rfh_state)) + GetSiteInstance()->decrement_active_frame_count(); + + // The active and swapped out state of the RVH is determined by its main + // frame, since subframes should have their own widgets. + if (frame_tree_node_->IsMainFrame()) { + render_view_host_->set_is_active(IsRFHStateActive(rfh_state)); + render_view_host_->set_is_swapped_out(rfh_state == STATE_SWAPPED_OUT); + } + + // Whenever we change the RFH state to and from active or swapped out state, + // we should not be waiting for beforeunload or close acks. We clear them + // here to be safe, since they can cause navigations to be ignored in + // OnDidCommitProvisionalLoad. + // TODO(creis): Move is_waiting_for_beforeunload_ack_ into the state machine. + if (rfh_state == STATE_DEFAULT || + rfh_state == STATE_SWAPPED_OUT || + rfh_state_ == STATE_DEFAULT || + rfh_state_ == STATE_SWAPPED_OUT) { + is_waiting_for_beforeunload_ack_ = false; + render_view_host_->is_waiting_for_close_ack_ = false; + } + rfh_state_ = rfh_state; +} + void RenderFrameHostImpl::SetPendingShutdown(const base::Closure& on_swap_out) { - render_view_host_->SetPendingShutdown(on_swap_out); + pending_shutdown_on_swap_out_ = on_swap_out; + SetState(STATE_PENDING_SHUTDOWN); } bool RenderFrameHostImpl::CanCommitURL(const GURL& url) { @@ -1186,8 +1248,8 @@ void RenderFrameHostImpl::Navigate(const FrameMsg_Navigate_Params& params) { suspended_nav_params_.reset(new FrameMsg_Navigate_Params(params)); } else { // Get back to a clean state, in case we start a new navigation without - // completing a RVH swap or unload handler. - render_view_host_->SetState(RenderViewHostImpl::STATE_DEFAULT); + // completing a RFH swap or unload handler. + SetState(RenderFrameHostImpl::STATE_DEFAULT); Send(new FrameMsg_Navigate(routing_id_, params)); } @@ -1230,9 +1292,8 @@ void RenderFrameHostImpl::DispatchBeforeUnload(bool for_cross_site_transition) { // TODO(creis): Support subframes. if (GetParent() || !IsRenderFrameLive()) { // We don't have a live renderer, so just skip running beforeunload. - render_view_host_->is_waiting_for_beforeunload_ack_ = true; - render_view_host_->unload_ack_is_for_cross_site_transition_ = - for_cross_site_transition; + is_waiting_for_beforeunload_ack_ = true; + unload_ack_is_for_cross_site_transition_ = for_cross_site_transition; base::TimeTicks now = base::TimeTicks::Now(); OnBeforeUnloadACK(true, now, now); return; @@ -1241,22 +1302,20 @@ void RenderFrameHostImpl::DispatchBeforeUnload(bool for_cross_site_transition) { // 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), and we only send the message once. - if (render_view_host_->is_waiting_for_beforeunload_ack_) { + if (is_waiting_for_beforeunload_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. - render_view_host_->unload_ack_is_for_cross_site_transition_ = - render_view_host_->unload_ack_is_for_cross_site_transition_ && - for_cross_site_transition; + 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. - render_view_host_->is_waiting_for_beforeunload_ack_ = true; - render_view_host_->unload_ack_is_for_cross_site_transition_ = - for_cross_site_transition; + is_waiting_for_beforeunload_ack_ = true; + unload_ack_is_for_cross_site_transition_ = for_cross_site_transition; // Increment the in-flight event count, to ensure that input events won't // cancel the timeout timer. render_view_host_->increment_in_flight_event_count(); @@ -1282,8 +1341,7 @@ void RenderFrameHostImpl::JavaScriptDialogClosed( const base::string16& user_input, bool dialog_was_suppressed) { GetProcess()->SetIgnoreInputEvents(false); - bool is_waiting = render_view_host_->is_waiting_for_beforeunload_ack() || - render_view_host_->IsWaitingForUnloadACK(); + bool is_waiting = is_waiting_for_beforeunload_ack_ || IsWaitingForUnloadACK(); // If we are executing as part of (before)unload event handling, we don't // want to use the regular hung_renderer_delay_ms_ if the user has agreed to @@ -1306,10 +1364,7 @@ void RenderFrameHostImpl::JavaScriptDialogClosed( // This must be done after sending the reply since RenderView can't close // correctly while waiting for a response. if (is_waiting && dialog_was_suppressed) - render_view_host_->delegate_->RendererUnresponsive( - render_view_host_, - render_view_host_->is_waiting_for_beforeunload_ack(), - render_view_host_->IsWaitingForUnloadACK()); + render_view_host_->delegate_->RendererUnresponsive(render_view_host_); } void RenderFrameHostImpl::NotificationClosed(int notification_id) { @@ -1452,7 +1507,7 @@ void RenderFrameHostImpl::SetNavigationsSuspended( // There's navigation message params waiting to be sent. Now that we're not // suspended anymore, resume navigation by sending them. If we were swapped // out, we should also stop filtering out the IPC messages now. - render_view_host_->SetState(RenderViewHostImpl::STATE_DEFAULT); + SetState(RenderFrameHostImpl::STATE_DEFAULT); DCHECK(!proceed_time.is_null()); suspended_nav_params_->browser_navigation_start = proceed_time; diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 60cdea9..341651a 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h @@ -15,6 +15,7 @@ #include "base/strings/string16.h" #include "base/time/time.h" #include "content/browser/accessibility/browser_accessibility_manager.h" +#include "content/browser/site_instance_impl.h" #include "content/common/accessibility_mode_enums.h" #include "content/common/content_export.h" #include "content/common/mojo/service_registry_impl.h" @@ -53,6 +54,7 @@ class RenderFrameProxyHost; class RenderProcessHost; class RenderViewHostImpl; class RenderWidgetHostImpl; +class TimeoutMonitor; struct ContextMenuParams; struct GlobalRequestID; struct Referrer; @@ -63,13 +65,37 @@ class CONTENT_EXPORT RenderFrameHostImpl : public RenderFrameHost, public BrowserAccessibilityDelegate { public: + // Keeps track of the state of the RenderFrameHostImpl, particularly with + // respect to swap out. + enum RenderFrameHostImplState { + // The standard state for a RFH handling the communication with an active + // RenderFrame. + STATE_DEFAULT = 0, + // The RFH has not received the SwapOutACK yet, but the new page has + // committed in a different RFH. The number of active frames of the RFH + // SiteInstanceImpl is not zero. Upon reception of the SwapOutACK, the RFH + // will be swapped out. + STATE_PENDING_SWAP_OUT, + // The RFH has not received the SwapOutACK yet, but the new page has + // committed in a different RFH. The number of active frames of the RFH + // SiteInstanceImpl is zero. Upon reception of the SwapOutACK, the RFH will + // be shutdown. + STATE_PENDING_SHUTDOWN, + // The RFH is swapped out and stored inside a RenderFrameProxyHost, being + // used as a placeholder to allow cross-process communication. + STATE_SWAPPED_OUT, + }; + // Helper function to determine whether the RFH state should contribute to the + // number of active frames of a SiteInstance or not. + static bool IsRFHStateActive(RenderFrameHostImplState rfh_state); + static RenderFrameHostImpl* FromID(int process_id, int routing_id); virtual ~RenderFrameHostImpl(); // RenderFrameHost virtual int GetRoutingID() OVERRIDE; - virtual SiteInstance* GetSiteInstance() OVERRIDE; + virtual SiteInstanceImpl* GetSiteInstance() OVERRIDE; virtual RenderProcessHost* GetProcess() OVERRIDE; virtual RenderFrameHost* GetParent() OVERRIDE; virtual const std::string& GetFrameName() OVERRIDE; @@ -192,14 +218,26 @@ class CONTENT_EXPORT RenderFrameHostImpl // but not until WasSwappedOut is called (when it is no longer visible). void SwapOut(RenderFrameProxyHost* proxy); - void OnSwappedOut(bool timed_out); - bool is_swapped_out() { return is_swapped_out_; } - void set_swapped_out(bool is_swapped_out) { - is_swapped_out_ = is_swapped_out; + bool is_waiting_for_beforeunload_ack() const { + return is_waiting_for_beforeunload_ack_; } - // Sets the RVH for |this| as pending shutdown. |on_swap_out| will be called - // when the SwapOutACK is received. + // Whether the RFH is waiting for an unload ACK from the renderer. + bool IsWaitingForUnloadACK() const; + + // Called when either the SwapOut request has been acknowledged or has timed + // out. + void OnSwappedOut(); + + // Whether this RenderFrameHost has been swapped out, such that the frame is + // now rendered by a RenderFrameHost in a different process. + bool is_swapped_out() const { return rfh_state_ == STATE_SWAPPED_OUT; } + + // The current state of this RFH. + RenderFrameHostImplState rfh_state() const { return rfh_state_; } + + // Set |this| as pending shutdown. |on_swap_out| will be called + // when the SwapOutACK is received, or when the unload timer times out. void SetPendingShutdown(const base::Closure& on_swap_out); // Sends the given navigation message. Use this rather than sending it @@ -387,6 +425,10 @@ class CONTENT_EXPORT RenderFrameHostImpl void OnHidePopup(); #endif + // Updates the state of this RenderFrameHost and clears any waiting state + // that is no longer relevant. + void SetState(RenderFrameHostImplState rfh_state); + // Returns whether the given URL is allowed to commit in the current process. // This is a more conservative check than RenderProcessHost::FilterURL, since // it will be used to kill processes that commit unauthorized URLs. @@ -451,7 +493,9 @@ class CONTENT_EXPORT RenderFrameHostImpl std::map<int, base::Closure> cancel_notification_callbacks_; int routing_id_; - bool is_swapped_out_; + + // The current state of this RenderFrameHost. + RenderFrameHostImplState rfh_state_; // Tracks whether the RenderFrame for this RenderFrameHost has been created in // the renderer process. Currently only used for subframes. @@ -474,6 +518,30 @@ class CONTENT_EXPORT RenderFrameHostImpl // When the last BeforeUnload message was sent. base::TimeTicks send_before_unload_start_time_; + // Set to true when there is a pending FrameMsg_ShouldClose message. This + // ensures we don't spam the renderer with multiple beforeunload requests. + // When either this value or IsWaitingForUnloadACK is true, the value of + // unload_ack_is_for_cross_site_transition_ indicates whether this is for a + // cross-site transition or a tab close attempt. + // TODO(clamy): Remove this boolean and add one more state to the state + // machine. + bool is_waiting_for_beforeunload_ack_; + + // Valid only when is_waiting_for_beforeunload_ack_ or + // IsWaitingForUnloadACK is true. This tells us if the unload request + // is for closing the entire tab ( = false), or only this RenderFrameHost in + // the case of a cross-site transition ( = true). + bool unload_ack_is_for_cross_site_transition_; + + // Used to swap out or shut down this RFH when the unload event is taking too + // long to execute, depending on the number of active frames in the + // SiteInstance. + scoped_ptr<TimeoutMonitor> swapout_event_monitor_timeout_; + + // Called after receiving the SwapOutACK when the RFH is in the pending + // shutdown state. Also called if the unload timer times out. + base::Closure pending_shutdown_on_swap_out_; + ServiceRegistryImpl service_registry_; scoped_ptr<BrowserAccessibilityManager> browser_accessibility_manager_; diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc index 0663346..8d557ac 100644 --- a/content/browser/frame_host/render_frame_host_manager.cc +++ b/content/browser/frame_host/render_frame_host_manager.cc @@ -283,15 +283,14 @@ bool RenderFrameHostManager::ShouldCloseTabOnUnresponsiveRenderer() { // If the tab becomes unresponsive during {before}unload while doing a // cross-site navigation, proceed with the navigation. (This assumes that // the pending RenderFrameHost is still responsive.) - if (render_frame_host_->render_view_host()->IsWaitingForUnloadACK()) { + if (render_frame_host_->IsWaitingForUnloadACK()) { // The request has been started and paused while we're waiting for the // unload handler to finish. We'll pretend that it did. The pending // renderer will then be swapped in as part of the usual DidNavigate logic. // (If the unload handler later finishes, this call will be ignored because // the pending_nav_params_ state will already be cleaned up.) - current_host()->OnSwappedOut(true); - } else if (render_frame_host_->render_view_host()-> - is_waiting_for_beforeunload_ack()) { + current_frame_host()->OnSwappedOut(); + } else if (render_frame_host_->is_waiting_for_beforeunload_ack()) { // Haven't gotten around to starting the request, because we're still // waiting for the beforeunload handler to finish. We'll pretend that it // did finish, to let the navigation proceed. Note that there's a danger @@ -690,8 +689,8 @@ bool RenderFrameHostManager::ClearProxiesInSiteInstance( RenderFrameProxyHost* proxy = iter->second; // If the RVH is pending swap out, it needs to switch state to // pending shutdown. Otherwise it is deleted. - if (proxy->GetRenderViewHost()->rvh_state() == - RenderViewHostImpl::STATE_PENDING_SWAP_OUT) { + if (proxy->render_frame_host()->rfh_state() == + RenderFrameHostImpl::STATE_PENDING_SWAP_OUT) { scoped_ptr<RenderFrameHostImpl> swapped_out_rfh = proxy->PassFrameHostOwnership(); @@ -1224,7 +1223,7 @@ bool RenderFrameHostManager::InitRenderView( } else { // Ensure that we don't create an unprivileged RenderView in a WebUI-enabled // process unless it's swapped out. - if (!render_view_host->IsSwappedOut()) { + if (render_view_host->is_active()) { CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings( render_view_host->GetProcess()->GetID())); } @@ -1332,10 +1331,9 @@ void RenderFrameHostManager::CommitPending() { SwapOutOldPage(old_render_frame_host.get()); // Schedule the old frame to shut down after it swaps out, if there are no - // other active views in its SiteInstance. - if (!static_cast<SiteInstanceImpl*>( - old_render_frame_host->GetSiteInstance())->active_view_count()) { - old_render_frame_host->render_view_host()->SetPendingShutdown(base::Bind( + // other active frames in its SiteInstance. + if (!old_render_frame_host->GetSiteInstance()->active_frame_count()) { + old_render_frame_host->SetPendingShutdown(base::Bind( &RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance, weak_factory_.GetWeakPtr(), old_site_instance_id, @@ -1369,18 +1367,17 @@ void RenderFrameHostManager::CommitPending() { // If the old RFH is live, we are swapping it out and should keep track of // it in case we navigate back to it, or it is waiting for the unload event // to execute in the background. - // TODO(creis): Swap out the subframe in --site-per-process. if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) { DCHECK(old_render_frame_host->is_swapped_out() || - !RenderViewHostImpl::IsRVHStateActive( - old_render_frame_host->render_view_host()->rvh_state())); + !RenderFrameHostImpl::IsRFHStateActive( + old_render_frame_host->rfh_state())); } // If the RenderViewHost backing the RenderFrameHost is pending shutdown, // the RenderFrameHost should be put in the map of RenderFrameHosts pending // shutdown. Otherwise, it is stored in the map of proxy hosts. - if (old_render_frame_host->render_view_host()->rvh_state() == - RenderViewHostImpl::STATE_PENDING_SHUTDOWN) { + if (old_render_frame_host->rfh_state() == + RenderFrameHostImpl::STATE_PENDING_SHUTDOWN) { // The proxy for this RenderFrameHost is created when sending the // SwapOut message, so check if it already exists and delete it. RenderFrameProxyHostMap::iterator iter = @@ -1400,12 +1397,11 @@ void RenderFrameHostManager::CommitPending() { CHECK(proxy_hosts_.find(render_frame_host_->GetSiteInstance()->GetId()) == proxy_hosts_.end()); - // Capture the active view count on the old RFH SiteInstance, since the + // Capture the active frame count on the old RFH SiteInstance, since the // ownership might be passed into the proxy and the pointer will be // invalid. - int active_view_count = - static_cast<SiteInstanceImpl*>(old_render_frame_host->GetSiteInstance()) - ->active_view_count(); + int active_frame_count = + old_render_frame_host->GetSiteInstance()->active_frame_count(); if (is_main_frame) { RenderFrameProxyHostMap::iterator iter = @@ -1414,23 +1410,23 @@ void RenderFrameHostManager::CommitPending() { iter->second->TakeFrameHostOwnership(old_render_frame_host.Pass()); } - // If there are no active views in this SiteInstance, it means that + // If there are no active frames in this SiteInstance, it means that // this RFH was the last active one in the SiteInstance. Now that we // know that all RFHs are swapped out, we can delete all the RFPHs and // RVHs in this SiteInstance. - if (!active_view_count) { + if (!active_frame_count) { ShutdownRenderFrameProxyHostsInSiteInstance(old_site_instance_id); - } else { - // If this is a subframe, it should have a CrossProcessFrameConnector - // created already and we just need to link it to the proper view in the - // new process. - if (!is_main_frame) { - RenderFrameProxyHost* proxy = GetProxyToParent(); - if (proxy) { - proxy->SetChildRWHView( - render_frame_host_->render_view_host()->GetView()); - } - } + } + } + + // If this is a subframe, it should have a CrossProcessFrameConnector + // created already and we just need to link it to the proper view in the + // new process. + if (!is_main_frame) { + RenderFrameProxyHost* proxy = GetProxyToParent(); + if (proxy) { + proxy->SetChildRWHView( + render_frame_host_->render_view_host()->GetView()); } } } @@ -1619,9 +1615,9 @@ void RenderFrameHostManager::CancelPending() { // If the SiteInstance for the pending RFH is being used by others, don't // delete the RFH, just swap it out and it can be reused at a later point. - SiteInstanceImpl* site_instance = static_cast<SiteInstanceImpl*>( - pending_render_frame_host->GetSiteInstance()); - if (site_instance->active_view_count() > 1) { + SiteInstanceImpl* site_instance = + pending_render_frame_host->GetSiteInstance(); + if (site_instance->active_frame_count() > 1) { // Any currently suspended navigations are no longer needed. pending_render_frame_host->CancelSuspendedNavigations(); @@ -1653,11 +1649,11 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::SetRenderFrameHost( // count top-level ones. This makes the value easier for consumers to // interpret. if (render_frame_host_) { - static_cast<SiteInstanceImpl*>(render_frame_host_->GetSiteInstance())-> + render_frame_host_->GetSiteInstance()-> IncrementRelatedActiveContentsCount(); } if (old_render_frame_host) { - static_cast<SiteInstanceImpl*>(old_render_frame_host->GetSiteInstance())-> + old_render_frame_host->GetSiteInstance()-> DecrementRelatedActiveContentsCount(); } } diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc index b23ecc9..69ea6f3 100644 --- a/content/browser/frame_host/render_frame_host_manager_unittest.cc +++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc @@ -14,6 +14,7 @@ #include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/navigator_impl.h" #include "content/browser/frame_host/render_frame_host_manager.h" +#include "content/browser/frame_host/render_frame_proxy_host.h" #include "content/browser/site_instance_impl.h" #include "content/browser/webui/web_ui_controller_factory_registry.h" #include "content/common/view_messages.h" @@ -281,46 +282,51 @@ class RenderFrameHostManagerTest controller().LoadURL( url, Referrer(), ui::PAGE_TRANSITION_LINK, std::string()); TestRenderViewHost* old_rvh = test_rvh(); + TestRenderFrameHost* old_rfh = main_test_rfh(); + TestRenderFrameHost* active_rfh = pending_main_rfh() ? + static_cast<TestRenderFrameHost*>(pending_main_rfh()) : + old_rfh; // Simulate the BeforeUnload_ACK that is received from the current renderer // for a cross-site navigation. - if (old_rvh != active_rvh()) { - old_rvh->SendBeforeUnloadACK(true); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, old_rvh->rvh_state()); + if (old_rfh != active_rfh) { + old_rfh->SendBeforeUnloadACK(true); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, old_rfh->rfh_state()); } // Commit the navigation with a new page ID. int32 max_page_id = contents()->GetMaxPageIDForSiteInstance( - active_rvh()->GetSiteInstance()); + active_rfh->GetSiteInstance()); // Use an observer to avoid accessing a deleted renderer later on when the // state is being checked. + RenderFrameHostDeletedObserver rfh_observer(old_rfh); RenderViewHostDeletedObserver rvh_observer(old_rvh); - active_test_rvh()->SendNavigate(max_page_id + 1, url); + active_rfh->SendNavigate(max_page_id + 1, url); // Make sure that we start to run the unload handler at the time of commit. - bool expecting_rvh_shutdown = false; - if (old_rvh != active_rvh() && !rvh_observer.deleted()) { - if (!static_cast<SiteInstanceImpl*>( - old_rvh->GetSiteInstance())->active_view_count()) { - expecting_rvh_shutdown = true; - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SHUTDOWN, - old_rvh->rvh_state()); + bool expecting_rfh_shutdown = false; + if (old_rfh != active_rfh && !rfh_observer.deleted()) { + if (!old_rfh->GetSiteInstance()->active_frame_count()) { + expecting_rfh_shutdown = true; + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SHUTDOWN, + old_rfh->rfh_state()); } else { - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, - old_rvh->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, + old_rfh->rfh_state()); } } // Simulate the swap out ACK coming from the pending renderer. This should - // either shut down the old RVH or leave it in a swapped out state. - if (old_rvh != active_rvh()) { - old_rvh->OnSwappedOut(false); - if (expecting_rvh_shutdown) { + // either shut down the old RFH or leave it in a swapped out state. + if (old_rfh != active_rfh) { + old_rfh->OnSwappedOut(); + if (expecting_rfh_shutdown) { + EXPECT_TRUE(rfh_observer.deleted()); EXPECT_TRUE(rvh_observer.deleted()); } else { - EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, - old_rvh->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_SWAPPED_OUT, + old_rfh->rfh_state()); } } } @@ -345,8 +351,8 @@ class RenderFrameHostManagerTest new_entry->IsViewSourceMode()); } - // Creates a test RenderViewHost that's swapped out. - TestRenderViewHost* CreateSwappedOutRenderViewHost() { + // Creates a test RenderFrameHost that's swapped out. + TestRenderFrameHost* CreateSwappedOutRenderFrameHost() { const GURL kChromeURL("chrome://foo"); const GURL kDestUrl("http://www.google.com/"); @@ -359,24 +365,23 @@ class RenderFrameHostManagerTest kDestUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string()); EXPECT_TRUE(contents()->cross_navigation_pending()); - // Manually increase the number of active views in the + // Manually increase the number of active frames in the // SiteInstance that ntp_rfh belongs to, to prevent it from being // destroyed when it gets swapped out. - static_cast<SiteInstanceImpl*>(ntp_rfh->GetSiteInstance())-> - increment_active_view_count(); + ntp_rfh->GetSiteInstance()->increment_active_frame_count(); TestRenderFrameHost* dest_rfh = contents()->GetPendingMainFrame(); CHECK(dest_rfh); EXPECT_NE(ntp_rfh, dest_rfh); // BeforeUnload finishes. - ntp_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + ntp_rfh->SendBeforeUnloadACK(true); dest_rfh->SendNavigate(101, kDestUrl); - ntp_rfh->OnSwappedOut(false); + ntp_rfh->OnSwappedOut(); - EXPECT_TRUE(ntp_rfh->GetRenderViewHost()->IsSwappedOut()); - return ntp_rfh->GetRenderViewHost(); + EXPECT_TRUE(ntp_rfh->is_swapped_out()); + return ntp_rfh; } NavigationRequest* GetNavigationRequestForRenderFrameManager( @@ -431,7 +436,7 @@ TEST_F(RenderFrameHostManagerTest, NewTabPageProcesses) { TestRenderFrameHost* dest_rfh2 = contents2->GetPendingMainFrame(); ASSERT_TRUE(dest_rfh2); - ntp_rfh2->GetRenderViewHost()->SendBeforeUnloadACK(true); + ntp_rfh2->SendBeforeUnloadACK(true); dest_rfh2->SendNavigate(101, kDestUrl); // The two RFH's should be different in every way. @@ -448,7 +453,7 @@ TEST_F(RenderFrameHostManagerTest, NewTabPageProcesses) { contents2->GetController().LoadURL( kChromeUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string()); - dest_rfh2->GetRenderViewHost()->SendBeforeUnloadACK(true); + dest_rfh2->SendBeforeUnloadACK(true); contents2->GetPendingMainFrame()->SendNavigate(102, kChromeUrl); EXPECT_NE(contents()->GetMainFrame()->GetSiteInstance(), @@ -480,11 +485,10 @@ TEST_F(RenderFrameHostManagerTest, FilterMessagesWhileSwappedOut) { ntp_rfh->GetRenderViewHost()->GetRoutingID(), icons))); EXPECT_TRUE(observer.favicon_received()); } - // Create one more view in the same SiteInstance where ntp_rfh + // Create one more frame in the same SiteInstance where ntp_rfh // exists so that it doesn't get deleted on navigation to another // site. - static_cast<SiteInstanceImpl*>(ntp_rfh->GetSiteInstance())-> - increment_active_view_count(); + ntp_rfh->GetSiteInstance()->increment_active_frame_count(); // Navigate to a cross-site URL. @@ -506,7 +510,7 @@ TEST_F(RenderFrameHostManagerTest, FilterMessagesWhileSwappedOut) { // The old renderer, being slow, now updates the favicon. It should be // filtered out and not take effect. - EXPECT_TRUE(ntp_rfh->GetRenderViewHost()->IsSwappedOut()); + EXPECT_TRUE(ntp_rfh->is_swapped_out()); { PluginFaviconMessageObserver observer(contents()); EXPECT_TRUE( @@ -555,28 +559,29 @@ TEST_F(RenderFrameHostManagerTest, FilterMessagesWhileSwappedOut) { } TEST_F(RenderFrameHostManagerTest, WhiteListSwapCompositorFrame) { - TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost(); + TestRenderFrameHost* swapped_out_rfh = CreateSwappedOutRenderFrameHost(); TestRenderWidgetHostView* swapped_out_rwhv = - static_cast<TestRenderWidgetHostView*>(swapped_out_rvh->GetView()); + static_cast<TestRenderWidgetHostView*>( + swapped_out_rfh->GetRenderViewHost()->GetView()); EXPECT_FALSE(swapped_out_rwhv->did_swap_compositor_frame()); MockRenderProcessHost* process_host = - static_cast<MockRenderProcessHost*>(swapped_out_rvh->GetProcess()); + static_cast<MockRenderProcessHost*>(swapped_out_rfh->GetProcess()); process_host->sink().ClearMessages(); cc::CompositorFrame frame; ViewHostMsg_SwapCompositorFrame msg( rvh()->GetRoutingID(), 0, frame, std::vector<IPC::Message>()); - EXPECT_TRUE(swapped_out_rvh->OnMessageReceived(msg)); + EXPECT_TRUE(swapped_out_rfh->render_view_host()->OnMessageReceived(msg)); EXPECT_TRUE(swapped_out_rwhv->did_swap_compositor_frame()); } // Test if RenderViewHost::GetRenderWidgetHosts() only returns active // widgets. TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) { - TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost(); - EXPECT_TRUE(swapped_out_rvh->IsSwappedOut()); + TestRenderFrameHost* swapped_out_rfh = CreateSwappedOutRenderFrameHost(); + EXPECT_TRUE(swapped_out_rfh->is_swapped_out()); scoped_ptr<RenderWidgetHostIterator> widgets( RenderWidgetHost::GetRenderWidgetHosts()); @@ -586,8 +591,7 @@ TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) { RenderWidgetHost* widget = widgets->GetNextHost(); EXPECT_FALSE(widgets->GetNextHost()); RenderViewHost* rvh = RenderViewHost::From(widget); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - static_cast<RenderViewHostImpl*>(rvh)->rvh_state()); + EXPECT_TRUE(static_cast<RenderViewHostImpl*>(rvh)->is_active()); } // Test if RenderViewHost::GetRenderWidgetHosts() returns a subset of @@ -597,8 +601,8 @@ TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) { // including swapped out ones. TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsWithinGetAllRenderWidgetHosts) { - TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost(); - EXPECT_TRUE(swapped_out_rvh->IsSwappedOut()); + TestRenderFrameHost* swapped_out_rfh = CreateSwappedOutRenderFrameHost(); + EXPECT_TRUE(swapped_out_rfh->is_swapped_out()); scoped_ptr<RenderWidgetHostIterator> widgets( RenderWidgetHost::GetRenderWidgetHosts()); @@ -617,19 +621,18 @@ TEST_F(RenderFrameHostManagerTest, } } -// Test if SiteInstanceImpl::active_view_count() is correctly updated -// as views in a SiteInstance get swapped out and in. -TEST_F(RenderFrameHostManagerTest, ActiveViewCountWhileSwappingInandOut) { +// Test if SiteInstanceImpl::active_frame_count() is correctly updated +// as frames in a SiteInstance get swapped out and in. +TEST_F(RenderFrameHostManagerTest, ActiveFrameCountWhileSwappingInAndOut) { const GURL kUrl1("http://www.google.com/"); const GURL kUrl2("http://www.chromium.org/"); // Navigate to an initial URL. contents()->NavigateAndCommit(kUrl1); - TestRenderViewHost* rvh1 = test_rvh(); + TestRenderFrameHost* rfh1 = main_test_rfh(); - SiteInstanceImpl* instance1 = - static_cast<SiteInstanceImpl*>(rvh1->GetSiteInstance()); - EXPECT_EQ(instance1->active_view_count(), 1U); + SiteInstanceImpl* instance1 = rfh1->GetSiteInstance(); + EXPECT_EQ(instance1->active_frame_count(), 1U); // Create 2 new tabs and simulate them being the opener chain for the main // tab. They should be in the same SiteInstance. @@ -641,26 +644,25 @@ TEST_F(RenderFrameHostManagerTest, ActiveViewCountWhileSwappingInandOut) { TestWebContents::Create(browser_context(), instance1)); opener1->SetOpener(opener2.get()); - EXPECT_EQ(instance1->active_view_count(), 3U); + EXPECT_EQ(instance1->active_frame_count(), 3U); // Navigate to a cross-site URL (different SiteInstance but same // BrowsingInstance). contents()->NavigateAndCommit(kUrl2); - TestRenderViewHost* rvh2 = test_rvh(); - SiteInstanceImpl* instance2 = - static_cast<SiteInstanceImpl*>(rvh2->GetSiteInstance()); + TestRenderFrameHost* rfh2 = main_test_rfh(); + SiteInstanceImpl* instance2 = rfh2->GetSiteInstance(); // rvh2 is on chromium.org which is different from google.com on // which other tabs are. - EXPECT_EQ(instance2->active_view_count(), 1U); + EXPECT_EQ(instance2->active_frame_count(), 1U); // There are two active views on google.com now. - EXPECT_EQ(instance1->active_view_count(), 2U); + EXPECT_EQ(instance1->active_frame_count(), 2U); // Navigate to the original origin (google.com). contents()->NavigateAndCommit(kUrl1); - EXPECT_EQ(instance1->active_view_count(), 3U); + EXPECT_EQ(instance1->active_frame_count(), 3U); } // This deletes a WebContents when the given RVH is deleted. This is @@ -812,7 +814,7 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { Source<WebContents>(web_contents.get())); RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting(); - RenderFrameHostImpl* host; + RenderFrameHostImpl* host = NULL; // 1) The first navigation. -------------------------- const GURL kUrl1("http://www.google.com/"); @@ -831,9 +833,8 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { // Commit to SiteInstance should be delayed until RenderView commit. EXPECT_TRUE(host == manager->current_frame_host()); ASSERT_TRUE(host); - EXPECT_FALSE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> - HasSite()); - static_cast<SiteInstanceImpl*>(host->GetSiteInstance())->SetSite(kUrl1); + EXPECT_FALSE(host->GetSiteInstance()->HasSite()); + host->GetSiteInstance()->SetSite(kUrl1); // 2) Navigate to next site. ------------------------- const GURL kUrl2("http://www.google.com/foo"); @@ -852,8 +853,7 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { manager->DidNavigateFrame(host); EXPECT_TRUE(host == manager->current_frame_host()); ASSERT_TRUE(host); - EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> - HasSite()); + EXPECT_TRUE(host->GetSiteInstance()->HasSite()); // 3) Cross-site navigate to next site. -------------- const GURL kUrl3("http://webkit.org/"); @@ -874,8 +874,7 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { manager->DidNavigateFrame(manager->pending_frame_host()); EXPECT_TRUE(host == manager->current_frame_host()); ASSERT_TRUE(host); - EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> - HasSite()); + EXPECT_TRUE(host->GetSiteInstance()->HasSite()); // Check the pending RenderFrameHost has been committed. EXPECT_FALSE(manager->pending_frame_host()); @@ -913,8 +912,7 @@ TEST_F(RenderFrameHostManagerTest, WebUI) { // as the navigation starts, rather than lazily after it commits, so we don't // try to re-use the SiteInstance/process for non Web UI things that may // get loaded in between. - EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> - HasSite()); + EXPECT_TRUE(host->GetSiteInstance()->HasSite()); EXPECT_EQ(kUrl, host->GetSiteInstance()->GetSiteURL()); // The Web UI is committed immediately because the RenderViewHost has not been @@ -1056,49 +1054,47 @@ TEST_F(RenderFrameHostManagerTest, NavigateAfterMissingSwapOutACK) { // Navigate to two pages. contents()->NavigateAndCommit(kUrl1); - TestRenderViewHost* rvh1 = test_rvh(); + TestRenderFrameHost* rfh1 = main_test_rfh(); - // Keep active_view_count nonzero so that no swapped out views in + // Keep active_frame_count nonzero so that no swapped out frames in // this SiteInstance get forcefully deleted. - static_cast<SiteInstanceImpl*>(rvh1->GetSiteInstance())-> - increment_active_view_count(); + rfh1->GetSiteInstance()->increment_active_frame_count(); contents()->NavigateAndCommit(kUrl2); - TestRenderViewHost* rvh2 = test_rvh(); - static_cast<SiteInstanceImpl*>(rvh2->GetSiteInstance())-> - increment_active_view_count(); + TestRenderFrameHost* rfh2 = main_test_rfh(); + rfh2->GetSiteInstance()->increment_active_frame_count(); // Now go back, but suppose the SwapOut_ACK isn't received. This shouldn't // happen, but we have seen it when going back quickly across many entries // (http://crbug.com/93427). contents()->GetController().GoBack(); - EXPECT_TRUE(rvh2->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(rfh2->is_waiting_for_beforeunload_ack()); contents()->ProceedWithCrossSiteNavigation(); - EXPECT_FALSE(rvh2->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(rfh2->is_waiting_for_beforeunload_ack()); // The back navigation commits. const NavigationEntry* entry1 = contents()->GetController().GetPendingEntry(); - rvh1->SendNavigate(entry1->GetPageID(), entry1->GetURL()); - EXPECT_TRUE(rvh2->IsWaitingForUnloadACK()); - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh2->rvh_state()); + rfh1->SendNavigate(entry1->GetPageID(), entry1->GetURL()); + EXPECT_TRUE(rfh2->IsWaitingForUnloadACK()); + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh2->rfh_state()); // We should be able to navigate forward. contents()->GetController().GoForward(); contents()->ProceedWithCrossSiteNavigation(); const NavigationEntry* entry2 = contents()->GetController().GetPendingEntry(); - rvh2->SendNavigate(entry2->GetPageID(), entry2->GetURL()); - EXPECT_EQ(rvh2, rvh()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh2->rvh_state()); - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh1->rvh_state()); - rvh1->OnSwappedOut(false); - EXPECT_TRUE(rvh1->IsSwappedOut()); - EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, rvh1->rvh_state()); + rfh2->SendNavigate(entry2->GetPageID(), entry2->GetURL()); + EXPECT_EQ(rfh2, main_test_rfh()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state()); + rfh1->OnSwappedOut(); + EXPECT_TRUE(rfh1->is_swapped_out()); + EXPECT_EQ(RenderFrameHostImpl::STATE_SWAPPED_OUT, rfh1->rfh_state()); } -// Test that we create swapped out RVHs for the opener chain when navigating an +// Test that we create swapped out RFHs for the opener chain when navigating an // opened tab cross-process. This allows us to support certain cross-process // JavaScript calls (http://crbug.com/99202). -TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRVHs) { +TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRFHs) { const GURL kUrl1("http://www.google.com/"); const GURL kUrl2("http://www.chromium.org/"); const GURL kChromeUrl("chrome://foo"); @@ -1106,18 +1102,19 @@ TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRVHs) { // Navigate to an initial URL. contents()->NavigateAndCommit(kUrl1); RenderFrameHostManager* manager = contents()->GetRenderManagerForTesting(); + TestRenderFrameHost* rfh1 = main_test_rfh(); TestRenderViewHost* rvh1 = test_rvh(); // Create 2 new tabs and simulate them being the opener chain for the main // tab. They should be in the same SiteInstance. scoped_ptr<TestWebContents> opener1( - TestWebContents::Create(browser_context(), rvh1->GetSiteInstance())); + TestWebContents::Create(browser_context(), rfh1->GetSiteInstance())); RenderFrameHostManager* opener1_manager = opener1->GetRenderManagerForTesting(); contents()->SetOpener(opener1.get()); scoped_ptr<TestWebContents> opener2( - TestWebContents::Create(browser_context(), rvh1->GetSiteInstance())); + TestWebContents::Create(browser_context(), rfh1->GetSiteInstance())); RenderFrameHostManager* opener2_manager = opener2->GetRenderManagerForTesting(); opener1->SetOpener(opener2.get()); @@ -1125,41 +1122,60 @@ TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRVHs) { // Navigate to a cross-site URL (different SiteInstance but same // BrowsingInstance). contents()->NavigateAndCommit(kUrl2); + TestRenderFrameHost* rfh2 = main_test_rfh(); TestRenderViewHost* rvh2 = test_rvh(); - EXPECT_NE(rvh1->GetSiteInstance(), rvh2->GetSiteInstance()); - EXPECT_TRUE(rvh1->GetSiteInstance()->IsRelatedSiteInstance( - rvh2->GetSiteInstance())); + EXPECT_NE(rfh1->GetSiteInstance(), rfh2->GetSiteInstance()); + EXPECT_TRUE(rfh1->GetSiteInstance()->IsRelatedSiteInstance( + rfh2->GetSiteInstance())); // Ensure rvh1 is placed on swapped out list of the current tab. + EXPECT_TRUE(manager->IsOnSwappedOutList(rfh1)); EXPECT_TRUE(manager->IsRVHOnSwappedOutList(rvh1)); + EXPECT_EQ(rfh1, + manager->GetRenderFrameProxyHost(rfh1->GetSiteInstance()) + ->render_frame_host()); EXPECT_EQ(rvh1, manager->GetSwappedOutRenderViewHost(rvh1->GetSiteInstance())); - // Ensure a swapped out RVH is created in the first opener tab. + // Ensure a swapped out RFH and RFH is created in the first opener tab. + RenderFrameProxyHost* opener1_proxy = + opener1_manager->GetRenderFrameProxyHost(rfh2->GetSiteInstance()); + RenderFrameHostImpl* opener1_rfh = opener1_proxy->render_frame_host(); TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>( opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance())); + EXPECT_TRUE(opener1_manager->IsOnSwappedOutList(opener1_rfh)); EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh)); - EXPECT_TRUE(opener1_rvh->IsSwappedOut()); + EXPECT_TRUE(opener1_rfh->is_swapped_out()); + EXPECT_FALSE(opener1_rvh->is_active()); - // Ensure a swapped out RVH is created in the second opener tab. + // Ensure a swapped out RFH and RVH is created in the second opener tab. + RenderFrameProxyHost* opener2_proxy = + opener2_manager->GetRenderFrameProxyHost(rfh2->GetSiteInstance()); + RenderFrameHostImpl* opener2_rfh = opener2_proxy->render_frame_host(); TestRenderViewHost* opener2_rvh = static_cast<TestRenderViewHost*>( opener2_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance())); + EXPECT_TRUE(opener2_manager->IsOnSwappedOutList(opener2_rfh)); EXPECT_TRUE(opener2_manager->IsRVHOnSwappedOutList(opener2_rvh)); - EXPECT_TRUE(opener2_rvh->IsSwappedOut()); + EXPECT_TRUE(opener2_rfh->is_swapped_out()); + EXPECT_FALSE(opener2_rvh->is_active()); // Navigate to a cross-BrowsingInstance URL. contents()->NavigateAndCommit(kChromeUrl); - TestRenderViewHost* rvh3 = test_rvh(); - EXPECT_NE(rvh1->GetSiteInstance(), rvh3->GetSiteInstance()); - EXPECT_FALSE(rvh1->GetSiteInstance()->IsRelatedSiteInstance( - rvh3->GetSiteInstance())); + TestRenderFrameHost* rfh3 = main_test_rfh(); + EXPECT_NE(rfh1->GetSiteInstance(), rfh3->GetSiteInstance()); + EXPECT_FALSE(rfh1->GetSiteInstance()->IsRelatedSiteInstance( + rfh3->GetSiteInstance())); // No scripting is allowed across BrowsingInstances, so we should not create // swapped out RVHs for the opener chain in this case. + EXPECT_FALSE(opener1_manager->GetRenderFrameProxyHost( + rfh3->GetSiteInstance())); EXPECT_FALSE(opener1_manager->GetSwappedOutRenderViewHost( - rvh3->GetSiteInstance())); + rfh3->GetSiteInstance())); + EXPECT_FALSE(opener2_manager->GetRenderFrameProxyHost( + rfh3->GetSiteInstance())); EXPECT_FALSE(opener2_manager->GetSwappedOutRenderViewHost( - rvh3->GetSiteInstance())); + rfh3->GetSiteInstance())); } // Test that a page can disown the opener of the WebContents. @@ -1369,11 +1385,16 @@ TEST_F(RenderFrameHostManagerTest, EnableWebUIWithSwappedOutOpener) { EXPECT_TRUE(rvh1->GetSiteInstance()->IsRelatedSiteInstance( rvh2->GetSiteInstance())); - // Ensure a swapped out RVH is created in the first opener tab. + // Ensure a swapped out RFH and RVH is created in the first opener tab. + RenderFrameProxyHost* opener1_proxy = + opener1_manager->GetRenderFrameProxyHost(rvh2->GetSiteInstance()); + RenderFrameHostImpl* opener1_rfh = opener1_proxy->render_frame_host(); TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>( opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance())); + EXPECT_TRUE(opener1_manager->IsOnSwappedOutList(opener1_rfh)); EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh)); - EXPECT_TRUE(opener1_rvh->IsSwappedOut()); + EXPECT_TRUE(opener1_rfh->is_swapped_out()); + EXPECT_FALSE(opener1_rvh->is_active()); // Ensure the new RVH has WebUI bindings. EXPECT_TRUE(rvh2->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI); @@ -1391,7 +1412,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) { RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting(); - RenderFrameHostImpl* host; + RenderFrameHostImpl* host = NULL; // 1) The first navigation. -------------------------- const GURL kUrl1("http://www.google.com/"); @@ -1411,8 +1432,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) { // Commit to SiteInstance should be delayed until RenderView commit. EXPECT_EQ(host, manager->current_frame_host()); ASSERT_TRUE(host); - EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> - HasSite()); + EXPECT_TRUE(host->GetSiteInstance()->HasSite()); // 2) Navigate to a different domain. ------------------------- // Guests stay in the same process on navigation. @@ -1432,8 +1452,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) { manager->DidNavigateFrame(host); EXPECT_EQ(host, manager->current_frame_host()); ASSERT_TRUE(host); - EXPECT_EQ(static_cast<SiteInstanceImpl*>(host->GetSiteInstance()), - instance); + EXPECT_EQ(host->GetSiteInstance(), instance); } // Test that we cancel a pending RVH if we close the tab while it's pending. @@ -1474,9 +1493,8 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) { // Commit to SiteInstance should be delayed until RenderFrame commits. EXPECT_EQ(host, manager->current_frame_host()); - EXPECT_FALSE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> - HasSite()); - static_cast<SiteInstanceImpl*>(host->GetSiteInstance())->SetSite(kUrl1); + EXPECT_FALSE(host->GetSiteInstance()->HasSite()); + host->GetSiteInstance()->SetSite(kUrl1); // 2) Cross-site navigate to next site. ------------------------- const GURL kUrl2("http://www.example.com"); @@ -1515,9 +1533,8 @@ TEST_F(RenderFrameHostManagerTest, DeleteFrameAfterSwapOutACK) { // Navigate to the first page. contents()->NavigateAndCommit(kUrl1); TestRenderFrameHost* rfh1 = contents()->GetMainFrame(); - RenderViewHostDeletedObserver rvh_deleted_observer(rfh1->GetRenderViewHost()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh1->GetRenderViewHost()->rvh_state()); + RenderFrameHostDeletedObserver rfh_deleted_observer(rfh1); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); // Navigate to new site, simulating onbeforeunload approval. controller().LoadURL( @@ -1526,32 +1543,28 @@ TEST_F(RenderFrameHostManagerTest, DeleteFrameAfterSwapOutACK) { contents()->GetMainFrame()->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); EXPECT_TRUE(contents()->cross_navigation_pending()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh1->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame(); // Simulate the swap out ack, unexpectedly early (before commit). It should // have no effect. - rfh1->OnSwappedOut(false); + rfh1->OnSwappedOut(); EXPECT_TRUE(contents()->cross_navigation_pending()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh1->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); // The new page commits. contents()->TestDidNavigate(rfh2, 1, kUrl2, ui::PAGE_TRANSITION_TYPED); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(rfh2, contents()->GetMainFrame()); EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh2->GetRenderViewHost()->rvh_state()); - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SHUTDOWN, - rfh1->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SHUTDOWN, rfh1->rfh_state()); // Simulate the swap out ack. - rfh1->OnSwappedOut(false); + rfh1->OnSwappedOut(); // rfh1 should have been deleted. - EXPECT_TRUE(rvh_deleted_observer.deleted()); + EXPECT_TRUE(rfh_deleted_observer.deleted()); rfh1 = NULL; } @@ -1564,14 +1577,12 @@ TEST_F(RenderFrameHostManagerTest, SwapOutFrameAfterSwapOutACK) { // Navigate to the first page. contents()->NavigateAndCommit(kUrl1); TestRenderFrameHost* rfh1 = contents()->GetMainFrame(); - RenderViewHostDeletedObserver rvh_deleted_observer(rfh1->GetRenderViewHost()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh1->GetRenderViewHost()->rvh_state()); + RenderFrameHostDeletedObserver rfh_deleted_observer(rfh1); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); - // Increment the number of active views in SiteInstanceImpl so that rfh1 is + // Increment the number of active frames in SiteInstanceImpl so that rfh1 is // not deleted on swap out. - static_cast<SiteInstanceImpl*>( - rfh1->GetSiteInstance())->increment_active_view_count(); + rfh1->GetSiteInstance()->increment_active_frame_count(); // Navigate to new site, simulating onbeforeunload approval. controller().LoadURL( @@ -1580,8 +1591,7 @@ TEST_F(RenderFrameHostManagerTest, SwapOutFrameAfterSwapOutACK) { contents()->GetMainFrame()->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); EXPECT_TRUE(contents()->cross_navigation_pending()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh1->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame(); // The new page commits. @@ -1589,17 +1599,15 @@ TEST_F(RenderFrameHostManagerTest, SwapOutFrameAfterSwapOutACK) { EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(rfh2, contents()->GetMainFrame()); EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh2->GetRenderViewHost()->rvh_state()); - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, - rfh1->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state()); // Simulate the swap out ack. - rfh1->OnSwappedOut(false); + rfh1->OnSwappedOut(); // rfh1 should be swapped out. - EXPECT_FALSE(rvh_deleted_observer.deleted()); - EXPECT_TRUE(rfh1->GetRenderViewHost()->IsSwappedOut()); + EXPECT_FALSE(rfh_deleted_observer.deleted()); + EXPECT_TRUE(rfh1->is_swapped_out()); } // Test that the RenderViewHost is properly swapped out if a navigation in the @@ -1614,14 +1622,12 @@ TEST_F(RenderFrameHostManagerTest, // Navigate to the first page. contents()->NavigateAndCommit(kUrl1); TestRenderFrameHost* rfh1 = contents()->GetMainFrame(); - RenderViewHostDeletedObserver rvh_deleted_observer(rfh1->GetRenderViewHost()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh1->GetRenderViewHost()->rvh_state()); + RenderFrameHostDeletedObserver rfh_deleted_observer(rfh1); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); - // Increment the number of active views in SiteInstanceImpl so that rfh1 is + // Increment the number of active frames in SiteInstanceImpl so that rfh1 is // not deleted on swap out. - static_cast<SiteInstanceImpl*>( - rfh1->GetSiteInstance())->increment_active_view_count(); + rfh1->GetSiteInstance()->increment_active_frame_count(); // Navigate to new site, simulating onbeforeunload approval. controller().LoadURL( @@ -1637,17 +1643,15 @@ TEST_F(RenderFrameHostManagerTest, EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(rfh2, contents()->GetMainFrame()); EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - rfh2->GetRenderViewHost()->rvh_state()); - EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, - rfh1->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state()); // Simulate the swap out ack. - rfh1->OnSwappedOut(false); + rfh1->OnSwappedOut(); // rfh1 should be swapped out. - EXPECT_FALSE(rvh_deleted_observer.deleted()); - EXPECT_TRUE(rfh1->GetRenderViewHost()->IsSwappedOut()); + EXPECT_FALSE(rfh_deleted_observer.deleted()); + EXPECT_TRUE(rfh1->is_swapped_out()); } // Test that a RenderFrameHost is properly deleted or swapped out when a @@ -1661,8 +1665,8 @@ TEST_F(RenderFrameHostManagerTest, // Navigate to the first page. contents()->NavigateAndCommit(kUrl1); - TestRenderViewHost* rvh1 = test_rvh(); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state()); + TestRenderFrameHost* rfh1 = main_test_rfh(); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state()); // Navigate to a new site, starting a cross-site navigation. controller().LoadURL( @@ -1670,7 +1674,7 @@ TEST_F(RenderFrameHostManagerTest, { pending_rfh = contents()->GetFrameTree()->root()->render_manager() ->pending_frame_host(); - RenderFrameHostDeletedObserver rvh_deleted_observer(pending_rfh); + RenderFrameHostDeletedObserver rfh_deleted_observer(pending_rfh); // Cancel the navigation by simulating a declined beforeunload dialog. contents()->GetMainFrame()->OnMessageReceived( @@ -1679,7 +1683,7 @@ TEST_F(RenderFrameHostManagerTest, // Since the pending RFH is the only one for the new SiteInstance, it should // be deleted. - EXPECT_TRUE(rvh_deleted_observer.deleted()); + EXPECT_TRUE(rfh_deleted_observer.deleted()); } // Start another cross-site navigation. @@ -1688,17 +1692,16 @@ TEST_F(RenderFrameHostManagerTest, { pending_rfh = contents()->GetFrameTree()->root()->render_manager() ->pending_frame_host(); - RenderFrameHostDeletedObserver rvh_deleted_observer(pending_rfh); + RenderFrameHostDeletedObserver rfh_deleted_observer(pending_rfh); - // Increment the number of active views in the new SiteInstance, which will + // Increment the number of active frames in the new SiteInstance, which will // cause the pending RFH to be swapped out instead of deleted. - static_cast<SiteInstanceImpl*>( - pending_rfh->GetSiteInstance())->increment_active_view_count(); + pending_rfh->GetSiteInstance()->increment_active_frame_count(); contents()->GetMainFrame()->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, false, now, now)); EXPECT_FALSE(contents()->cross_navigation_pending()); - EXPECT_FALSE(rvh_deleted_observer.deleted()); + EXPECT_FALSE(rfh_deleted_observer.deleted()); } } @@ -1775,6 +1778,7 @@ TEST_F(RenderFrameHostManagerTest, // The main RFH should not have been changed, and the renderer should have // been initialized. EXPECT_EQ(rfh, main_test_rfh()); + EXPECT_TRUE(main_test_rfh()->IsRenderFrameLive()); EXPECT_TRUE(main_test_rfh()->render_view_host()->IsRenderViewLive()); } @@ -1789,9 +1793,8 @@ TEST_F(RenderFrameHostManagerTest, // when CommitNavigation is properly implemented. // Navigate to the first page. contents()->NavigateAndCommit(kUrl1); - TestRenderViewHost* rvh1 = test_rvh(); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state()); RenderFrameHostImpl* rfh = main_test_rfh(); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh->rfh_state()); RenderFrameHostManager* render_manager = main_test_rfh()->frame_tree_node()->render_manager(); @@ -1807,6 +1810,7 @@ TEST_F(RenderFrameHostManagerTest, commit_info.navigation_request_id = main_request->navigation_request_id(); render_manager->CommitNavigation(commit_info); EXPECT_NE(main_test_rfh(), rfh); + EXPECT_TRUE(main_test_rfh()->IsRenderFrameLive()); EXPECT_TRUE(main_test_rfh()->render_view_host()->IsRenderViewLive()); } diff --git a/content/browser/frame_host/render_frame_proxy_host.cc b/content/browser/frame_host/render_frame_proxy_host.cc index 35ed286..1e99ff7 100644 --- a/content/browser/frame_host/render_frame_proxy_host.cc +++ b/content/browser/frame_host/render_frame_proxy_host.cc @@ -86,6 +86,12 @@ RenderViewHostImpl* RenderFrameProxyHost::GetRenderViewHost() { site_instance_.get()); } +void RenderFrameProxyHost::TakeFrameHostOwnership( + scoped_ptr<RenderFrameHostImpl> render_frame_host) { + render_frame_host_ = render_frame_host.Pass(); + render_frame_host_->set_render_frame_proxy_host(this); +} + scoped_ptr<RenderFrameHostImpl> RenderFrameProxyHost::PassFrameHostOwnership() { render_frame_host_->set_render_frame_proxy_host(NULL); return render_frame_host_.Pass(); diff --git a/content/browser/frame_host/render_frame_proxy_host.h b/content/browser/frame_host/render_frame_proxy_host.h index 2a34fc2..f2ea92a 100644 --- a/content/browser/frame_host/render_frame_proxy_host.h +++ b/content/browser/frame_host/render_frame_proxy_host.h @@ -89,9 +89,7 @@ class RenderFrameProxyHost RenderViewHostImpl* GetRenderViewHost(); void TakeFrameHostOwnership( - scoped_ptr<RenderFrameHostImpl> render_frame_host) { - render_frame_host_ = render_frame_host.Pass(); - } + scoped_ptr<RenderFrameHostImpl> render_frame_host); scoped_ptr<RenderFrameHostImpl> PassFrameHostOwnership(); // IPC::Sender diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h index 5c15250..2e467bc 100644 --- a/content/browser/renderer_host/render_view_host_delegate.h +++ b/content/browser/renderer_host/render_view_host_delegate.h @@ -156,9 +156,7 @@ class CONTENT_EXPORT RenderViewHostDelegate { // Notification that the renderer has become unresponsive. The // delegate can use this notification to show a warning to the user. - virtual void RendererUnresponsive(RenderViewHost* render_view_host, - bool is_during_before_unload, - bool is_during_unload) {} + virtual void RendererUnresponsive(RenderViewHost* render_view_host) {} // Notification that a previously unresponsive renderer has become // responsive again. The delegate can use this notification to end the diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc index 396be95..6e15855 100644 --- a/content/browser/renderer_host/render_view_host_impl.cc +++ b/content/browser/renderer_host/render_view_host_impl.cc @@ -34,7 +34,6 @@ #include "content/browser/host_zoom_map_impl.h" #include "content/browser/loader/resource_dispatcher_host_impl.h" #include "content/browser/renderer_host/dip_util.h" -#include "content/browser/renderer_host/input/timeout_monitor.h" #include "content/browser/renderer_host/media/audio_renderer_host.h" #include "content/browser/renderer_host/render_process_host_impl.h" #include "content/browser/renderer_host/render_view_host_delegate.h" @@ -136,14 +135,6 @@ const int RenderViewHostImpl::kUnloadTimeoutMS = 1000; // RenderViewHost, public: // static -bool RenderViewHostImpl::IsRVHStateActive(RenderViewHostImplState rvh_state) { - if (rvh_state == STATE_DEFAULT || - rvh_state == STATE_WAITING_FOR_CLOSE) - return true; - return false; -} - -// static RenderViewHost* RenderViewHost::FromID(int render_process_id, int render_view_id) { return RenderViewHostImpl::FromID(render_process_id, render_view_id); @@ -186,11 +177,12 @@ RenderViewHostImpl::RenderViewHostImpl( waiting_for_drag_context_response_(false), enabled_bindings_(0), page_id_(-1), + is_active_(!swapped_out), + is_swapped_out_(swapped_out), main_frame_routing_id_(main_frame_routing_id), run_modal_reply_msg_(NULL), run_modal_opener_id_(MSG_ROUTING_NONE), - is_waiting_for_beforeunload_ack_(false), - unload_ack_is_for_cross_site_transition_(false), + is_waiting_for_close_ack_(false), sudden_termination_allowed_(false), render_view_termination_status_(base::TERMINATION_STATUS_STILL_RUNNING), virtual_keyboard_requested_(false), @@ -202,13 +194,6 @@ RenderViewHostImpl::RenderViewHostImpl( GetProcess()->EnableSendQueue(); - if (swapped_out) { - rvh_state_ = STATE_SWAPPED_OUT; - } else { - rvh_state_ = STATE_DEFAULT; - instance_->increment_active_view_count(); - } - if (ResourceDispatcherHostImpl::Get()) { BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, @@ -220,9 +205,6 @@ RenderViewHostImpl::RenderViewHostImpl( #if defined(ENABLE_BROWSER_CDMS) media_web_contents_observer_.reset(new MediaWebContentsObserver(this)); #endif - - unload_event_monitor_timeout_.reset(new TimeoutMonitor(base::Bind( - &RenderViewHostImpl::OnSwappedOut, weak_factory_.GetWeakPtr(), true))); } RenderViewHostImpl::~RenderViewHostImpl() { @@ -235,18 +217,13 @@ RenderViewHostImpl::~RenderViewHostImpl() { } delegate_->RenderViewDeleted(this); - - // If this was swapped out, it already decremented the active view - // count of the SiteInstance it belongs to. - if (IsRVHStateActive(rvh_state_)) - instance_->decrement_active_view_count(); } RenderViewHostDelegate* RenderViewHostImpl::GetDelegate() const { return delegate_; } -SiteInstance* RenderViewHostImpl::GetSiteInstance() const { +SiteInstanceImpl* RenderViewHostImpl::GetSiteInstance() const { return instance_.get(); } @@ -292,7 +269,7 @@ bool RenderViewHostImpl::CreateRenderView( params.frame_name = frame_name; // Ensure the RenderView sets its opener correctly. params.opener_route_id = opener_route_id; - params.swapped_out = !IsRVHStateActive(rvh_state_); + params.swapped_out = !is_active_; params.proxy_routing_id = proxy_route_id; params.hidden = is_hidden(); params.never_visible = delegate_->IsNeverVisible(); @@ -490,73 +467,8 @@ void RenderViewHostImpl::SuppressDialogsUntilSwapOut() { Send(new ViewMsg_SuppressDialogsUntilSwapOut(GetRoutingID())); } -void RenderViewHostImpl::OnSwappedOut(bool timed_out) { - // Ignore spurious swap out ack. - if (!IsWaitingForUnloadACK()) - return; - - TRACE_EVENT0("navigation", "RenderViewHostImpl::OnSwappedOut"); - unload_event_monitor_timeout_->Stop(); - if (timed_out) { - base::ProcessHandle process_handle = GetProcess()->GetHandle(); - int views = 0; - - // Count the number of active widget hosts for the process, which - // is equivalent to views using the process as of this writing. - scoped_ptr<RenderWidgetHostIterator> widgets( - RenderWidgetHost::GetRenderWidgetHosts()); - while (RenderWidgetHost* widget = widgets->GetNextHost()) { - if (widget->GetProcess()->GetID() == GetProcess()->GetID()) - ++views; - } - - if (!RenderProcessHost::run_renderer_in_process() && - process_handle && views <= 1) { - // The process can safely be terminated, only if WebContents sets - // SuddenTerminationAllowed, which indicates that the timer has expired. - // This is not the case if we load data URLs or about:blank. The reason - // is that those have no network requests and this code is hit without - // setting the unresponsiveness timer. This allows a corner case where a - // navigation to a data URL will leave a process running, if the - // beforeunload handler completes fine, but the unload handler hangs. - // At this time, the complexity to solve this edge case is not worthwhile. - if (SuddenTerminationAllowed()) { - // We should kill the process, but for now, just log the data so we can - // diagnose the kill rate and investigate if separate timer is needed. - // http://crbug.com/104346. - - // Log a histogram point to help us diagnose how many of those kills - // we have performed. 1 is the enum value for RendererType Normal for - // the histogram. - UMA_HISTOGRAM_PERCENTAGE( - "BrowserRenderProcessHost.ChildKillsUnresponsive", 1); - } - } - // This is going to be incorrect for subframes and will only hit if - // --site-per-process is specified. - TRACE_EVENT_ASYNC_END0("navigation", "RenderFrameHostImpl::SwapOut", this); - } - - switch (rvh_state_) { - case STATE_PENDING_SWAP_OUT: - SetState(STATE_SWAPPED_OUT); - break; - case STATE_PENDING_SHUTDOWN: - DCHECK(!pending_shutdown_on_swap_out_.is_null()); - pending_shutdown_on_swap_out_.Run(); - break; - default: - NOTREACHED(); - } -} - -void RenderViewHostImpl::SetPendingShutdown(const base::Closure& on_swap_out) { - pending_shutdown_on_swap_out_ = on_swap_out; - SetState(STATE_PENDING_SHUTDOWN); -} - void RenderViewHostImpl::ClosePage() { - SetState(STATE_WAITING_FOR_CLOSE); + is_waiting_for_close_ack_ = true; StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS)); if (IsRenderViewLive()) { @@ -582,7 +494,7 @@ void RenderViewHostImpl::ClosePage() { void RenderViewHostImpl::ClosePageIgnoringUnloadEvents() { StopHangMonitorTimeout(); - is_waiting_for_beforeunload_ack_ = false; + is_waiting_for_close_ack_ = false; sudden_termination_allowed_ = true; delegate_->Close(this); @@ -873,7 +785,7 @@ bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) { // Filter out most IPC messages if this renderer is swapped out. // We still want to handle certain ACKs to keep our state consistent. - if (IsSwappedOut()) { + if (is_swapped_out_) { if (!SwappedOutMessages::CanHandleWhileSwappedOut(msg)) { // If this is a synchronous message and we decided not to handle it, // we must send an error reply, or else the renderer will be stuck @@ -1017,7 +929,7 @@ void RenderViewHostImpl::OnShowView(int route_id, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture) { - if (IsRVHStateActive(rvh_state_)) { + if (is_active_) { delegate_->ShowCreatedWindow( route_id, disposition, initial_pos, user_gesture); } @@ -1026,13 +938,13 @@ void RenderViewHostImpl::OnShowView(int route_id, void RenderViewHostImpl::OnShowWidget(int route_id, const gfx::Rect& initial_pos) { - if (IsRVHStateActive(rvh_state_)) + if (is_active_) delegate_->ShowCreatedWidget(route_id, initial_pos); Send(new ViewMsg_Move_ACK(route_id)); } void RenderViewHostImpl::OnShowFullscreenWidget(int route_id) { - if (IsRVHStateActive(rvh_state_)) + if (is_active_) delegate_->ShowCreatedFullscreenWidget(route_id); Send(new ViewMsg_Move_ACK(route_id)); } @@ -1095,7 +1007,7 @@ void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) { } void RenderViewHostImpl::OnUpdateTargetURL(const GURL& url) { - if (IsRVHStateActive(rvh_state_)) + if (is_active_) delegate_->UpdateTargetURL(url); // Send a notification back to the renderer that we are ready to @@ -1110,7 +1022,7 @@ void RenderViewHostImpl::OnClose() { } void RenderViewHostImpl::OnRequestMove(const gfx::Rect& pos) { - if (IsRVHStateActive(rvh_state_)) + if (is_active_) delegate_->RequestMove(pos); Send(new ViewMsg_Move_ACK(GetRoutingID())); } @@ -1260,8 +1172,7 @@ void RenderViewHostImpl::OnClosePageACK() { } void RenderViewHostImpl::NotifyRendererUnresponsive() { - delegate_->RendererUnresponsive( - this, is_waiting_for_beforeunload_ack_, IsWaitingForUnloadACK()); + delegate_->RendererUnresponsive(this); } void RenderViewHostImpl::NotifyRendererResponsive() { @@ -1335,12 +1246,6 @@ void RenderViewHostImpl::ForwardKeyboardEvent( RenderWidgetHostImpl::ForwardKeyboardEvent(key_event); } -bool RenderViewHostImpl::IsWaitingForUnloadACK() const { - return rvh_state_ == STATE_WAITING_FOR_CLOSE || - rvh_state_ == STATE_PENDING_SHUTDOWN || - rvh_state_ == STATE_PENDING_SWAP_OUT; -} - void RenderViewHostImpl::OnTextSurroundingSelectionResponse( const base::string16& content, size_t start_offset, @@ -1469,27 +1374,6 @@ void RenderViewHostImpl::OnFocusedNodeTouched(bool editable) { #endif } -void RenderViewHostImpl::SetState(RenderViewHostImplState rvh_state) { - // We update the number of RenderViews in a SiteInstance when the - // swapped out status of this RenderView gets flipped to/from live. - if (!IsRVHStateActive(rvh_state_) && IsRVHStateActive(rvh_state)) - instance_->increment_active_view_count(); - else if (IsRVHStateActive(rvh_state_) && !IsRVHStateActive(rvh_state)) - instance_->decrement_active_view_count(); - - // Whenever we change the RVH state to and from live or swapped out state, we - // should not be waiting for beforeunload or unload acks. We clear them here - // to be safe, since they can cause navigations to be ignored in OnNavigate. - if (rvh_state == STATE_DEFAULT || - rvh_state == STATE_SWAPPED_OUT || - rvh_state_ == STATE_DEFAULT || - rvh_state_ == STATE_SWAPPED_OUT) { - is_waiting_for_beforeunload_ack_ = false; - } - rvh_state_ = rvh_state; - -} - bool RenderViewHostImpl::CanAccessFilesOfPageState( const PageState& state) const { ChildProcessSecurityPolicyImpl* policy = diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h index dde5a36..2499c64 100644 --- a/content/browser/renderer_host/render_view_host_impl.h +++ b/content/browser/renderer_host/render_view_host_impl.h @@ -56,7 +56,6 @@ class RenderWidgetHostDelegate; class SessionStorageNamespace; class SessionStorageNamespaceImpl; class TestRenderViewHost; -class TimeoutMonitor; struct FileChooserParams; #if defined(COMPILER_MSVC) @@ -89,32 +88,6 @@ class CONTENT_EXPORT RenderViewHostImpl : public RenderViewHost, public RenderWidgetHostImpl { public: - // Keeps track of the state of the RenderViewHostImpl, particularly with - // respect to swap out. - enum RenderViewHostImplState { - // The standard state for a RVH handling the communication with a - // RenderView. - STATE_DEFAULT = 0, - // The RVH is waiting for the CloseACK from the RenderView. - STATE_WAITING_FOR_CLOSE, - // The RVH has not received the SwapOutACK yet, but the new page has - // committed in a different RVH. The number of active views of the RVH - // SiteInstanceImpl is not zero. Upon reception of the SwapOutACK, the RVH - // will be swapped out. - STATE_PENDING_SWAP_OUT, - // The RVH has not received the SwapOutACK yet, but the new page has - // committed in a different RVH. The number of active views of the RVH - // SiteInstanceImpl is zero. Upon reception of the SwapOutACK, the RVH will - // be shutdown. - STATE_PENDING_SHUTDOWN, - // The RVH is swapped out, and it is being used as a placeholder to allow - // for cross-process communication. - STATE_SWAPPED_OUT, - }; - // Helper function to determine whether the RVH state should contribute to the - // number of active views of a SiteInstance or not. - static bool IsRVHStateActive(RenderViewHostImplState rvh_state); - // Convenience function, just like RenderViewHost::FromID. static RenderViewHostImpl* FromID(int render_process_id, int render_view_id); @@ -186,7 +159,7 @@ class CONTENT_EXPORT RenderViewHostImpl FileChooserParams::Mode permissions) OVERRIDE; virtual RenderViewHostDelegate* GetDelegate() const OVERRIDE; virtual int GetEnabledBindings() const OVERRIDE; - virtual SiteInstance* GetSiteInstance() const OVERRIDE; + virtual SiteInstanceImpl* GetSiteInstance() const OVERRIDE; virtual bool IsRenderViewLive() const OVERRIDE; virtual void NotifyMoveOrResizeStarted() OVERRIDE; virtual void SetWebUIProperty(const std::string& name, @@ -236,12 +209,17 @@ class CONTENT_EXPORT RenderViewHostImpl // Returns the content specific prefs for this RenderViewHost. WebPreferences ComputeWebkitPrefs(const GURL& url); - // Whether this RenderViewHost has been swapped out to be displayed by a - // different process. - bool IsSwappedOut() const { return rvh_state_ == STATE_SWAPPED_OUT; } + // Tracks whether this RenderViewHost is in an active state (rather than + // pending swap out, pending deletion, or swapped out), according to its main + // frame RenderFrameHost. + bool is_active() const { return is_active_; } + void set_is_active(bool is_active) { is_active_ = is_active; } - // The current state of this RVH. - RenderViewHostImplState rvh_state() const { return rvh_state_; } + // Tracks whether this RenderViewHost is swapped out, according to its main + // frame RenderFrameHost. + void set_is_swapped_out(bool is_swapped_out) { + is_swapped_out_ = is_swapped_out; + } // Tells the renderer that this RenderView will soon be swapped out, and thus // not to create any new modal dialogs until it happens. This must be done @@ -249,14 +227,6 @@ class CONTENT_EXPORT RenderViewHostImpl // longer on the stack when we attempt to swap it out. void SuppressDialogsUntilSwapOut(); - // Called when either the SwapOut request has been acknowledged or has timed - // out. - void OnSwappedOut(bool timed_out); - - // Set |this| as pending shutdown. |on_swap_out| will be called - // when the SwapOutACK is received, or when the unload timer times out. - void SetPendingShutdown(const base::Closure& on_swap_out); - // Close the page ignoring whether it has unload events registers. // This is called after the beforeunload and unload events have fired // and the user has agreed to continue with closing the page. @@ -330,13 +300,6 @@ class CONTENT_EXPORT RenderViewHostImpl return main_frame_routing_id_; } - bool is_waiting_for_beforeunload_ack() { - return is_waiting_for_beforeunload_ack_; - } - - // Whether the RVH is waiting for the unload ack from the renderer. - bool IsWaitingForUnloadACK() const; - void OnTextSurroundingSelectionResponse(const base::string16& content, size_t start_offset, size_t end_offset); @@ -425,10 +388,6 @@ class CONTENT_EXPORT RenderViewHostImpl // to fire. static const int kUnloadTimeoutMS; - // Updates the state of this RenderViewHost and clears any waiting state - // that is no longer relevant. - void SetState(RenderViewHostImplState rvh_state); - bool CanAccessFilesOfPageState(const PageState& state) const; // The number of RenderFrameHosts which have a reference to this RVH. @@ -438,7 +397,7 @@ class CONTENT_EXPORT RenderViewHostImpl RenderViewHostDelegate* delegate_; // The SiteInstance associated with this RenderViewHost. All pages drawn - // in this RenderViewHost are part of this SiteInstance. Should not change + // in this RenderViewHost are part of this SiteInstance. Cannot change // over time. scoped_refptr<SiteInstanceImpl> instance_; @@ -455,9 +414,16 @@ class CONTENT_EXPORT RenderViewHostImpl // TODO(creis): Allocate this in WebContents/NavigationController instead. int32 page_id_; - // The current state of this RVH. - // TODO(nasko): Move to RenderFrameHost, as this is per-frame state. - RenderViewHostImplState rvh_state_; + // Tracks whether this RenderViewHost is in an active state. False if the + // main frame is pending swap out, pending deletion, or swapped out, because + // it is not visible to the user in any of these cases. + bool is_active_; + + // Tracks whether the main frame RenderFrameHost is swapped out. Unlike + // is_active_, this is false when the frame is pending swap out or deletion. + // TODO(creis): Remove this when we no longer use swappedout://. + // See http://crbug.com/357747. + bool is_swapped_out_; // Routing ID for the main frame's RenderFrameHost. int main_frame_routing_id_; @@ -468,22 +434,10 @@ class CONTENT_EXPORT RenderViewHostImpl // This will hold the routing id of the RenderView that opened us. int run_modal_opener_id_; - // Set to true when there is a pending ViewMsg_ShouldClose message. This - // ensures we don't spam the renderer with multiple beforeunload requests. - // When either this value or IsWaitingForUnloadACK is true, the value of - // unload_ack_is_for_cross_site_transition_ indicates whether this is for a - // cross-site transition or a tab close attempt. - // TODO(clamy): Remove this boolean and add one more state to the state - // machine. - // TODO(nasko): Move to RenderFrameHost, as this is per-frame state. - bool is_waiting_for_beforeunload_ack_; - - // Valid only when is_waiting_for_beforeunload_ack_ or - // IsWaitingForUnloadACK 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). - // TODO(nasko): Move to RenderFrameHost, as this is per-frame state. - bool unload_ack_is_for_cross_site_transition_; + // Set to true when waiting for a ViewHostMsg_ClosePageACK. + // TODO(creis): Move to RenderFrameHost and RenderWidgetHost. + // See http://crbug.com/418265. + bool is_waiting_for_close_ack_; // True if the render view can be shut down suddenly. bool sudden_termination_allowed_; @@ -499,17 +453,6 @@ class CONTENT_EXPORT RenderViewHostImpl scoped_ptr<MediaWebContentsObserver> media_web_contents_observer_; #endif - // Used to swap out or shutdown this RVH when the unload event is taking too - // long to execute, depending on the number of active views in the - // SiteInstance. - // TODO(nasko): Move to RenderFrameHost, as this is per-frame state. - scoped_ptr<TimeoutMonitor> unload_event_monitor_timeout_; - - // Called after receiving the SwapOutACK when the RVH is in state pending - // shutdown. Also called if the unload timer times out. - // TODO(nasko): Move to RenderFrameHost, as this is per-frame state. - base::Closure pending_shutdown_on_swap_out_; - // True if the current focused element is editable. bool is_focused_element_editable_; diff --git a/content/browser/renderer_host/render_view_host_unittest.cc b/content/browser/renderer_host/render_view_host_unittest.cc index b2cd8a8..be0a1d8 100644 --- a/content/browser/renderer_host/render_view_host_unittest.cc +++ b/content/browser/renderer_host/render_view_host_unittest.cc @@ -99,11 +99,11 @@ TEST_F(RenderViewHostTest, ResetUnloadOnReload) { url2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string()); // Simulate the ClosePage call which is normally sent by the net::URLRequest. rvh()->ClosePage(); - // Needed so that navigations are not suspended on the RVH. - test_rvh()->SendBeforeUnloadACK(true); + // Needed so that navigations are not suspended on the RFH. + main_test_rfh()->SendBeforeUnloadACK(true); contents()->Stop(); controller().Reload(false); - EXPECT_FALSE(test_rvh()->IsWaitingForUnloadACK()); + EXPECT_FALSE(main_test_rfh()->IsWaitingForUnloadACK()); } // Ensure we do not grant bindings to a process shared with unprivileged views. diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc index f9fb437cd..1d1db0d 100644 --- a/content/browser/renderer_host/render_widget_host_impl.cc +++ b/content/browser/renderer_host/render_widget_host_impl.cc @@ -284,8 +284,7 @@ scoped_ptr<RenderWidgetHostIterator> RenderWidgetHost::GetRenderWidgetHosts() { // Add only active RenderViewHosts. RenderViewHost* rvh = RenderViewHost::From(widget); - if (RenderViewHostImpl::IsRVHStateActive( - static_cast<RenderViewHostImpl*>(rvh)->rvh_state())) + if (static_cast<RenderViewHostImpl*>(rvh)->is_active()) hosts->Add(widget); } diff --git a/content/browser/site_instance_impl.cc b/content/browser/site_instance_impl.cc index 987dd5c..f1555dc 100644 --- a/content/browser/site_instance_impl.cc +++ b/content/browser/site_instance_impl.cc @@ -25,7 +25,7 @@ int32 SiteInstanceImpl::next_site_instance_id_ = 1; SiteInstanceImpl::SiteInstanceImpl(BrowsingInstance* browsing_instance) : id_(next_site_instance_id_++), - active_view_count_(0), + active_frame_count_(0), browsing_instance_(browsing_instance), process_(NULL), has_site_(false) { diff --git a/content/browser/site_instance_impl.h b/content/browser/site_instance_impl.h index 69197ee..9086e09 100644 --- a/content/browser/site_instance_impl.h +++ b/content/browser/site_instance_impl.h @@ -45,28 +45,27 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance, // navigating to the URL. bool HasWrongProcessForURL(const GURL& url); - // Increase the number of active views in this SiteInstance. This is - // increased when a view is created, or a currently swapped out view + // Increase the number of active frames in this SiteInstance. This is + // increased when a frame is created, or a currently swapped out frame // is swapped in. - void increment_active_view_count() { active_view_count_++; } + void increment_active_frame_count() { active_frame_count_++; } - // Decrease the number of active views in this SiteInstance. This is - // decreased when a view is destroyed, or a currently active view is + // Decrease the number of active frames in this SiteInstance. This is + // decreased when a frame is destroyed, or a currently active frame is // swapped out. - void decrement_active_view_count() { active_view_count_--; } + void decrement_active_frame_count() { active_frame_count_--; } - // Get the number of active views which belong to this - // SiteInstance. If there is no active view left in this - // SiteInstance, all view in this SiteInstance can be safely - // discarded to save memory. - size_t active_view_count() { return active_view_count_; } + // Get the number of active frames which belong to this SiteInstance. If + // there are no active frames left, all frames in this SiteInstance can be + // safely discarded. + size_t active_frame_count() { return active_frame_count_; } // Increase the number of active WebContentses using this SiteInstance. Note - // that, unlike active_view_count, this does not count pending RVHs. + // that, unlike active_frame_count, this does not count pending RFHs. void IncrementRelatedActiveContentsCount(); // Decrease the number of active WebContentses using this SiteInstance. Note - // that, unlike active_view_count, this does not count pending RVHs. + // that, unlike active_frame_count, this does not count pending RFHs. void DecrementRelatedActiveContentsCount(); // Sets the global factory used to create new RenderProcessHosts. It may be @@ -113,8 +112,8 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance, // A unique ID for this SiteInstance. int32 id_; - // The number of active views under this SiteInstance. - size_t active_view_count_; + // The number of active frames in this SiteInstance. + size_t active_frame_count_; // BrowsingInstance to which this SiteInstance belongs. scoped_refptr<BrowsingInstance> browsing_instance_; diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 2be629a..a448d34 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -895,14 +895,15 @@ void WebContentsImpl::CopyMaxPageIDsFrom(WebContents* web_contents) { max_page_ids_ = contents->max_page_ids_; } -SiteInstance* WebContentsImpl::GetSiteInstance() const { +SiteInstanceImpl* WebContentsImpl::GetSiteInstance() const { return GetRenderManager()->current_host()->GetSiteInstance(); } -SiteInstance* WebContentsImpl::GetPendingSiteInstance() const { - RenderViewHost* dest_rvh = GetRenderManager()->pending_render_view_host() ? - GetRenderManager()->pending_render_view_host() : - GetRenderManager()->current_host(); +SiteInstanceImpl* WebContentsImpl::GetPendingSiteInstance() const { + RenderViewHostImpl* dest_rvh = + GetRenderManager()->pending_render_view_host() ? + GetRenderManager()->pending_render_view_host() : + GetRenderManager()->current_host(); return dest_rvh->GetSiteInstance(); } @@ -3414,8 +3415,7 @@ void WebContentsImpl::RunJavaScriptMessage( // showing an interstitial as it's shown over the previous page and we don't // want the hidden page's dialogs to interfere with the interstitial. bool suppress_this_message = - static_cast<RenderViewHostImpl*>(render_frame_host->GetRenderViewHost())-> - IsSwappedOut() || + static_cast<RenderFrameHostImpl*>(render_frame_host)->is_swapped_out() || ShowingInterstitialPage() || !delegate_ || delegate_->ShouldSuppressDialogs() || @@ -3460,13 +3460,11 @@ void WebContentsImpl::RunBeforeUnloadConfirm( IPC::Message* reply_msg) { RenderFrameHostImpl* rfhi = static_cast<RenderFrameHostImpl*>(render_frame_host); - RenderViewHostImpl* rvhi = - static_cast<RenderViewHostImpl*>(render_frame_host->GetRenderViewHost()); if (delegate_) delegate_->WillRunBeforeUnloadConfirm(); bool suppress_this_message = - rvhi->rvh_state() != RenderViewHostImpl::STATE_DEFAULT || + rfhi->rfh_state() != RenderFrameHostImpl::STATE_DEFAULT || !delegate_ || delegate_->ShouldSuppressDialogs() || !delegate_->GetJavaScriptDialogManager(); @@ -3525,7 +3523,7 @@ void WebContentsImpl::RenderViewCreated(RenderViewHost* render_view_host) { // Don't send notifications if we are just creating a swapped-out RVH for // the opener chain. These won't be used for view-source or WebUI, so it's // ok to return early. - if (static_cast<RenderViewHostImpl*>(render_view_host)->IsSwappedOut()) + if (!static_cast<RenderViewHostImpl*>(render_view_host)->is_active()) return; if (delegate_) @@ -3980,14 +3978,14 @@ void WebContentsImpl::OnIgnoredUIEvent() { FOR_EACH_OBSERVER(WebContentsObserver, observers_, DidGetIgnoredUIEvent()); } -void WebContentsImpl::RendererUnresponsive(RenderViewHost* rvh, - bool is_during_beforeunload, - bool is_during_unload) { +void WebContentsImpl::RendererUnresponsive(RenderViewHost* render_view_host) { // Don't show hung renderer dialog for a swapped out RVH. - if (rvh != GetRenderViewHost()) + if (render_view_host != GetRenderViewHost()) return; - RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(rvh); + RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(render_view_host); + RenderFrameHostImpl* rfhi = + static_cast<RenderFrameHostImpl*>(rvhi->GetMainFrame()); // Ignore renderer unresponsive event if debugger is attached to the tab // since the event may be a result of the renderer sitting on a breakpoint. @@ -3995,7 +3993,8 @@ void WebContentsImpl::RendererUnresponsive(RenderViewHost* rvh, if (DevToolsAgentHost::IsDebuggerAttached(this)) return; - if (is_during_beforeunload || is_during_unload) { + if (rfhi->is_waiting_for_beforeunload_ack() || + rfhi->IsWaitingForUnloadACK()) { // Hang occurred while firing the beforeunload/unload handler. // Pretend the handler fired so tab closing continues as if it had. rvhi->set_sudden_termination_allowed(true); @@ -4010,11 +4009,11 @@ void WebContentsImpl::RendererUnresponsive(RenderViewHost* rvh, // close. Otherwise, pretend the unload listeners have all fired and close // the tab. bool close = true; - if (is_during_beforeunload && delegate_) { + if (rfhi->is_waiting_for_beforeunload_ack() && delegate_) { delegate_->BeforeUnloadFired(this, true, &close); } if (close) - Close(rvh); + Close(rvhi); return; } diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 2e5a073..f29d27c 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -232,8 +232,8 @@ class CONTENT_EXPORT WebContentsImpl virtual int32 GetMaxPageID() OVERRIDE; virtual int32 GetMaxPageIDForSiteInstance( SiteInstance* site_instance) OVERRIDE; - virtual SiteInstance* GetSiteInstance() const OVERRIDE; - virtual SiteInstance* GetPendingSiteInstance() const OVERRIDE; + virtual SiteInstanceImpl* GetSiteInstance() const OVERRIDE; + virtual SiteInstanceImpl* GetPendingSiteInstance() const OVERRIDE; virtual bool IsLoading() const OVERRIDE; virtual bool IsLoadingToDifferentDocument() const OVERRIDE; virtual bool IsWaitingForResponse() const OVERRIDE; @@ -434,9 +434,7 @@ class CONTENT_EXPORT WebContentsImpl virtual WebPreferences ComputeWebkitPrefs() OVERRIDE; virtual void OnUserGesture() OVERRIDE; virtual void OnIgnoredUIEvent() OVERRIDE; - virtual void RendererUnresponsive(RenderViewHost* render_view_host, - bool is_during_beforeunload, - bool is_during_unload) OVERRIDE; + virtual void RendererUnresponsive(RenderViewHost* render_view_host) OVERRIDE; virtual void RendererResponsive(RenderViewHost* render_view_host) OVERRIDE; virtual void LoadStateChanged(const GURL& url, const net::LoadStateWithParam& load_state, diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc index a9ce61d..c5e50da 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc @@ -478,10 +478,9 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED); - // Keep the number of active views in orig_rfh's SiteInstance non-zero so that - // orig_rfh doesn't get deleted when it gets swapped out. - static_cast<SiteInstanceImpl*>(orig_rfh->GetSiteInstance())-> - increment_active_view_count(); + // Keep the number of active frames in orig_rfh's SiteInstance non-zero so + // that orig_rfh doesn't get deleted when it gets swapped out. + orig_rfh->GetSiteInstance()->increment_active_frame_count(); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(orig_rfh->GetRenderViewHost(), contents()->GetRenderViewHost()); @@ -502,7 +501,7 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { // Navigations should be suspended in pending_rfh until BeforeUnloadACK. EXPECT_TRUE(pending_rfh->are_navigations_suspended()); - orig_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + orig_rfh->SendBeforeUnloadACK(true); EXPECT_FALSE(pending_rfh->are_navigations_suspended()); // DidNavigate from the pending page @@ -510,11 +509,10 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { pending_rfh, 1, url2, ui::PAGE_TRANSITION_TYPED); SiteInstance* instance2 = contents()->GetSiteInstance(); - // Keep the number of active views in pending_rfh's SiteInstance + // Keep the number of active frames in pending_rfh's SiteInstance // non-zero so that orig_rfh doesn't get deleted when it gets // swapped out. - static_cast<SiteInstanceImpl*>(pending_rfh->GetSiteInstance())-> - increment_active_view_count(); + pending_rfh->GetSiteInstance()->increment_active_frame_count(); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(pending_rfh, contents()->GetMainFrame()); @@ -537,7 +535,7 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { // Navigations should be suspended in goback_rfh until BeforeUnloadACK. EXPECT_TRUE(goback_rfh->are_navigations_suspended()); - pending_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + pending_rfh->SendBeforeUnloadACK(true); EXPECT_FALSE(goback_rfh->are_navigations_suspended()); // DidNavigate from the back action @@ -549,7 +547,7 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { EXPECT_TRUE(contents()->GetRenderManagerForTesting()-> IsOnSwappedOutList(pending_rfh)); EXPECT_EQ(pending_rvh_delete_count, 0); - pending_rfh->OnSwappedOut(false); + pending_rfh->OnSwappedOut(); // Close contents and ensure RVHs are deleted. DeleteContents(); @@ -631,7 +629,7 @@ TEST_F(WebContentsImplTest, NavigateTwoTabsCrossSite) { const GURL url2a("http://www.yahoo.com"); controller().LoadURL( url2a, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - orig_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + orig_rfh->SendBeforeUnloadACK(true); TestRenderFrameHost* pending_rfh_a = contents()->GetPendingMainFrame(); contents()->TestDidNavigate( pending_rfh_a, 1, url2a, ui::PAGE_TRANSITION_TYPED); @@ -644,7 +642,7 @@ TEST_F(WebContentsImplTest, NavigateTwoTabsCrossSite) { ui::PAGE_TRANSITION_TYPED, std::string()); TestRenderFrameHost* rfh2 = contents2->GetMainFrame(); - rfh2->GetRenderViewHost()->SendBeforeUnloadACK(true); + rfh2->SendBeforeUnloadACK(true); TestRenderFrameHost* pending_rfh_b = contents2->GetPendingMainFrame(); EXPECT_TRUE(pending_rfh_b != NULL); EXPECT_TRUE(contents2->cross_navigation_pending()); @@ -672,8 +670,7 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) { TestRenderFrameHost* orig_rfh = contents()->GetMainFrame(); int orig_rvh_delete_count = 0; orig_rfh->GetRenderViewHost()->set_delete_counter(&orig_rvh_delete_count); - SiteInstanceImpl* orig_instance = - static_cast<SiteInstanceImpl*>(contents()->GetSiteInstance()); + SiteInstanceImpl* orig_instance = contents()->GetSiteInstance(); browser_client.set_assign_site_for_url(false); // Navigate to an URL that will not assign a new SiteInstance. @@ -702,11 +699,10 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) { EXPECT_FALSE(contents()->GetPendingMainFrame()); contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED); - // Keep the number of active views in orig_rfh's SiteInstance + // Keep the number of active frames in orig_rfh's SiteInstance // non-zero so that orig_rfh doesn't get deleted when it gets // swapped out. - static_cast<SiteInstanceImpl*>(orig_rfh->GetSiteInstance())-> - increment_active_view_count(); + orig_rfh->GetSiteInstance()->increment_active_frame_count(); EXPECT_EQ(orig_instance, contents()->GetSiteInstance()); EXPECT_TRUE( @@ -727,7 +723,7 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) { // Navigations should be suspended in pending_rvh until BeforeUnloadACK. EXPECT_TRUE(pending_rfh->are_navigations_suspended()); - orig_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + orig_rfh->SendBeforeUnloadACK(true); EXPECT_FALSE(pending_rfh->are_navigations_suspended()); // DidNavigate from the pending page. @@ -745,7 +741,7 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) { EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList( orig_rfh)); EXPECT_EQ(orig_rvh_delete_count, 0); - orig_rfh->OnSwappedOut(false); + orig_rfh->OnSwappedOut(); // Close contents and ensure RVHs are deleted. DeleteContents(); @@ -759,8 +755,7 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) { TEST_F(WebContentsImplTest, NavigateFromRestoredSitelessUrl) { WebContentsImplTestBrowserClient browser_client; SetBrowserClientForTesting(&browser_client); - SiteInstanceImpl* orig_instance = - static_cast<SiteInstanceImpl*>(contents()->GetSiteInstance()); + SiteInstanceImpl* orig_instance = contents()->GetSiteInstance(); TestRenderFrameHost* orig_rfh = contents()->GetMainFrame(); // Restore a navigation entry for URL that should not assign site to the @@ -803,8 +798,7 @@ TEST_F(WebContentsImplTest, NavigateFromRestoredSitelessUrl) { TEST_F(WebContentsImplTest, NavigateFromRestoredRegularUrl) { WebContentsImplTestBrowserClient browser_client; SetBrowserClientForTesting(&browser_client); - SiteInstanceImpl* orig_instance = - static_cast<SiteInstanceImpl*>(contents()->GetSiteInstance()); + SiteInstanceImpl* orig_instance = contents()->GetSiteInstance(); TestRenderFrameHost* orig_rfh = contents()->GetMainFrame(); // Restore a navigation entry for a regular URL ensuring that the embedder @@ -856,7 +850,7 @@ TEST_F(WebContentsImplTest, FindOpenerRVHWhenPending) { const GURL url2("http://www.yahoo.com"); controller().LoadURL( url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - orig_rfh->GetRenderViewHost()->SendBeforeUnloadACK(true); + orig_rfh->SendBeforeUnloadACK(true); TestRenderFrameHost* pending_rfh = contents()->GetPendingMainFrame(); // While it is still pending, simulate opening a new tab with the first tab @@ -937,24 +931,22 @@ TEST_F(WebContentsImplTest, CrossSiteUnloadHandlers) { const GURL url2("http://www.yahoo.com"); controller().LoadURL( url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(orig_rfh->is_waiting_for_beforeunload_ack()); base::TimeTicks now = base::TimeTicks::Now(); orig_rfh->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, false, now, now)); - EXPECT_FALSE( - orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(orig_rfh->is_waiting_for_beforeunload_ack()); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(orig_rfh, contents()->GetMainFrame()); // Navigate again, but simulate an onbeforeunload approval. controller().LoadURL( url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(orig_rfh->is_waiting_for_beforeunload_ack()); now = base::TimeTicks::Now(); orig_rfh->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); - EXPECT_FALSE( - orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(orig_rfh->is_waiting_for_beforeunload_ack()); EXPECT_TRUE(contents()->cross_navigation_pending()); TestRenderFrameHost* pending_rfh = contents()->GetPendingMainFrame(); @@ -989,7 +981,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationPreempted) { const GURL url2("http://www.yahoo.com"); controller().LoadURL( url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(orig_rfh->is_waiting_for_beforeunload_ack()); base::TimeTicks now = base::TimeTicks::Now(); orig_rfh->OnMessageReceived(FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); EXPECT_TRUE(contents()->cross_navigation_pending()); @@ -998,8 +990,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationPreempted) { orig_rfh->SendNavigate(2, GURL("http://www.google.com/foo")); // Verify that the pending navigation is cancelled. - EXPECT_FALSE( - orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(orig_rfh->is_waiting_for_beforeunload_ack()); SiteInstance* instance2 = contents()->GetSiteInstance(); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(orig_rfh, contents()->GetMainFrame()); @@ -1033,7 +1024,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationBackPreempted) { TestRenderFrameHost* google_rfh = contents()->GetPendingMainFrame(); // Simulate beforeunload approval. - EXPECT_TRUE(ntp_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(ntp_rfh->is_waiting_for_beforeunload_ack()); base::TimeTicks now = base::TimeTicks::Now(); ntp_rfh->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); @@ -1084,8 +1075,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationBackPreempted) { EXPECT_EQ(entry1, controller().GetPendingEntry()); // Simulate beforeunload approval. - EXPECT_TRUE( - google_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(google_rfh->is_waiting_for_beforeunload_ack()); now = base::TimeTicks::Now(); google_rfh->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); @@ -1132,15 +1122,14 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationNotPreemptedByFrame) { TestRenderFrameHost* child_rfh = orig_rfh->AppendChild("subframe"); child_rfh->SendNavigateWithTransition( 1, GURL("http://google.com/frame"), ui::PAGE_TRANSITION_AUTO_SUBFRAME); - EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(orig_rfh->is_waiting_for_beforeunload_ack()); // Now simulate the onbeforeunload approval and verify the navigation is // not canceled. base::TimeTicks now = base::TimeTicks::Now(); orig_rfh->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); - EXPECT_FALSE( - orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(orig_rfh->is_waiting_for_beforeunload_ack()); EXPECT_TRUE(contents()->cross_navigation_pending()); } @@ -1162,7 +1151,7 @@ TEST_F(WebContentsImplTest, CrossSiteNotPreemptedDuringBeforeUnload) { url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); TestRenderFrameHost* pending_rfh = contents()->GetPendingMainFrame(); EXPECT_TRUE(contents()->cross_navigation_pending()); - EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(orig_rfh->is_waiting_for_beforeunload_ack()); // Suppose the first navigation tries to commit now, with a // FrameMsg_Stop in flight. This should not cancel the pending navigation, @@ -1170,8 +1159,7 @@ TEST_F(WebContentsImplTest, CrossSiteNotPreemptedDuringBeforeUnload) { orig_rfh->SendNavigate(1, GURL("chrome://blah")); EXPECT_TRUE(contents()->cross_navigation_pending()); EXPECT_EQ(orig_rfh, contents()->GetMainFrame()); - EXPECT_FALSE( - orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(orig_rfh->is_waiting_for_beforeunload_ack()); // The pending navigation should be able to commit successfully. contents()->TestDidNavigate(pending_rfh, 1, url2, ui::PAGE_TRANSITION_TYPED); @@ -1197,14 +1185,14 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationCanceled) { const GURL url2("http://www.yahoo.com"); controller().LoadURL( url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); - EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_TRUE(orig_rfh->is_waiting_for_beforeunload_ack()); base::TimeTicks now = base::TimeTicks::Now(); orig_rfh->OnMessageReceived( FrameHostMsg_BeforeUnload_ACK(0, true, now, now)); EXPECT_TRUE(contents()->cross_navigation_pending()); // Simulate swap out message when the response arrives. - orig_rfh->OnSwappedOut(false); + orig_rfh->OnSwappedOut(); // Suppose the navigation doesn't get a chance to commit, and the user // navigates in the current RFH's SiteInstance. @@ -1213,13 +1201,11 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationCanceled) { // Verify that the pending navigation is cancelled and the renderer is no // longer swapped out. - EXPECT_FALSE( - orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack()); + EXPECT_FALSE(orig_rfh->is_waiting_for_beforeunload_ack()); SiteInstance* instance2 = contents()->GetSiteInstance(); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(orig_rfh, contents()->GetMainFrame()); - EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, - orig_rfh->GetRenderViewHost()->rvh_state()); + EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, orig_rfh->rfh_state()); EXPECT_EQ(instance1, instance2); EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL); } |