From f25c0cac19485073965e907f151dc9bc0b85996a Mon Sep 17 00:00:00 2001 From: melandory Date: Wed, 7 Jan 2015 09:01:59 -0800 Subject: Collects data about interaction with "Allow to collect URL?" bubble. Add histogram which collects data on how user interacted with "Allow to collect URL?". BUG=435080 R=vabr@chromium.org,jar@chromium.org Review URL: https://codereview.chromium.org/816353003 Cr-Commit-Position: refs/heads/master@{#310307} --- .../ui/passwords/manage_passwords_bubble_model.cc | 6 +++++ .../manage_passwords_bubble_model_unittest.cc | 26 ++++++++++++---------- .../core/browser/password_manager_metrics_util.cc | 11 +++++++++ .../core/browser/password_manager_metrics_util.h | 11 +++++++++ tools/metrics/histograms/histograms.xml | 14 ++++++++++++ 5 files changed, 56 insertions(+), 12 deletions(-) diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc index 0a4cf08..a73851c 100644 --- a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc +++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc @@ -205,7 +205,13 @@ void ManagePasswordsBubbleModel::OnBubbleHidden() { if (password_manager::ui::IsAskSubmitURLState(state_)) { state_ = password_manager::ui::ASK_USER_REPORT_URL_BUBBLE_SHOWN_STATE; + metrics_util::LogAllowToCollectURLBubbleUIDismissalReason( + dismissal_reason_); + // Return since we do not want to include "Allow to collect URL?" bubble + // data in other PasswordManager metrics. + return; } + metrics_util::LogUIDismissalReason(dismissal_reason_); // Other use cases have been reported in the callbacks like OnSaveClicked(). if (dismissal_reason_ == metrics_util::NO_DIRECT_INTERACTION) diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc index ee8a2ca..debc516 100644 --- a/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc @@ -19,6 +19,8 @@ #include "testing/gtest/include/gtest/gtest.h" const char kUIDismissalReasonMetric[] = "PasswordManager.UIDismissalReason"; +const char kAllowToCollectURLBubbleUIDismissalReasonMetric[] = + "PasswordManager.AllowToCollectURLBubble.UIDismissalReason"; class ManagePasswordsBubbleModelTest : public testing::Test { public: @@ -298,8 +300,8 @@ TEST_F(ManagePasswordsBubbleModelTest, ClickCollectURL) { model_->state()); histogram_tester.ExpectUniqueSample( - kUIDismissalReasonMetric, - password_manager::metrics_util::CLICKED_COLLECT_URL, 1); + kAllowToCollectURLBubbleUIDismissalReasonMetric, + password_manager::metrics_util::COLLECT_URL, 1); } TEST_F(ManagePasswordsBubbleModelTest, ClickDoNotCollectURL) { @@ -313,8 +315,8 @@ TEST_F(ManagePasswordsBubbleModelTest, ClickDoNotCollectURL) { model_->state()); histogram_tester.ExpectUniqueSample( - kUIDismissalReasonMetric, - password_manager::metrics_util::CLICKED_DO_NOT_COLLECT_URL, 1); + kAllowToCollectURLBubbleUIDismissalReasonMetric, + password_manager::metrics_util::DO_NOT_COLLECT_URL, 1); } TEST_F(ManagePasswordsBubbleModelTest, @@ -327,8 +329,8 @@ TEST_F(ManagePasswordsBubbleModelTest, EXPECT_EQ(password_manager::ui::ASK_USER_REPORT_URL_BUBBLE_SHOWN_STATE, model_->state()); histogram_tester.ExpectUniqueSample( - kUIDismissalReasonMetric, - password_manager::metrics_util::NO_DIRECT_INTERACTION, 1); + kAllowToCollectURLBubbleUIDismissalReasonMetric, + password_manager::metrics_util::NO_INTERACTION, 1); } TEST_F(ManagePasswordsBubbleModelTest, ClickCollectURLBeforeNavigation) { @@ -344,8 +346,8 @@ TEST_F(ManagePasswordsBubbleModelTest, ClickCollectURLBeforeNavigation) { model_->state()); histogram_tester.ExpectUniqueSample( - kUIDismissalReasonMetric, - password_manager::metrics_util::CLICKED_COLLECT_URL, 1); + kAllowToCollectURLBubbleUIDismissalReasonMetric, + password_manager::metrics_util::COLLECT_URL, 1); } TEST_F(ManagePasswordsBubbleModelTest, ClickDoNotCollectURLBeforeNavigation) { @@ -361,8 +363,8 @@ TEST_F(ManagePasswordsBubbleModelTest, ClickDoNotCollectURLBeforeNavigation) { model_->state()); histogram_tester.ExpectUniqueSample( - kUIDismissalReasonMetric, - password_manager::metrics_util::CLICKED_DO_NOT_COLLECT_URL, 1); + kAllowToCollectURLBubbleUIDismissalReasonMetric, + password_manager::metrics_util::DO_NOT_COLLECT_URL, 1); } TEST_F(ManagePasswordsBubbleModelTest, @@ -377,8 +379,8 @@ TEST_F(ManagePasswordsBubbleModelTest, EXPECT_EQ(password_manager::ui::ASK_USER_REPORT_URL_BUBBLE_SHOWN_STATE, model_->state()); histogram_tester.ExpectUniqueSample( - kUIDismissalReasonMetric, - password_manager::metrics_util::NO_DIRECT_INTERACTION, 1); + kAllowToCollectURLBubbleUIDismissalReasonMetric, + password_manager::metrics_util::NO_INTERACTION, 1); } TEST_F(ManagePasswordsBubbleModelTest, DismissCredential) { diff --git a/components/password_manager/core/browser/password_manager_metrics_util.cc b/components/password_manager/core/browser/password_manager_metrics_util.cc index bc2b317..5794d51 100644 --- a/components/password_manager/core/browser/password_manager_metrics_util.cc +++ b/components/password_manager/core/browser/password_manager_metrics_util.cc @@ -85,6 +85,17 @@ size_t MonitoredDomainGroupId(const std::string& url_host, return 0; } +void LogAllowToCollectURLBubbleUIDismissalReason(UIDismissalReason reason) { + AllowToCollectURLBubbleUIDismissalReason dismissal_reason = NO_INTERACTION; + if (reason == CLICKED_COLLECT_URL) + dismissal_reason = COLLECT_URL; + else if (reason == CLICKED_DO_NOT_COLLECT_URL) + dismissal_reason = DO_NOT_COLLECT_URL; + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.AllowToCollectURLBubble.UIDismissalReason", + dismissal_reason, NUM_ALLOW_TO_COLLECT_BUBBLE_DISMISSAL_REASON); +} + void LogUMAHistogramEnumeration(const std::string& name, int sample, int boundary_value) { diff --git a/components/password_manager/core/browser/password_manager_metrics_util.h b/components/password_manager/core/browser/password_manager_metrics_util.h index 7eefde2..d4ea6c3 100644 --- a/components/password_manager/core/browser/password_manager_metrics_util.h +++ b/components/password_manager/core/browser/password_manager_metrics_util.h @@ -56,6 +56,14 @@ enum UIDismissalReason { NOT_DISPLAYED }; +// Metrics: "PasswordManager.AllowToCollectURLBubble.UIDismissalReason" +enum AllowToCollectURLBubbleUIDismissalReason { + NO_INTERACTION = 0, + COLLECT_URL, + DO_NOT_COLLECT_URL, + NUM_ALLOW_TO_COLLECT_BUBBLE_DISMISSAL_REASON, +}; + // 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 @@ -72,6 +80,9 @@ const size_t kGroupsPerDomain = 10u; size_t MonitoredDomainGroupId(const std::string& url_host, PrefService* pref_service); +// Log the |reason| a user dismissed the "Allow to collect URL?" bubble. +void LogAllowToCollectURLBubbleUIDismissalReason(UIDismissalReason reason); + // 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, diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index c083678..d26292e 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -23006,6 +23006,13 @@ Therefore, the affected-histogram name has to have at least one dot in it. + + melandory@chromium.org + vasilii@chromium.org + Why was "Allow to collect URL?" bubble closed? + + gcasto@chromium.org vabr@chromium.org @@ -53060,6 +53067,13 @@ To add a new entry, add it with any value and run test to compute valid value. / form submit succeeded"/> + + + + + + -- cgit v1.1