Adding instrument to NotificationRegistrar to check

Add() and Remove() are called from same thread.

Note I already checked in a fix for the bug. The CL
is trying to check whether the issue exists in other
code path.

TEST=none
BUG=27834

Review URL: http://codereview.chromium.org/449044

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

4 files changed, 42 insertions, 0 deletions

diff --git a/chrome/browser/chrome_thread.cc b/chrome/browser/chrome_thread.cc
index f279ed3..a5cbe89 100644
--- a/chrome/browser/chrome_thread.cc
+++ b/chrome/browser/chrome_thread.cc
@@ -59,6 +59,13 @@ ChromeThread::~ChromeThread() {
 }
 
 // static
+bool ChromeThread::IsWellKnownThread(ID identifier) {
+  AutoLock lock(lock_);
+  return (identifier >= 0 && identifier < ID_COUNT &&
+          chrome_threads_[identifier]);
+}
+
+// static
 bool ChromeThread::CurrentlyOn(ID identifier) {
   AutoLock lock(lock_);
   DCHECK(identifier >= 0 && identifier < ID_COUNT);
diff --git a/chrome/browser/chrome_thread.h b/chrome/browser/chrome_thread.h
index e890790..a8fe44b 100644
--- a/chrome/browser/chrome_thread.h
+++ b/chrome/browser/chrome_thread.h
@@ -114,6 +114,10 @@ class ChromeThread : public base::Thread {
         identifier, from_here, new ReleaseTask<T>(object));
   }
 
+  // Callable on any thread.  Returns whether the given ID corresponds to a well
+  // known thread.
+  static bool IsWellKnownThread(ID identifier);
+
   // Callable on any thread.  Returns whether you're currently on a particular
   // thread.
   static bool CurrentlyOn(ID identifier);
diff --git a/chrome/common/notification_registrar.cc b/chrome/common/notification_registrar.cc
index 4da68ee..a124f0d 100644
--- a/chrome/common/notification_registrar.cc
+++ b/chrome/common/notification_registrar.cc
@@ -24,6 +24,11 @@ bool NotificationRegistrar::Record::operator==(const Record& other) const {
 }
 
 NotificationRegistrar::NotificationRegistrar() {
+  if (!ChromeThread::GetCurrentThreadIdentifier(&thread_id_)) {
+    // If we are not created in well known thread, GetCurrentThreadIdentifier
+    // fails. Assign thread_id_ to an ID not used.
+    thread_id_ = ChromeThread::ID_COUNT;
+  }
 }
 
 NotificationRegistrar::~NotificationRegistrar() {
@@ -38,6 +43,16 @@ void NotificationRegistrar::Add(NotificationObserver* observer,
          registered_.end()) << "Duplicate registration.";
   registered_.push_back(record);
 
+  if (ChromeThread::IsWellKnownThread(thread_id_)) {
+    if (!ChromeThread::CurrentlyOn(thread_id_)) {
+      // We are created on a well known thread, but this function is called
+      // on a different thread. This could be a bug, or maybe the object is
+      // passed around.
+      // To be safe, reset thread_id_ so we don't call CHECK during remove.
+      thread_id_ = ChromeThread::ID_COUNT;
+    }
+  }
+
   NotificationService::current()->AddObserver(observer, type, source);
 }
 
@@ -54,6 +69,8 @@ void NotificationRegistrar::Remove(NotificationObserver* observer,
   }
   registered_.erase(found);
 
+  CheckCalledOnValidWellKnownThread();
+
   // This can be NULL if our owner outlives the NotificationService, e.g. if our
   // owner is a Singleton.
   NotificationService* service = NotificationService::current();
@@ -71,6 +88,8 @@ void NotificationRegistrar::RemoveAll() {
   if (registered_.empty())
     return;
 
+  CheckCalledOnValidWellKnownThread();
+
   // This can be NULL if our owner outlives the NotificationService, e.g. if our
   // owner is a Singleton.
   NotificationService* service = NotificationService::current();
@@ -87,3 +106,9 @@ void NotificationRegistrar::RemoveAll() {
 bool NotificationRegistrar::IsEmpty() const {
   return registered_.empty();
 }
+
+void NotificationRegistrar::CheckCalledOnValidWellKnownThread() {
+  if (ChromeThread::IsWellKnownThread(thread_id_)) {
+    CHECK(ChromeThread::CurrentlyOn(thread_id_));
+  }
+}
diff --git a/chrome/common/notification_registrar.h b/chrome/common/notification_registrar.h
index ef21ff2..7f6956a 100644
--- a/chrome/common/notification_registrar.h
+++ b/chrome/common/notification_registrar.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/common/notification_observer.h"
 
 // Aids in registering for notifications and ensures that all registered
@@ -39,6 +40,8 @@ class NotificationRegistrar {
   bool IsEmpty() const;
 
  private:
+  void CheckCalledOnValidWellKnownThread();
+
   struct Record;
 
   // We keep registered notifications in a simple vector. This means we'll do
@@ -50,6 +53,9 @@ class NotificationRegistrar {
   // Lists all notifications we're currently registered for.
   RecordVector registered_;
 
+  // The thread creating this object.
+  ChromeThread::ID thread_id_;
+
   DISALLOW_COPY_AND_ASSIGN(NotificationRegistrar);
 };