From 1f3157f5c807cae9cddd92fb8b6e4908da9adefb Mon Sep 17 00:00:00 2001 From: "pkasting@chromium.org" Date: Fri, 22 May 2009 22:57:48 +0000 Subject: Use a NotificationRegistrar to listen for notifications. BUG=2381 Review URL: http://codereview.chromium.org/113792 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16806 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/gtk/browser_window_gtk.cc | 10 +---- chrome/browser/gtk/browser_window_gtk.h | 4 +- chrome/browser/gtk/infobar_container_gtk.cc | 20 +++------- chrome/browser/gtk/infobar_container_gtk.h | 4 +- chrome/browser/gtk/tab_contents_container_gtk.cc | 47 ++++++------------------ chrome/browser/gtk/tab_contents_container_gtk.h | 4 +- 6 files changed, 29 insertions(+), 60 deletions(-) diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index 34e42f0..24d4509 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -354,17 +354,11 @@ BrowserWindowGtk::BrowserWindowGtk(Browser* browser) HideUnsupportedWindowFeatures(); - NotificationService* ns = NotificationService::current(); - ns->AddObserver(this, NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, - NotificationService::AllSources()); + registrar_.Add(this, NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, + NotificationService::AllSources()); } BrowserWindowGtk::~BrowserWindowGtk() { - NotificationService* ns = NotificationService::current(); - ns->RemoveObserver(this, - NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, - NotificationService::AllSources()); - browser_->tabstrip_model()->RemoveObserver(this); } diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h index 9982f35..ac51bec 100644 --- a/chrome/browser/gtk/browser_window_gtk.h +++ b/chrome/browser/gtk/browser_window_gtk.h @@ -12,7 +12,7 @@ #include "base/timer.h" #include "chrome/browser/browser_window.h" #include "chrome/browser/tabs/tab_strip_model.h" -#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" class BookmarkBarGtk; class BrowserToolbarGtk; @@ -167,6 +167,8 @@ class BrowserWindowGtk : public BrowserWindow, bool IsToolbarSupported(); + NotificationRegistrar registrar_; + gfx::Rect bounds_; GdkWindowState state_; diff --git a/chrome/browser/gtk/infobar_container_gtk.cc b/chrome/browser/gtk/infobar_container_gtk.cc index a44ef65..9489026 100644 --- a/chrome/browser/gtk/infobar_container_gtk.cc +++ b/chrome/browser/gtk/infobar_container_gtk.cc @@ -61,26 +61,18 @@ InfoBarContainerGtk::~InfoBarContainerGtk() { } void InfoBarContainerGtk::ChangeTabContents(TabContents* contents) { - if (tab_contents_) { - NotificationService::current()->RemoveObserver( - this, NotificationType::TAB_CONTENTS_INFOBAR_ADDED, - Source(tab_contents_)); - NotificationService::current()->RemoveObserver( - this, NotificationType::TAB_CONTENTS_INFOBAR_REMOVED, - Source(tab_contents_)); - } + if (tab_contents_) + registrar_.RemoveAll(); gtk_util::RemoveAllChildren(widget()); tab_contents_ = contents; if (tab_contents_) { UpdateInfoBars(); - NotificationService::current()->AddObserver( - this, NotificationType::TAB_CONTENTS_INFOBAR_ADDED, - Source(tab_contents_)); - NotificationService::current()->AddObserver( - this, NotificationType::TAB_CONTENTS_INFOBAR_REMOVED, - Source(tab_contents_)); + Source source(tab_contents_); + registrar_.Add(this, NotificationType::TAB_CONTENTS_INFOBAR_ADDED, source); + registrar_.Add(this, NotificationType::TAB_CONTENTS_INFOBAR_REMOVED, + source); } } diff --git a/chrome/browser/gtk/infobar_container_gtk.h b/chrome/browser/gtk/infobar_container_gtk.h index 58e0c52..a8fbeb9 100644 --- a/chrome/browser/gtk/infobar_container_gtk.h +++ b/chrome/browser/gtk/infobar_container_gtk.h @@ -6,7 +6,7 @@ #define CHROME_BROWSER_GTK_INFOBAR_CONTAINER_GTK_H_ #include "base/basictypes.h" -#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" #include "chrome/common/owned_widget_gtk.h" class BrowserWindow; @@ -57,6 +57,8 @@ class InfoBarContainerGtk : public NotificationObserver { // will be animated. void RemoveInfoBar(InfoBarDelegate* delegate); + NotificationRegistrar registrar_; + // The BrowserView that hosts this InfoBarContainer. BrowserWindow* browser_window_; diff --git a/chrome/browser/gtk/tab_contents_container_gtk.cc b/chrome/browser/gtk/tab_contents_container_gtk.cc index 4305519..7b4e6b4 100644 --- a/chrome/browser/gtk/tab_contents_container_gtk.cc +++ b/chrome/browser/gtk/tab_contents_container_gtk.cc @@ -41,8 +41,6 @@ TabContentsContainerGtk::TabContentsContainerGtk(StatusBubbleGtk* status_bubble) } TabContentsContainerGtk::~TabContentsContainerGtk() { - if (tab_contents_) - RemoveObservers(); } void TabContentsContainerGtk::AddContainerToBox(GtkWidget* box) { @@ -57,7 +55,10 @@ void TabContentsContainerGtk::SetTabContents(TabContents* tab_contents) { tab_contents_->WasHidden(); - RemoveObservers(); + registrar_.Remove(this, NotificationType::RENDER_VIEW_HOST_CHANGED, + Source(&tab_contents_->controller())); + registrar_.Remove(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source(tab_contents_)); } tab_contents_ = tab_contents; @@ -65,7 +66,14 @@ void TabContentsContainerGtk::SetTabContents(TabContents* tab_contents) { // When detaching the last tab of the browser SetTabContents is invoked // with NULL. Don't attempt to do anything in that case. if (tab_contents_) { - AddObservers(); + // TabContents can change their RenderViewHost and hence the GtkWidget that + // is shown. I'm not entirely sure that we need to observe this event under + // GTK, but am putting a stub implementation and a comment saying that if + // we crash after that NOTIMPLEMENTED(), we'll need it. + registrar_.Add(this, NotificationType::RENDER_VIEW_HOST_CHANGED, + Source(&tab_contents_->controller())); + registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source(tab_contents_)); gfx::NativeView widget = tab_contents_->GetNativeView(); if (widget) { @@ -111,37 +119,6 @@ void TabContentsContainerGtk::Observe(NotificationType type, } } -void TabContentsContainerGtk::AddObservers() { - DCHECK(tab_contents_); - - // TabContents can change their RenderViewHost and hence the GtkWidget that - // is shown. I'm not entirely sure that we need to observe this event under - // GTK, but am putting a stub implementation and a comment saying that if - // we crash after that NOTIMPLEMENTED(), we'll need it. - NotificationService::current()->AddObserver( - this, NotificationType::RENDER_VIEW_HOST_CHANGED, - Source(&tab_contents_->controller())); - - NotificationService::current()->AddObserver( - this, - NotificationType::TAB_CONTENTS_DESTROYED, - Source(tab_contents_)); -} - -void TabContentsContainerGtk::RemoveObservers() { - DCHECK(tab_contents_); - - NotificationService::current()->RemoveObserver( - this, - NotificationType::RENDER_VIEW_HOST_CHANGED, - Source(&tab_contents_->controller())); - - NotificationService::current()->RemoveObserver( - this, - NotificationType::TAB_CONTENTS_DESTROYED, - Source(tab_contents_)); -} - void TabContentsContainerGtk::RenderViewHostChanged(RenderViewHost* old_host, RenderViewHost* new_host) { // TODO(port): Remove this method and the logic where we subscribe to the diff --git a/chrome/browser/gtk/tab_contents_container_gtk.h b/chrome/browser/gtk/tab_contents_container_gtk.h index 8915b67..457748c 100644 --- a/chrome/browser/gtk/tab_contents_container_gtk.h +++ b/chrome/browser/gtk/tab_contents_container_gtk.h @@ -8,7 +8,7 @@ #include #include "base/basictypes.h" -#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" class RenderViewHost; class StatusBubbleGtk; @@ -58,6 +58,8 @@ class TabContentsContainerGtk : public NotificationObserver { GtkAllocation* allocation, TabContentsContainerGtk* container); + NotificationRegistrar registrar_; + // The currently visible TabContents. TabContents* tab_contents_; -- cgit v1.1