[UMA] Fix a crash on logging user actions posted from a non-UI thread.

Along the way, add DCHECKs to verify that user actions are always posted from
the UI thread.

BUG=360271
TEST=none
R=asvitkine@chromium.org, avi@chromium.org

Review URL: https://codereview.chromium.org/229873002

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

2 files changed, 46 insertions, 15 deletions

diff --git a/base/metrics/user_metrics.cc b/base/metrics/user_metrics.cc
index a3b122a..9db5840 100644
--- a/base/metrics/user_metrics.cc
+++ b/base/metrics/user_metrics.cc
@@ -7,39 +7,68 @@
 #include <vector>
 
 #include "base/lazy_instance.h"
+#include "base/threading/thread_checker.h"
 
 namespace base {
 namespace {
 
-base::LazyInstance<std::vector<ActionCallback> > g_action_callbacks =
-    LAZY_INSTANCE_INITIALIZER;
+// A helper class for tracking callbacks and ensuring thread-safety.
+class Callbacks {
+ public:
+  Callbacks() {}
 
-void Record(const char *action) {
-  for (size_t i = 0; i < g_action_callbacks.Get().size(); i++)
-    g_action_callbacks.Get()[i].Run(action);
-}
+  // Records the |action|.
+  void Record(const std::string& action) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    for (size_t i = 0; i < callbacks_.size(); ++i) {
+      callbacks_[i].Run(action);
+    }
+  }
+
+  // Adds |callback| to the list of |callbacks_|.
+  void AddCallback(const ActionCallback& callback) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    callbacks_.push_back(callback);
+  }
+
+  // Removes the first instance of |callback| from the list of |callbacks_|, if
+  // there is one.
+  void RemoveCallback(const ActionCallback& callback) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    for (size_t i = 0; i < callbacks_.size(); ++i) {
+      if (callbacks_[i].Equals(callback)) {
+        callbacks_.erase(callbacks_.begin() + i);
+        return;
+      }
+    }
+  }
+
+ private:
+  base::ThreadChecker thread_checker_;
+  std::vector<ActionCallback> callbacks_;
+
+  DISALLOW_COPY_AND_ASSIGN(Callbacks);
+};
+
+base::LazyInstance<Callbacks> g_callbacks = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 void RecordAction(const UserMetricsAction& action) {
-  Record(action.str_);
+  g_callbacks.Get().Record(action.str_);
 }
 
 void RecordComputedAction(const std::string& action) {
-  Record(action.c_str());
+  g_callbacks.Get().Record(action);
 }
 
 void AddActionCallback(const ActionCallback& callback) {
-  g_action_callbacks.Get().push_back(callback);
+  g_callbacks.Get().AddCallback(callback);
 }
 
 void RemoveActionCallback(const ActionCallback& callback) {
-  for (size_t i = 0; i < g_action_callbacks.Get().size(); i++) {
-    if (g_action_callbacks.Get()[i].Equals(callback)) {
-      g_action_callbacks.Get().erase(g_action_callbacks.Get().begin() + i);
-      return;
-    }
-  }
+  g_callbacks.Get().RemoveCallback(callback);
+
 }
 
 }  // namespace base
diff --git a/base/metrics/user_metrics.h b/base/metrics/user_metrics.h
index 57356c3..bcfefb80 100644
--- a/base/metrics/user_metrics.h
+++ b/base/metrics/user_metrics.h
@@ -17,6 +17,8 @@ namespace base {
 // the user metrics system.
 
 // Record that the user performed an action.
+// This method *must* be called from the main thread.
+//
 // "Action" here means a user-generated event:
 //   good: "Reload", "CloseTab", and "IMEInvoked"
 //   not good: "SSLDialogShown", "PageLoaded", "DiskFull"