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) {}