Add DBusStatistics and DBusLogSource to log and show dbus stats

dbus/dbus_statistics.h

+// 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.
+
+#ifndef DBUS_DBUS_STATISTICS_H_
+#define DBUS_DBUS_STATISTICS_H_
+
+#include <set>
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/time.h"
+#include "dbus/dbus_export.h"
+
+namespace dbus {
+
+class CHROME_DBUS_EXPORT DbusStatistics {

[satorux1, 2012/11/12 00:40:43]

Please write some class comment.

[satorux1, 2012/11/12 00:40:43]

Dbus -> DBus, to be consistent with other classes in this directory.

[stevenjb, 2012/11/12 19:46:40]

Done.

[stevenjb, 2012/11/12 19:46:40]

Done.

+public:
+  // Enum to specify which details to show in GetAsString
+  enum ShowInString {
+    SHOW_SERVICE = 0,  // Service totals only
+    SHOW_INTERFACE = 1,  // Service + interface totals
+    SHOW_METHOD = 2,  // Service + interface + method totals
+  };
+
+  struct Stat {
+    Stat(const std::string& service,
+         const std::string& interface,
+         const std::string& method);
+    std::string service;
+    std::string interface;
+    std::string method;
+    int sent_calls;
+    int received_calls;
+
+    bool Compare(const Stat& other) const;

[satorux1, 2012/11/12 00:40:43]

Returns true when?

[stevenjb, 2012/11/12 19:46:40]

Compare == < is pretty standard, but added a comment at the top of the struct.

+
+    struct PtrCompare {
+      bool operator()(Stat* lhs, Stat* rhs) const;
+    };
+  };
+
+  DbusStatistics();

[hashimoto, 2012/11/12 04:33:16]

Since all public methods are static, how about hiding ctor/dtor and other
private methods of this class in the unnamed namespace of dbus_statictics.cc?
Or, how about add a static method "GetInstance()" as DBusThreadManager is doing
and make all methods non-static?

[satorux1, 2012/11/12 04:43:17]

Good ideas. I think we can hide the DbusStatistics class from .h file completely
like:

namespace dbus {
namespace statistics {

void AddSentMethodCall(...);

...

[stevenjb, 2012/11/12 19:46:40]

That makes sense. I think I started this expecting that more of this would need
to be exposed as a class.
Done.

+  ~DbusStatistics();
+
+  // Initializes / shuts down dbus statistics gathering. Calling Initialize
+  // more than once will reset the statistics.
+  static void Initialize();
+  static void Shutdown();
+
+  // Static methods do nothing uless Initialize() was called.

[satorux1, 2012/11/12 00:40:43]

Please also write what these functions do.

[stevenjb, 2012/11/12 19:46:40]

Done.

+  static void AddSentCall(const std::string& service,

[satorux1, 2012/11/12 00:40:43]

Call -> MethodCall to be a bit more descriptive?

[stevenjb, 2012/11/12 19:46:40]

Done.

+                          const std::string& interface,
+                          const std::string& method);
+  static void AddReceivedCall(const std::string& service,

[satorux1, 2012/11/12 00:40:43]

ditto.

[stevenjb, 2012/11/12 19:46:40]

Done.

+                              const std::string& interface,
+                              const std::string& method);
+
+  // Output the calls into a formatted string. |show| determines what level
+  // of detail to show: one line per service, per interface, or per method.
+  // If |show_per_minute| is true include per minute stats.
+  static std::string GetAsString(ShowInString show, bool show_per_minute);

[satorux1, 2012/11/12 00:40:43]

boolean parameter like this makes the client code cryptic. (it's unclear what
'true' means). Maybe just go with one parameter like:

GetAsString(ShowOptions options);

Where ShowOptions is a bit-field of options, like 

  SHOW_INTERFACE = 1,
  SHOW_METHOD = 1 << 1,
  SHOW_PER_MINUTE = 1 << 2,

  

[stevenjb, 2012/11/12 19:46:40]

I find bitfields to be error-prone; C++ doesn't handle them very well. Changed
show_per_minute as a separate enum.

+
+  // Testing

[satorux1, 2012/11/12 00:40:43]

Please write a function comment.

[stevenjb, 2012/11/12 19:46:40]

Done.

+  static bool GetCallsForTesting(const std::string& service,
+                                 const std::string& interface,
+                                 const std::string& method,
+                                 int* sent,
+                                 int* received);
+
+ private:
+  // Eunum to specify which field in Stat to incrament in AddStat
+  enum StatType {
+    TYPE_SENT,
+    TYPE_RECEIVED
+  };
+
+  typedef std::set<Stat*, Stat::PtrCompare> StatSet;
+
+  // 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);
+
+  // 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);
+
+  StatSet stats_;
+  base::Time start_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(DbusStatistics);
+};
+
+}  // namespace dbus
+
+#endif  // DBUS_DBUS_STATISTICS_H_