Use a NotificationRegistrar to listen for notifications.

Also clean up a bunch of code to make it shorter/clearer/more style-guide compliant.

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

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

3 files changed, 85 insertions, 218 deletions

diff --git a/chrome/browser/debugger/debugger_host_impl.cpp b/chrome/browser/debugger/debugger_host_impl.cpp
index 09b4e24..e8b586a 100644
--- a/chrome/browser/debugger/debugger_host_impl.cpp
+++ b/chrome/browser/debugger/debugger_host_impl.cpp
@@ -11,55 +11,36 @@
 #include "chrome/browser/debugger/debugger_wrapper.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
 
 class TabContentsReference : public NotificationObserver {
  public:
   TabContentsReference(TabContents *c) : navigation_controller_(NULL) {
     navigation_controller_ = &c->controller();
-
-    NotificationService* service = NotificationService::current();
-    DCHECK(service);
-    service->AddObserver(this,
-                         NotificationType::TAB_CLOSING,
-                         Source<NavigationController>(navigation_controller_));
-    observing_ = true;
+    registrar_.Add(this, NotificationType::TAB_CLOSING,
+                   Source<NavigationController>(navigation_controller_));
   }
 
   virtual ~TabContentsReference() {
-    StopObserving();
   }
 
   // NotificationObserver impl
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details) {
-    StopObserving();
+    registrar_.RemoveAll();
     navigation_controller_ = NULL;
   }
 
   TabContents* GetTabContents() {
-    if (navigation_controller_) {
-      return navigation_controller_->tab_contents();
-    } else {
-      return NULL;
-    }
-  }
- private:
-  void StopObserving() {
-    if (observing_ && navigation_controller_) {
-      NotificationService* service = NotificationService::current();
-      DCHECK(service);
-      service->RemoveObserver(
-          this,
-          NotificationType::TAB_CLOSING,
-          Source<NavigationController>(navigation_controller_));
-      observing_ = false;
-    }
+    return navigation_controller_ ?
+        navigation_controller_->tab_contents() : NULL;
   }
 
+ private:
+  NotificationRegistrar registrar_;
   NavigationController* navigation_controller_;
-  bool observing_;
 
   DISALLOW_COPY_AND_ASSIGN(TabContentsReference);
 };
diff --git a/chrome/browser/debugger/debugger_node.cc b/chrome/browser/debugger/debugger_node.cc
index 3322500..d6ba0e6 100644
--- a/chrome/browser/debugger/debugger_node.cc
+++ b/chrome/browser/debugger/debugger_node.cc
@@ -15,13 +15,14 @@
 #include "chrome/browser/debugger/debugger_shell.h"
 #include "chrome/common/notification_service.h"
 
-DebuggerNode::DebuggerNode() : data_(NULL), valid_(true), observing_(false) {
+DebuggerNode::DebuggerNode() : data_(NULL), valid_(true) {
 }
 
 void DebuggerNode::Observe(NotificationType type,
                            const NotificationSource& source,
                            const NotificationDetails& details) {
-  StopObserving();
+  registrar_.RemoveAll();
+  data_ = NULL;
   Invalidate();
 }
 
@@ -29,19 +30,6 @@ DebuggerNode::~DebuggerNode() {
 }
 
 
-void DebuggerNode::StopObserving() {
-  if (observing_ && valid_) {
-    NotificationService* service = NotificationService::current();
-    DCHECK(service);
-    StopObserving(service);
-    observing_ = false;
-  }
-  data_ = NULL;
-}
-
-void DebuggerNode::StopObserving(NotificationService *service) {
-}
-
 v8::Handle<v8::Value> DebuggerNode::IndexGetter(uint32_t index,
                                                 const v8::AccessorInfo& info) {
   return v8::Undefined();
@@ -89,11 +77,8 @@ v8::Handle<v8::Value> DebuggerNode::NodeGetter(v8::Local<v8::String> prop,
   DebuggerNodeWrapper* w = static_cast<DebuggerNodeWrapper*>(v8::External::Cast(
         *info.Data())->Value());
   DebuggerNode* n = w->node();
-  if (n->IsValid() && n->IsObject()) {
-    return n->PropGetter(prop, info);
-  } else {
-    return v8::Undefined();
-  }
+  return (n->IsValid() && n->IsObject()) ? n->PropGetter(prop, info) :
+      static_cast<v8::Handle<v8::Value> >(v8::Undefined());
 }
 
 v8::Handle<v8::Value> DebuggerNode::NodeIndex(uint32_t index,
@@ -101,26 +86,19 @@ v8::Handle<v8::Value> DebuggerNode::NodeIndex(uint32_t index,
   DebuggerNodeWrapper* w = static_cast<DebuggerNodeWrapper*>(v8::External::Cast(
       *info.Data())->Value());
   DebuggerNode* n = w->node();
-  if (n->IsValid() && n->IsCollection()) {
-    return n->IndexGetter(index, info);
-  } else {
-    return v8::Undefined();
-  }
+  return (n->IsValid() && n->IsCollection()) ? n->IndexGetter(index, info) :
+      static_cast<v8::Handle<v8::Value> >(v8::Undefined());
 }
 
 v8::Handle<v8::Value> DebuggerNode::NodeFunc(const v8::Arguments& args) {
   DebuggerNodeWrapper* w = static_cast<DebuggerNodeWrapper*>(v8::External::Cast(
       *args.Data())->Value());
   DebuggerNode* n = w->node();
-  if (n->IsValid() && n->IsFunction()) {
-    return n->Function(args);
-  } else {
-    return v8::Undefined();
-  }
+  return (n->IsValid() && n->IsFunction()) ? n->Function(args) :
+      static_cast<v8::Handle<v8::Value> >(v8::Undefined());
 }
 
 
-
 /////////////////////////////////////////////
 
 ChromeNode::ChromeNode(DebuggerShell* debugger) {
@@ -132,58 +110,36 @@ ChromeNode::~ChromeNode() {
 
 v8::Handle<v8::Value> ChromeNode::PropGetter(v8::Handle<v8::String> prop,
                                              const v8::AccessorInfo& info) {
-  if (prop->Equals(v8::String::New("pid"))) {
+  if (prop->Equals(v8::String::New("pid")))
     return v8::Number::New(base::GetCurrentProcId());
-  } else if (prop->Equals(v8::String::New("browser"))) {
-    BrowserListNode *node = BrowserListNode::BrowserList();
-    return node->NewInstance();
-  } else if (prop->Equals(v8::String::New("setDebuggerReady"))) {
-    FunctionNode<DebuggerShell>* f =
-      new FunctionNode<DebuggerShell>(DebuggerShell::SetDebuggerReady,
-                                      debugger_);
-    return f->NewInstance();
-  } else if (prop->Equals(v8::String::New("setDebuggerBreak"))) {
-    FunctionNode<DebuggerShell>* f =
-      new FunctionNode<DebuggerShell>(DebuggerShell::SetDebuggerBreak,
-                                      debugger_);
-    return f->NewInstance();
-  } else if (prop->Equals(v8::String::New("foo"))) {
-    return v8::Undefined();
-  } else {
-    return prop;
+  if (prop->Equals(v8::String::New("browser")))
+    return BrowserListNode::BrowserList()->NewInstance();
+  if (prop->Equals(v8::String::New("setDebuggerReady"))) {
+    return (new FunctionNode<DebuggerShell>(DebuggerShell::SetDebuggerReady,
+                                            debugger_))->NewInstance();
   }
-}
-
-void ChromeNode::StopObserving(NotificationService *service) {
+  if (prop->Equals(v8::String::New("setDebuggerBreak"))) {
+    return (new FunctionNode<DebuggerShell>(DebuggerShell::SetDebuggerBreak,
+                                            debugger_))->NewInstance();
+  }
+  return (prop->Equals(v8::String::New("foo"))) ?
+      static_cast<v8::Handle<v8::Value> >(v8::Undefined()) : prop;
 }
 
 /////////////////////////////////////////////
 
 BrowserNode::BrowserNode(Browser *b) {
   data_ = b;
-
-  NotificationService* service = NotificationService::current();
-  DCHECK(service);
-  service->AddObserver(
-      this, NotificationType::BROWSER_CLOSED, Source<Browser>(b));
-  observing_ = true;
-}
-
-void BrowserNode::StopObserving(NotificationService *service) {
-  Browser *b = static_cast<Browser*>(data_);
-  service->RemoveObserver(
-      this, NotificationType::BROWSER_CLOSED, Source<Browser>(b));
+  registrar_.Add(this, NotificationType::BROWSER_CLOSED, Source<Browser>(b));
 }
 
 BrowserNode* BrowserNode::BrowserAtIndex(int index) {
   if (index >= 0) {
     BrowserList::const_iterator iter = BrowserList::begin();
-
-    for (; (iter != BrowserList::end()) && (index > 0); ++iter, --index);
-
-    if (iter != BrowserList::end()) {
+    for (; (iter != BrowserList::end()) && (index > 0); ++iter, --index)
+      ;
+    if (iter != BrowserList::end())
       return new BrowserNode(*iter);
-    }
   }
   return NULL;
 }
@@ -192,26 +148,19 @@ BrowserNode::~BrowserNode() {
 }
 
 Browser* BrowserNode::GetBrowser() {
-  if (IsValid()) {
-    return static_cast<Browser *>(data_);
-  } else {
-    return NULL;
-  }
+  return IsValid() ? static_cast<Browser*>(data_) : NULL;
 }
 
 v8::Handle<v8::Value> BrowserNode::PropGetter(v8::Handle<v8::String> prop,
                                               const v8::AccessorInfo& info) {
-  Browser *b = GetBrowser();
+  Browser* b = GetBrowser();
   if (b != NULL) {
     if (prop->Equals(v8::String::New("title"))) {
-      const TabContents *t = b->GetSelectedTabContents();
-      std::wstring title = UTF16ToWideHack(t->GetTitle());
-      std::string title2 = WideToUTF8(title);
-      return v8::String::New(title2.c_str());
-    } else if (prop->Equals(v8::String::New("tab"))) {
-      TabListNode* node = TabListNode::TabList(b);
-      return node->NewInstance();
+      return v8::String::New(UTF16ToUTF8(
+          b->GetSelectedTabContents()->GetTitle()).c_str());
     }
+    if (prop->Equals(v8::String::New("tab")))
+      return TabListNode::TabList(b)->NewInstance();
   }
   return v8::Undefined();
 }
@@ -233,25 +182,15 @@ v8::Handle<v8::Value> BrowserListNode::IndexGetter(
     uint32_t index,
     const v8::AccessorInfo& info) {
   BrowserNode* b = BrowserNode::BrowserAtIndex(index);
-  if (!b) {
-    return v8::Undefined();
-  }
-  return b->NewInstance();
-}
-
-void BrowserListNode::StopObserving(NotificationService *service) {
+  return b ? b->NewInstance() :
+      static_cast<v8::Handle<v8::Value> >(v8::Undefined());
 }
 
 /////////////////////////////////////////////
 
 TabListNode::TabListNode(Browser* b) {
   data_ = b;
-
-  NotificationService* service = NotificationService::current();
-  DCHECK(service);
-  service->AddObserver(
-      this, NotificationType::BROWSER_CLOSED, Source<Browser>(b));
-  observing_ = true;
+  registrar_.Add(this, NotificationType::BROWSER_CLOSED, Source<Browser>(b));
 }
 
 TabListNode::~TabListNode() {
@@ -262,17 +201,7 @@ TabListNode* TabListNode::TabList(Browser* b) {
 }
 
 Browser* TabListNode::GetBrowser() {
-  if (IsValid()) {
-    return static_cast<Browser *>(data_);
-  } else {
-    return NULL;
-  }
-}
-
-void TabListNode::StopObserving(NotificationService *service) {
-  Browser *b = static_cast<Browser*>(data_);
-  service->RemoveObserver(
-      this, NotificationType::BROWSER_CLOSED, Source<Browser>(b));
+  return IsValid() ? static_cast<Browser*>(data_) : NULL;
 }
 
 v8::Handle<v8::Value> TabListNode::IndexGetter(uint32_t index,
@@ -280,41 +209,27 @@ v8::Handle<v8::Value> TabListNode::IndexGetter(uint32_t index,
   Browser* b = GetBrowser();
   if (b != NULL) {
     TabContents* tab_contents = b->GetTabContentsAt(index);
-    if (tab_contents) {
-      TabNode* node = new TabNode(tab_contents);
-      return node->NewInstance();
-    }
+    if (tab_contents)
+      return (new TabNode(tab_contents))->NewInstance();
   }
   return v8::Undefined();
 }
 
 /////////////////////////////////////////////
 
-TabNode::TabNode(TabContents *c) {
-  data_ = &c->controller();
-
-  NotificationService* service = NotificationService::current();
-  DCHECK(service);
-  service->AddObserver(this, NotificationType::TAB_CLOSING,
-                       Source<NavigationController>(&c->controller()));
-  observing_ = true;
+TabNode::TabNode(TabContents* c) {
+  NavigationController* controller = &c->controller();
+  data_ = controller;
+  registrar_.Add(this, NotificationType::TAB_CLOSING,
+                 Source<NavigationController>(controller));
 }
 
 TabNode::~TabNode() {
 }
 
-void TabNode::StopObserving(NotificationService *service) {
-  NavigationController *c = static_cast<NavigationController*>(data_);
-  service->RemoveObserver(this, NotificationType::TAB_CLOSING,
-                          Source<NavigationController>(c));
-}
-
 TabContents* TabNode::GetTab() {
-  if (IsValid()) {
-    return static_cast<NavigationController*>(data_)->tab_contents();
-  } else {
-    return NULL;
-  }
+  return IsValid() ?
+      static_cast<NavigationController*>(data_)->tab_contents() : NULL;
 }
 
 v8::Handle<v8::Value> TabNode::SendToDebugger(const v8::Arguments& args,
@@ -322,8 +237,7 @@ v8::Handle<v8::Value> TabNode::SendToDebugger(const v8::Arguments& args,
   RenderViewHost* host = tab->render_view_host();
   if (args.Length() == 1) {
     std::wstring cmd;
-    v8::Handle<v8::Value> obj;
-    obj = args[0];
+    v8::Handle<v8::Value> obj = args[0];
     DebuggerShell::ObjectToString(obj, &cmd);
     host->DebugCommand(cmd);
   }
@@ -334,26 +248,20 @@ v8::Handle<v8::Value> TabNode::Attach(const v8::Arguments& args,
                                       TabContents* tab) {
   RenderViewHost* host = tab->render_view_host();
   host->DebugAttach();
-  RenderProcessHost* proc = host->process();
-  return v8::Int32::New(proc->process().pid());
+  return v8::Int32::New(host->process()->process().pid());
 }
 
 v8::Handle<v8::Value> TabNode::Detach(const v8::Arguments& args,
                                       TabContents* tab) {
   RenderViewHost* host = tab->render_view_host();
   host->DebugDetach();
-  RenderProcessHost* proc = host->process();
-  return v8::Int32::New(proc->process().pid());
+  return v8::Int32::New(host->process()->process().pid());
 }
 
 v8::Handle<v8::Value> TabNode::Break(const v8::Arguments& args,
                                      TabContents* tab) {
-  RenderViewHost* host = tab->render_view_host();
-  bool force = false;
-  if (args.Length() >= 1) {
-    force = args[0]->BooleanValue();
-  }
-  host->DebugBreak(force);
+  tab->render_view_host()->DebugBreak((args.Length() >= 1) ?
+      args[0]->BooleanValue() : false);
   return v8::Undefined();
 }
 
@@ -361,27 +269,23 @@ v8::Handle<v8::Value> TabNode::PropGetter(v8::Handle<v8::String> prop,
                                           const v8::AccessorInfo& info) {
   TabContents* tab = GetTab();
   if (tab != NULL) {
-    if (prop->Equals(v8::String::New("title"))) {
-      std::string title = UTF16ToUTF8(tab->GetTitle());
-      return v8::String::New(title.c_str());
-    } else {
-      if (prop->Equals(v8::String::New("attach"))) {
-        FunctionNode<TabContents>* f =
-            new FunctionNode<TabContents>(TabNode::Attach, tab);
-        return f->NewInstance();
-      } else if (prop->Equals(v8::String::New("detach"))) {
-        FunctionNode<TabContents>* f =
-            new FunctionNode<TabContents>(TabNode::Detach, tab);
-        return f->NewInstance();
-      } else if (prop->Equals(v8::String::New("sendToDebugger"))) {
-        FunctionNode<TabContents>* f =
-            new FunctionNode<TabContents>(TabNode::SendToDebugger, tab);
-        return f->NewInstance();
-      } else if (prop->Equals(v8::String::New("debugBreak"))) {
-        FunctionNode<TabContents>* f =
-            new FunctionNode<TabContents>(TabNode::Break, tab);
-        return f->NewInstance();
-      }
+    if (prop->Equals(v8::String::New("title")))
+      return v8::String::New(UTF16ToUTF8(tab->GetTitle()).c_str());
+    if (prop->Equals(v8::String::New("attach"))) {
+      return (new FunctionNode<TabContents>(TabNode::Attach,
+                                            tab))->NewInstance();
+    }
+    if (prop->Equals(v8::String::New("detach"))) {
+      return (new FunctionNode<TabContents>(TabNode::Detach,
+                                            tab))->NewInstance();
+    }
+    if (prop->Equals(v8::String::New("sendToDebugger"))) {
+      return (new FunctionNode<TabContents>(TabNode::SendToDebugger,
+                                            tab))->NewInstance();
+    }
+    if (prop->Equals(v8::String::New("debugBreak"))) {
+      return (new FunctionNode<TabContents>(TabNode::Break,
+                                            tab))->NewInstance();
     }
   }
   return v8::Undefined();
diff --git a/chrome/browser/debugger/debugger_node.h b/chrome/browser/debugger/debugger_node.h
index f62a006..8b243f6 100644
--- a/chrome/browser/debugger/debugger_node.h
+++ b/chrome/browser/debugger/debugger_node.h
@@ -15,8 +15,9 @@
 #define CHROME_BROWSER_DEBUGGER_DEBUGGER_NODE_H_
 
 #include "base/basictypes.h"
+#include "base/scoped_ptr.h"
 #include "base/ref_counted.h"
-#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_registrar.h"
 #include "v8/include/v8.h"
 
 class Browser;
@@ -48,8 +49,6 @@ class DebuggerNode : public NotificationObserver {
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
-  void StopObserving();
-  virtual void StopObserving(NotificationService *service);
 
   // Index getter callback from V8 for objects where IsCollection is true
   virtual v8::Handle<v8::Value> IndexGetter(uint32_t index,
@@ -73,17 +72,13 @@ class DebuggerNode : public NotificationObserver {
   static v8::Handle<v8::Value> NodeFunc(const v8::Arguments& args);
 
  protected:
-  void *data_;
+  NotificationRegistrar registrar_;
+  void* data_;
   bool valid_;
-  bool observing_;
-
- private:
 };
 
-// A wrapper around the proxy to handle two issues:
-// - call virtual methods to stop observing at destruction time
-// - call virtual methods during callbacks from V8 after a static_cast
-//   from void*
+// A wrapper around the proxy to handle calling virtual methods during callbacks
+// from V8 after a static_cast from void*.
 // The point here is that we'd like to be able to stick DebuggerNode* objects
 // into V8.  To do that, we need to cast them to void*, which means we need
 // this additional layer of wrapper to protect them from the harmful effects
@@ -93,13 +88,10 @@ class DebuggerNode : public NotificationObserver {
 class DebuggerNodeWrapper : public base::RefCounted<DebuggerNodeWrapper> {
  public:
   DebuggerNodeWrapper(DebuggerNode* node) : node_(node) {}
-  virtual ~DebuggerNodeWrapper() {
-    node_->StopObserving();
-    delete node_;
-  }
-  DebuggerNode* node() { return node_; }
+  virtual ~DebuggerNodeWrapper() {}
+  DebuggerNode* node() { return node_.get(); }
  private:
-  DebuggerNode* node_;
+  scoped_ptr<DebuggerNode> node_;
 };
 
 // top level chrome object implements:
@@ -117,10 +109,7 @@ class ChromeNode : public DebuggerNode {
   virtual v8::Handle<v8::Value> PropGetter(v8::Handle<v8::String> prop,
                                            const v8::AccessorInfo& info);
 
-  virtual void StopObserving(NotificationService *service);
-
  private:
-
   DebuggerShell* debugger_;
 };
 
@@ -133,8 +122,6 @@ class BrowserListNode : public DebuggerNode {
 
   virtual v8::Handle<v8::Value> IndexGetter(uint32_t index,
                                             const v8::AccessorInfo& info);
-  virtual void StopObserving(NotificationService *service);
-
   static BrowserListNode* BrowserList();
 
  private:
@@ -153,8 +140,6 @@ class BrowserNode : public DebuggerNode {
 
   static BrowserNode* BrowserAtIndex(int index);
 
-  virtual void StopObserving(NotificationService *service);
-
   virtual v8::Handle<v8::Value> PropGetter(v8::Handle<v8::String> prop,
                                            const v8::AccessorInfo& info);
 
@@ -171,8 +156,6 @@ class TabListNode : public DebuggerNode {
   bool IsFunction() { return false; }
   bool IsObject() { return false; }
 
-  virtual void StopObserving(NotificationService *service);
-
   virtual v8::Handle<v8::Value> IndexGetter(uint32_t index,
                                             const v8::AccessorInfo& info);
   static TabListNode* TabList(Browser* b);
@@ -195,7 +178,6 @@ class TabNode : public DebuggerNode {
   bool IsObject() { return true; }
 
   TabNode(TabContents* c);
-  virtual void StopObserving(NotificationService* service);
   virtual v8::Handle<v8::Value> PropGetter(v8::Handle<v8::String> prop,
                                            const v8::AccessorInfo& info);
  private:
@@ -222,8 +204,8 @@ class FunctionNode : public DebuggerNode {
 
   typedef v8::Handle<v8::Value> (*Callback)(const v8::Arguments& args, T* data);
 
-  FunctionNode(Callback f, T* data) :
-      function_(f), data_(data) {};
+  FunctionNode(Callback f, T* data)
+      : function_(f), data_(data) {}
 
 private:
   // Functor callback from V8 for objects where IsFunction is true