Sync Promo: Add more UMA metrics.

chrome/browser/ui/webui/sync_promo_handler.cc

Index: chrome/browser/ui/webui/sync_promo_handler.cc
diff --git a/chrome/browser/ui/webui/sync_promo_handler.cc b/chrome/browser/ui/webui/sync_promo_handler.cc
index fbee2dc2dbb191ce649e899bd91c0078dcef62a1..8dcbdb89c92bb67da0b2752c25483decaf29d085 100644
--- a/chrome/browser/ui/webui/sync_promo_handler.cc
+++ b/chrome/browser/ui/webui/sync_promo_handler.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/metrics/histogram.h"
+#include "base/time.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/profile_sync_service.h"
@@ -20,8 +21,21 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/browser/tab_contents/tab_contents.h"
-#include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+
+namespace {
+
+// This was added because of the need to change the existing UMA enum for the
+// sync promo mid-flight. Ideally these values would be contiguous, but the
+// real world is not always ideal.
+static bool IsValidUserFlowAction(int action) {
+  return (action >= extension_misc::SYNC_PROMO_FIRST_VALID_JS_ACTION &&
+          action <= extension_misc::SYNC_PROMO_LAST_VALID_JS_ACTION) ||
+         action == extension_misc::SYNC_PROMO_LEFT_DURING_THROBBER;
+}
+
+}  // namespace
 
 SyncPromoHandler::SyncPromoHandler(ProfileManager* profile_manager)
     : SyncSetupHandler(profile_manager),
@@ -67,12 +81,18 @@ void SyncPromoHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback("SyncPromo:Initialize",
       base::Bind(&SyncPromoHandler::HandleInitializeSyncPromo,
                  base::Unretained(this)));
-  web_ui_->RegisterMessageCallback("SyncPromo:UserFlowAction",
-      base::Bind(&SyncPromoHandler::HandleUserFlowAction,
+  web_ui_->RegisterMessageCallback("SyncPromo:RecordSignInAttempts",
+      base::Bind(&SyncPromoHandler::HandleRecordSignInAttempts,
+                 base::Unretained(this)));
+  web_ui_->RegisterMessageCallback("SyncPromo:RecordThrobberTime",
+      base::Bind(&SyncPromoHandler::HandleRecordThrobberTime,
                  base::Unretained(this)));
   web_ui_->RegisterMessageCallback("SyncPromo:ShowAdvancedSettings",
       base::Bind(&SyncPromoHandler::HandleShowAdvancedSettings,
                  base::Unretained(this)));
+  web_ui_->RegisterMessageCallback("SyncPromo:UserFlowAction",
+      base::Bind(&SyncPromoHandler::HandleUserFlowAction,
+                 base::Unretained(this)));
   SyncSetupHandler::RegisterMessages();
 }
 
@@ -168,17 +188,33 @@ void SyncPromoHandler::HandleShowAdvancedSettings(
   RecordUserFlowAction(extension_misc::SYNC_PROMO_ADVANCED_CLICKED);
 }
 
+// TODO(dbeam): Replace with metricsHandler:recordHistogramTime when it exists.
+void SyncPromoHandler::HandleRecordThrobberTime(const base::ListValue* args) {
+  double time_double;
+  CHECK(args->GetDouble(0, &time_double));
+  UMA_HISTOGRAM_TIMES("SyncPromo.ThrobberTime",
+                      base::TimeDelta::FromMilliseconds(time_double));
+}
+
+// TODO(dbeam): Replace with metricsHandler:recordHistogramCount when it exists.
+void SyncPromoHandler::HandleRecordSignInAttempts(const base::ListValue* args) {
+  double count_double;
+  CHECK(args->GetDouble(0, &count_double));
+  UMA_HISTOGRAM_COUNTS("SyncPromo.SignInAttempts", count_double);
+}
+
 void SyncPromoHandler::HandleUserFlowAction(const base::ListValue* args) {
   double action_double;
   CHECK(args->GetDouble(0, &action_double));
   int action = static_cast<int>(action_double);
-  if (action >= extension_misc::SYNC_PROMO_FIRST_VALID_JS_ACTION &&
-      action <= extension_misc::SYNC_PROMO_LAST_VALID_JS_ACTION) {
+
+  if (IsValidUserFlowAction(action))
     RecordUserFlowAction(action);
-  } else {
+  else
     NOTREACHED() << "Attempt to record invalid user flow action on sync promo.";
-  }
 
+  // TODO(dbeam): Should this be here or in a separate handler? This method was

[Evan Stade, 2011/11/15 17:45:19]

agree it should be separate

[Dan Beam, 2011/11/15 18:37:13]

Done.

+  // meant solely for UMA reporting.
   if (action == extension_misc::SYNC_PROMO_SKIP_CLICKED)
     SyncPromoUI::SetUserSkippedSyncPromo(Profile::FromWebUI(web_ui_));
 }