Commit 845ecd2b authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Report readonly password fields with UKM

When Chrome parses FormData of password forms into PasswordForm
structures, it omits, under certain conditions, password fields marked
as readonly. To understand the potential impact of removing this
special handling, this CL adds a UKM metric to report when this
occurs. This will provide both the frequency and some sample sites to
understand the usage of readonly password fields.

Privacy review of this metric:
The whole UKM event "PasswordForm" has been reviewed in crbug.com/728707
and go/gkmnc, where the privacy TL agreed that adding new metrics for
this event is OK under certain conditions (see the linked doc). The
new twin metrics for the readonly attribute satisfy such conditions, in
the opinion of the CL author, and hence are covered by that review. The
new metrics have been added to the requested spreadsheet listing all
passwords-related metrics.

Choice of metrics:
The core of the metrics is the ReadonlyPasswordFields enum, which
describes different situations which can happen wrt. readonly fields
during parsing. Those are in the UKM combined with the bit indicating
whether the parsing overall was successful, because this is also
relevant to the issue.
Further, the metrics can be collected at two stages: when a form is
parsed to be filled, and when it is parsed to be saved. Here there
was a choice to either record this as an additional bit in one metric,
or create two "twin" metrics. The advantage of the former is the
ability to study how these two events correlate. The advantage of the
latter is easier reading of the particular metrics and a lower range
of values per metric. Because the correlation is unlikely to be of
significant use, the latter approach was chosen.

Bug: 883633
Change-Id: Ib63e52908ae3a257546646c1b255a6cd8fd48b6b
Reviewed-on: https://chromium-review.googlesource.com/c/1261984Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597561}
parent 29bdbadd
...@@ -29,6 +29,9 @@ specific_include_rules = { ...@@ -29,6 +29,9 @@ specific_include_rules = {
"hsts_query_unittest\.cc": [ "hsts_query_unittest\.cc": [
"+services/network", "+services/network",
], ],
"new_password_form_manager_unittest\.cc": [
"+components/ukm",
],
"password_autofill_manager_unittest\.cc": [ "password_autofill_manager_unittest\.cc": [
"+components/ukm", "+components/ukm",
], ],
......
...@@ -269,23 +269,31 @@ std::unique_ptr<SignificantFields> ParseUsingAutocomplete( ...@@ -269,23 +269,31 @@ std::unique_ptr<SignificantFields> ParseUsingAutocomplete(
// useless). This ignores all passwords with Interactability below // useless). This ignores all passwords with Interactability below
// |best_interactability| and also fields with names which sound like CVC // |best_interactability| and also fields with names which sound like CVC
// fields. Stores the iterator to the first relevant password in // fields. Stores the iterator to the first relevant password in
// |first_relevant_password|. // |first_relevant_password|. |readonly_status| will be updated according to the
// processing of the parsed fields; it must not be null.
std::vector<const FormFieldData*> GetRelevantPasswords( std::vector<const FormFieldData*> GetRelevantPasswords(
const std::vector<ProcessedField>& processed_fields, const std::vector<ProcessedField>& processed_fields,
FormDataParser::Mode mode, FormDataParser::Mode mode,
Interactability best_interactability, Interactability best_interactability,
std::vector<ProcessedField>::const_iterator* first_relevant_password) { std::vector<ProcessedField>::const_iterator* first_relevant_password,
FormDataParser::ReadonlyPasswordFields* readonly_status) {
DCHECK(first_relevant_password); DCHECK(first_relevant_password);
*first_relevant_password = processed_fields.end(); *first_relevant_password = processed_fields.end();
std::vector<const FormFieldData*> result; std::vector<const FormFieldData*> result;
result.reserve(processed_fields.size()); result.reserve(processed_fields.size());
DCHECK(readonly_status);
const bool consider_only_non_empty = mode == FormDataParser::Mode::kSaving; const bool consider_only_non_empty = mode == FormDataParser::Mode::kSaving;
// These two counters are used to determine the ReadonlyPassowrdFields value
// corresponding to this form.
size_t all_passwords_seen = 0;
size_t ignored_readonly = 0;
for (auto it = processed_fields.begin(); it != processed_fields.end(); ++it) { for (auto it = processed_fields.begin(); it != processed_fields.end(); ++it) {
const ProcessedField& processed_field = *it; const ProcessedField& processed_field = *it;
if (!processed_field.is_password) if (!processed_field.is_password)
continue; continue;
++all_passwords_seen;
if (!MatchesInteractability(processed_field, best_interactability)) if (!MatchesInteractability(processed_field, best_interactability))
continue; continue;
if (consider_only_non_empty && processed_field.field->value.empty()) if (consider_only_non_empty && processed_field.field->value.empty())
...@@ -299,6 +307,7 @@ std::vector<const FormFieldData*> GetRelevantPasswords( ...@@ -299,6 +307,7 @@ std::vector<const FormFieldData*> GetRelevantPasswords(
!(processed_field.field->properties_mask & !(processed_field.field->properties_mask &
(FieldPropertiesFlags::USER_TYPED | (FieldPropertiesFlags::USER_TYPED |
FieldPropertiesFlags::AUTOFILLED))) { FieldPropertiesFlags::AUTOFILLED))) {
++ignored_readonly;
continue; continue;
} }
if (IsFieldCVC(*processed_field.field)) if (IsFieldCVC(*processed_field.field))
...@@ -308,6 +317,15 @@ std::vector<const FormFieldData*> GetRelevantPasswords( ...@@ -308,6 +317,15 @@ std::vector<const FormFieldData*> GetRelevantPasswords(
result.push_back(processed_field.field); result.push_back(processed_field.field);
} }
DCHECK_NE(0u, all_passwords_seen);
DCHECK_LE(ignored_readonly, all_passwords_seen);
if (ignored_readonly == 0)
*readonly_status = FormDataParser::ReadonlyPasswordFields::kNoneIgnored;
else if (ignored_readonly < all_passwords_seen)
*readonly_status = FormDataParser::ReadonlyPasswordFields::kSomeIgnored;
else
*readonly_status = FormDataParser::ReadonlyPasswordFields::kAllIgnored;
return result; return result;
} }
...@@ -432,12 +450,14 @@ uint32_t ExtractUniqueId(const FormFieldData* field) { ...@@ -432,12 +450,14 @@ uint32_t ExtractUniqueId(const FormFieldData* field) {
// complete it. The result is stored back in |found_fields|. The best // complete it. The result is stored back in |found_fields|. The best
// interactability for usernames, which depends on position of the found // interactability for usernames, which depends on position of the found
// passwords as well, is returned through |username_max| to be used in other // passwords as well, is returned through |username_max| to be used in other
// kinds of analysis. // kinds of analysis. If password fields end up being parsed, |readonly_status|
// will be updated according to that processing.
void ParseUsingBaseHeuristics( void ParseUsingBaseHeuristics(
const std::vector<ProcessedField>& processed_fields, const std::vector<ProcessedField>& processed_fields,
FormDataParser::Mode mode, FormDataParser::Mode mode,
SignificantFields* found_fields, SignificantFields* found_fields,
Interactability* username_max) { Interactability* username_max,
FormDataParser::ReadonlyPasswordFields* readonly_status) {
// If there is both the username and the minimal set of fields to build a // If there is both the username and the minimal set of fields to build a
// PasswordForm, return early -- no more work to do. // PasswordForm, return early -- no more work to do.
if (found_fields->HasPasswords() && found_fields->username) if (found_fields->HasPasswords() && found_fields->username)
...@@ -458,8 +478,9 @@ void ParseUsingBaseHeuristics( ...@@ -458,8 +478,9 @@ void ParseUsingBaseHeuristics(
// Try to find password elements (current, new, confirmation) among those // Try to find password elements (current, new, confirmation) among those
// with best interactability. // with best interactability.
first_relevant_password = processed_fields.end(); first_relevant_password = processed_fields.end();
std::vector<const FormFieldData*> passwords = GetRelevantPasswords( std::vector<const FormFieldData*> passwords =
processed_fields, mode, password_max, &first_relevant_password); GetRelevantPasswords(processed_fields, mode, password_max,
&first_relevant_password, readonly_status);
if (passwords.empty()) if (passwords.empty())
return; return;
LocateSpecificPasswords(passwords, &found_fields->password, LocateSpecificPasswords(passwords, &found_fields->password,
...@@ -692,6 +713,7 @@ FormDataParser::~FormDataParser() = default; ...@@ -692,6 +713,7 @@ FormDataParser::~FormDataParser() = default;
std::unique_ptr<PasswordForm> FormDataParser::Parse( std::unique_ptr<PasswordForm> FormDataParser::Parse(
const autofill::FormData& form_data, const autofill::FormData& form_data,
Mode mode) { Mode mode) {
readonly_status_ = ReadonlyPasswordFields::kNoHeuristics;
autofill::ValueElementVector all_possible_passwords; autofill::ValueElementVector all_possible_passwords;
autofill::ValueElementVector all_possible_usernames; autofill::ValueElementVector all_possible_usernames;
std::vector<ProcessedField> processed_fields = ProcessFields( std::vector<ProcessedField> processed_fields = ProcessFields(
...@@ -731,7 +753,7 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse( ...@@ -731,7 +753,7 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(
// Try to parse with base heuristic. // Try to parse with base heuristic.
Interactability username_max = Interactability::kUnlikely; Interactability username_max = Interactability::kUnlikely;
ParseUsingBaseHeuristics(processed_fields, mode, significant_fields.get(), ParseUsingBaseHeuristics(processed_fields, mode, significant_fields.get(),
&username_max); &username_max, &readonly_status_);
if (username_detection_method == if (username_detection_method ==
UsernameDetectionMethod::kNoUsernameDetected && UsernameDetectionMethod::kNoUsernameDetected &&
significant_fields && significant_fields->username) { significant_fields && significant_fields->username) {
......
...@@ -42,6 +42,29 @@ class FormDataParser { ...@@ -42,6 +42,29 @@ class FormDataParser {
kCount kCount
}; };
// Records whether password fields with a "readonly" attribute were ignored
// during form parsing.
enum class ReadonlyPasswordFields {
// Local heuristics, which only consider the readonly attribute, were not
// used for the parsing. This means that either the form was unsuitable for
// parsing (e.g., no fields at all), or some of the more trusted methods
// (server hints, autocomplete attributes) succeeded.
kNoHeuristics = 0,
//
// The rest of the values refer to the case when local heuristics were used.
// In that case there are always some password fields.
//
// No password fields with "readonly" ignored but some password fields
// present.
kNoneIgnored = 2,
// At least one password with "readonly" was ignored and at least one other
// password field was not ignored (whether readonly or not).
kSomeIgnored = 3,
// At least one password with "readonly" was ignored and every password
// field was ignored because of being readonly.
kAllIgnored = 4,
};
FormDataParser(); FormDataParser();
~FormDataParser(); ~FormDataParser();
...@@ -52,6 +75,8 @@ class FormDataParser { ...@@ -52,6 +75,8 @@ class FormDataParser {
const base::Optional<FormPredictions>& predictions() { return predictions_; } const base::Optional<FormPredictions>& predictions() { return predictions_; }
ReadonlyPasswordFields readonly_status() { return readonly_status_; }
// Parse DOM information |form_data| into Password Manager's form // Parse DOM information |form_data| into Password Manager's form
// representation PasswordForm. Return nullptr when parsing is unsuccessful. // representation PasswordForm. Return nullptr when parsing is unsuccessful.
std::unique_ptr<autofill::PasswordForm> Parse( std::unique_ptr<autofill::PasswordForm> Parse(
...@@ -63,6 +88,11 @@ class FormDataParser { ...@@ -63,6 +88,11 @@ class FormDataParser {
// types. // types.
base::Optional<FormPredictions> predictions_; base::Optional<FormPredictions> predictions_;
// Records whether readonly password fields were seen during the last call to
// Parse().
ReadonlyPasswordFields readonly_status_ =
ReadonlyPasswordFields::kNoHeuristics;
DISALLOW_COPY_AND_ASSIGN(FormDataParser); DISALLOW_COPY_AND_ASSIGN(FormDataParser);
}; };
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <set> #include <set>
#include <utility> #include <utility>
#include "base/optional.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
...@@ -87,6 +88,7 @@ struct FormParsingTestCase { ...@@ -87,6 +88,7 @@ struct FormParsingTestCase {
const autofill::ValueElementVector* all_possible_passwords = nullptr; const autofill::ValueElementVector* all_possible_passwords = nullptr;
const autofill::ValueElementVector* all_possible_usernames = nullptr; const autofill::ValueElementVector* all_possible_usernames = nullptr;
bool username_may_use_prefilled_placeholder = false; bool username_may_use_prefilled_placeholder = false;
base::Optional<FormDataParser::ReadonlyPasswordFields> readonly_status;
}; };
// Returns numbers which are distinct from each other within the scope of one // Returns numbers which are distinct from each other within the scope of one
...@@ -353,6 +355,9 @@ void CheckTestData(const std::vector<FormParsingTestCase>& test_cases) { ...@@ -353,6 +355,9 @@ void CheckTestData(const std::vector<FormParsingTestCase>& test_cases) {
parsed_form->other_possible_usernames); parsed_form->other_possible_usernames);
} }
} }
if (test_case.readonly_status) {
EXPECT_EQ(*test_case.readonly_status, parser.readonly_status());
}
} }
} }
} }
...@@ -1335,6 +1340,83 @@ TEST(FormParserTest, CVC) { ...@@ -1335,6 +1340,83 @@ TEST(FormParserTest, CVC) {
}); });
} }
// Check that "readonly status" is reported accordingly.
TEST(FormParserTest, ReadonlyStatus) {
CheckTestData({
{
"Server hints prevent heuristics from using readonly.",
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
.prediction = {.type = autofill::PASSWORD},
.is_readonly = true,
.form_control_type = "password"},
},
.readonly_status =
FormDataParser::ReadonlyPasswordFields::kNoHeuristics,
},
{
"Autocomplete attributes prevent heuristics from using readonly.",
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
.autocomplete_attribute = "current-password",
.is_readonly = true,
.form_control_type = "password"},
},
.readonly_status =
FormDataParser::ReadonlyPasswordFields::kNoHeuristics,
},
{
"No password fields are a special case of not going through local "
"heuristics.",
{
{.form_control_type = "text"},
},
.readonly_status =
FormDataParser::ReadonlyPasswordFields::kNoHeuristics,
},
{
"No readonly passwords ignored.",
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
// While readonly, this field is not ignored because it was
// autofilled before.
.is_readonly = true,
.properties_mask = FieldPropertiesFlags::AUTOFILLED_ON_PAGELOAD,
.form_control_type = "password"},
{.role = ElementRole::NEW_PASSWORD,
.is_readonly = false,
.form_control_type = "password"},
},
.readonly_status =
FormDataParser::ReadonlyPasswordFields::kNoneIgnored,
},
{
"Some readonly passwords ignored.",
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.is_readonly = true, .form_control_type = "password"},
{.role = ElementRole::CURRENT_PASSWORD,
.is_readonly = false,
.form_control_type = "password"},
},
.readonly_status =
FormDataParser::ReadonlyPasswordFields::kSomeIgnored,
},
{
"All readonly passwords ignored.",
{
{.form_control_type = "text"},
{.is_readonly = true, .form_control_type = "password"},
},
.readonly_status =
FormDataParser::ReadonlyPasswordFields::kAllIgnored,
},
});
}
TEST(FormParserTest, HistogramsForUsernameDetectionMethod) { TEST(FormParserTest, HistogramsForUsernameDetectionMethod) {
struct HistogramTestCase { struct HistogramTestCase {
FormParsingTestCase parsing_data; FormParsingTestCase parsing_data;
......
...@@ -405,6 +405,8 @@ bool NewPasswordFormManager::SetSubmittedFormIfIsManaged( ...@@ -405,6 +405,8 @@ bool NewPasswordFormManager::SetSubmittedFormIfIsManaged(
is_submitted_ = true; is_submitted_ = true;
parsed_submitted_form_ = parsed_submitted_form_ =
ParseFormAndMakeLogging(submitted_form_, FormDataParser::Mode::kSaving); ParseFormAndMakeLogging(submitted_form_, FormDataParser::Mode::kSaving);
RecordMetricOnReadonly(parser_.readonly_status(), !!parsed_submitted_form_,
FormDataParser::Mode::kSaving);
CreatePendingCredentials(); CreatePendingCredentials();
return true; return true;
} }
...@@ -433,6 +435,8 @@ void NewPasswordFormManager::Fill() { ...@@ -433,6 +435,8 @@ void NewPasswordFormManager::Fill() {
// parse result, but to parse each time again. // parse result, but to parse each time again.
std::unique_ptr<PasswordForm> observed_password_form = std::unique_ptr<PasswordForm> observed_password_form =
ParseFormAndMakeLogging(observed_form_, FormDataParser::Mode::kFilling); ParseFormAndMakeLogging(observed_form_, FormDataParser::Mode::kFilling);
RecordMetricOnReadonly(parser_.readonly_status(), !!observed_password_form,
FormDataParser::Mode::kFilling);
if (!observed_password_form) if (!observed_password_form)
return; return;
...@@ -490,6 +494,25 @@ void NewPasswordFormManager::RecordMetricOnCompareParsingResult( ...@@ -490,6 +494,25 @@ void NewPasswordFormManager::RecordMetricOnCompareParsingResult(
} }
} }
void NewPasswordFormManager::RecordMetricOnReadonly(
FormDataParser::ReadonlyPasswordFields readonly_status,
bool parsing_successful,
FormDataParser::Mode mode) {
// The reported value is combined of the |readonly_status| shifted by one bit
// to the left, and the success bit put in the least significant bit. Note:
// C++ guarantees that bool->int conversions map false to 0 and true to 1.
uint64_t value = static_cast<uint64_t>(parsing_successful) +
(static_cast<uint64_t>(readonly_status) << 1);
switch (mode) {
case FormDataParser::Mode::kSaving:
metrics_recorder_->RecordReadonlyWhenSaving(value);
break;
case FormDataParser::Mode::kFilling:
metrics_recorder_->RecordReadonlyWhenFilling(value);
break;
}
}
void NewPasswordFormManager::ReportTimeBetweenStoreAndServerUMA() { void NewPasswordFormManager::ReportTimeBetweenStoreAndServerUMA() {
if (!received_stored_credentials_time_.is_null()) { if (!received_stored_credentials_time_.is_null()) {
UMA_HISTOGRAM_TIMES("PasswordManager.TimeBetweenStoreAndServer", UMA_HISTOGRAM_TIMES("PasswordManager.TimeBetweenStoreAndServer",
......
...@@ -157,6 +157,14 @@ class NewPasswordFormManager : public PasswordFormManagerInterface, ...@@ -157,6 +157,14 @@ class NewPasswordFormManager : public PasswordFormManagerInterface,
void RecordMetricOnCompareParsingResult( void RecordMetricOnCompareParsingResult(
const autofill::PasswordForm& parsed_form); const autofill::PasswordForm& parsed_form);
// Records the status of readonly fields during parsing, combined with the
// overall success of the parsing. It reports through two different metrics,
// depending on whether |mode| indicates parsing for saving or filling.
void RecordMetricOnReadonly(
FormDataParser::ReadonlyPasswordFields readonly_status,
bool parsing_successful,
FormDataParser::Mode mode);
// Report the time between receiving credentials from the password store and // Report the time between receiving credentials from the password store and
// the autofill server responding to the lookup request. // the autofill server responding to the lookup request.
void ReportTimeBetweenStoreAndServerUMA(); void ReportTimeBetweenStoreAndServerUMA();
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "components/password_manager/core/browser/stub_form_saver.h" #include "components/password_manager/core/browser/stub_form_saver.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h" #include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h" #include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/ukm/test_ukm_recorder.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -97,8 +98,6 @@ class MockFormSaver : public StubFormSaver { ...@@ -97,8 +98,6 @@ class MockFormSaver : public StubFormSaver {
DISALLOW_COPY_AND_ASSIGN(MockFormSaver); DISALLOW_COPY_AND_ASSIGN(MockFormSaver);
}; };
} // namespace
// TODO(https://crbug.com/831123): Test sending metrics. // TODO(https://crbug.com/831123): Test sending metrics.
// TODO(https://crbug.com/831123): Test create pending credentials when // TODO(https://crbug.com/831123): Test create pending credentials when
// generation happened. // generation happened.
...@@ -840,4 +839,121 @@ TEST_F(NewPasswordFormManagerTest, Clone) { ...@@ -840,4 +839,121 @@ TEST_F(NewPasswordFormManagerTest, Clone) {
cloned_manager->metrics_recorder()); cloned_manager->metrics_recorder());
} }
// Extracts the information whether parsing was successful from a metric
// specified by |metric_name| stored in |entry|. The metric name should be one
// of ukm::builders::PasswordForm::kReadonlyWhenSavingName and
// ukm::builders::PasswordForm::kReadonlyWhenFillingName.
bool ParsingSuccessReported(const ukm::mojom::UkmEntry* entry,
base::StringPiece metric_name) {
const int64_t* value =
ukm::TestUkmRecorder::GetEntryMetric(entry, metric_name);
EXPECT_TRUE(value);
// Ideally, an ASSERT_TRUE above would prevent the test suite from crashing on
// dereferencing |value| below. But ASSERT_* is not available in non-void
// returning functions, so the null value is handled explicitly.
if (!value)
return false; // Value does not matter, the test already failed.
return 1 == (1 & *value);
}
// Test that an attempt to log to ReadonlyWhenFilling UKM is made when filling.
TEST_F(NewPasswordFormManagerTest, RecordReadonlyWhenFilling) {
TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
EXPECT_CALL(driver_, AllowPasswordGenerationForForm(_));
EXPECT_CALL(driver_, FillPasswordForm(_));
fetcher_->SetNonFederated({&saved_match_}, 0u);
task_runner_->FastForwardUntilNoTasksRemain();
// Destroy the form manager to destroy the UKM recorder it owns. The recorder
// only records metrics in its destructor.
form_manager_.reset();
auto entries = test_ukm_recorder.GetEntriesByName(
ukm::builders::PasswordForm::kEntryName);
ASSERT_EQ(1u, entries.size());
EXPECT_TRUE(ParsingSuccessReported(
entries[0], ukm::builders::PasswordForm::kReadonlyWhenFillingName));
}
// Test that an attempt to log to ReadonlyWhenFilling UKM is made when filling,
// even when the parsing itself is unsuccessful.
TEST_F(NewPasswordFormManagerTest, RecordReadonlyWhenFilling_ParsingFailed) {
TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
FormData malformed_form = observed_form_;
malformed_form.fields.clear();
CreateFormManager(malformed_form);
// Only create the recorder after the current form manager is created,
// otherwise the destruction of the previous one will add unwanted UKM entries
// in it.
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
fetcher_->SetNonFederated({&saved_match_}, 0u);
task_runner_->FastForwardUntilNoTasksRemain();
// Destroy the form manager to destroy the UKM recorder it owns. The recorder
// only records metrics in its destructor.
form_manager_.reset();
auto entries = test_ukm_recorder.GetEntriesByName(
ukm::builders::PasswordForm::kEntryName);
ASSERT_EQ(1u, entries.size());
EXPECT_FALSE(ParsingSuccessReported(
entries[0], ukm::builders::PasswordForm::kReadonlyWhenFillingName));
}
// Test that an attempt to log to ReadonlyWhenSaving UKM is made when creating
// pending credentials.
TEST_F(NewPasswordFormManagerTest, RecordReadonlyWhenSaving) {
// The scoped context is needed for the UKM recorder.
TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
fetcher_->SetNonFederated({&saved_match_}, 0u);
EXPECT_TRUE(
form_manager_->SetSubmittedFormIfIsManaged(submitted_form_, &driver_));
// Destroy the form manager to destroy the UKM recorder it owns. The recorder
// only records metrics in its destructor.
form_manager_.reset();
auto entries = test_ukm_recorder.GetEntriesByName(
ukm::builders::PasswordForm::kEntryName);
ASSERT_EQ(1u, entries.size());
EXPECT_TRUE(ParsingSuccessReported(
entries[0], ukm::builders::PasswordForm::kReadonlyWhenSavingName));
}
// Test that an attempt to log to ReadonlyWhenSaving UKM is made when creating
// pending credentials, even when their parsing itself is unsuccessful.
TEST_F(NewPasswordFormManagerTest, RecordReadonlyWhenSaving_ParsingFailed) {
// The scoped context is needed for the UKM recorder.
TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
fetcher_->SetNonFederated({&saved_match_}, 0u);
FormData malformed_form = observed_form_;
malformed_form.fields.clear();
EXPECT_TRUE(
form_manager_->SetSubmittedFormIfIsManaged(malformed_form, &driver_));
// Destroy the form manager to destroy the UKM recorder it owns. The recorder
// only records metrics in its destructor.
form_manager_.reset();
auto entries = test_ukm_recorder.GetEntriesByName(
ukm::builders::PasswordForm::kEntryName);
ASSERT_EQ(1u, entries.size());
EXPECT_FALSE(ParsingSuccessReported(
entries[0], ukm::builders::PasswordForm::kReadonlyWhenSavingName));
}
} // namespace
} // namespace password_manager } // namespace password_manager
...@@ -288,6 +288,14 @@ void PasswordFormMetricsRecorder::RecordParsingOnSavingDifference( ...@@ -288,6 +288,14 @@ void PasswordFormMetricsRecorder::RecordParsingOnSavingDifference(
ukm_entry_builder_.SetParsingOnSavingDifference(comparison_result); ukm_entry_builder_.SetParsingOnSavingDifference(comparison_result);
} }
void PasswordFormMetricsRecorder::RecordReadonlyWhenFilling(uint64_t value) {
ukm_entry_builder_.SetReadonlyWhenFilling(value);
}
void PasswordFormMetricsRecorder::RecordReadonlyWhenSaving(uint64_t value) {
ukm_entry_builder_.SetReadonlyWhenSaving(value);
}
void PasswordFormMetricsRecorder::RecordShowManualFallbackForSaving( void PasswordFormMetricsRecorder::RecordShowManualFallbackForSaving(
bool has_generated_password, bool has_generated_password,
bool is_update) { bool is_update) {
......
...@@ -291,6 +291,18 @@ class PasswordFormMetricsRecorder ...@@ -291,6 +291,18 @@ class PasswordFormMetricsRecorder
// |comparison_result| is a bitmask of values from ParsingOnSavingDifference. // |comparison_result| is a bitmask of values from ParsingOnSavingDifference.
void RecordParsingOnSavingDifference(uint64_t comparison_result); void RecordParsingOnSavingDifference(uint64_t comparison_result);
// Records the readonly status encoded with parsing success after parsing for
// filling. The |value| is constructed as follows: The least significant bit
// says whether parsing succeeded (1) or not (0). The rest, shifted by one
// bit to the right is the FormDataParser::ReadonlyPasswordFields
// representation of the readonly status.
void RecordReadonlyWhenFilling(uint64_t value);
// Records the readonly status encoded with parsing success after parsing for
// creating pending credentials. See RecordReadonlyWhenFilling for the meaning
// of |value|.
void RecordReadonlyWhenSaving(uint64_t value);
// Records that Chrome noticed that it should show a manual fallback for // Records that Chrome noticed that it should show a manual fallback for
// saving. // saving.
void RecordShowManualFallbackForSaving(bool has_generated_password, void RecordShowManualFallbackForSaving(bool has_generated_password,
......
...@@ -3153,6 +3153,22 @@ be describing additional metrics about the same event. ...@@ -3153,6 +3153,22 @@ be describing additional metrics about the same event.
bitmask can be combined. bitmask can be combined.
</summary> </summary>
</metric> </metric>
<metric name="ReadonlyWhenFilling">
<summary>
Records the readonly status encoded with parsing success after parsing for
filling. The |value| is constructed as follows: The least significant bit
says whether parsing succeeded (1) or not (0). The rest, shifted by one
bit to the right is the FormDataParser::ReadonlyPasswordFields
representation of the readonly status.
</summary>
</metric>
<metric name="ReadonlyWhenSaving">
<summary>
Records the readonly status encoded with parsing success after parsing for
creating pending credentials. The |value| is constructed the same way as
for ReadonlyWhenFilling.
</summary>
</metric>
<metric name="Saving.Prompt.Interaction"> <metric name="Saving.Prompt.Interaction">
<summary> <summary>
Records how the user interacted with a saving prompt. Recorded values Records how the user interacted with a saving prompt. Recorded values
......
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