Added of new UMA signals in order to be able to discover early if the "save...

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

According the http://crbug.com/262215: Detect, fix and monitor
usability problems of password management

New Boolean :   PasswordManager.InforbarDisplayed
		PasswordManager.InforbarDisplayedTooShortly
		PasswordManager.PreviousPasswordSavedRecently

BUG=262215,268980

Review URL: https://chromiumcodereview.appspot.com/23140005

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

chrome/browser/password_manager/password_manager.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager_delegate.h"
+#include "chrome/browser/password_manager/password_manager_metrics_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
@@ -55,12 +56,7 @@ void ReportMetrics(bool password_manager_enabled) {
     return;
   ran_once = true;
 
-  // TODO(isherman): This does not actually measure a user action.  It should be
-  // a boolean histogram.
-  if (password_manager_enabled)
-    content::RecordAction(UserMetricsAction("PasswordManager_Enabled"));
-  else
-    content::RecordAction(UserMetricsAction("PasswordManager_Disabled"));
+  UMA_HISTOGRAM_BOOLEAN("PasswordManager.Enabled", password_manager_enabled);
 }
 
 }  // namespace
@@ -139,13 +135,13 @@ bool PasswordManager::IsSavingEnabled() const {
 
 void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
   if (!IsSavingEnabled()) {
-    RecordFailure(SAVING_DISABLED);
+    RecordFailure(SAVING_DISABLED, form.origin.host());
     return;
   }
 
   // No password to save? Then don't.
   if (form.password_value.empty()) {
-    RecordFailure(EMPTY_PASSWORD);
+    RecordFailure(EMPTY_PASSWORD, form.origin.host());
     return;
   }
 
@@ -178,7 +174,7 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
     manager.reset(*matched_manager_it);
     pending_login_managers_.weak_erase(matched_manager_it);
   } else {
-    RecordFailure(NO_MATCHING_FORM);
+    RecordFailure(NO_MATCHING_FORM, form.origin.host());
     return;
   }
 
@@ -187,20 +183,20 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
   // results for the given form and autofill. If this is the case, we just
   // give up.
   if (!manager->HasCompletedMatching()) {
-    RecordFailure(MATCHING_NOT_COMPLETE);
+    RecordFailure(MATCHING_NOT_COMPLETE, form.origin.host());
     return;
   }
 
   // Also get out of here if the user told us to 'never remember' passwords for
   // this form.
   if (manager->IsBlacklisted()) {
-    RecordFailure(FORM_BLACKLISTED);
+    RecordFailure(FORM_BLACKLISTED, form.origin.host());
     return;
   }
 
   // Bail if we're missing any of the necessary form components.
   if (!manager->HasValidPasswordForm()) {
-    RecordFailure(INVALID_FORM);
+    RecordFailure(INVALID_FORM, form.origin.host());
     return;
   }
 
@@ -208,7 +204,7 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
   // Chrome to manage such passwords. For other passwords, respect the
   // autocomplete attribute.
   if (!manager->HasGeneratedPassword() && !form.password_autocomplete_set) {
-    RecordFailure(AUTOCOMPLETE_OFF);
+    RecordFailure(AUTOCOMPLETE_OFF, form.origin.host());
     return;
   }
 
@@ -224,9 +220,18 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
   provisional_save_manager_.swap(manager);
 }
 
-void PasswordManager::RecordFailure(ProvisionalSaveFailure failure) {
+void PasswordManager::RecordFailure(ProvisionalSaveFailure failure,
+                                    const std::string& form_origin) {
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.ProvisionalSaveFailure",
                             failure, MAX_FAILURE_VALUE);
+
+  std::string group_name = password_manager_metrics_util::GroupIdToString(
+      password_manager_metrics_util::MonitoredDomainGroupId(form_origin));
+  if (!group_name.empty()) {
+    password_manager_metrics_util::LogUMAHistogramEnumeration(
+        "PasswordManager.ProvisionalSaveFailure_" + group_name, failure,
+        MAX_FAILURE_VALUE);
+  }
 }
 
 void PasswordManager::AddSubmissionCallback(
@@ -301,8 +306,8 @@ void PasswordManager::OnPasswordFormsParsed(
 
 bool PasswordManager::ShouldShowSavePasswordInfoBar() const {
   return provisional_save_manager_->IsNewLogin() &&
-      !provisional_save_manager_->HasGeneratedPassword() &&
-      !provisional_save_manager_->IsPendingCredentialsPublicSuffixMatch();
+         !provisional_save_manager_->HasGeneratedPassword() &&
+         !provisional_save_manager_->IsPendingCredentialsPublicSuffixMatch();
 }
 
 void PasswordManager::OnPasswordFormsRendered(

chrome/browser/password_manager/password_manager.h

@@ -113,8 +113,10 @@ class PasswordManager : public LoginModel,
     MAX_FAILURE_VALUE
   };
 
-  // Log failure for UMA
-  void RecordFailure(ProvisionalSaveFailure failure);
+  // Log failure for UMA. Logs additional metrics if the |form_origin|
+  // corresponds to one of the top, explicitly monitored websites.
+  void RecordFailure(ProvisionalSaveFailure failure,
+                     const std::string& form_origin);
 
   // Possibly set up FieldTrial for testing other possible usernames. This only
   // happens if there are other_possible_usernames to be shown and the

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_;
 
+  // Measures the "Save password?" prompt lifetime. Used to report an UMA
+  // signal.
+  PerfTimer timer_;
+
+  // The group name corresponding to the domain name of |form_to_save_| if the
+  // form is on a monitored domain. Otherwise, an empty string.
+  const std::string uma_histogram_suffix_;
+
   DISALLOW_COPY_AND_ASSIGN(SavePasswordInfoBarDelegate);
 };
 
@@ -105,12 +115,36 @@ SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate(
     PasswordFormManager* form_to_save)
     : ConfirmInfoBarDelegate(infobar_service),
       form_to_save_(form_to_save),
-      infobar_response_(NO_RESPONSE) {
+      infobar_response_(NO_RESPONSE),
+      uma_histogram_suffix_(password_manager_metrics_util::GroupIdToString(
+          password_manager_metrics_util::MonitoredDomainGroupId(
+              form_to_save->realm()))) {
+  if (!uma_histogram_suffix_.empty()) {
+    password_manager_metrics_util::LogUMAHistogramBoolean(
+        "PasswordManager.SavePasswordPromptDisplayed_" + uma_histogram_suffix_,
+        true);
+  }
 }
 
 SavePasswordInfoBarDelegate::~SavePasswordInfoBarDelegate() {
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.InfoBarResponse",
                             infobar_response_, NUM_RESPONSE_TYPES);
+
+  // The shortest period for which the prompt needs to live, so that we don't
+  // consider it killed prematurely, as might happen, e.g., if a pre-rendered
+  // page gets swapped in (and the current WebContents is destroyed).
+  const base::TimeDelta kMinimumPromptDisplayTime =
+      base::TimeDelta::FromSeconds(1);
+
+  if (!uma_histogram_suffix_.empty()) {
+    password_manager_metrics_util::LogUMAHistogramEnumeration(
+        "PasswordManager.SavePasswordPromptResponse_" + uma_histogram_suffix_,
+        infobar_response_, NUM_RESPONSE_TYPES);
+    password_manager_metrics_util::LogUMAHistogramBoolean(
+        "PasswordManager.SavePasswordPromptDisappearedQuickly_" +
+            uma_histogram_suffix_,
+        timer_.Elapsed() < kMinimumPromptDisplayTime);
+  }
 }
 
 int SavePasswordInfoBarDelegate::GetIconID() const {

chrome/browser/password_manager/password_manager_metrics_util.cc

+// Copyright 2013 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_metrics_util.h"
+
+#include "base/basictypes.h"
+#include "base/metrics/histogram.h"
+#include "base/rand_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/time/time.h"
+#include "url/gurl.h"
+
+namespace password_manager_metrics_util {
+
+namespace {
+
+// The number of domain groups.
+const size_t kNumGroups = 2 * kGroupsPerDomain;
+
+// The group index used to determine the group id attributed to the user
+// in order to report the behaviour of the save password prompt. When
+// |g_random_group_index| is equal to |kGroupsPerDomain| the index is
+// generated randomly in range [0, |kGroupsPerDomain|). The user keeps his
+// group index till the end of each session.
+size_t g_random_group_index = kGroupsPerDomain;
+
+// Contains a monitored domain name and all the group IDs which contain
+// domain name.
+struct DomainGroupsPair {
+  const char* const domain_name;
+  const size_t group_ids[kGroupsPerDomain];
+};
+
+// Generate a group index in range [0, |kGroupsPerDomain|).
+// If the |g_random_group_index| is not equal to its initial value
+// (|kGroupsPerDomain|), the value of |g_random_group_index| is not changed.
+void GenerateRandomIndex() {
+  // For each session, the user uses only one group for each website.
+  if (g_random_group_index == kGroupsPerDomain)
+    g_random_group_index = base::RandGenerator(kGroupsPerDomain);
+}
+
+}  // namespace
+
+size_t MonitoredDomainGroupId(const std::string& url_host) {
+  // This array contains each monitored website together with all ids of
+  // groups which contain the website. Each website appears in
+  // |kGroupsPerDomain| groups, and each group includes an equal number of
+  // websites, so that no two websites have the same set of groups that they
+  // belong to. All ids are in the range [1, |kNumGroups|].
+  // For more information about the algorithm used see http://goo.gl/vUuFd5.
+  static const DomainGroupsPair kDomainMapping[] = {
+      {"google.com", {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}},
+      {"yahoo.com", {1, 2, 3, 4, 5, 11, 12, 13, 14, 15}},
+      {"baidu.com", {1, 2, 3, 4, 6, 7, 11, 12, 16, 17}},
+      {"wikipedia.org", {1, 2, 3, 4, 5, 6, 11, 12, 16, 18}},
+      {"linkedin.com", {1, 6, 8, 11, 13, 14, 15, 16, 17, 19}},
+      {"twitter.com", {5, 6, 7, 8, 9, 11, 13, 17, 19, 20}},
+      {"live.com", {7, 8, 9, 10, 13, 14, 16, 17, 18, 20}},
+      {"amazon.com", {2, 5, 9, 10, 12, 14, 15, 18, 19, 20}},
+      {"ebay.com", {3, 7, 9, 10, 14, 15, 17, 18, 19, 20}},
+      {"tumblr.com", {4, 8, 10, 12, 13, 15, 16, 18, 19, 20}},
+  };
+  const size_t kDomainMappingLength = arraysize(kDomainMapping);
+
+  GenerateRandomIndex();
+
+  GURL url(url_host);
+  for (size_t i = 0; i < kDomainMappingLength; ++i) {
+    if (url.DomainIs(kDomainMapping[i].domain_name))
+      return kDomainMapping[i].group_ids[g_random_group_index];
+  }
+  return 0;
+}
+
+void LogUMAHistogramEnumeration(const std::string& name,
+                                int sample,
+                                int boundary_value) {
+  DCHECK_LT(sample, boundary_value);
+
+  // Note: This leaks memory, which is expected behavior.
+  base::HistogramBase* histogram =
+      base::LinearHistogram::FactoryGet(
+          name,
+          1,
+          boundary_value,
+          boundary_value + 1,
+          base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram->Add(sample);
+}
+
+void LogUMAHistogramBoolean(const std::string& name, bool sample) {
+  // Note: This leaks memory, which is expected behavior.
+  base::HistogramBase* histogram =
+      base::BooleanHistogram::FactoryGet(
+          name,
+          base::Histogram::kNoFlags);
+          histogram->AddBoolean(sample);
+}
+
+void SetRandomIndexForTesting(size_t value) {
+  g_random_group_index = value;
+}
+
+std::string GroupIdToString(size_t group_id) {
+  DCHECK_LE(group_id, kNumGroups);
+  if (group_id > 0)
+    return "group_" + base::IntToString(group_id);
+  return std::string();
+}
+
+}  // namespace password_manager_metrics_util

chrome/browser/password_manager/password_manager_metrics_util.h

+// Copyright 2013 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.
+
+#ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_METRICS_UTIL_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_METRICS_UTIL_H_
+
+#include <string>
+
+namespace password_manager_metrics_util {
+
+// We monitor the performance of the save password heuristic for a handful of
+// domains. For privacy reasons we are not reporting UMA signals by domain, but
+// by a domain group. A domain group can contain multiple domains, and a domain
+// can be contained in multiple groups.
+// For more information see http://goo.gl/vUuFd5.
+
+// The number of groups in which each monitored website appears.
+// It is a half of the total number of groups.
+const size_t kGroupsPerDomain = 10u;
+
+// Check whether the |url_host| is monitored or not. If yes, we return
+// the id of the group which contains the domain name otherwise
+// returns 0.
+size_t MonitoredDomainGroupId(const std::string& url_host);
+
+// A version of the UMA_HISTOGRAM_ENUMERATION macro that allows the |name|
+// to vary over the program's runtime.
+void LogUMAHistogramEnumeration(const std::string& name,
+                                int sample,
+                                int boundary_value);
+
+// A version of the UMA_HISTOGRAM_BOOLEAN macro that allows the |name|
+// to vary over the program's runtime.
+void LogUMAHistogramBoolean(const std::string& name, bool sample);
+
+// Sets the random seed used to generate group ids for monitored domains.
+void SetRandomIndexForTesting(size_t value);
+
+// Returns a string which contains group_|group_id|. If the
+// |group_id| corresponds to an unmonitored domain returns an empty string.
+std::string GroupIdToString(size_t group_id);
+
+}  // namespace password_manager_metrics_util
+
+#endif  // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_METRICS_UTIL_H_

chrome/browser/password_manager/password_manager_metrics_util_unittest.cc

+// Copyright 2013 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_metrics_util.h"
+
+#include <iterator>
+#include <map>
+
+#include "base/basictypes.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+bool IsMonitored(const char* url_host) {
+  return password_manager_metrics_util::MonitoredDomainGroupId(url_host) > 0;
+}
+
+}  // namespace
+
+TEST(PasswordManagerMetricsUtilTest, MonitoredDomainGroupAssigmentTest) {
+  const char* const kMonitoredWebsites[] = {
+      "https://www.google.com",
+      "https://www.yahoo.com",
+      "https://www.baidu.com",
+      "https://www.wikipedia.org",
+      "https://www.linkedin.com",
+      "https://www.twitter.com",
+      "https://www.live.com",
+      "https://www.amazon.com",
+      "https://www.ebay.com",
+      "https://www.tumblr.com",
+  };
+  const size_t kMonitoredWebsitesLength = arraysize(kMonitoredWebsites);
+
+  // The |groups| map contains the group id and the number of times
+  // it get assigned.
+  std::map<size_t, size_t> groups;
+
+  // Provide all possible values of the group id parameter for each monitored
+  // website.
+  for (size_t i = 0; i < kMonitoredWebsitesLength; ++i) {
+    for (size_t j = 0; j < password_manager_metrics_util::kGroupsPerDomain;
+         ++j) {
+      password_manager_metrics_util::SetRandomIndexForTesting(j);
+      ++groups[password_manager_metrics_util::MonitoredDomainGroupId(
+            kMonitoredWebsites[i])];
+    }
+  }
+
+  // Check if all groups get assigned the same number of times.
+  size_t number_of_assigment = groups.begin()->second;
+  for (std::map<size_t, size_t>::iterator it = groups.begin();
+       it != groups.end(); ++it) {
+    EXPECT_EQ(it->second, number_of_assigment) << " group id = " << it->first;
+  }
+}
+
+TEST(PasswordManagerMetricsUtilTest, MonitoredDomainGroupTest) {
+  EXPECT_TRUE(IsMonitored("https://www.linkedin.com"));
+  EXPECT_TRUE(IsMonitored("https://www.amazon.com"));
+  EXPECT_FALSE(IsMonitored("https://www.facebook.com"));
+  EXPECT_TRUE(IsMonitored("http://wikipedia.org"));
+  EXPECT_FALSE(IsMonitored("http://thisisnotwikipedia.org"));
+}

chrome/chrome_browser.gypi

@@ -1322,6 +1322,8 @@
         'browser/password_manager/password_manager_delegate.h',
         'browser/password_manager/password_manager_delegate_impl.cc',
         'browser/password_manager/password_manager_delegate_impl.h',
+        'browser/password_manager/password_manager_metrics_util.cc',
+        'browser/password_manager/password_manager_metrics_util.h', 
         'browser/password_manager/password_store.cc',
         'browser/password_manager/password_store.h',
         'browser/password_manager/password_store_consumer.cc',

chrome/chrome_tests_unit.gypi

@@ -1029,6 +1029,7 @@
         'browser/password_manager/native_backend_kwallet_x_unittest.cc',
         'browser/password_manager/password_form_manager_unittest.cc',
         'browser/password_manager/password_generation_manager_unittest.cc',
+        'browser/password_manager/password_manager_metrics_util_unittest.cc',
         'browser/password_manager/password_manager_unittest.cc',
         'browser/password_manager/password_store_unittest.cc',
         'browser/password_manager/password_store_default_unittest.cc',

tools/metrics/histograms/histograms.xml

@@ -10477,6 +10477,13 @@ other types of suffix sets.
   </summary>
 </histogram>
 
+<histogram name="PasswordManager.Enabled" enum="BooleanEnabled">
+  <summary>
+    Indicates whether the password manager is enabled when a tab is opened. This
+    includes prerendered tabs.
+  </summary>
+</histogram>
+
 <histogram name="PasswordManager.OtherPossibleUsernamesUsage"
     enum="OtherPossibleUsernamesUsage">
   <summary>
@@ -10493,6 +10500,27 @@ other types of suffix sets.
   </summary>
 </histogram>
 
+<histogram name="PasswordManager.SavePasswordPromptDisappearedQuickly"
+    enum="Boolean">
+  <summary>
+    Indicates whether the save password prompt disappeared in less than one
+    second. This most likely indicates that the prompt was dismissed
+    automatically, e.g. due to a page navigation, before the user was able to
+    respond to the infobar.
+  </summary>
+</histogram>
+
+<histogram name="PasswordManager.SavePasswordPromptDisplayed" enum="Boolean">
+  <summary>Indicates whether the save password prompt was displayed.</summary>
+</histogram>
+
+<histogram name="PasswordManager.SavePasswordPromptResponse"
+    enum="SavePasswordPromptResponseType">
+  <summary>
+    Breakdown of which response the user selected from the save password prompt.
+  </summary>
+</histogram>
+
 <histogram name="PasswordManager.TimesGeneratedPasswordUsed">
   <summary>
     The number of times each generated password has been used to log in.
@@ -24279,6 +24307,12 @@ other types of suffix sets.
   </int>
 </enum>
 
+<enum name="SavePasswordPromptResponseType" type="int">
+  <int value="0" label="NO_RESPONSE"/>
+  <int value="1" label="REMEMBER_PASSWORD"/>
+  <int value="2" label="DONT_REMEMBER_PASSWORD"/>
+</enum>
+
 <enum name="SB2BloomFailure" type="int">
   <int value="0" label="READ_OPEN"/>
   <int value="1" label="READ_VERSION"/>
@@ -27538,6 +27572,35 @@ other types of suffix sets.
   <affected-histogram name="Renderer4.BeginToFinishDoc"/>
 </fieldtrial>
 
+<fieldtrial name="PasswordManagerMonitor">
+  <group name="group_1" label="group 1"/>
+  <group name="group_2" label="group 2"/>
+  <group name="group_3" label="group 3"/>
+  <group name="group_4" label="group 4"/>
+  <group name="group_5" label="group 5"/>
+  <group name="group_6" label="group 6"/>
+  <group name="group_7" label="group 7"/>
+  <group name="group_8" label="group 8"/>
+  <group name="group_9" label="group 9"/>
+  <group name="group_10" label="group 10"/>
+  <group name="group_11" label="group 11"/>
+  <group name="group_12" label="group 12"/>
+  <group name="group_13" label="group 13"/>
+  <group name="group_14" label="group 14"/>
+  <group name="group_15" label="group 15"/>
+  <group name="group_16" label="group 16"/>
+  <group name="group_17" label="group 17"/>
+  <group name="group_18" label="group 18"/>
+  <group name="group_19" label="group 19"/>
+  <group name="group_20" label="group 20"/>
+  <group name="" label=""/>
+  <affected-histogram name="PasswordManager.ProvisionalSaveFailure"/>
+  <affected-histogram
+      name="PasswordManager.SavePasswordPromptDisappearedQuickly"/>
+  <affected-histogram name="PasswordManager.SavePasswordPromptDisplayed"/>
+  <affected-histogram name="PasswordManager.SavePasswordPromptResponse"/>
+</fieldtrial>
+
 <fieldtrial name="PpapiPluginName">
   <group name="libpepflashplayer.so" label="Flash player on Linux or Cros"/>
   <group name="libwidevinecdmadapter.so" label="Widevine CDM on Linux or Cros"/>