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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/password_manager/password_manager_delegate_impl.h"
 
 #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"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager.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"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/common/autofill_messages.h"
@@ skipping 27 matching lines @@
                      PasswordFormManager* form_to_save);
 
  private:
   enum ResponseType {
     NO_RESPONSE = 0,
     REMEMBER_PASSWORD,
     DONT_REMEMBER_PASSWORD,
     NUM_RESPONSE_TYPES,
   };
 
+  PerfTimer timer_;
+
+  std::string domain_name_;
+
   SavePasswordInfoBarDelegate(InfoBarService* infobar_service,
                               PasswordFormManager* form_to_save);
   virtual ~SavePasswordInfoBarDelegate();
 
   // ConfirmInfoBarDelegate
   virtual int GetIconID() const OVERRIDE;
   virtual Type GetInfoBarType() const OVERRIDE;
   virtual string16 GetMessageText() const OVERRIDE;
   virtual string16 GetButtonLabel(InfoBarButton button) const OVERRIDE;
   virtual bool Accept() OVERRIDE;
@@ skipping 33 matching lines @@
   infobar_service->AddInfoBar(scoped_ptr<InfoBarDelegate>(
       new SavePasswordInfoBarDelegate(infobar_service, form_to_save)));
 }
 
 SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate(
     InfoBarService* infobar_service,
     PasswordFormManager* form_to_save)
     : 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 {
   return IDR_INFOBAR_SAVE_PASSWORD;
 }
 
 InfoBarDelegate::Type SavePasswordInfoBarDelegate::GetInfoBarType() const {
   return PAGE_ACTION_TYPE;
 }
 
@@ skipping 59 matching lines @@
 bool PasswordManagerDelegateImpl::DidLastPageLoadEncounterSSLErrors() {
   content::NavigationEntry* entry =
       web_contents_->GetController().GetActiveEntry();
   if (!entry) {
     NOTREACHED();
     return false;
   }
 
   return net::IsCertStatusError(entry->GetSSL().cert_status);
 }