Skip to content

Commit

Permalink
[Autofill] Log the number of profiles considered/removed during dedupe.
Browse files Browse the repository at this point in the history
BUG=618356
TEST=PersonalDataManagerTest

Review-Url: https://codereview.chromium.org/2061303002
Cr-Commit-Position: refs/heads/master@{#399991}
  • Loading branch information
mathp authored and Commit bot committed Jun 15, 2016
1 parent 652392b commit f31a7d4
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 0 deletions.
16 changes: 16 additions & 0 deletions components/autofill/core/browser/autofill_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,22 @@ void AutofillMetrics::LogParseFormTiming(const base::TimeDelta& duration) {
UMA_HISTOGRAM_TIMES("Autofill.Timing.ParseForm", duration);
}

// static
void AutofillMetrics::LogNumberOfProfilesConsideredForDedupe(
size_t num_considered) {
// A maximum of 50 is enforced to reduce the number of generated buckets.
UMA_HISTOGRAM_COUNTS_1000("Autofill.NumberOfProfilesConsideredForDedupe",
std::min(int(num_considered), 50));
}

// static
void AutofillMetrics::LogNumberOfProfilesRemovedDuringDedupe(
size_t num_removed) {
// A maximum of 50 is enforced to reduce the number of generated buckets.
UMA_HISTOGRAM_COUNTS_1000("Autofill.NumberOfProfilesRemovedDuringDedupe",
std::min(int(num_removed), 50));
}

AutofillMetrics::FormEventLogger::FormEventLogger(bool is_for_credit_card)
: is_for_credit_card_(is_for_credit_card),
is_server_data_available_(false),
Expand Down
6 changes: 6 additions & 0 deletions components/autofill/core/browser/autofill_metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,12 @@ class AutofillMetrics {
// This should be called when parsing each form.
static void LogParseFormTiming(const base::TimeDelta& duration);

// Log how many profiles were considered for the deduplication process.
static void LogNumberOfProfilesConsideredForDedupe(size_t num_considered);

// Log how many profiles were removed as part of the deduplication process.
static void LogNumberOfProfilesRemovedDuringDedupe(size_t num_removed);

// Utility to autofill form events in the relevant histograms depending on
// the presence of server and/or local data.
class FormEventLogger {
Expand Down
5 changes: 5 additions & 0 deletions components/autofill/core/browser/personal_data_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,8 @@ void PersonalDataManager::FindAndMergeDuplicateProfiles(
const std::vector<AutofillProfile*>& existing_profiles,
AutofillProfile* profile_to_merge,
std::vector<std::string>* profile_guids_to_delete) {
AutofillMetrics::LogNumberOfProfilesConsideredForDedupe(
existing_profiles.size());
for (AutofillProfile* existing_profile : existing_profiles) {
// Don't try to merge a profile with itself or with any profile with a
// different PrimaryValue.
Expand All @@ -1583,6 +1585,9 @@ void PersonalDataManager::FindAndMergeDuplicateProfiles(
}
}
}

AutofillMetrics::LogNumberOfProfilesRemovedDuringDedupe(
profile_guids_to_delete->size());
}

} // namespace autofill
22 changes: 22 additions & 0 deletions components/autofill/core/browser/personal_data_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/metrics/field_trial.h"
#include "base/strings/utf_string_conversions.h"
#include "base/synchronization/waitable_event.h"
#include "base/test/histogram_tester.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "build/build_config.h"
Expand Down Expand Up @@ -4451,6 +4452,7 @@ TEST_F(PersonalDataManagerTest, DedupeOnInsert) {
"homer.simpson@abc.com", "", "742. Evergreen Terrace",
"", "Springfield", "IL", "91601", "US", "12345678910");

base::HistogramTester histogram_tester;
// Save the imported profile (use it).
personal_data_->SaveImportedProfile(imported_profile);

Expand All @@ -4463,6 +4465,12 @@ TEST_F(PersonalDataManagerTest, DedupeOnInsert) {
// The imported profile and saved profiles 1 and 2 should be merged together.
// Therefore there should only be 3 saved profiles.
ASSERT_EQ(3U, profiles.size());
// 4 profiles were considered for dedupe.
histogram_tester.ExpectUniqueSample(
"Autofill.NumberOfProfilesConsideredForDedupe", 4, 1);
// 1 profile was removed.
histogram_tester.ExpectUniqueSample(
"Autofill.NumberOfProfilesRemovedDuringDedupe", 1, 1);

// Sort the profiles by frecency to have a deterministic order.
base::Time comparison_time = base::Time::Now();
Expand Down Expand Up @@ -4542,9 +4550,16 @@ TEST_F(PersonalDataManagerTest,
existing_profiles.push_back(&profile4);
existing_profiles.push_back(&profile5);

base::HistogramTester histogram_tester;
std::vector<std::string> guids_to_delete;
personal_data_->FindAndMergeDuplicateProfiles(existing_profiles, &profile1,
&guids_to_delete);
// 5 profiles were considered for dedupe.
histogram_tester.ExpectUniqueSample(
"Autofill.NumberOfProfilesConsideredForDedupe", 5, 1);
// 2 profiles were removed.
histogram_tester.ExpectUniqueSample(
"Autofill.NumberOfProfilesRemovedDuringDedupe", 2, 1);

// Profile1 should be deleted because it was sent as the profile to merge and
// thus was merged into profile3 and then into profile5.
Expand Down Expand Up @@ -4602,6 +4617,7 @@ TEST_F(PersonalDataManagerTest,
"homer.simpson@abc.com", "", "742. Evergreen Terrace",
"", "Springfield", "IL", "91601", "US", "");

base::HistogramTester histogram_tester;
personal_data_->SaveImportedProfile(imported_profile);

EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
Expand All @@ -4613,6 +4629,12 @@ TEST_F(PersonalDataManagerTest,
// The imported profile and saved profiles 1 and 2 should be merged together.
// Therefore there should only be 1 saved profile.
ASSERT_EQ(1U, profiles.size());
// 2 profiles were considered for dedupe.
histogram_tester.ExpectUniqueSample(
"Autofill.NumberOfProfilesConsideredForDedupe", 2, 1);
// 1 profile was removed.
histogram_tester.ExpectUniqueSample(
"Autofill.NumberOfProfilesRemovedDuringDedupe", 1, 1);

// Since profiles with higher frecency scores are merged into profiles with
// lower frecency scores, the result of the merge should be contained in
Expand Down
14 changes: 14 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2849,6 +2849,20 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram name="Autofill.NumberOfProfilesConsideredForDedupe" units="profiles">
<owner>mathp@chromium.org</owner>
<summary>
The number of Autofill profiles that have been considered for deduplication.
</summary>
</histogram>

<histogram name="Autofill.NumberOfProfilesRemovedDuringDedupe" units="profiles">
<owner>mathp@chromium.org</owner>
<summary>
The number of Autofill profiles that have been removed during deduplication.
</summary>
</histogram>

<histogram name="Autofill.PasswordFormQueryVolume"
enum="PasswordFormQueryVolume">
<owner>dvadym@chromium.org</owner>
Expand Down

0 comments on commit f31a7d4

Please sign in to comment.