Commit 061a338c authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Add reporting UsernameDetectionMethod from new parser

The old FormData -> PasswordForm parser reported to the
PasswordManager.UsernameDetectionMethod histogram, which method was used
to detect the username in the form.

This CL teaches the new parser to do the same.

The supporting enum is copied from the old one and modernised. There is
not point in attempting to share that code given that the old parser is
going away soon.

Bug: 845426
Change-Id: I7737cb8dc598c11b42c0e7e0f929afed5e830d7c
Reviewed-on: https://chromium-review.googlesource.com/1149866Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577841}
parent 2a084e47
......@@ -51,6 +51,7 @@ source_set("unit_tests") {
deps = [
":form_parsing",
"//base",
"//base/test:test_support",
"//components/autofill/core/browser",
"//components/autofill/core/browser/proto",
"//components/autofill/core/common",
......
......@@ -12,6 +12,7 @@
#include <utility>
#include <vector>
#include "base/metrics/histogram_macros.h"
#include "base/no_destructor.h"
#include "base/stl_util.h"
#include "base/strings/string16.h"
......@@ -685,15 +686,27 @@ std::unique_ptr<PasswordForm> ParseFormData(
return nullptr;
std::unique_ptr<SignificantFields> significant_fields;
UsernameDetectionMethod username_detection_method =
UsernameDetectionMethod::kNoUsernameDetected;
// (1) First, try to parse with server predictions.
if (form_predictions)
if (form_predictions) {
significant_fields =
ParseUsingPredictions(processed_fields, *form_predictions);
if (significant_fields && significant_fields->username) {
username_detection_method =
UsernameDetectionMethod::kServerSidePrediction;
}
}
// (2) If that failed, try to parse with autocomplete attributes.
if (!significant_fields)
if (!significant_fields) {
significant_fields = ParseUsingAutocomplete(processed_fields);
if (significant_fields && significant_fields->username) {
username_detection_method =
UsernameDetectionMethod::kAutocompleteAttribute;
}
}
// (3) Now try to fill the gaps.
if (!significant_fields)
......@@ -705,6 +718,11 @@ std::unique_ptr<PasswordForm> ParseFormData(
Interactability username_max = Interactability::kUnlikely;
ParseUsingBaseHeuristics(processed_fields, mode, significant_fields.get(),
&username_max);
if (username_detection_method ==
UsernameDetectionMethod::kNoUsernameDetected &&
significant_fields && significant_fields->username) {
username_detection_method = UsernameDetectionMethod::kBaseHeuristic;
}
// Additionally, and based on the best interactability computed by base
// heuristics, try to improve the username based on the context of the
......@@ -719,9 +737,20 @@ std::unique_ptr<PasswordForm> ParseFormData(
!(mode == FormParsingMode::SAVING &&
username_field_by_context->value.empty())) {
significant_fields->username = username_field_by_context;
if (username_detection_method ==
UsernameDetectionMethod::kNoUsernameDetected ||
username_detection_method ==
UsernameDetectionMethod::kBaseHeuristic) {
username_detection_method =
UsernameDetectionMethod::kHtmlBasedClassifier;
}
}
}
UMA_HISTOGRAM_ENUMERATION("PasswordManager.UsernameDetectionMethod",
username_detection_method,
UsernameDetectionMethod::kCount);
return AssemblePasswordForm(form_data, significant_fields.get(),
std::move(all_possible_passwords),
form_predictions);
......
......@@ -19,8 +19,18 @@ namespace password_manager {
enum class FormParsingMode { FILLING, SAVING };
// TODO(crbug.com/845426): Add the UsernameDetectionMethod enum and log data
// into the "PasswordManager.UsernameDetectionMethod" histogram.
// This needs to be in sync with the histogram enumeration
// UsernameDetectionMethod, because the values are reported in the
// "PasswordManager.UsernameDetectionMethod" histogram. Don't remove or shift
// existing values in the enum, only append and mark as obsolete as needed.
enum class UsernameDetectionMethod {
kNoUsernameDetected = 0,
kBaseHeuristic = 1,
kHtmlBasedClassifier = 2,
kAutocompleteAttribute = 3,
kServerSidePrediction = 4,
kCount
};
// Parse DOM information |form_data| into Password Manager's form representation
// PasswordForm. |form_predictions| are an optional source of server-side
......
......@@ -12,6 +12,7 @@
#include "base/strings/string16.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "build/build_config.h"
#include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h"
......@@ -1279,6 +1280,115 @@ TEST(FormParserTest, CVC) {
});
}
TEST(FormParserTest, HistogramsForUsernameDetectionMethod) {
struct HistogramTestCase {
FormParsingTestCase parsing_data;
UsernameDetectionMethod expected_method;
} kHistogramTestCases[] = {
{
{
"No username",
{
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password",
.prediction = {.type = autofill::PASSWORD}},
},
},
UsernameDetectionMethod::kNoUsernameDetected,
},
{
{
"Reporting server analysis",
{
{.role = ElementRole::USERNAME,
.form_control_type = "text",
.prediction = {.type = autofill::USERNAME}},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password",
.prediction = {.type = autofill::PASSWORD}},
},
},
UsernameDetectionMethod::kServerSidePrediction,
},
{
{
"Reporting autocomplete analysis",
{
{.role = ElementRole::USERNAME,
.form_control_type = "text",
.autocomplete_attribute = "username"},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password",
.autocomplete_attribute = "current-password"},
},
},
UsernameDetectionMethod::kAutocompleteAttribute,
},
{
{
"Reporting HTML classifier",
{
{.role = ElementRole::USERNAME,
.form_control_type = "text",
.predicted_username = 0},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password"},
},
},
UsernameDetectionMethod::kHtmlBasedClassifier,
},
{
{
"Reporting basic heuristics",
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password"},
},
},
UsernameDetectionMethod::kBaseHeuristic,
},
{
{
"Mixing server analysis on password and HTML classifier on "
"username is reported as HTML classifier",
{
{.role = ElementRole::USERNAME,
.form_control_type = "text",
.predicted_username = 0},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password",
.prediction = {.type = autofill::PASSWORD}},
},
},
UsernameDetectionMethod::kHtmlBasedClassifier,
},
{
{
"Mixing autocomplete analysis on password and basic heuristics "
"on username is reported as basic heuristics",
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password",
.autocomplete_attribute = "current-password"},
},
},
UsernameDetectionMethod::kBaseHeuristic,
},
};
for (const HistogramTestCase& histogram_test_case : kHistogramTestCases) {
base::HistogramTester tester;
CheckTestData({histogram_test_case.parsing_data});
// Expect two samples, because parsing is done once for filling and once for
// saving mode.
SCOPED_TRACE(histogram_test_case.parsing_data.description_for_logging);
tester.ExpectUniqueSample("PasswordManager.UsernameDetectionMethod",
histogram_test_case.expected_method,
2);
}
}
} // namespace
} // namespace password_manager
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment