NTP4/Sync Promo:

- rename sync promo -> NTP sign-in promo
- re-arrange UMA to correctly report stuff given new code path
- re-fix pane switcher issue.

R=sail@chromium.org,estade@chromium.org
TBR=mirandac@chromium.org
BUG=99540
TEST=Unit tests still pass, code still compiles, UMA gets reported, and pane switchers don't obscure.


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

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

3 files changed, 26 insertions, 13 deletions

diff --git a/chrome/browser/ui/webui/ntp/ntp_login_handler.cc b/chrome/browser/ui/webui/ntp/ntp_login_handler.cc
index 2a46eb3..7d767c5 100644
--- a/chrome/browser/ui/webui/ntp/ntp_login_handler.cc
+++ b/chrome/browser/ui/webui/ntp/ntp_login_handler.cc
@@ -64,9 +64,6 @@ void NTPLoginHandler::Observe(int type,
 
 void NTPLoginHandler::HandleInitializeSyncLogin(const ListValue* args) {
   UpdateLogin();
-  UMA_HISTOGRAM_ENUMERATION("SyncPromo.NTPPromo",
-                            SYNC_PROMO_NTP_PROMO_VIEWED,
-                            SYNC_PROMO_NTP_PROMO_BUCKET_BOUNDARY);
 }
 
 void NTPLoginHandler::HandleShowSyncLoginUI(const ListValue* args) {
@@ -77,12 +74,10 @@ void NTPLoginHandler::HandleShowSyncLoginUI(const ListValue* args) {
   if (username.empty()) {
     // The user isn't signed in, show the sync promo.
     if (SyncPromoUI::ShouldShowSyncPromo(profile)) {
-      UMA_HISTOGRAM_ENUMERATION("SyncPromo.NTPPromo",
-                                SYNC_PROMO_NTP_PROMO_CLICKED,
-                                SYNC_PROMO_NTP_PROMO_BUCKET_BOUNDARY);
       web_ui_->tab_contents()->OpenURL(GURL(chrome::kChromeUISyncPromoURL),
                                        GURL(), CURRENT_TAB,
                                        content::PAGE_TRANSITION_LINK);
+      RecordInHistogram(NTP_SIGN_IN_PROMO_CLICKED);
     }
   } else if (args->GetSize() == 4) {
     // The user is signed in, show the profiles menu.
@@ -107,6 +102,17 @@ void NTPLoginHandler::HandleShowSyncLoginUI(const ListValue* args) {
   }
 }
 
+void NTPLoginHandler::RecordInHistogram(int type) {
+  // Invalid type to record.
+  if (type < NTP_SIGN_IN_PROMO_VIEWED ||
+      type > NTP_SIGN_IN_PROMO_CLICKED) {
+    NOTREACHED();
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("SyncPromo.NTPPromo", type,
+                              NTP_SIGN_IN_PROMO_BUCKET_BOUNDARY);
+  }
+}
+
 void NTPLoginHandler::UpdateLogin() {
   Profile* profile = Profile::FromWebUI(web_ui_);
   std::string username = profile->GetPrefs()->GetString(
@@ -116,7 +122,7 @@ void NTPLoginHandler::UpdateLogin() {
     header = UTF8ToUTF16(username);
   } else if (SyncPromoUI::ShouldShowSyncPromo(profile) &&
              (SyncPromoUI::UserHasSeenSyncPromoAtStartup(profile) ||
-              PromoResourceService::CanShowSyncPromo(profile))) {
+              PromoResourceService::CanShowNTPSignInPromo(profile))) {
     string16 signed_in_link = l10n_util::GetStringUTF16(
         IDS_SYNC_PROMO_NOT_SIGNED_IN_STATUS_LINK);
     signed_in_link = ASCIIToUTF16("<span class='link-span'>") + signed_in_link +
@@ -126,6 +132,8 @@ void NTPLoginHandler::UpdateLogin() {
         l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_NAME));
     sub_header = l10n_util::GetStringFUTF16(
         IDS_SYNC_PROMO_NOT_SIGNED_IN_STATUS_SUB_HEADER, signed_in_link);
+    // Record that the user was shown the promo.
+    RecordInHistogram(NTP_SIGN_IN_PROMO_VIEWED);
   }
 
   StringValue header_value(header);
@@ -134,6 +142,7 @@ void NTPLoginHandler::UpdateLogin() {
       "updateLogin", header_value, sub_header_value);
 }
 
+// static
 bool NTPLoginHandler::ShouldShow(Profile* profile) {
 #if defined(OS_CHROMEOS)
   // For now we don't care about showing sync status on Chrome OS. The promo
diff --git a/chrome/browser/ui/webui/ntp/ntp_login_handler.h b/chrome/browser/ui/webui/ntp/ntp_login_handler.h
index 3169ba0..890a34c 100644
--- a/chrome/browser/ui/webui/ntp/ntp_login_handler.h
+++ b/chrome/browser/ui/webui/ntp/ntp_login_handler.h
@@ -38,10 +38,10 @@ class NTPLoginHandler : public WebUIMessageHandler,
 
  private:
   // User actions while on the NTP when clicking on or viewing the sync promo.
-  enum SyncPromoNTPPromoBuckets {
-    SYNC_PROMO_NTP_PROMO_VIEWED,
-    SYNC_PROMO_NTP_PROMO_CLICKED,
-    SYNC_PROMO_NTP_PROMO_BUCKET_BOUNDARY,
+  enum NTPSignInPromoBuckets {
+    NTP_SIGN_IN_PROMO_VIEWED,
+    NTP_SIGN_IN_PROMO_CLICKED,
+    NTP_SIGN_IN_PROMO_BUCKET_BOUNDARY,
   };
 
   // Called from JS when the NTP is loaded. |args| is the list of arguments
@@ -53,6 +53,9 @@ class NTPLoginHandler : public WebUIMessageHandler,
   // arguments passed from JS and should be an empty list.
   void HandleShowSyncLoginUI(const ListValue* args);
 
+  // Records actions in SyncPromo.NTPPromo histogram.
+  void RecordInHistogram(int type);
+
   // Internal helper method
   void UpdateLogin();
 
diff --git a/chrome/browser/ui/webui/sync_promo_ui.cc b/chrome/browser/ui/webui/sync_promo_ui.cc
index fe58dea..17d980e 100644
--- a/chrome/browser/ui/webui/sync_promo_ui.cc
+++ b/chrome/browser/ui/webui/sync_promo_ui.cc
@@ -36,7 +36,7 @@ const char kSyncPromoQueryKeyShowTitle[]  = "show_title";
 const char kSyncPromoQueryKeyNextPage[]  = "next_page";
 
 // The maximum number of times we want to show the sync promo at startup.
-const int kSyncPromoShowAtStartupMaxiumum = 10;
+const int kSyncPromoShowAtStartupMaximum = 10;
 
 // Checks we want to show the sync promo for the given brand.
 bool AllowPromoAtStartupForCurrentBrand() {
@@ -120,6 +120,7 @@ SyncPromoUI::SyncPromoUI(TabContents* contents) : ChromeWebUI(contents) {
   profile->GetChromeURLDataManager()->AddDataSource(html_source);
 }
 
+// static
 bool SyncPromoUI::ShouldShowSyncPromo(Profile* profile) {
 #if defined(OS_CHROMEOS)
   // There's no need to show the sync promo on cros since cros users are logged
@@ -176,7 +177,7 @@ bool SyncPromoUI::ShouldShowSyncPromoAtStartup(Profile* profile,
     return false;
 
   int show_count = prefs->GetInteger(prefs::kSyncPromoStartupCount);
-  if (show_count >= kSyncPromoShowAtStartupMaxiumum)
+  if (show_count >= kSyncPromoShowAtStartupMaximum)
     return false;
 
   // This pref can be set in the master preferences file to allow or disallow