chromeos: Simplify user action code.

chrome/browser/chromeos/external_metrics.cc

Index: chrome/browser/chromeos/external_metrics.cc
diff --git a/chrome/browser/chromeos/external_metrics.cc b/chrome/browser/chromeos/external_metrics.cc
index 73e24d82a873bc6e35f688486e2144f6b3d6f70c..08be59bc3565e8025156c3e14a23b5917e525d1c 100644
--- a/chrome/browser/chromeos/external_metrics.cc
+++ b/chrome/browser/chromeos/external_metrics.cc
@@ -21,55 +21,36 @@
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/metrics/user_metrics.h"
 
-// Steps to add an action.
-//
-// 1. Enter a helper function that calls UserMetrics::RecordAction.
-//
-// 2. Add a line for that function in InitializeUserActions.
-//
-// 3. Enjoy the recompilation.
-//
-// TODO(semenzato): should see if it is possible to avoid recompiling code
-// every time a new user action is added, and register it in some other way.
-
 namespace chromeos {
 
 // The interval between external metrics collections, in milliseconds.
 static const int kExternalMetricsCollectionIntervalMs = 30 * 1000;
 
-// There is one of the following functions for every user action as we have to
-// call RecordAction in a way that gets picked up by the processing scripts.
-static void RecordTabOverviewKeystroke() {
-  UserMetrics::RecordAction(UserMetricsAction("TabOverview_Keystroke"));
-}
-
-static void RecordTabOverviewExitMouse() {
-  UserMetrics::RecordAction(UserMetricsAction("TabOverview_ExitMouse"));
+ExternalMetrics::ExternalMetrics()
+    : test_recorder_(NULL) {
 }
 
 void ExternalMetrics::Start() {
-  InitializeUserActions();
-  ScheduleCollector();
-}
+  // Register user actions external to the browser.
+  // chrome/tools/extract_actions.py won't understand these lines, so all of
+  // these are explicitly added in that script.
+  // TODO(derat): We shouldn't need to verify actions before reporting them;

[petkov, 2011/01/21 18:03:42]

Or we can modify the regular expression in extract_actions.py:GrepForActions to
recognize the pattern below too. This way we guarantee that we never send
unknown actions to the server and at the same time keep only one place under
Chrome that needs to be updated whenever a new action needs to be registered.

Maybe add this as an alternative to the TODO.

+  // remove all of this once http://crosbug.com/11125 is fixed.
+  valid_user_actions_.insert("Accel_NextWindow_Tab");
+  valid_user_actions_.insert("Accel_PrevWindow_Tab");
+  valid_user_actions_.insert("Accel_NextWindow_F5");
+  valid_user_actions_.insert("Accel_PrevWindow_F5");
+  valid_user_actions_.insert("Accel_BrightnessDown_F6");
+  valid_user_actions_.insert("Accel_BrightnessUp_F7");
 
-void ExternalMetrics::DefineUserAction(const std::string& name,
-                                       RecordFunctionType f) {
-  DCHECK(action_recorders_.find(name) == action_recorders_.end());
-  action_recorders_[name] = f;
-}
-
-void ExternalMetrics::InitializeUserActions() {
-  DefineUserAction("TabOverviewExitMouse", RecordTabOverviewExitMouse);
-  DefineUserAction("TabOverviewKeystroke", RecordTabOverviewKeystroke);
+  ScheduleCollector();
 }
 
 void ExternalMetrics::RecordActionUI(std::string action_string) {
-  base::hash_map<std::string, RecordFunctionType>::const_iterator iterator;
-  iterator = action_recorders_.find(action_string);
-  if (iterator == action_recorders_.end()) {
-    LOG(ERROR) << "undefined UMA action: " << action_string;
+  if (valid_user_actions_.count(action_string)) {
+    UserMetrics::RecordComputedAction(action_string);
   } else {
-    iterator->second();
+    LOG(ERROR) << "undefined UMA action: " << action_string;
   }
 }
 
@@ -77,7 +58,7 @@ void ExternalMetrics::RecordAction(const char* action) {
   std::string action_string(action);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &ExternalMetrics::RecordActionUI, action));
+      NewRunnableMethod(this, &ExternalMetrics::RecordActionUI, action_string));
 }
 
 void ExternalMetrics::RecordHistogram(const char* histogram_data) {