Commit d981f048 authored by Christoph Schwering's avatar Christoph Schwering Committed by Commit Bot

[Autofill] Removed FormStructure::GetServerTypes().

FormStructure::GetServerTypes() created a vector that was always
passed to data_util::DetermineGroups().

This CL removes FormStructure::GetServerTypes() and moves the relevant
computation of the server into the loop in
data_util::DetermineGroups().

We need to keep an overload data_util::DetermineGroups() that takes a
vector because of one reference in LabelFormatter::Create().

Bug: 1007974
Change-Id: Idea5c2d4ea484701ac6a898b4eb6b667859de488
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2266173
Commit-Queue: Christoph Schwering <schwering@google.com>
Reviewed-by: default avatarMatthias Körber <koerber@google.com>
Cr-Commit-Position: refs/heads/master@{#796331}
parent 7be9f143
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill/core/browser/data_model/autofill_profile.h"
#include "components/autofill/core/browser/data_model/credit_card.h" #include "components/autofill/core/browser/data_model/credit_card.h"
#include "components/autofill/core/browser/field_types.h" #include "components/autofill/core/browser/field_types.h"
#include "components/autofill/core/browser/form_structure.h"
#include "components/autofill/core/browser/geo/autofill_country.h" #include "components/autofill/core/browser/geo/autofill_country.h"
#include "components/autofill/core/browser/webdata/autofill_table.h" #include "components/autofill/core/browser/webdata/autofill_table.h"
#include "components/grit/components_scaled_resources.h" #include "components/grit/components_scaled_resources.h"
...@@ -246,6 +247,27 @@ bool SplitCJKName(const std::vector<base::StringPiece16>& name_tokens, ...@@ -246,6 +247,27 @@ bool SplitCJKName(const std::vector<base::StringPiece16>& name_tokens,
return false; return false;
} }
void AddGroupToBitmask(uint32_t* group_bitmask, ServerFieldType type) {
const FieldTypeGroup group =
AutofillType(AutofillType(type).GetStorableType()).group();
switch (group) {
case autofill::NAME:
*group_bitmask |= kName;
break;
case autofill::ADDRESS_HOME:
*group_bitmask |= kAddress;
break;
case autofill::EMAIL:
*group_bitmask |= kEmail;
break;
case autofill::PHONE_HOME:
*group_bitmask |= kPhone;
break;
default:
break;
}
}
} // namespace } // namespace
bool ContainsName(uint32_t groups) { bool ContainsName(uint32_t groups) {
...@@ -264,27 +286,19 @@ bool ContainsPhone(uint32_t groups) { ...@@ -264,27 +286,19 @@ bool ContainsPhone(uint32_t groups) {
return groups & kPhone; return groups & kPhone;
} }
uint32_t DetermineGroups(const FormStructure& form) {
uint32_t group_bitmask = 0;
for (const auto& field : form) {
ServerFieldType type = field->Type().GetStorableType();
AddGroupToBitmask(&group_bitmask, type);
}
return group_bitmask;
}
uint32_t DetermineGroups(const std::vector<ServerFieldType>& types) { uint32_t DetermineGroups(const std::vector<ServerFieldType>& types) {
uint32_t group_bitmask = 0; uint32_t group_bitmask = 0;
for (const ServerFieldType& type : types) { for (const auto& type : types) {
const FieldTypeGroup group = AddGroupToBitmask(&group_bitmask, type);
AutofillType(AutofillType(type).GetStorableType()).group();
switch (group) {
case autofill::NAME:
group_bitmask |= kName;
break;
case autofill::ADDRESS_HOME:
group_bitmask |= kAddress;
break;
case autofill::EMAIL:
group_bitmask |= kEmail;
break;
case autofill::PHONE_HOME:
group_bitmask |= kPhone;
break;
default:
break;
}
} }
return group_bitmask; return group_bitmask;
} }
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
namespace autofill { namespace autofill {
class AutofillProfile; class AutofillProfile;
class FormStructure;
namespace data_util { namespace data_util {
...@@ -54,7 +55,9 @@ bool ContainsEmail(uint32_t groups); ...@@ -54,7 +55,9 @@ bool ContainsEmail(uint32_t groups);
bool ContainsPhone(uint32_t groups); bool ContainsPhone(uint32_t groups);
// Returns a bitmask indicating which of the name, address, email address, and // Returns a bitmask indicating which of the name, address, email address, and
// phone number FieldTypeGroups are associated with the given |types|. // phone number FieldTypeGroups are associated with the given |form|'s storable
// types or |types|, respectively.
uint32_t DetermineGroups(const FormStructure& form);
uint32_t DetermineGroups(const std::vector<ServerFieldType>& types); uint32_t DetermineGroups(const std::vector<ServerFieldType>& types);
// Returns true if a form has address fields or has least two supported // Returns true if a form has address fields or has least two supported
......
...@@ -871,8 +871,7 @@ void AutofillManager::OnTextFieldDidChangeImpl(const FormData& form, ...@@ -871,8 +871,7 @@ void AutofillManager::OnTextFieldDidChangeImpl(const FormData& form,
if (!user_did_type_ || autofill_field->is_autofilled) { if (!user_did_type_ || autofill_field->is_autofilled) {
form_interactions_ukm_logger_->LogTextFieldDidChange(*form_structure, form_interactions_ukm_logger_->LogTextFieldDidChange(*form_structure,
*autofill_field); *autofill_field);
profile_form_bitmask = profile_form_bitmask = data_util::DetermineGroups(*form_structure);
data_util::DetermineGroups(form_structure->GetServerFieldTypes());
} }
if (!autofill_field->is_autofilled) { if (!autofill_field->is_autofilled) {
...@@ -1198,9 +1197,7 @@ void AutofillManager::OnDidFillAutofillFormData(const FormData& form, ...@@ -1198,9 +1197,7 @@ void AutofillManager::OnDidFillAutofillFormData(const FormData& form,
} }
uint32_t profile_form_bitmask = uint32_t profile_form_bitmask =
form_structure form_structure ? data_util::DetermineGroups(*form_structure) : 0;
? data_util::DetermineGroups(form_structure->GetServerFieldTypes())
: 0;
AutofillMetrics::LogUserHappinessMetric( AutofillMetrics::LogUserHappinessMetric(
AutofillMetrics::USER_DID_AUTOFILL, form_types, AutofillMetrics::USER_DID_AUTOFILL, form_types,
...@@ -1233,8 +1230,7 @@ void AutofillManager::DidShowSuggestions(bool has_autofill_suggestions, ...@@ -1233,8 +1230,7 @@ void AutofillManager::DidShowSuggestions(bool has_autofill_suggestions,
if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field)) if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field))
return; return;
uint32_t profile_form_bitmask = uint32_t profile_form_bitmask = data_util::DetermineGroups(*form_structure);
data_util::DetermineGroups(form_structure->GetServerFieldTypes());
AutofillMetrics::LogUserHappinessMetric( AutofillMetrics::LogUserHappinessMetric(
AutofillMetrics::SUGGESTIONS_SHOWN, autofill_field->Type().group(), AutofillMetrics::SUGGESTIONS_SHOWN, autofill_field->Type().group(),
client_->GetSecurityLevelForUmaHistograms(), profile_form_bitmask); client_->GetSecurityLevelForUmaHistograms(), profile_form_bitmask);
...@@ -1890,11 +1886,10 @@ void AutofillManager::FillOrPreviewDataModelForm( ...@@ -1890,11 +1886,10 @@ void AutofillManager::FillOrPreviewDataModelForm(
// Fill the non-empty value from |data_model| into the result vector, which // Fill the non-empty value from |data_model| into the result vector, which
// will be sent to the renderer. // will be sent to the renderer.
FillFieldWithValue( FillFieldWithValue(cached_field, data_model, &result.fields[i],
cached_field, data_model, &result.fields[i], should_notify, should_notify, optional_cvc ? *optional_cvc : kEmptyCvc,
optional_cvc ? *optional_cvc : kEmptyCvc, data_util::DetermineGroups(*form_structure),
data_util::DetermineGroups(form_structure->GetServerFieldTypes()), &failure_to_fill);
&failure_to_fill);
bool has_value_after = !result.fields[i].value.empty(); bool has_value_after = !result.fields[i].value.empty();
bool is_autofilled_after = result.fields[i].is_autofilled; bool is_autofilled_after = result.fields[i].is_autofilled;
...@@ -2056,8 +2051,7 @@ void AutofillManager::OnFormsParsed(const std::vector<const FormData*>& forms, ...@@ -2056,8 +2051,7 @@ void AutofillManager::OnFormsParsed(const std::vector<const FormData*>& forms,
continue; continue;
} }
if (data_util::ContainsPhone(data_util::DetermineGroups( if (data_util::ContainsPhone(data_util::DetermineGroups(*form_structure))) {
form_structure->GetServerFieldTypes()))) {
has_observed_phone_number_field_ = true; has_observed_phone_number_field_ = true;
} }
......
...@@ -1184,7 +1184,7 @@ void FormStructure::LogQualityMetrics( ...@@ -1184,7 +1184,7 @@ void FormStructure::LogQualityMetrics(
AutofillMetrics::LogUserHappinessMetric( AutofillMetrics::LogUserHappinessMetric(
AutofillMetrics::USER_DID_ENTER_UPI_VPA, field->Type().group(), AutofillMetrics::USER_DID_ENTER_UPI_VPA, field->Type().group(),
security_state::SecurityLevel::SECURITY_LEVEL_COUNT, security_state::SecurityLevel::SECURITY_LEVEL_COUNT,
data_util::DetermineGroups(GetServerFieldTypes())); data_util::DetermineGroups(*this));
} }
form_interactions_ukm_logger->LogFieldFillStatus(*this, *field, form_interactions_ukm_logger->LogFieldFillStatus(*this, *field,
...@@ -2287,14 +2287,6 @@ std::set<FormType> FormStructure::GetFormTypes() const { ...@@ -2287,14 +2287,6 @@ std::set<FormType> FormStructure::GetFormTypes() const {
return form_types; return form_types;
} }
std::vector<ServerFieldType> FormStructure::GetServerFieldTypes() const {
std::vector<ServerFieldType> types(field_count());
std::transform(begin(), end(), types.begin(), [&](const auto& field) {
return field->Type().GetStorableType();
});
return types;
}
base::string16 FormStructure::GetIdentifierForRefill() const { base::string16 FormStructure::GetIdentifierForRefill() const {
if (!form_name().empty()) if (!form_name().empty())
return form_name(); return form_name();
......
...@@ -285,10 +285,6 @@ class FormStructure { ...@@ -285,10 +285,6 @@ class FormStructure {
// Returns the possible form types. // Returns the possible form types.
std::set<FormType> GetFormTypes() const; std::set<FormType> GetFormTypes() const;
// Returns a collection of ServerFieldTypes corresponding to this
// FormStructure's fields.
std::vector<ServerFieldType> GetServerFieldTypes() const;
bool passwords_were_revealed() const { return passwords_were_revealed_; } bool passwords_were_revealed() const { return passwords_were_revealed_; }
void set_passwords_were_revealed(bool passwords_were_revealed) { void set_passwords_were_revealed(bool passwords_were_revealed) {
passwords_were_revealed_ = passwords_were_revealed; passwords_were_revealed_ = passwords_were_revealed;
......
...@@ -84,14 +84,7 @@ void AddressFormEventLogger::OnSubsequentRefillAttempt( ...@@ -84,14 +84,7 @@ void AddressFormEventLogger::OnSubsequentRefillAttempt(
void AddressFormEventLogger::OnLog(const std::string& name, void AddressFormEventLogger::OnLog(const std::string& name,
FormEvent event, FormEvent event,
const FormStructure& form) const { const FormStructure& form) const {
std::vector<ServerFieldType> types; uint32_t groups = data_util::DetermineGroups(form);
std::transform(
form.begin(), form.end(), std::back_inserter(types),
[&](const std::unique_ptr<AutofillField>& field) -> ServerFieldType {
return field->Type().GetStorableType();
});
uint32_t groups = data_util::DetermineGroups(types);
base::UmaHistogramEnumeration( base::UmaHistogramEnumeration(
name + data_util::GetSuffixForProfileFormType(groups), event, name + data_util::GetSuffixForProfileFormType(groups), event,
NUM_FORM_EVENTS); NUM_FORM_EVENTS);
......
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