Make DBusStatistics only run on the main thread

dbus/dbus_statistics.cc

Index: dbus/dbus_statistics.cc
diff --git a/dbus/dbus_statistics.cc b/dbus/dbus_statistics.cc
index 015e20ea61c769896940ea4bfc7071da677fec3a..448407e8b9cc4a84b1c21d64992e6ac11789323d 100644
--- a/dbus/dbus_statistics.cc
+++ b/dbus/dbus_statistics.cc
@@ -6,10 +6,15 @@
 
 #include <set>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
+#include "base/threading/platform_thread.h"
 #include "base/time.h"
 
 namespace dbus {
@@ -60,10 +65,17 @@ typedef std::set<Stat*, Stat::PtrCompare> StatSet;
 // Simple class for gathering DBus usage statistics.
 class DBusStatistics {
  public:
-  DBusStatistics() : start_time_(base::Time::Now()) {
+  DBusStatistics()
+      : start_time_(base::Time::Now()),
+        origin_thread_id_(base::PlatformThread::CurrentId()),
+        ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+    // MessageLoop::current() might be NULL in tests.

[hashimoto, 2013/01/04 11:20:10]

How about setting up the message loop in tests instead of allowing its absence?
I think it will make the code clearer.

[stevenjb, 2013/01/04 20:30:37]

Replacing this with a CHECK fails for a handful of tests; trying to change all
of them didn't seem to make sense (and may not be feasible in all instances). We
already have similar code elsewhere.

[hashimoto, 2013/01/07 13:28:00]

OK.
Please fix the failing browser tests.

+    if (MessageLoop::current())
+      origin_message_loop_proxy_ = MessageLoop::current()->message_loop_proxy();

[hashimoto, 2013/01/04 11:20:10]

nit: How about using MessageLoopProxy::current()?

[stevenjb, 2013/01/04 20:30:37]

Done.

   }
 
   ~DBusStatistics() {
+    CHECK_EQ(origin_thread_id_, base::PlatformThread::CurrentId());
     STLDeleteContainerPointers(stats_.begin(), stats_.end());
   }
 
@@ -79,8 +91,18 @@ class DBusStatistics {
                const std::string& interface,
                const std::string& method,
                StatType type) {
+    if (base::PlatformThread::CurrentId() != origin_thread_id_) {

[hashimoto, 2013/01/04 11:20:10]

How about using SingleThreadTaskRunner:: BelongsToCurrentThread()?
(MessageLoopProxy is a SingleThreadTaskRunner)
The same goes for the CHECKs in other places.

[stevenjb, 2013/01/04 20:30:37]

As long as origin_message_loop_proxy_ might be NULL we can't use that, and I
really don't want to modify any tests just for the sake of this code.

+      if (origin_message_loop_proxy_.get()) {
+        origin_message_loop_proxy_->PostTask(
+            FROM_HERE,
+            base::Bind(&DBusStatistics::AddStat,
+                       weak_ptr_factory_.GetWeakPtr(),
+                       service, interface, method, type));
+      }
+      return;
+    }
     Stat* stat = GetStat(service, interface, method, true);
-    DCHECK(stat);
+    CHECK(stat);
     if (type == TYPE_SENT_METHOD_CALLS)
       ++stat->sent_method_calls;
     else if (type == TYPE_RECEIVED_SIGNALS)
@@ -97,6 +119,7 @@ class DBusStatistics {
                 const std::string& interface,
                 const std::string& method,
                 bool add_stat) {
+    CHECK_EQ(origin_thread_id_, base::PlatformThread::CurrentId());
     scoped_ptr<Stat> stat(new Stat(service, interface, method));
     StatSet::iterator found = stats_.find(stat.get());
     if (found != stats_.end())
@@ -107,12 +130,18 @@ class DBusStatistics {
     return *found;
   }
 
-  StatSet& stats() { return stats_; }
+  StatSet& GetStatSet() {
+    CHECK_EQ(origin_thread_id_, base::PlatformThread::CurrentId());
+    return stats_;
+  }
   base::Time start_time() { return start_time_; }
 
  private:
   StatSet stats_;
   base::Time start_time_;
+  base::PlatformThreadId origin_thread_id_;
+  scoped_refptr<base::MessageLoopProxy> origin_message_loop_proxy_;
+  base::WeakPtrFactory<DBusStatistics> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DBusStatistics);
 };
@@ -170,7 +199,7 @@ std::string GetAsString(ShowInString show, FormatString format) {
   if (!g_dbus_statistics)
     return "DBusStatistics not initialized.";
 
-  const StatSet& stats = g_dbus_statistics->stats();
+  const StatSet& stats = g_dbus_statistics->GetStatSet();
   if (stats.empty())
     return "No DBus calls.";