[Password Manager] Introduce UMA histogram for username detection method
Bug: 786404
Change-Id: Ib04d8064a16369b4daf162cb1deee4c98b44f398
Reviewed-on: https://chromium-review.googlesource.com/776668
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518192}diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc
index 20594f4..f7ba02e7 100644
--- a/components/autofill/content/renderer/password_form_conversion_utils.cc
+++ b/components/autofill/content/renderer/password_form_conversion_utils.cc
@@ -43,6 +43,7 @@
using blink::WebString;
namespace autofill {
+
namespace {
// PasswordForms can be constructed for both WebFormElements and for collections
@@ -439,6 +440,8 @@
const FormsPredictionsMap* form_predictions,
UsernameDetectorCache* username_detector_cache) {
WebInputElement username_element;
+ UsernameDetectionMethod username_detection_method =
+ UsernameDetectionMethod::NO_USERNAME_DETECTED;
password_form->username_marked_by_site = false;
std::vector<WebInputElement> passwords;
@@ -457,6 +460,18 @@
if (form_predictions) {
FindPredictedElements(form, password_form->form_data, *form_predictions,
&predicted_elements);
+ WebInputElement predicted_username_element;
+ bool map_has_username_prediction = MapContainsPrediction(
+ predicted_elements, PREDICTION_USERNAME, &predicted_username_element);
+
+ // Let server predictions override the selection of the username field. This
+ // allows instant adjusting without changing Chromium code.
+ if (map_has_username_prediction) {
+ username_element = predicted_username_element;
+ password_form->was_parsed_using_autofill_predictions = true;
+ username_detection_method =
+ UsernameDetectionMethod::SERVER_SIDE_PREDICTION;
+ }
}
bool found_password_with_user_input = false;
@@ -581,8 +596,14 @@
// we might have made before). Furthermore, drop all other possible
// usernames we have accrued so far: they come from fields not marked
// with the autocomplete attribute, making them unlikely alternatives.
- username_element = *input_element;
- password_form->username_marked_by_site = true;
+
+ // Don't override the server-side prediction if any.
+ if (username_element.IsNull()) {
+ username_element = *input_element;
+ password_form->username_marked_by_site = true;
+ username_detection_method =
+ UsernameDetectionMethod::AUTOCOMPLETE_ATTRIBUTE;
+ }
}
} else {
if (password_form->username_marked_by_site) {
@@ -611,6 +632,7 @@
passwords = passwords_without_heuristics;
last_text_input_before_password = last_text_input_without_heuristics;
}
+ DCHECK_EQ(passwords.size(), last_text_input_before_password.size());
// |passwords| must be non-empty. Just in case the heuristics above have a
// bug, return now.
@@ -626,6 +648,9 @@
GetUsernameFieldBasedOnHtmlAttributes(
all_possible_usernames, password_form->form_data, &username_element,
username_detector_cache);
+ if (!username_element.IsNull())
+ username_detection_method =
+ UsernameDetectionMethod::HTML_BASED_CLASSIFIER;
}
}
@@ -662,29 +687,32 @@
}
// Base heuristic for username detection.
- DCHECK_EQ(passwords.size(), last_text_input_before_password.size());
+ WebInputElement base_heuristic_prediction;
+ if (!password.IsNull())
+ base_heuristic_prediction = last_text_input_before_password[password];
+ if (base_heuristic_prediction.IsNull() && !new_password.IsNull())
+ base_heuristic_prediction = last_text_input_before_password[new_password];
+
if (username_element.IsNull()) {
- if (!password.IsNull())
- username_element = last_text_input_before_password[password];
- if (username_element.IsNull() && !new_password.IsNull())
- username_element = last_text_input_before_password[new_password];
+ username_element = base_heuristic_prediction;
+ if (!base_heuristic_prediction.IsNull())
+ username_detection_method = UsernameDetectionMethod::BASE_HEURISTIC;
+ } else if (base_heuristic_prediction == username_element &&
+ username_detection_method !=
+ UsernameDetectionMethod::AUTOCOMPLETE_ATTRIBUTE) {
+ // TODO(crbug.com/786404): when the bug is fixed, remove this block and
+ // calculate |base_heuristic_prediction| only if |username_element.IsNull()|
+ // This block was added to measure the impact of server-side predictions and
+ // HTML based classifier compared to "old classifiers" (the based heuristic
+ // and 'autocomplete' attribute).
+ username_detection_method = UsernameDetectionMethod::BASE_HEURISTIC;
}
+ UMA_HISTOGRAM_ENUMERATION(
+ "PasswordManager.UsernameDetectionMethod", username_detection_method,
+ UsernameDetectionMethod::USERNAME_DETECTION_METHOD_COUNT);
+
password_form->layout = SequenceToLayout(layout_sequence);
- WebInputElement predicted_username_element;
- bool map_has_username_prediction = MapContainsPrediction(
- predicted_elements, PREDICTION_USERNAME, &predicted_username_element);
-
- // Let server predictions override the selection of the username field. This
- // allows instant adjusting without changing Chromium code.
- auto username_element_iterator = predicted_elements.find(username_element);
- if (map_has_username_prediction &&
- (username_element_iterator == predicted_elements.end() ||
- username_element_iterator->second != PREDICTION_USERNAME)) {
- username_element = predicted_username_element;
- password_form->was_parsed_using_autofill_predictions = true;
- }
-
if (!username_element.IsNull()) {
password_form->username_element =
FieldName(username_element, "anonymous_username");
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.h b/components/autofill/content/renderer/password_form_conversion_utils.h
index a64b9da..d310b30 100644
--- a/components/autofill/content/renderer/password_form_conversion_utils.h
+++ b/components/autofill/content/renderer/password_form_conversion_utils.h
@@ -33,6 +33,15 @@
struct PasswordForm;
+enum UsernameDetectionMethod {
+ NO_USERNAME_DETECTED,
+ BASE_HEURISTIC,
+ HTML_BASED_CLASSIFIER,
+ AUTOCOMPLETE_ATTRIBUTE,
+ SERVER_SIDE_PREDICTION,
+ USERNAME_DETECTION_METHOD_COUNT
+};
+
// The caller of this function is responsible for deleting the returned object.
re2::RE2* CreateMatcher(void* instance, const char* pattern);
diff --git a/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc b/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
index edf9c60..dd139c5 100644
--- a/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
+++ b/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
@@ -10,6 +10,7 @@
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/test/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "components/autofill/content/renderer/form_autofill_util.h"
@@ -597,12 +598,16 @@
// No signals from HTML attributes. The classifier found nothing and cached
// it.
+ base::HistogramTester histogram_tester;
std::unique_ptr<PasswordForm> password_form = CreatePasswordFormFromWebForm(
form, nullptr, nullptr, &username_detector_cache);
EXPECT_TRUE(password_form);
ASSERT_EQ(1u, username_detector_cache.size());
EXPECT_EQ(form, username_detector_cache.begin()->first);
EXPECT_EQ(blink::WebInputElement(), username_detector_cache.begin()->second);
+ histogram_tester.ExpectUniqueSample("PasswordManager.UsernameDetectionMethod",
+ UsernameDetectionMethod::BASE_HEURISTIC,
+ 1);
// Changing attributes would change the classifier's output. But the output
// will be the same because it was cached in |username_detector_cache|.
@@ -615,6 +620,9 @@
ASSERT_EQ(1u, username_detector_cache.size());
EXPECT_EQ(form, username_detector_cache.begin()->first);
EXPECT_EQ(blink::WebInputElement(), username_detector_cache.begin()->second);
+ histogram_tester.ExpectUniqueSample("PasswordManager.UsernameDetectionMethod",
+ UsernameDetectionMethod::BASE_HEURISTIC,
+ 2);
// Clear the cache. The classifier will find username field and cache it.
username_detector_cache.clear();
@@ -627,6 +635,11 @@
ASSERT_FALSE(username_detector_cache.begin()->second.IsNull());
EXPECT_EQ("login",
username_detector_cache.begin()->second.NameForAutofill().Utf8());
+ EXPECT_THAT(
+ histogram_tester.GetAllSamples("PasswordManager.UsernameDetectionMethod"),
+ testing::UnorderedElementsAre(
+ base::Bucket(UsernameDetectionMethod::BASE_HEURISTIC, 2),
+ base::Bucket(UsernameDetectionMethod::HTML_BASED_CLASSIFIER, 1)));
// Change the attributes again ("username" is stronger signal than "login"),
// but keep the cache. The classifier's output should be the same.
@@ -639,6 +652,11 @@
ASSERT_FALSE(username_detector_cache.begin()->second.IsNull());
EXPECT_EQ("login",
username_detector_cache.begin()->second.NameForAutofill().Utf8());
+ EXPECT_THAT(
+ histogram_tester.GetAllSamples("PasswordManager.UsernameDetectionMethod"),
+ testing::UnorderedElementsAre(
+ base::Bucket(UsernameDetectionMethod::BASE_HEURISTIC, 2),
+ base::Bucket(UsernameDetectionMethod::HTML_BASED_CLASSIFIER, 2)));
}
TEST_F(MAYBE_PasswordFormConversionUtilsTest,
@@ -1275,6 +1293,26 @@
}
}
+TEST_F(MAYBE_PasswordFormConversionUtilsTest,
+ UsernameDetection_AutocompleteAttribute) {
+ PasswordFormBuilder builder(kTestFormActionURL);
+ builder.AddTextField("username", "JohnSmith", "username");
+ builder.AddTextField("Full name", "John A. Smith", nullptr);
+ builder.AddPasswordField("password", "secret", nullptr);
+ builder.AddSubmitButton("submit");
+ std::string html = builder.ProduceHTML();
+
+ base::HistogramTester histogram_tester;
+ std::unique_ptr<PasswordForm> password_form =
+ LoadHTMLAndConvertForm(html, nullptr, false);
+ ASSERT_TRUE(password_form);
+ EXPECT_EQ(base::UTF8ToUTF16("username"), password_form->username_element);
+ EXPECT_EQ(base::UTF8ToUTF16("JohnSmith"), password_form->username_value);
+ histogram_tester.ExpectUniqueSample(
+ "PasswordManager.UsernameDetectionMethod",
+ UsernameDetectionMethod::AUTOCOMPLETE_ATTRIBUTE, 1);
+}
+
TEST_F(MAYBE_PasswordFormConversionUtilsTest, IgnoreInvisibledTextFields) {
PasswordFormBuilder builder(kTestFormActionURL);
@@ -1745,6 +1783,31 @@
password_form->password_value);
}
+TEST_F(MAYBE_PasswordFormConversionUtilsTest, UsernamePredictionFromServer) {
+ PasswordFormBuilder builder(kTestFormActionURL);
+ builder.AddTextField("username", "JohnSmith", nullptr);
+ // 'autocomplete' attribute cannot override server's prediction.
+ builder.AddTextField("Full name", "John A. Smith", "username");
+ builder.AddPasswordField("password", "secret", nullptr);
+ builder.AddSubmitButton("submit");
+ std::string html = builder.ProduceHTML();
+
+ std::map<int, PasswordFormFieldPredictionType> predictions_positions;
+ predictions_positions[0] = PREDICTION_USERNAME;
+ FormsPredictionsMap predictions;
+ SetPredictions(html, &predictions, predictions_positions);
+
+ base::HistogramTester histogram_tester;
+ std::unique_ptr<PasswordForm> password_form =
+ LoadHTMLAndConvertForm(html, &predictions, false);
+ ASSERT_TRUE(password_form);
+ EXPECT_EQ(base::UTF8ToUTF16("username"), password_form->username_element);
+ EXPECT_EQ(base::UTF8ToUTF16("JohnSmith"), password_form->username_value);
+ histogram_tester.ExpectUniqueSample(
+ "PasswordManager.UsernameDetectionMethod",
+ UsernameDetectionMethod::SERVER_SIDE_PREDICTION, 1);
+}
+
TEST_F(MAYBE_PasswordFormConversionUtilsTest,
CreditCardVerificationNumberWithTypePasswordForm) {
PasswordFormBuilder builder(kTestFormActionURL);
@@ -2124,6 +2187,7 @@
builder.AddPasswordField("password", "secret", nullptr);
builder.AddSubmitButton("submit");
std::string html = builder.ProduceHTML();
+ base::HistogramTester histogram_tester;
std::unique_ptr<PasswordForm> password_form =
LoadHTMLAndConvertForm(html, nullptr, false);
@@ -2134,6 +2198,9 @@
EXPECT_TRUE(password_form->username_value.empty());
EXPECT_EQ(base::UTF8ToUTF16("password"), password_form->password_element);
EXPECT_EQ(base::UTF8ToUTF16("secret"), password_form->password_value);
+ histogram_tester.ExpectUniqueSample(
+ "PasswordManager.UsernameDetectionMethod",
+ UsernameDetectionMethod::NO_USERNAME_DETECTED, 1);
}
} // namespace autofill
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index e9b058dc..44ab738 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -41142,6 +41142,14 @@
<int value="0" label="WiFi Scan"/>
</enum>
+<enum name="UsernameDetectionMethod">
+ <int value="0" label="No username detected"/>
+ <int value="1" label="Base heuristic"/>
+ <int value="2" label="HTML based classifier"/>
+ <int value="3" label="'autocomplete' attribute"/>
+ <int value="4" label="Server-side prediction"/>
+</enum>
+
<enum name="UserPodsDisplay">
<int value="0" label="Enabled by local settings"/>
<int value="1" label="Enabled by domain policy"/>
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 9b4b377c..a967c9f9 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -58980,6 +58980,20 @@
</summary>
</histogram>
+<histogram name="PasswordManager.UsernameDetectionMethod"
+ enum="UsernameDetectionMethod">
+ <owner>kolos@chromium.org</owner>
+ <summary>
+ Measures what method was used for username field detection in the renderer
+ code. The metric may be recorded several times for page visit because it is
+ recorded at every PasswordForm creation. If a site changes HTML attributes
+ of fields or server-side predictions is received, different values can be
+ recorded for the same form. If an outcome of HTML classifier or a
+ server-side prediction coincides with the outcome of base heuristic, the
+ metric points to base heuristic method.
+ </summary>
+</histogram>
+
<histogram name="PasswordManager.UserStoredPasswordWithInvalidSSLCert"
enum="Boolean">
<obsolete>
