Added of new UMA signals in order to be able to discover early if the "save password" feature gets …

chrome/browser/password_manager/password_manager_delegate_impl.cc

Index: chrome/browser/password_manager/password_manager_delegate_impl.cc
diff --git a/chrome/browser/password_manager/password_manager_delegate_impl.cc b/chrome/browser/password_manager/password_manager_delegate_impl.cc
index f7c42bf88110d2718864d978996f9d8f2010a8d7..1e317ef931549a99e122ef6f4686b8db98c951b4 100644
--- a/chrome/browser/password_manager/password_manager_delegate_impl.cc
+++ b/chrome/browser/password_manager/password_manager_delegate_impl.cc
@@ -7,10 +7,12 @@
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/perftimer.h"
 #include "chrome/browser/infobars/confirm_infobar_delegate.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager.h"
+#include "chrome/browser/password_manager/password_manager_metrics_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 #include "components/autofill/content/browser/autofill_driver_impl.h"
@@ -74,6 +76,14 @@ class SavePasswordInfoBarDelegate : public ConfirmInfoBarDelegate {
   // Used to track the results we get from the info bar.
   ResponseType infobar_response_;
 
+  // Save password prompt lifetime needed for a UMA signal.
+  PerfTimer timer_;
+
+  // The bucket name corresponding to the domain name of
+  // |form_to_save_| if the form is on a monitored domain.
+  // Otherwise, an empty string.
+  std::string bucket_name_;

[Ilya Sherman, 2013/09/10 22:30:54]

nit: Perhaps name this |uma_histogram_suffix_| or |domain_type_for_metrics_| or
something a little more descriptive like these two suggestions.

[jdomingos, 2013/09/11 15:50:30]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(SavePasswordInfoBarDelegate);
 };
 
@@ -106,11 +116,27 @@ SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate(
     : ConfirmInfoBarDelegate(infobar_service),
       form_to_save_(form_to_save),
       infobar_response_(NO_RESPONSE) {
+  bucket_name_ = password_manager_metrics_util::IsDomainNameMonitored(
+      form_to_save->realm());
+  if (!bucket_name_.empty()) {
+    password_manager_metrics_util::LogUMAHistogramCount(
+        "PasswordManager.SavePasswordPromptDisplayed_" + bucket_name_,
+        1);

[Ilya Sherman, 2013/09/10 22:30:54]

It's wasteful to allocate 50 buckets and only use one of them as you are doing
here.  Please either create a single enumerated histogram with one bucket per
category, or use fewer buckets per histogram.

[jdomingos, 2013/09/11 15:50:30]

Done.

+  }
 }
 
 SavePasswordInfoBarDelegate::~SavePasswordInfoBarDelegate() {
+

[Ilya Sherman, 2013/09/10 22:30:54]

nit: Spurious newline.

[jdomingos, 2013/09/11 15:50:30]

Done.

   UMA_HISTOGRAM_ENUMERATION("PasswordManager.InfoBarResponse",
                             infobar_response_, NUM_RESPONSE_TYPES);
+  if (!bucket_name_.empty()) {
+    password_manager_metrics_util::LogUMAHistogramEnumeration(
+        "PasswordManager.SavePasswordPromptResponse_" + bucket_name_,
+        infobar_response_, NUM_RESPONSE_TYPES);
+    password_manager_metrics_util::LogUMAHistogramTimes(
+        "PasswordManager.SavePasswordPromptLifetime_" + bucket_name_,
+        timer_.Elapsed());

[Ilya Sherman, 2013/09/10 22:30:54]

Why is this histogram tracker per-category, rather than having just a single
histogram for all possible infobars?  Are you really trying to measure a derived
signal that can be inferred from the timer value -- e.g. did the infobar
disappear before the user could act on it -- that could be measured as a boolean
histogram on the client rather than needing to fill out the complete time
distribution?

+  }
 }
 
 int SavePasswordInfoBarDelegate::GetIconID() const {