diff options
author | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-07-11 20:22:32 +0000 |
---|---|---|
committer | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-07-11 20:22:32 +0000 |
commit | eda48c7a4e569aaf2f2327e0226762868346f531 (patch) | |
tree | eb1e837d6ff4a77add2df32c8286601c530a51b3 | |
parent | 34717bc08d6b56f276e49675acbbb5eb2fc48fbf (diff) | |
download | chromium_src-eda48c7a4e569aaf2f2327e0226762868346f531.zip chromium_src-eda48c7a4e569aaf2f2327e0226762868346f531.tar.gz chromium_src-eda48c7a4e569aaf2f2327e0226762868346f531.tar.bz2 |
Get rid of RenderWidgetHostDestroyed method in the various classes. RenderWidgetHost fires a notification and also calls RenderWidgetHostView. The notification was being observed by TabContents, which calls TabContentsView which tells the RenderWidgetHostView and RenderViewHostDelegateHelper. So I took out the extra indirection which was redundant, and just made RenderViewHostDelegateHelper observe the notification itself.
BUG=87702
Review URL: http://codereview.chromium.org/7324057
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@92045 0039d316-1c4b-4281-b951-d872f2087c98
11 files changed, 33 insertions, 70 deletions
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.h b/chrome/browser/renderer_host/render_widget_host_view_mac.h index 1d032e7..93862f8 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_mac.h +++ b/chrome/browser/renderer_host/render_widget_host_view_mac.h @@ -202,7 +202,6 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView { const std::vector<gfx::Rect>& copy_rects) OVERRIDE; virtual void RenderViewGone(base::TerminationStatus status, int error_code) OVERRIDE; - virtual void WillDestroyRenderWidget(RenderWidgetHost* rwh) OVERRIDE {}; virtual void Destroy() OVERRIDE; virtual void SetTooltipText(const std::wstring& tooltip_text) OVERRIDE; virtual void SelectionChanged(const std::string& text, diff --git a/chrome/browser/renderer_host/render_widget_host_view_views.h b/chrome/browser/renderer_host/render_widget_host_view_views.h index f1a44ea..86ea396 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_views.h +++ b/chrome/browser/renderer_host/render_widget_host_view_views.h @@ -76,7 +76,6 @@ class RenderWidgetHostViewViews : public RenderWidgetHostView, virtual void RenderViewGone(base::TerminationStatus status, int error_code) OVERRIDE; virtual void Destroy() OVERRIDE; - virtual void WillDestroyRenderWidget(RenderWidgetHost* rwh) OVERRIDE {} virtual void SetTooltipText(const std::wstring& tooltip_text) OVERRIDE; virtual void SelectionChanged(const std::string& text, const ui::Range& range) OVERRIDE; diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc index 7934837..f2c5e33 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_win.cc +++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc @@ -691,11 +691,6 @@ void RenderWidgetHostViewWin::WillWmDestroy() { CleanupCompositorWindow(); } -void RenderWidgetHostViewWin::WillDestroyRenderWidget(RenderWidgetHost* rwh) { - if (rwh == render_widget_host_) - render_widget_host_ = NULL; -} - void RenderWidgetHostViewWin::Destroy() { // We've been told to destroy. // By clearing close_on_deactivate_, we prevent further deactivations @@ -703,6 +698,7 @@ void RenderWidgetHostViewWin::Destroy() { // triggering further destructions. The deletion of this is handled by // OnFinalMessage(); close_on_deactivate_ = false; + render_widget_host_ = NULL; being_destroyed_ = true; CleanupCompositorWindow(); DestroyWindow(); @@ -1688,8 +1684,7 @@ LRESULT RenderWidgetHostViewWin::OnParentNotify(UINT message, WPARAM wparam, void RenderWidgetHostViewWin::OnFinalMessage(HWND window) { // When the render widget host is being destroyed, it ends up calling - // WillDestroyRenderWidget (through the RENDER_WIDGET_HOST_DESTROYED - // notification) which NULLs render_widget_host_. + // Destroy() which NULLs render_widget_host_. // Note: the following bug http://crbug.com/24248 seems to report that // OnFinalMessage is called with a deleted |render_widget_host_|. It is not // clear how this could happen, hence the NULLing of render_widget_host_ diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.h b/chrome/browser/renderer_host/render_widget_host_view_win.h index a079c55..d557f97 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_win.h +++ b/chrome/browser/renderer_host/render_widget_host_view_win.h @@ -159,7 +159,6 @@ class RenderWidgetHostViewWin int error_code) OVERRIDE; // called by TabContents before DestroyWindow virtual void WillWmDestroy() OVERRIDE; - virtual void WillDestroyRenderWidget(RenderWidgetHost* rwh) OVERRIDE; virtual void Destroy() OVERRIDE; virtual void SetTooltipText(const std::wstring& tooltip_text) OVERRIDE; virtual BackingStore* AllocBackingStore(const gfx::Size& size) OVERRIDE; diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc index 2b61535..3f23de0 100644 --- a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc +++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc @@ -32,11 +32,30 @@ #include "content/browser/tab_contents/tab_contents.h" #include "content/browser/tab_contents/tab_contents_view.h" #include "content/browser/webui/web_ui.h" +#include "content/common/notification_service.h" -RenderViewHostDelegateViewHelper::RenderViewHostDelegateViewHelper() {} +RenderViewHostDelegateViewHelper::RenderViewHostDelegateViewHelper() { + registrar_.Add(this, content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED, + NotificationService::AllSources()); +} RenderViewHostDelegateViewHelper::~RenderViewHostDelegateViewHelper() {} +void RenderViewHostDelegateViewHelper::Observe( + int type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK_EQ(type, content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED); + RenderWidgetHost* host = Source<RenderWidgetHost>(source).ptr(); + for (PendingWidgetViews::iterator i = pending_widget_views_.begin(); + i != pending_widget_views_.end(); ++i) { + if (host->view() == i->second) { + pending_widget_views_.erase(i); + break; + } + } +} + BackgroundContents* RenderViewHostDelegateViewHelper::MaybeCreateBackgroundContents( int route_id, @@ -202,17 +221,6 @@ RenderWidgetHostView* RenderViewHostDelegateViewHelper::GetCreatedWidget( return widget_host_view; } -void RenderViewHostDelegateViewHelper::RenderWidgetHostDestroyed( - RenderWidgetHost* host) { - for (PendingWidgetViews::iterator i = pending_widget_views_.begin(); - i != pending_widget_views_.end(); ++i) { - if (host->view() == i->second) { - pending_widget_views_.erase(i); - return; - } - } -} - // static WebPreferences RenderViewHostDelegateHelper::GetWebkitPrefs( Profile* profile, bool is_web_ui) { diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.h b/chrome/browser/tab_contents/render_view_host_delegate_helper.h index e0525fb..a2b2bf6 100644 --- a/chrome/browser/tab_contents/render_view_host_delegate_helper.h +++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.h @@ -10,6 +10,8 @@ #include "base/basictypes.h" #include "content/browser/webui/web_ui.h" +#include "content/common/notification_observer.h" +#include "content/common/notification_registrar.h" #include "content/common/window_container_type.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebPopupType.h" #include "ui/gfx/rect.h" @@ -28,7 +30,7 @@ class TabContents; // Provides helper methods that provide common implementations of some // RenderViewHostDelegate::View methods. -class RenderViewHostDelegateViewHelper { +class RenderViewHostDelegateViewHelper : public NotificationObserver { public: RenderViewHostDelegateViewHelper(); virtual ~RenderViewHostDelegateViewHelper(); @@ -66,10 +68,12 @@ class RenderViewHostDelegateViewHelper { // map. virtual TabContents* GetCreatedWindow(int route_id); - // Removes |host| from the internal map of pending RenderWidgets. - void RenderWidgetHostDestroyed(RenderWidgetHost* host); - private: + // NotificationObserver implementation + virtual void Observe(int type, + const NotificationSource& source, + const NotificationDetails& details) OVERRIDE; + BackgroundContents* MaybeCreateBackgroundContents( int route_id, Profile* profile, @@ -87,6 +91,9 @@ class RenderViewHostDelegateViewHelper { typedef std::map<int, RenderWidgetHostView*> PendingWidgetViews; PendingWidgetViews pending_widget_views_; + // Registers and unregisters us for notifications. + NotificationRegistrar registrar_; + DISALLOW_COPY_AND_ASSIGN(RenderViewHostDelegateViewHelper); }; diff --git a/content/browser/renderer_host/render_widget_host_view.h b/content/browser/renderer_host/render_widget_host_view.h index 56979b0..41cb005 100644 --- a/content/browser/renderer_host/render_widget_host_view.h +++ b/content/browser/renderer_host/render_widget_host_view.h @@ -170,9 +170,6 @@ class RenderWidgetHostView { virtual void RenderViewGone(base::TerminationStatus status, int error_code) = 0; - // Notifies the View that the renderer will be delete soon. - virtual void WillDestroyRenderWidget(RenderWidgetHost* rwh) = 0; - // Tells the View to destroy itself. virtual void Destroy() = 0; diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc index e6fbba1..98f03f9 100644 --- a/content/browser/tab_contents/tab_contents.cc +++ b/content/browser/tab_contents/tab_contents.cc @@ -199,9 +199,6 @@ TabContents::TabContents(Profile* profile, view_->CreateView(base_tab_contents ? base_tab_contents->view()->GetContainerSize() : gfx::Size()); - registrar_.Add(this, content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED, - NotificationService::AllSources()); - // Can only add observers after render_manager_.Init() is called, since that's // what sets up the render_view_host which TabContentObserver's constructor // uses to get the routing_id. @@ -211,9 +208,6 @@ TabContents::TabContents(Profile* profile, TabContents::~TabContents() { is_being_destroyed_ = true; - // We don't want any notifications while we're running our destructor. - registrar_.RemoveAll(); - // Clear out any JavaScript state. if (delegate_) delegate_->GetJavaScriptDialogCreator()->ResetJavaScriptState(this); @@ -1810,20 +1804,6 @@ bool TabContents::CreateRenderViewForRenderManager( return true; } -void TabContents::Observe(int type, - const NotificationSource& source, - const NotificationDetails& details) { - switch (type) { - case content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED: - view_->RenderWidgetHostDestroyed(Source<RenderWidgetHost>(source).ptr()); - break; - default: - NOTREACHED(); - } -} - -// Overridden from JavaScriptDialogDelegate - void TabContents::OnDialogClosed(IPC::Message* reply_msg, bool success, const string16& user_input) { diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h index 63e7506..018fc7a 100644 --- a/content/browser/tab_contents/tab_contents.h +++ b/content/browser/tab_contents/tab_contents.h @@ -23,7 +23,6 @@ #include "content/browser/tab_contents/render_view_host_manager.h" #include "content/browser/tab_contents/tab_contents_observer.h" #include "content/browser/webui/web_ui.h" -#include "content/common/notification_registrar.h" #include "content/common/property_bag.h" #include "content/common/renderer_preferences.h" #include "net/base/load_states.h" @@ -58,7 +57,6 @@ class WebUI; // Describes what goes in the main content area of a tab. TabContents is // the only type of TabContents, and these should be merged together. class TabContents : public PageNavigator, - public NotificationObserver, public RenderViewHostDelegate, public RenderViewHostManager::Delegate, public content::JavaScriptDialogDelegate, @@ -699,12 +697,6 @@ class TabContents : public PageNavigator, virtual bool CreateRenderViewForRenderManager( RenderViewHost* render_view_host); - // NotificationObserver ------------------------------------------------------ - - virtual void Observe(int type, - const NotificationSource& source, - const NotificationDetails& details); - // NetworkChangeNotifier::OnlineStateObserver: virtual void OnOnlineStateChanged(bool online); @@ -732,9 +724,6 @@ class TabContents : public PageNavigator, // Manages creation and swapping of render views. RenderViewHostManager render_manager_; - // Registers and unregisters us for notifications. - NotificationRegistrar registrar_; - // Data for loading state ---------------------------------------------------- // Indicates whether we're currently loading a resource. diff --git a/content/browser/tab_contents/tab_contents_view.cc b/content/browser/tab_contents/tab_contents_view.cc index 0019441..7458f54 100644 --- a/content/browser/tab_contents/tab_contents_view.cc +++ b/content/browser/tab_contents/tab_contents_view.cc @@ -20,12 +20,6 @@ TabContentsView::TabContentsView(TabContents* tab_contents) TabContentsView::~TabContentsView() {} -void TabContentsView::RenderWidgetHostDestroyed(RenderWidgetHost* host) { - if (host->view()) - host->view()->WillDestroyRenderWidget(host); - delegate_view_helper_.RenderWidgetHostDestroyed(host); -} - void TabContentsView::CreateNewWindow( int route_id, const ViewHostMsg_CreateWindow_Params& params) { diff --git a/content/browser/tab_contents/tab_contents_view.h b/content/browser/tab_contents/tab_contents_view.h index 1e6d73f..16ae3ab 100644 --- a/content/browser/tab_contents/tab_contents_view.h +++ b/content/browser/tab_contents/tab_contents_view.h @@ -93,10 +93,6 @@ class TabContentsView : public RenderViewHostDelegate::View { // TabContents without the special code. virtual void SizeContents(const gfx::Size& size) = 0; - // Invoked from the platform dependent web contents view when a - // RenderWidgetHost is deleted. Removes |host| from internal maps. - void RenderWidgetHostDestroyed(RenderWidgetHost* host); - // Invoked when the TabContents is notified that the RenderView has been // fully created. virtual void RenderViewCreated(RenderViewHost* host) {} |