diff options
author | creis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-11 21:56:16 +0000 |
---|---|---|
committer | creis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-11 21:56:16 +0000 |
commit | 111ed4259b2771579f5cf7a7f31a885b6047f062 (patch) | |
tree | da6055d094db33d66e224dbe244e5c657c105988 /content | |
parent | b969efa462d851d9d2483d811166665ca778d8a2 (diff) | |
download | chromium_src-111ed4259b2771579f5cf7a7f31a885b6047f062.zip chromium_src-111ed4259b2771579f5cf7a7f31a885b6047f062.tar.gz chromium_src-111ed4259b2771579f5cf7a7f31a885b6047f062.tar.bz2 |
Avoid exiting the renderer process if it has a pending render view.
This addresses an issue for a future CL when we switch to swapping
RenderViewHosts in and out, rather than throwing them away each time.
BUG=65953
TEST=None
Review URL: http://codereview.chromium.org/6927014
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85054 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
7 files changed, 92 insertions, 1 deletions
diff --git a/content/browser/renderer_host/browser_render_process_host.cc b/content/browser/renderer_host/browser_render_process_host.cc index 2bbfadd..4868e89 100644 --- a/content/browser/renderer_host/browser_render_process_host.cc +++ b/content/browser/renderer_host/browser_render_process_host.cc @@ -786,6 +786,8 @@ bool BrowserRenderProcessHost::OnMessageReceived(const IPC::Message& msg) { // Dispatch control messages. bool msg_is_ok = true; IPC_BEGIN_MESSAGE_MAP_EX(BrowserRenderProcessHost, msg, msg_is_ok) + IPC_MESSAGE_HANDLER(ChildProcessHostMsg_ShutdownRequest, + OnShutdownRequest) IPC_MESSAGE_HANDLER(ViewHostMsg_UpdatedCacheStats, OnUpdatedCacheStats) IPC_MESSAGE_HANDLER(ViewHostMsg_SuddenTerminationChanged, @@ -825,6 +827,10 @@ void BrowserRenderProcessHost::OnChannelConnected(int32 peer_pid) { Send(new ChildProcessMsg_SetIPCLoggingEnabled( IPC::Logging::GetInstance()->Enabled())); #endif + + // Make sure the child checks with us before exiting, so that we do not try + // to schedule a new navigation in a swapped out and exiting renderer. + Send(new ChildProcessMsg_AskBeforeShutdown()); } void BrowserRenderProcessHost::OnChannelError() { @@ -880,6 +886,20 @@ void BrowserRenderProcessHost::OnChannelError() { // TODO(darin): clean this up } +void BrowserRenderProcessHost::OnShutdownRequest() { + // Don't shutdown if there are pending RenderViews being swapped back in. + if (pending_views_) + return; + + // Notify any tabs that might have swapped out renderers from this process. + // They should not attempt to swap them back in. + NotificationService::current()->Notify( + NotificationType::RENDERER_PROCESS_CLOSING, + Source<RenderProcessHost>(this), NotificationService::NoDetails()); + + Send(new ChildProcessMsg_Shutdown()); +} + void BrowserRenderProcessHost::OnUpdatedCacheStats( const WebCache::UsageStats& stats) { WebCacheManager::GetInstance()->ObserveStats(id(), stats); diff --git a/content/browser/renderer_host/browser_render_process_host.h b/content/browser/renderer_host/browser_render_process_host.h index c97648c..fb46c82 100644 --- a/content/browser/renderer_host/browser_render_process_host.h +++ b/content/browser/renderer_host/browser_render_process_host.h @@ -81,6 +81,7 @@ class BrowserRenderProcessHost : public RenderProcessHost, void CreateMessageFilters(); // Control message handlers. + void OnShutdownRequest(); void OnUpdatedCacheStats(const WebKit::WebCache::UsageStats& stats); void SuddenTerminationChanged(bool enabled); void OnUserMetricsRecordAction(const std::string& action); diff --git a/content/browser/renderer_host/render_process_host.cc b/content/browser/renderer_host/render_process_host.cc index f435e7c..5466004 100644 --- a/content/browser/renderer_host/render_process_host.cc +++ b/content/browser/renderer_host/render_process_host.cc @@ -95,6 +95,7 @@ RenderProcessHost::RenderProcessHost(Profile* profile) fast_shutdown_started_(false), deleting_soon_(false), is_extension_process_(false), + pending_views_(0), id_(ChildProcessInfo::GenerateChildProcessUniqueId()), profile_(profile), sudden_termination_allowed_(true), @@ -144,6 +145,15 @@ void RenderProcessHost::ReportExpectingClose(int32 listener_id) { listeners_expecting_close_.insert(listener_id); } +void RenderProcessHost::AddPendingView() { + pending_views_++; +} + +void RenderProcessHost::RemovePendingView() { + DCHECK(pending_views_); + pending_views_--; +} + void RenderProcessHost::UpdateMaxPageID(int32 page_id) { if (page_id > max_page_id_) max_page_id_ = page_id; diff --git a/content/browser/renderer_host/render_process_host.h b/content/browser/renderer_host/render_process_host.h index 4a76d1f..9d3b8fc 100644 --- a/content/browser/renderer_host/render_process_host.h +++ b/content/browser/renderer_host/render_process_host.h @@ -104,6 +104,12 @@ class RenderProcessHost : public IPC::Channel::Sender, // goes away we'll know that it was intentional rather than a crash. void ReportExpectingClose(int32 listener_id); + // Track the count of pending views that are being swapped back in. Called + // by listeners to register and unregister pending views to prevent the + // process from exiting. + void AddPendingView(); + void RemovePendingView(); + // Allows iteration over this RenderProcessHost's RenderViewHost listeners. // Use from UI thread only. typedef IDMap<IPC::Channel::Listener>::const_iterator listeners_iterator; @@ -282,6 +288,11 @@ class RenderProcessHost : public IPC::Channel::Sender, // when running in single-process mode. bool is_extension_process_; + // The count of currently swapped out but pending RenderViews. We have + // started to swap these in, so the renderer process should not exit if + // this count is non-zero. + int32 pending_views_; + private: // The globally-unique identifier for this RPH. int id_; diff --git a/content/browser/tab_contents/render_view_host_manager.cc b/content/browser/tab_contents/render_view_host_manager.cc index 7effc09..442e91c8 100644 --- a/content/browser/tab_contents/render_view_host_manager.cc +++ b/content/browser/tab_contents/render_view_host_manager.cc @@ -62,6 +62,10 @@ void RenderViewHostManager::Init(Profile* profile, render_view_host_ = RenderViewHostFactory::Create( site_instance, render_view_delegate_, routing_id, delegate_-> GetControllerForRenderManager().session_storage_namespace()); + + // Keep track of renderer processes as they start to shut down. + registrar_.Add(this, NotificationType::RENDERER_PROCESS_CLOSING, + NotificationService::AllSources()); } RenderWidgetHostView* RenderViewHostManager::GetRenderWidgetHostView() const { @@ -215,6 +219,12 @@ void RenderViewHostManager::RendererAbortedProvisionalLoad( // the response is not a download. } +void RenderViewHostManager::RendererProcessClosing( + RenderProcessHost* render_process_host) { + // TODO(creis): Don't schedule new navigations in RenderViewHosts of this + // process. (Part of http://crbug.com/65953.) +} + void RenderViewHostManager::ShouldClosePage(bool for_cross_site_transition, bool proceed) { if (for_cross_site_transition) { @@ -278,6 +288,19 @@ void RenderViewHostManager::OnCrossSiteNavigationCanceled() { CancelPending(); } +void RenderViewHostManager::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch (type.value) { + case NotificationType::RENDERER_PROCESS_CLOSING: + RendererProcessClosing(Source<RenderProcessHost>(source).ptr()); + break; + + default: + NOTREACHED(); + } +} + bool RenderViewHostManager::ShouldTransitionCrossSite() { // True if we are using process-per-site-instance (default) or // process-per-site (kProcessPerSite). @@ -463,6 +486,9 @@ bool RenderViewHostManager::CreatePendingRenderView( instance, render_view_delegate_, MSG_ROUTING_NONE, delegate_-> GetControllerForRenderManager().session_storage_namespace()); + // Prevent the process from exiting while we're trying to use it. + pending_render_view_host_->process()->AddPendingView(); + bool success = InitRenderView(pending_render_view_host_, entry); if (success) { // Don't show the view until we get a DidNavigate from it. @@ -521,6 +547,9 @@ void RenderViewHostManager::CommitPending() { render_view_host_ = pending_render_view_host_; pending_render_view_host_ = NULL; + // The process will no longer try to exit, so we can decrement the count. + render_view_host_->process()->RemovePendingView(); + // If the view is gone, then this RenderViewHost died while it was hidden. // We ignored the RenderViewGone call at the time, so we should send it now // to make sure the sad tab shows up, etc. @@ -655,6 +684,10 @@ RenderViewHost* RenderViewHostManager::UpdateRendererStateForNavigate( void RenderViewHostManager::CancelPending() { RenderViewHost* pending_render_view_host = pending_render_view_host_; + + // We no longer need to prevent the process from exiting. + pending_render_view_host->process()->RemovePendingView(); + pending_render_view_host_ = NULL; pending_render_view_host->Shutdown(); diff --git a/content/browser/tab_contents/render_view_host_manager.h b/content/browser/tab_contents/render_view_host_manager.h index 01ddbc8..e2033d15 100644 --- a/content/browser/tab_contents/render_view_host_manager.h +++ b/content/browser/tab_contents/render_view_host_manager.h @@ -10,6 +10,7 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "content/browser/renderer_host/render_view_host_delegate.h" +#include "content/common/notification_observer.h" #include "content/common/notification_registrar.h" class WebUI; @@ -25,7 +26,8 @@ class SiteInstance; // it is easy to do. But we can also have transitions of processes (and hence // RenderViewHosts) that can get complex. class RenderViewHostManager - : public RenderViewHostDelegate::RendererManagement { + : public RenderViewHostDelegate::RendererManagement, + public NotificationObserver { public: // Functions implemented by our owner that we need. // @@ -171,6 +173,11 @@ class RenderViewHostManager int new_request_id); virtual void OnCrossSiteNavigationCanceled(); + // NotificationObserver implementation. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + // Called when a RenderViewHost is about to be deleted. void RenderViewDeleted(RenderViewHost* rvh); @@ -224,6 +231,10 @@ class RenderViewHostManager RenderViewHost* UpdateRendererStateForNavigate(const NavigationEntry& entry); + // Called when a renderer process is starting to close. We should not + // schedule new navigations in its swapped out RenderViewHosts after this. + void RendererProcessClosing(RenderProcessHost* render_process_host); + // Our delegate, not owned by us. Guaranteed non-NULL. Delegate* delegate_; diff --git a/content/common/notification_type.h b/content/common/notification_type.h index 85a829b..c7e4d6a 100644 --- a/content/common/notification_type.h +++ b/content/common/notification_type.h @@ -408,6 +408,11 @@ class NotificationType { // RenderProcessHost that corresponds to the process. RENDERER_PROCESS_TERMINATED, + // Indicates that a render process is starting to exit, such that it should + // not be used for future navigations. The source will be the + // RenderProcessHost that corresponds to the process. + RENDERER_PROCESS_CLOSING, + // Indicates that a render process was closed (meaning it exited, but the // RenderProcessHost might be reused). The source will be the corresponding // RenderProcessHost. The details will be a RendererClosedDetails struct. |