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 95ce6b81ab845a42af0b2c5712b4be28dbc1b363..1ed19bb39e8ac5f913d3d47744b0514380e5258e 100644
--- a/chrome/browser/password_manager/password_manager_delegate_impl.cc
+++ b/chrome/browser/password_manager/password_manager_delegate_impl.cc
@@ -6,6 +6,7 @@
 
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
+#include "base/perftimer.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/infobars/confirm_infobar_delegate.h"
 #include "chrome/browser/infobars/infobar_service.h"
@@ -53,6 +54,10 @@ class SavePasswordInfoBarDelegate : public ConfirmInfoBarDelegate {
     NUM_RESPONSE_TYPES,
   };
 
+  PerfTimer timer_;
+
+  std::string domain_name_;
+
   SavePasswordInfoBarDelegate(InfoBarService* infobar_service,
                               PasswordFormManager* form_to_save);
   virtual ~SavePasswordInfoBarDelegate();
@@ -106,11 +111,22 @@ SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate(
     : ConfirmInfoBarDelegate(infobar_service),
       form_to_save_(form_to_save),
       infobar_response_(NO_RESPONSE) {
+  GURL gurl(form_to_save->realm());
+  domain_name_ = gurl.host();

[vabr (Chromium), 2013/08/14 14:39:58]

Here again, only for a handful of hosts we want to log the UMA signal.

+  if (domain_name_.size())
+    domain_name_.insert(0, "_");
+  UMA_HISTOGRAM_BOOLEAN("PasswordManager.InfobarDisplayed" + domain_name_,
+                        true);
 }
 
 SavePasswordInfoBarDelegate::~SavePasswordInfoBarDelegate() {
+  // Time in milliseconds.

[vabr (Chromium), 2013/08/14 14:39:58]

Time of what? Please comment (in the code, not in the code-review tool) on what
his constant means and why it was chosen.

+  const int kTime = 200;
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.InfoBarResponse",

[vabr (Chromium), 2013/08/14 14:39:58]

This comments is on behalf of Jordy, as a reminder:

"Add the enum definition of InfoBarResponse from
http://chromegw/viewvc/chrome-internal/trunk/tools/histograms/histograms.xml?...

Note that I'm not sure about that actually. It looks like a couple of enums are
defined in the above internal histograms.xml, but used also in the public one.
Garrett, do you know what is the policy about using enums from the deprecated
internal histograms.xml?

[Garrett Casto, 2013/08/14 23:02:47]

It sounds like they are just migrating from the internal to the external one,
and you can use data from either, but this is not something I know too much
about. asvitkine@ knows better.

                             infobar_response_, NUM_RESPONSE_TYPES);
+  UMA_HISTOGRAM_BOOLEAN(
+      "PasswordManager.InfobarDislayedTooShortly" + domain_name_,
+      timer_.Elapsed().InMilliseconds() > kTime ? true : false);

[vabr (Chromium), 2013/08/14 14:39:58]

1) Omit the ternary operator.
   boolean ? true : false
   can be written simply just as 
   boolean
2) Did you mean to negate the result? In other words is "too shortly" equivalent
to "timer_.Elapsed().InMilliseconds() < kTime"?

 }
 
 int SavePasswordInfoBarDelegate::GetIconID() const {