Make DBusStatistics only run on the main thread

dbus/dbus_statistics.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "dbus/dbus_statistics.h"
 
 #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 {
 
 namespace {
 
 // Used to store dbus statistics sorted alphabetically by service, interface,
 // then method (using std::string <).
 struct Stat {
   Stat(const std::string& service,
@@ skipping 30 matching lines @@
 };
 
 typedef std::set<Stat*, Stat::PtrCompare> StatSet;
 
 //------------------------------------------------------------------------------
 // DBusStatistics
 
 // 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());
   }
 
   // Enum to specify which field in Stat to increment in AddStat
   enum StatType {
     TYPE_SENT_METHOD_CALLS,
     TYPE_RECEIVED_SIGNALS,
     TYPE_SENT_BLOCKING_METHOD_CALLS
   };
 
   // Add a call to |method| for |interface|. See also MethodCall in message.h.
   void AddStat(const std::string& service,
                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)
       ++stat->received_signals;
     else if (type == TYPE_SENT_BLOCKING_METHOD_CALLS)
       ++stat->sent_blocking_method_calls;
     else
       NOTREACHED();
   }
 
   // Look up the Stat entry in |stats_|. If |add_stat| is true, add a new entry
   // if one does not already exist.
   Stat* GetStat(const std::string& service,
                 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())
       return *found;
     if (!add_stat)
       return NULL;
     found = stats_.insert(stat.release()).first;
     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);
 };
 
 DBusStatistics* g_dbus_statistics = NULL;
 
 }  // namespace
 
 //------------------------------------------------------------------------------
 
@@ skipping 37 matching lines @@
       service, interface, method,
       DBusStatistics::TYPE_SENT_BLOCKING_METHOD_CALLS);
 }
 
 // NOTE: If the output format is changed, be certain to change the test
 // expectations as well.
 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.";
 
   base::TimeDelta dtime = base::Time::Now() - g_dbus_statistics->start_time();
   int dminutes = dtime.InMinutes();
   dminutes = std::max(dminutes, 1);
 
   std::string result;
   int sent = 0, received = 0, sent_blocking = 0;
   // Stats are stored in order by service, then interface, then method.
@@ skipping 78 matching lines @@
   *sent = stat->sent_method_calls;
   *received = stat->received_signals;
   *blocking = stat->sent_blocking_method_calls;
   return true;
 }
 
 }  // namespace testing
 
 }  // namespace statistics
 }  // namespace dbus