Add metrics to help measure effects of installing default apps in a profile.

chrome/browser/ui/webui/ntp/new_tab_page_handler.cc

Index: chrome/browser/ui/webui/ntp/new_tab_page_handler.cc
===================================================================
--- chrome/browser/ui/webui/ntp/new_tab_page_handler.cc	(revision 103881)
+++ chrome/browser/ui/webui/ntp/new_tab_page_handler.cc	(working copy)
@@ -6,6 +6,9 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
+#include "chrome/browser/extensions/default_apps_trial.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/profile_sync_service.h"
@@ -18,33 +21,55 @@
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
-static const int kIntroDisplayMax = 10;
+namespace {
+const int kIntroDisplayMax = 10;
+}  // anonymous

[Evan Stade, 2011/10/04 16:58:33]

is there a difference?

[Roger Tawa OOO till Jul 10th, 2011/10/04 17:38:46]

I believe unnamed namespaces are preferred, but I can revert this if you like.

[Evan Stade, 2011/10/04 17:54:35]

I think it's the same thing, but for very short namespaces static is less
verbose

[Roger Tawa OOO till Jul 10th, 2011/10/04 18:15:25]

Put back static.

 
 // The URL of a knowledge-base article about the new NTP.
 static const char kNTP4IntroURL[] =
   "http://www.google.com/support/chrome/bin/answer.py?answer=95451";
 
+WebUIMessageHandler* NewTabPageHandler::Attach(WebUI* web_ui) {
+  // Record an open of the NTP with its default page type.
+  PrefService* prefs = Profile::FromWebUI(web_ui)->GetPrefs();
+  int shown_page_type = prefs->GetInteger(prefs::kNTPShownPage) >>
+      PAGE_ID_OFFSET;
+  UMA_HISTOGRAM_ENUMERATION("NtpHandler.AttachShownPageType",
+                            shown_page_type, 4);
+
+  static bool default_apps_trial_exists =
+      base::FieldTrialList::TrialExists(kDefaultAppsTrial_Name);

[Evan Stade, 2011/10/04 16:58:33]

why does this variable have an underscore in it

[Roger Tawa OOO till Jul 10th, 2011/10/04 17:38:46]

Because that's the way its declared in default_apps_trial.h

[Evan Stade, 2011/10/04 17:54:35]

right. I couldn't comment there because it's not in this changelist. Pretend I
put this comment in the right place. There shouldn't be an underscore unless
there's a style rule I'm unaware of.

[Roger Tawa OOO till Jul 10th, 2011/10/04 18:15:25]

That can be addressed in a follow up CL.

+  if (default_apps_trial_exists) {
+    UMA_HISTOGRAM_ENUMERATION(
+        base::FieldTrial::MakeName("NtpHandler.AttachShownPageType",
+                                   kDefaultAppsTrial_Name),
+        shown_page_type, 4);
+  }
+
+  return WebUIMessageHandler::Attach(web_ui);
+}
+
 void NewTabPageHandler::RegisterMessages() {
-  web_ui_->RegisterMessageCallback("closeNotificationPromo",
+  web_ui()->RegisterMessageCallback("closeNotificationPromo",
       base::Bind(&NewTabPageHandler::HandleCloseNotificationPromo,
                  base::Unretained(this)));
-  web_ui_->RegisterMessageCallback("notificationPromoViewed",
+  web_ui()->RegisterMessageCallback("notificationPromoViewed",
       base::Bind(&NewTabPageHandler::HandleNotificationPromoViewed,
                  base::Unretained(this)));
-  web_ui_->RegisterMessageCallback("pageSelected",
+  web_ui()->RegisterMessageCallback("pageSelected",
       base::Bind(&NewTabPageHandler::HandlePageSelected,
                  base::Unretained(this)));
-  web_ui_->RegisterMessageCallback("introMessageDismissed",
+  web_ui()->RegisterMessageCallback("introMessageDismissed",
       base::Bind(&NewTabPageHandler::HandleIntroMessageDismissed,
                  base::Unretained(this)));
-  web_ui_->RegisterMessageCallback("introMessageSeen",
+  web_ui()->RegisterMessageCallback("introMessageSeen",
       base::Bind(&NewTabPageHandler::HandleIntroMessageSeen,
                  base::Unretained(this)));
 }
 
 void NewTabPageHandler::HandleCloseNotificationPromo(const ListValue* args) {
   NotificationPromo notification_promo(
-      Profile::FromWebUI(web_ui_)->GetPrefs(), NULL);
+      Profile::FromWebUI(web_ui())->GetPrefs(), NULL);
   notification_promo.HandleClosed();
   NotifyPromoResourceChanged();
 }
@@ -66,17 +91,30 @@
   CHECK(args->GetDouble(1, &index_double));
   int index = static_cast<int>(index_double);
 
-  PrefService* prefs = Profile::FromWebUI(web_ui_)->GetPrefs();
+  PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
   prefs->SetInteger(prefs::kNTPShownPage, page_id | index);
+
+  int shown_page_type = page_id >> PAGE_ID_OFFSET;
+  UMA_HISTOGRAM_ENUMERATION("NtpHandler.SelectedShownPageType",

[Evan Stade, 2011/10/04 16:58:33]

don't you have to add this name to a list somewhere? Have you done this and have
you pinged jar about this review?

[Roger Tawa OOO till Jul 10th, 2011/10/04 17:38:46]

That change is in a separate repo.

+                            shown_page_type, 4);
+
+  static bool default_apps_trial_exists =
+      base::FieldTrialList::TrialExists(kDefaultAppsTrial_Name);
+  if (default_apps_trial_exists) {
+    UMA_HISTOGRAM_ENUMERATION(
+        base::FieldTrial::MakeName("NtpHandler.SelectedShownPageType",
+                                   kDefaultAppsTrial_Name),
+        shown_page_type, 4);
+  }
 }
 
 void NewTabPageHandler::HandleIntroMessageDismissed(const ListValue* args) {
-  PrefService* prefs = Profile::FromWebUI(web_ui_)->GetPrefs();
+  PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
   prefs->SetInteger(prefs::kNTP4IntroDisplayCount, kIntroDisplayMax + 1);
 }
 
 void NewTabPageHandler::HandleIntroMessageSeen(const ListValue* args) {
-  PrefService* prefs = Profile::FromWebUI(web_ui_)->GetPrefs();
+  PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
   int intro_displays = prefs->GetInteger(prefs::kNTP4IntroDisplayCount);
   prefs->SetInteger(prefs::kNTP4IntroDisplayCount, intro_displays + 1);
 }