chromeos: Simplify user action code.

This makes us (almost) just forward through user actions that
we receive from other programs rather than having them listed
in Chrome's source.  Note that new actions that aren't
present in Chrome still need to be added to
chrome/tools/extract_actions.py, which needs to be rerun to
generate new hashes.

I say "almost" above because metrics still need to be added
to external_metrics.cc due to http://crosbug.com/11095.  Once
it's fixed, that requirement can be easily removed.

I also cleaned up some earlier additions that I made to
extract_actions.py and re-ran it to pick up missing actions.

BUG=chromium-os:10403
TEST=built it.  checked that i see no errors in chrome's log when using the window manager to report a valid user action, but that i still see an error when running "metrics_client -u BogusMetric"

Review URL: http://codereview.chromium.org/6266011

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

chrome/browser/chromeos/external_metrics.cc

@@ -23,55 +23,36 @@
 #include "chrome/browser/metrics/metrics_service.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;
+  // 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;
   }
 }
 
@@ -79,7 +60,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::RecordCrashUI(const std::string& crash_kind) {

chrome/browser/chromeos/external_metrics.h

@@ -26,7 +26,7 @@ class ExternalMetrics : public base::RefCountedThreadSafe<ExternalMetrics> {
   friend class base::RefCountedThreadSafe<ExternalMetrics>;
 
  public:
-  ExternalMetrics() : test_recorder_(NULL) {}
+  ExternalMetrics();
 
   // Begins the external data collection.  This service is started and stopped
   // by the chrome metrics service.  Calls to RecordAction originate in the
@@ -44,13 +44,6 @@ class ExternalMetrics : public base::RefCountedThreadSafe<ExternalMetrics> {
 
   ~ExternalMetrics() {}
 
-  // Registers a user action by associating the action name with a function
-  // that records instances of that action.
-  void DefineUserAction(const std::string& name, RecordFunctionType f);
-
-  // Registers all user actions external to the browser.
-  void InitializeUserActions();
-
   // Passes an action event to the UMA service on the UI thread.
   void RecordActionUI(std::string action_string);
 
@@ -85,6 +78,9 @@ class ExternalMetrics : public base::RefCountedThreadSafe<ExternalMetrics> {
   // Maps histogram or action names to recorder structs.
   base::hash_map<std::string, RecordFunctionType> action_recorders_;
 
+  // Set containing known user actions.
+  base::hash_set<std::string> valid_user_actions_;
+
   // Used for testing only.
   RecorderType test_recorder_;
   FilePath test_path_;

chrome/tools/chromeactions.txt

@@ -931,6 +931,11 @@
 0x9abd34cb9a857b9e	Options_UseSuggestCheckbox_Enable
 0x24604cce8816243c	Options_VertEdgeScrollCheckbox_Disable
 0x8d45d4c781319d53	Options_VertEdgeScrollCheckbox_Enable
+0x40a2640625760310	OutdatedPluginInfobar.AllowThisTime
+0xa0361f9250c19197	OutdatedPluginInfobar.Closed
+0x883c4259bb3beddd	OutdatedPluginInfobar.LearnMore
+0xaf62158873b29888	OutdatedPluginInfobar.Shown
+0x7e7073691a152241	OutdatedPluginInfobar.Update
 0x016839db155e0468	Outdent
 0x69fae31849862d21	OverrideEncoding
 0xdcb4676e49e95259	PDF.FitToHeightButton

chrome/tools/extract_actions.py

@@ -133,16 +133,6 @@ def AddComputedActions(actions):
     actions.add('LanguageOptions_UiLanguageChange_%s' % language_code)
     actions.add('LanguageOptions_SpellCheckLanguageChange_%s' % language_code)
 
-  # Actions sent by the Chrome OS window manager.
-  actions.add('Accel_NextWindow_Tab')
-  actions.add('Accel_PrevWindow_Tab')
-  actions.add('Accel_NextWindow_F5')
-  actions.add('Accel_PrevWindow_F5')
-
-  # Actions sent by the Chrome OS power manager.
-  actions.add('Accel_BrightnessDown_F6')
-  actions.add('Accel_BrightnessUp_F7')
-
 def AddWebKitEditorActions(actions):
   """Add editor actions from editor_client_impl.cc.
 
@@ -192,6 +182,22 @@ def AddAboutFlagsActions(actions):
       print >>sys.stderr, 'WARNING: This line is marked for recording ' + \
           'about:flags metrics, but is not in the proper format:\n' + line
 
+def AddChromeOSActions(actions):
+  """Add actions reported by non-Chrome processes in Chrome OS.
+
+  Arguments:
+    actions: set of actions to add to.
+  """
+  # Actions sent by the Chrome OS window manager.
+  actions.add('Accel_NextWindow_Tab')
+  actions.add('Accel_PrevWindow_Tab')
+  actions.add('Accel_NextWindow_F5')
+  actions.add('Accel_PrevWindow_F5')
+
+  # Actions sent by the Chrome OS power manager.
+  actions.add('Accel_BrightnessDown_F6')
+  actions.add('Accel_BrightnessUp_F7')
+
 def GrepForActions(path, actions):
   """Grep a source file for calls to UserMetrics functions.
 
@@ -264,6 +270,7 @@ def main(argv):
   # print "Found {0} entries".format(len(actions))
 
   AddClosedSourceActions(actions)
+  AddChromeOSActions(actions)
 
   if hash_output:
     f = open(chromeactions_path, "w")