Merge 110354 - Sync Promo: Add more UMA metrics.

R=estade@chromium.org
BUG=103822
TEST=+1 action in SyncPromo.UserFlowAction and +2 new histograms.


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

TBR=dbeam@chromium.org
Review URL: http://codereview.chromium.org/8586010

git-svn-id: svn://svn.chromium.org/chrome/branches/912/src@110404 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 154 insertions, 48 deletions

diff --git a/chrome/browser/resources/sync_promo.js b/chrome/browser/resources/sync_promo.js
index d387518..03341f3 100644
--- a/chrome/browser/resources/sync_promo.js
+++ b/chrome/browser/resources/sync_promo.js
@@ -32,6 +32,7 @@ cr.define('sync_promo', function() {
       CONFIRMED_AFTER_SIGN_IN: i++,
       CLOSED_TAB: i++,
       CLOSED_WINDOW: i++,
+      LEFT_DURING_THROBBER: i++,
     };
   }());
 
@@ -64,7 +65,7 @@ cr.define('sync_promo', function() {
       var self = this;
 
       $('promo-skip-button').addEventListener('click', function() {
-        chrome.send('SyncPromo:UserFlowAction', [actions.SKIP_CLICKED]);
+        chrome.send('SyncPromo:UserSkipped');
         self.closeOverlay_();
       });
 
@@ -104,6 +105,7 @@ cr.define('sync_promo', function() {
       // we also track users that use the keyboard and press enter.
       var signInAttemptedAlready = false;
       $('gaia-login-form').addEventListener('submit', function() {
+        ++self.signInAttempts_;
         if (!signInAttemptedAlready)
           chrome.send('SyncPromo:UserFlowAction', [actions.SIGN_IN_ATTEMPTED]);
         signInAttemptedAlready = true;
@@ -142,6 +144,17 @@ cr.define('sync_promo', function() {
     },
 
     /**
+     * Called when the page is unloading to record number of times a user tried
+     * to sign in and if they left while a throbber was running.
+     * @private
+     */
+    recordPageViewActions_: function() {
+      chrome.send('SyncPromo:RecordSignInAttempts', [this.signInAttempts_]);
+      if (this.throbberStart_)
+        chrome.send('SyncPromo:UserFlowAction', [actions.LEFT_DURING_THROBBER]);
+    },
+
+    /**
      * Remove the [hidden] attribute from the node that was not previously
      * transitioning.
      * @param {Event} e A -webkit-transition end event.
@@ -188,7 +201,36 @@ cr.define('sync_promo', function() {
      */
     setPromoTitleVisible_: function(visible) {
       $('promo-title').hidden = !visible;
-    }
+    },
+
+    /** @inheritDoc */
+    setThrobbersVisible_: function(visible) {
+      if (visible) {
+        this.throbberStart_ = Date.now();
+      } else {
+        if (this.throbberStart_) {
+          chrome.send('SyncPromo:RecordThrobberTime',
+                      [Date.now() - this.throbberStart_]);
+        }
+        this.throbberStart_ = 0;
+      }
+      // Pass through to SyncSetupOverlay to handle display logic.
+      options.SyncSetupOverlay.prototype.setThrobbersVisible_.apply(
+          this, arguments);
+    },
+
+    /**
+     * Number of times a user attempted to sign in to GAIA during this page
+     * view.
+     * @private
+     */
+    signInAttempts_: 0,
+
+    /**
+     * The start time of a throbber on the page.
+     * @private
+     */
+    throbberStart_: 0,
   };
 
   SyncPromo.showErrorUI = function() {
@@ -222,14 +264,20 @@ cr.define('sync_promo', function() {
 
   SyncPromo.setPromoTitleVisible = function(visible) {
     SyncPromo.getInstance().setPromoTitleVisible_(visible);
-  }
+  };
+
+  SyncPromo.recordPageViewActions = function() {
+    SyncPromo.getInstance().recordPageViewActions_();
+  };
 
   // Export
   return {
-    SyncPromo : SyncPromo
+    SyncPromo: SyncPromo
   };
 });
 
 var OptionsPage = options.OptionsPage;
 var SyncSetupOverlay = sync_promo.SyncPromo;
 window.addEventListener('DOMContentLoaded', sync_promo.SyncPromo.initialize);
+window.addEventListener('beforeunload',
+    sync_promo.SyncPromo.recordPageViewActions.bind(sync_promo.SyncPromo));
diff --git a/chrome/browser/resources/sync_setup_overlay.js b/chrome/browser/resources/sync_setup_overlay.js
index 8e3a955..920c84f 100644
--- a/chrome/browser/resources/sync_setup_overlay.js
+++ b/chrome/browser/resources/sync_setup_overlay.js
@@ -545,10 +545,15 @@ cr.define('options', function() {
         this.showOverlay_();
     },
 
+    /**
+     * Changes the visibility of throbbers on this page.
+     * @param {boolean} visible Whether or not to set all throbber nodes
+     *     visible.
+     */
     setThrobbersVisible_: function(visible) {
       var throbbers = document.getElementsByClassName("throbber");
-        for (var i = 0; i < throbbers.length; i++)
-          throbbers[i].style.visibility = visible ? "visible" : "hidden";
+      for (var i = 0; i < throbbers.length; i++)
+        throbbers[i].style.visibility = visible ? "visible" : "hidden";
     },
 
     loginSetFocus_: function() {
@@ -751,7 +756,7 @@ cr.define('options', function() {
       $('captcha-value').disabled = true;
       $('access-code').disabled = true;
 
-      $('logging-in-throbber').style.visibility = "visible";
+      this.setThrobbersVisible_(true);
 
       var f = $('gaia-login-form');
       var email = $('gaia-email');
diff --git a/chrome/browser/ui/webui/metrics_handler.h b/chrome/browser/ui/webui/metrics_handler.h
index b86dbf2..2f6c693 100644
--- a/chrome/browser/ui/webui/metrics_handler.h
+++ b/chrome/browser/ui/webui/metrics_handler.h
@@ -33,6 +33,8 @@ class MetricsHandler : public WebUIMessageHandler {
   // user action.
   void HandleRecordAction(const base::ListValue* args);
 
+  // TODO(dbeam): http://crbug.com/104338
+
   // Callback for the "metricsHandler:recordInHistogram" message. This records
   // into a histogram. |args| contains the histogram name, the value to record,
   // and the maximum allowed value, which can be at most 4000. The histogram
diff --git a/chrome/browser/ui/webui/sync_promo_handler.cc b/chrome/browser/ui/webui/sync_promo_handler.cc
index ed69584..7540790 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,42 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/browser/tab_contents/tab_contents.h"
-#include "content/common/notification_details.h"
-#include "content/common/notification_service.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+
+namespace {
+
+// User actions on the sync promo (aka "Sign in to Chrome").
+enum SyncPromoUserFlowActionEnums {
+  SYNC_PROMO_VIEWED,
+  SYNC_PROMO_LEARN_MORE_CLICKED,
+  SYNC_PROMO_ACCOUNT_HELP_CLICKED,
+  SYNC_PROMO_CREATE_ACCOUNT_CLICKED,
+  SYNC_PROMO_SKIP_CLICKED,
+  SYNC_PROMO_SIGN_IN_ATTEMPTED,
+  SYNC_PROMO_SIGNED_IN_SUCCESSFULLY,
+  SYNC_PROMO_ADVANCED_CLICKED,
+  SYNC_PROMO_ENCRYPTION_HELP_CLICKED,
+  SYNC_PROMO_CANCELLED_AFTER_SIGN_IN,
+  SYNC_PROMO_CONFIRMED_AFTER_SIGN_IN,
+  SYNC_PROMO_CLOSED_TAB,
+  SYNC_PROMO_CLOSED_WINDOW,
+  SYNC_PROMO_LEFT_DURING_THROBBER,
+  SYNC_PROMO_BUCKET_BOUNDARY,
+  SYNC_PROMO_FIRST_VALID_JS_ACTION = SYNC_PROMO_LEARN_MORE_CLICKED,
+  SYNC_PROMO_LAST_VALID_JS_ACTION = SYNC_PROMO_CONFIRMED_AFTER_SIGN_IN,
+};
+
+// 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 >= SYNC_PROMO_FIRST_VALID_JS_ACTION &&
+          action <= SYNC_PROMO_LAST_VALID_JS_ACTION) ||
+         action == SYNC_PROMO_LEFT_DURING_THROBBER;
+}
+
+}  // namespace
 
 SyncPromoHandler::SyncPromoHandler(ProfileManager* profile_manager)
     : SyncSetupHandler(profile_manager),
@@ -66,12 +101,21 @@ 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)));
+  web_ui_->RegisterMessageCallback("SyncPromo:UserSkipped",
+      base::Bind(&SyncPromoHandler::HandleUserSkipped,
+                 base::Unretained(this)));
   SyncSetupHandler::RegisterMessages();
 }
 
@@ -100,7 +144,7 @@ void SyncPromoHandler::Observe(int type,
   switch (type) {
     case content::NOTIFICATION_TAB_CLOSING: {
       if (!window_already_closed_)
-        RecordUserFlowAction(extension_misc::SYNC_PROMO_CLOSED_TAB);
+        RecordUserFlowAction(SYNC_PROMO_CLOSED_TAB);
       break;
     }
     case chrome::NOTIFICATION_BROWSER_CLOSING: {
@@ -108,7 +152,7 @@ void SyncPromoHandler::Observe(int type,
       Browser* browser = Source<Browser>(source).ptr();
       if (browser->tabstrip_model()->GetWrapperIndex(
               web_ui_->tab_contents()) != TabStripModel::kNoTab) {
-        RecordUserFlowAction(extension_misc::SYNC_PROMO_CLOSED_WINDOW);
+        RecordUserFlowAction(SYNC_PROMO_CLOSED_WINDOW);
         window_already_closed_ = true;
       }
       break;
@@ -148,7 +192,7 @@ void SyncPromoHandler::HandleInitializeSyncPromo(const base::ListValue* args) {
 
   OpenSyncSetup();
   // We don't need to compute anything for this, just do this every time.
-  RecordUserFlowAction(extension_misc::SYNC_PROMO_VIEWED);
+  RecordUserFlowAction(SYNC_PROMO_VIEWED);
   // Increment view count first and show natural numbers in stats rather than 0
   // based starting point (if it happened to be our first time showing this).
   IncrementViewCountBy(1);
@@ -164,22 +208,38 @@ void SyncPromoHandler::HandleShowAdvancedSettings(
   url += chrome::kSyncSetupSubPage;
   web_ui_->tab_contents()->OpenURL(GURL(url), GURL(), CURRENT_TAB,
                                    content::PAGE_TRANSITION_LINK);
-  RecordUserFlowAction(extension_misc::SYNC_PROMO_ADVANCED_CLICKED);
+  RecordUserFlowAction(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.";
-  }
+}
 
-  if (action == extension_misc::SYNC_PROMO_SKIP_CLICKED)
-    SyncPromoUI::SetUserSkippedSyncPromo(Profile::FromWebUI(web_ui_));
+void SyncPromoHandler::HandleUserSkipped(const base::ListValue* args) {
+  SyncPromoUI::SetUserSkippedSyncPromo(Profile::FromWebUI(web_ui_));
+  RecordUserFlowAction(SYNC_PROMO_SKIP_CLICKED);
 }
 
 int SyncPromoHandler::GetViewCount() const {
@@ -198,5 +258,5 @@ int SyncPromoHandler::IncrementViewCountBy(unsigned int amount) {
 void SyncPromoHandler::RecordUserFlowAction(int action) {
   // Send an enumeration to our single user flow histogram.
   UMA_HISTOGRAM_ENUMERATION("SyncPromo.UserFlow", action,
-                            extension_misc::SYNC_PROMO_BUCKET_BOUNDARY);
+                            SYNC_PROMO_BUCKET_BOUNDARY);
 }
diff --git a/chrome/browser/ui/webui/sync_promo_handler.h b/chrome/browser/ui/webui/sync_promo_handler.h
index bd1c1b1..b128bed 100644
--- a/chrome/browser/ui/webui/sync_promo_handler.h
+++ b/chrome/browser/ui/webui/sync_promo_handler.h
@@ -11,7 +11,7 @@
 class NotificationSource;
 class NotificationDetails;
 
-// The handler for Javascript messages related to the "sync promo" page.
+// The handler for JavaScript messages related to the "sync promo" page.
 class SyncPromoHandler : public SyncSetupHandler {
  public:
   explicit SyncPromoHandler(ProfileManager* profile_manager);
@@ -37,19 +37,30 @@ class SyncPromoHandler : public SyncSetupHandler {
   virtual void ShowSetupUI() OVERRIDE;
 
  private:
-  // Javascript callback handler to close the sync promo.
+  // JavaScript callback handler to close the sync promo.
   void HandleCloseSyncPromo(const base::ListValue* args);
 
-  // Javascript callback handler to initialize the sync promo.
+  // JavaScript callback handler to initialize the sync promo.
   void HandleInitializeSyncPromo(const base::ListValue* args);
 
-  // Javascript callback handler to switch the advanced sync settings. |args| is
+  // JavaScript handler to record the duration for which the throbber was
+  // visible during an attempted sign-in flow.
+  void HandleRecordThrobberTime(const base::ListValue* args);
+
+  // JavaScript handler to record the number of times a user attempted to sign
+  // in to chrome while they were on the sync promo page.
+  void HandleRecordSignInAttempts(const base::ListValue* args);
+
+  // JavaScript callback handler to switch the advanced sync settings. |args| is
   // the list of arguments passed from JS and should be an empty list.
   void HandleShowAdvancedSettings(const base::ListValue* args);
 
-  // Javascript callback handler to record user actions on the sync promo.
+  // JavaScript callback handler to record user actions on the sync promo.
   void HandleUserFlowAction(const base::ListValue* args);
 
+  // JavaScript callback handler for when a user clicks skip.
+  void HandleUserSkipped(const base::ListValue* args);
+
   // Return the number of times the user with the current profile has seen the
   // sync promo.
   int GetViewCount() const;
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index 804fb0e..7dd3315 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -455,26 +455,6 @@ namespace extension_misc {
     UNLOAD_REASON_UPDATE,     // Extension is being updated to a newer version.
     UNLOAD_REASON_UNINSTALL,  // Extension is being uninstalled.
   };
-
-  // User actions on the sync promo (aka "Sign in to Chrome").
-  enum SyncPromoBuckets {
-    SYNC_PROMO_VIEWED,
-    SYNC_PROMO_LEARN_MORE_CLICKED,
-    SYNC_PROMO_ACCOUNT_HELP_CLICKED,
-    SYNC_PROMO_CREATE_ACCOUNT_CLICKED,
-    SYNC_PROMO_SKIP_CLICKED,
-    SYNC_PROMO_SIGN_IN_ATTEMPTED,
-    SYNC_PROMO_SIGNED_IN_SUCCESSFULLY,
-    SYNC_PROMO_ADVANCED_CLICKED,
-    SYNC_PROMO_ENCRYPTION_HELP_CLICKED,
-    SYNC_PROMO_CANCELLED_AFTER_SIGN_IN,
-    SYNC_PROMO_CONFIRMED_AFTER_SIGN_IN,
-    SYNC_PROMO_CLOSED_TAB,
-    SYNC_PROMO_CLOSED_WINDOW,
-    SYNC_PROMO_BUCKET_BOUNDARY,
-    SYNC_PROMO_FIRST_VALID_JS_ACTION = SYNC_PROMO_LEARN_MORE_CLICKED,
-    SYNC_PROMO_LAST_VALID_JS_ACTION = SYNC_PROMO_CONFIRMED_AFTER_SIGN_IN,
-  };
-} // extension_misc
+}  // extension_misc
 
 #endif  // CHROME_COMMON_EXTENSIONS_EXTENSION_CONSTANTS_H_