Make the NotificationRegistrar safe for use in Singletons, which may outlive the NotificationService instances, by checking whether the service exists before calling RemoveObserver() on it.

Also add comments in NotificationService telling people to use NotificationRegistrar.

BUG=2381
Review URL: http://codereview.chromium.org/115601

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16554 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 26 insertions, 5 deletions

diff --git a/chrome/common/notification_registrar.cc b/chrome/common/notification_registrar.cc
index b9ea3d7..73635b5 100644
--- a/chrome/common/notification_registrar.cc
+++ b/chrome/common/notification_registrar.cc
@@ -57,15 +57,27 @@ void NotificationRegistrar::Remove(NotificationObserver* observer,
     NOTREACHED();
   }
 
-  NotificationService::current()->RemoveObserver(observer, type, source);
+  // This can be NULL if our owner outlives the NotificationService, e.g. if our
+  // owner is a Singleton.
+  NotificationService* service = NotificationService::current();
+  if (service)
+    service->RemoveObserver(observer, type, source);
 }
 
 void NotificationRegistrar::RemoveAll() {
+  // This can be NULL if our owner outlives the NotificationService, e.g. if our
+  // owner is a Singleton.
   NotificationService* service = NotificationService::current();
-  for (size_t i = 0; i < registered_.size(); i++) {
-    service->RemoveObserver(registered_[i].observer,
-                            registered_[i].type,
-                            registered_[i].source);
+  if (service) {
+    for (size_t i = 0; i < registered_.size(); i++) {
+      service->RemoveObserver(registered_[i].observer,
+                              registered_[i].type,
+                              registered_[i].source);
+    }
   }
   registered_.clear();
 }
+
+bool NotificationRegistrar::IsEmpty() const {
+  return registered_.empty();
+}
diff --git a/chrome/common/notification_registrar.h b/chrome/common/notification_registrar.h
index c79d732..ef21ff2 100644
--- a/chrome/common/notification_registrar.h
+++ b/chrome/common/notification_registrar.h
@@ -35,6 +35,9 @@ class NotificationRegistrar {
   // Unregisters all notifications.
   void RemoveAll();
 
+  // Returns true if no notifications are registered.
+  bool IsEmpty() const;
+
  private:
   struct Record;
 
diff --git a/chrome/common/notification_service.h b/chrome/common/notification_service.h
index 706cc945..0f99949 100644
--- a/chrome/common/notification_service.h
+++ b/chrome/common/notification_service.h
@@ -30,6 +30,9 @@ class NotificationService {
   NotificationService();
   ~NotificationService();
 
+  // NOTE: Rather than using this directly, you you probably want to use a
+  // NotificationRegistrar.  It's generally easier and less error-prone.
+  //
   // Registers a NotificationObserver to be called whenever a matching
   // notification is posted.  Observer is a pointer to an object subclassing
   // NotificationObserver to be notified when an event matching the other two
@@ -48,6 +51,9 @@ class NotificationService {
   void AddObserver(NotificationObserver* observer,
                    NotificationType type, const NotificationSource& source);
 
+  // NOTE: Rather than using this directly, you you probably want to use a
+  // NotificationRegistrar.  It's generally easier and less error-prone.
+  //
   // Removes the object pointed to by observer from receiving notifications
   // that match type and source.  If no object matching the parameters is
   // currently registered, this method is a no-op.