Skip to content

GitLab

T
tangled
Commit ae1c6296 authored by Marian Fechete's avatar Marian Fechete Committed by Commit Bot
Browse files
Options

[Autofill Assistant] Refactor RequiredFieldsHandler to make FallbackData and RequiredField generic. 

This CL removes the hard-coded fields from the Fallback data for credit
card and address. Instead, a map is used, and the required fields can
now be specified by their enum values.

This allows us to remove the callback for the field getters and retrieve
the fallback values directly from the map.

Bug: b/141362833
Change-Id: I6cd8f7371557d096f82921d0bb8a21527c33e9f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1886820
Commit-Queue: Marian Fechete <marianfe@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#710723}
parent a73d100b
Hide whitespace changes
Inline Side-by-side
Showing with 154 additions and 174 deletions
components/autofill_assistant/browser/actions/required_fields_fallback_handler.cc
View file @ ae1c6296
......@@ -18,16 +18,8 @@ namespace autofill_assistant {
RequiredFieldsFallbackHandler::RequiredFieldsFallbackHandler(
const std::vector<RequiredField>& required_fields,
base::RepeatingCallback<std::string(const RequiredField&,
const FallbackData&)>
field_value_getter,
base::OnceCallback<void(const ClientStatus&,
const base::Optional<ClientStatus>&)>
status_update_callback,
ActionDelegate* action_delegate) {
required_fields_.assign(required_fields.begin(), required_fields.end());
field_value_getter_ = std::move(field_value_getter);
status_update_callback_ = std::move(status_update_callback);
action_delegate_ = action_delegate;
}
......@@ -35,10 +27,25 @@ RequiredFieldsFallbackHandler::~RequiredFieldsFallbackHandler() {}
RequiredFieldsFallbackHandler::FallbackData::FallbackData() {}
RequiredFieldsFallbackHandler::FallbackData::~FallbackData() {}
base::Optional<std::string>
RequiredFieldsFallbackHandler::FallbackData::GetValue(int key) {
auto it = field_values.find(key);
if (it != field_values.end()) {
return it->second;
}
return base::nullopt;
}
void RequiredFieldsFallbackHandler::CheckAndFallbackRequiredFields(
const ClientStatus& initial_autofill_status,
std::unique_ptr<FallbackData> fallback_data) {
std::unique_ptr<FallbackData> fallback_data,
base::OnceCallback<void(const ClientStatus&,
const base::Optional<ClientStatus>&)>
status_update_callback) {
initial_autofill_status_ = initial_autofill_status;
status_update_callback_ = std::move(status_update_callback);
if (required_fields_.empty()) {
if (!initial_autofill_status.ok()) {
......@@ -114,10 +121,11 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone(
// immediately.
bool has_fallbacks = false;
for (const RequiredField& required_field : required_fields_) {
if (!required_field.ShouldFallback(/* has_fallback_data= */ true))
if (!required_field.ShouldFallback(/* has_fallback_data= */ true)) {
continue;
}
if (!field_value_getter_.Run(required_field, *fallback_data).empty()) {
if (fallback_data->GetValue(required_field.fallback_key).has_value()) {
has_fallbacks = true;
}
}
......@@ -151,18 +159,17 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially(
// Set the next field to its fallback value.
const RequiredField& required_field = required_fields_[required_fields_index];
std::string fallback_value =
field_value_getter_.Run(required_field, *fallback_data);
if (fallback_value.empty()) {
auto fallback_value = fallback_data->GetValue(required_field.fallback_key);
if (!fallback_value.has_value()) {
DVLOG(3) << "No fallback for " << required_field.selector;
// If there is no fallback value, we skip this failed field.
return SetFallbackFieldValuesSequentially(++required_fields_index,
std::move(fallback_data));
}
DVLOG(3) << "Setting fallback value for " << required_field.selector;
DVLOG(3) << "Setting fallback value for " << required_field.selector;
action_delegate_->SetFieldValue(
required_field.selector, fallback_value,
required_field.selector, fallback_value.value(),
required_field.simulate_key_presses, required_field.delay_in_millisecond,
base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue,
weak_ptr_factory_.GetWeakPtr(), required_fields_index,
......@@ -172,8 +179,8 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially(
void RequiredFieldsFallbackHandler::OnSetFallbackFieldValue(
size_t required_fields_index,
std::unique_ptr<FallbackData> fallback_data,
const ClientStatus& setFieldStatus) {
if (!setFieldStatus.ok()) {
const ClientStatus& set_field_status) {
if (!set_field_status.ok()) {
// Fallback failed: we stop the script without checking the fields.
std::move(status_update_callback_)
.Run(ClientStatus(MANUAL_FALLBACK), initial_autofill_status_);
......
components/autofill_assistant/browser/actions/required_fields_fallback_handler.h
View file @ ae1c6296
......@@ -18,10 +18,6 @@
#include "components/autofill_assistant/browser/actions/action.h"
#include "components/autofill_assistant/browser/batch_element_checker.h"
namespace autofill {
class AutofillProfile;
} // namespace autofill
namespace autofill_assistant {
class ClientStatus;
......@@ -37,12 +33,7 @@ class RequiredFieldsFallbackHandler {
bool forced = false;
FieldValueStatus status = UNKNOWN;
// When filling in credit card, card_field must be set. When filling in
// address, address_field must be set.
UseAddressProto::RequiredField::AddressField address_field =
UseAddressProto::RequiredField::UNDEFINED;
UseCreditCardProto::RequiredField::CardField card_field =
UseCreditCardProto::RequiredField::UNDEFINED;
int fallback_key;
// Returns true if fallback is required for this field.
bool ShouldFallback(bool has_fallback_data) const {
......@@ -56,17 +47,13 @@ class RequiredFieldsFallbackHandler {
// TODO(marianfe): Refactor this to use a map instead.
struct FallbackData {
FallbackData();
~FallbackData() = default;
~FallbackData();
// autofill profile.
const autofill::AutofillProfile* profile;
// the key of the map should be the same as the |fallback_key| of the
// required field.
std::map<int, std::string> field_values;
// Card information for UseCreditCard fallback.
std::string cvc;
std::string card_holder_name;
std::string card_number;
int expiration_year = 0;
int expiration_month = 0;
base::Optional<std::string> GetValue(int key);
private:
DISALLOW_COPY_AND_ASSIGN(FallbackData);
......@@ -74,12 +61,6 @@ class RequiredFieldsFallbackHandler {
explicit RequiredFieldsFallbackHandler(
const std::vector<RequiredField>& required_fields,
base::RepeatingCallback<std::string(const RequiredField&,
const FallbackData&)>
field_value_getter,
base::OnceCallback<void(const ClientStatus&,
const base::Optional<ClientStatus>&)>
status_update_callback,
ActionDelegate* delegate);
~RequiredFieldsFallbackHandler();
......@@ -88,7 +69,10 @@ class RequiredFieldsFallbackHandler {
// action.
void CheckAndFallbackRequiredFields(
const ClientStatus& initial_autofill_status,
std::unique_ptr<FallbackData> fallback_data);
std::unique_ptr<FallbackData> fallback_data,
base::OnceCallback<void(const ClientStatus&,
const base::Optional<ClientStatus>&)>
status_update_callback);
private:
// Check whether all required fields have a non-empty value. If it is the
......@@ -125,9 +109,6 @@ class RequiredFieldsFallbackHandler {
ClientStatus initial_autofill_status_;
std::vector<RequiredField> required_fields_;
base::RepeatingCallback<std::string(const RequiredField&,
const FallbackData&)>
field_value_getter_;
base::OnceCallback<void(const ClientStatus&,
const base::Optional<ClientStatus>&)>
status_update_callback_;
......
components/autofill_assistant/browser/actions/use_address_action.cc
View file @ ae1c6296
......@@ -33,9 +33,15 @@ UseAddressAction::UseAddressAction(ActionDelegate* delegate,
std::vector<RequiredField> required_fields;
for (const auto& required_field_proto :
proto_.use_address().required_fields()) {
if (required_field_proto.address_field() ==
UseAddressProto::RequiredField::UNDEFINED) {
DVLOG(1) << "address_field enum not set, skipping required field";
continue;
}
required_fields.emplace_back();
RequiredField& required_field = required_fields.back();
required_field.address_field = required_field_proto.address_field();
required_field.fallback_key = (int)required_field_proto.address_field();
required_field.selector = Selector(required_field_proto.element());
required_field.simulate_key_presses =
required_field_proto.simulate_key_presses();
......@@ -45,12 +51,8 @@ UseAddressAction::UseAddressAction(ActionDelegate* delegate,
}
required_fields_fallback_handler_ =
std::make_unique<RequiredFieldsFallbackHandler>(
required_fields,
base::BindRepeating(&UseAddressAction::GetFallbackValue,
base::Unretained(this)),
base::BindOnce(&UseAddressAction::EndAction, base::Unretained(this)),
delegate);
std::make_unique<RequiredFieldsFallbackHandler>(required_fields,
delegate);
selector_ = Selector(proto.use_address().form_field_element());
selector_.MustBeVisible();
DCHECK(!selector_.empty());
......@@ -108,8 +110,7 @@ void UseAddressAction::OnWaitForElement(const ClientStatus& element_status) {
const autofill::AutofillProfile* profile =
delegate_->GetClientMemory()->selected_address(name_);
DCHECK(profile);
auto fallback_data = std::make_unique<FallbackData>();
fallback_data->profile = profile;
auto fallback_data = CreateFallbackData(*profile);
delegate_->FillAddressForm(
profile, selector_,
base::BindOnce(&UseAddressAction::OnFormFilled,
......@@ -119,51 +120,61 @@ void UseAddressAction::OnWaitForElement(const ClientStatus& element_status) {
void UseAddressAction::OnFormFilled(std::unique_ptr<FallbackData> fallback_data,
const ClientStatus& status) {
required_fields_fallback_handler_->CheckAndFallbackRequiredFields(
status, std::move(fallback_data));
}
std::string UseAddressAction::GetFallbackValue(
const RequiredField& required_field,
const FallbackData& fallback_data) {
return base::UTF16ToUTF8(
GetFieldValue(fallback_data.profile, required_field.address_field));
status, std::move(fallback_data),
base::BindOnce(&UseAddressAction::EndAction,
weak_ptr_factory_.GetWeakPtr()));
}
base::string16 UseAddressAction::GetFieldValue(
const autofill::AutofillProfile* profile,
const UseAddressProto::RequiredField::AddressField& address_field) {
// TODO(crbug.com/806868): Get the actual application locale.
std::unique_ptr<FallbackData> UseAddressAction::CreateFallbackData(
const autofill::AutofillProfile& profile) {
// TODO(crbug.com/806868): Get the locale from the backend.
std::string app_locale = "en-US";
switch (address_field) {
case UseAddressProto::RequiredField::FIRST_NAME:
return profile->GetInfo(autofill::NAME_FIRST, app_locale);
case UseAddressProto::RequiredField::LAST_NAME:
return profile->GetInfo(autofill::NAME_LAST, app_locale);
case UseAddressProto::RequiredField::FULL_NAME:
return profile->GetInfo(autofill::NAME_FULL, app_locale);
case UseAddressProto::RequiredField::PHONE_NUMBER:
return profile->GetInfo(autofill::PHONE_HOME_WHOLE_NUMBER, app_locale);
case UseAddressProto::RequiredField::EMAIL:
return profile->GetInfo(autofill::EMAIL_ADDRESS, app_locale);
case UseAddressProto::RequiredField::ORGANIZATION:
return profile->GetInfo(autofill::COMPANY_NAME, app_locale);
case UseAddressProto::RequiredField::COUNTRY_CODE:
return profile->GetInfo(autofill::ADDRESS_HOME_COUNTRY, app_locale);
case UseAddressProto::RequiredField::REGION:
return profile->GetInfo(autofill::ADDRESS_HOME_STATE, app_locale);
case UseAddressProto::RequiredField::STREET_ADDRESS:
return profile->GetInfo(autofill::ADDRESS_HOME_STREET_ADDRESS,
app_locale);
case UseAddressProto::RequiredField::LOCALITY:
return profile->GetInfo(autofill::ADDRESS_HOME_CITY, app_locale);
case UseAddressProto::RequiredField::DEPENDANT_LOCALITY:
return profile->GetInfo(autofill::ADDRESS_HOME_DEPENDENT_LOCALITY,
app_locale);
case UseAddressProto::RequiredField::POSTAL_CODE:
return profile->GetInfo(autofill::ADDRESS_HOME_ZIP, app_locale);
case UseAddressProto::RequiredField::UNDEFINED:
NOTREACHED();
return base::string16();
}
auto fallback_data = std::make_unique<FallbackData>();
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::FIRST_NAME,
base::UTF16ToUTF8(profile.GetInfo(autofill::NAME_FIRST, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::LAST_NAME,
base::UTF16ToUTF8(profile.GetInfo(autofill::NAME_LAST, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::FULL_NAME,
base::UTF16ToUTF8(profile.GetInfo(autofill::NAME_FIRST, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::PHONE_NUMBER,
base::UTF16ToUTF8(
profile.GetInfo(autofill::PHONE_HOME_WHOLE_NUMBER, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::EMAIL,
base::UTF16ToUTF8(profile.GetInfo(autofill::EMAIL_ADDRESS, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::ORGANIZATION,
base::UTF16ToUTF8(profile.GetInfo(autofill::COMPANY_NAME, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::COUNTRY_CODE,
base::UTF16ToUTF8(
profile.GetInfo(autofill::ADDRESS_HOME_COUNTRY, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::REGION,
base::UTF16ToUTF8(
profile.GetInfo(autofill::ADDRESS_HOME_STATE, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::STREET_ADDRESS,
base::UTF16ToUTF8(
profile.GetInfo(autofill::ADDRESS_HOME_STREET_ADDRESS, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::LOCALITY,
base::UTF16ToUTF8(
profile.GetInfo(autofill::ADDRESS_HOME_CITY, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::DEPENDANT_LOCALITY,
base::UTF16ToUTF8(profile.GetInfo(
autofill::ADDRESS_HOME_DEPENDENT_LOCALITY, app_locale)));
fallback_data->field_values.emplace(
(int)UseAddressProto::RequiredField::POSTAL_CODE,
base::UTF16ToUTF8(
profile.GetInfo(autofill::ADDRESS_HOME_ZIP, app_locale)));
return fallback_data;
}
} // namespace autofill_assistant
components/autofill_assistant/browser/actions/use_address_action.h
View file @ ae1c6296
......@@ -47,16 +47,9 @@ class UseAddressAction : public Action {
fallback_data,
const ClientStatus& status);
// Gets the fallback value.
std::string GetFallbackValue(
const RequiredFieldsFallbackHandler::RequiredField& required_field,
const RequiredFieldsFallbackHandler::FallbackData& fallback_data);
// Get the value of |address_field| associated to profile |profile|. Return
// empty string if there is no data available.
base::string16 GetFieldValue(
const autofill::AutofillProfile* profile,
const UseAddressProto::RequiredField::AddressField& address_field);
// Create fallback data.
std::unique_ptr<RequiredFieldsFallbackHandler::FallbackData>
CreateFallbackData(const autofill::AutofillProfile& profile);
// Usage of the autofilled address.
std::string name_;
......
components/autofill_assistant/browser/actions/use_credit_card_action.cc
View file @ ae1c6296
......@@ -33,9 +33,15 @@ UseCreditCardAction::UseCreditCardAction(ActionDelegate* delegate,
prompt_ = proto.use_card().prompt();
std::vector<RequiredField> required_fields;
for (const auto& required_field_proto : proto_.use_card().required_fields()) {
if (required_field_proto.card_field() ==
UseCreditCardProto::RequiredField::UNDEFINED) {
DVLOG(1) << "card_field enum not set, skipping required field";
continue;
}
required_fields.emplace_back();
RequiredField& required_field = required_fields.back();
required_field.card_field = required_field_proto.card_field();
required_field.fallback_key = (int)required_field_proto.card_field();
required_field.selector = Selector(required_field_proto.element());
required_field.simulate_key_presses =
required_field_proto.simulate_key_presses();
......@@ -45,13 +51,8 @@ UseCreditCardAction::UseCreditCardAction(ActionDelegate* delegate,
}
required_fields_fallback_handler_ =
std::make_unique<RequiredFieldsFallbackHandler>(
required_fields,
base::BindRepeating(&UseCreditCardAction::GetFallbackValue,
base::Unretained(this)),
base::BindOnce(&UseCreditCardAction::EndAction,
base::Unretained(this)),
delegate);
std::make_unique<RequiredFieldsFallbackHandler>(required_fields,
delegate);
selector_ = Selector(proto.use_card().form_field_element());
selector_.MustBeVisible();
DCHECK(!selector_.empty());
......@@ -109,15 +110,7 @@ void UseCreditCardAction::OnGetFullCard(
return;
}
auto fallback_data = std::make_unique<FallbackData>();
fallback_data->cvc = base::UTF16ToUTF8(cvc);
fallback_data->expiration_month = card->expiration_month();
fallback_data->expiration_year = card->expiration_year();
fallback_data->card_holder_name =
base::UTF16ToUTF8(card->GetRawInfo(autofill::CREDIT_CARD_NAME_FULL));
fallback_data->card_number =
base::UTF16ToUTF8(card->GetRawInfo(autofill::CREDIT_CARD_NUMBER));
auto fallback_data = CreateFallbackData(cvc, *card);
delegate_->FillCardForm(
std::move(card), cvc, selector_,
base::BindOnce(&UseCreditCardAction::OnFormFilled,
......@@ -128,52 +121,47 @@ void UseCreditCardAction::OnFormFilled(
std::unique_ptr<FallbackData> fallback_data,
const ClientStatus& status) {
required_fields_fallback_handler_->CheckAndFallbackRequiredFields(
status, std::move(fallback_data));
status, std::move(fallback_data),
base::BindOnce(&UseCreditCardAction::EndAction,
weak_ptr_factory_.GetWeakPtr()));
}
std::string UseCreditCardAction::GetFallbackValue(
const RequiredField& required_field,
const FallbackData& fallback_data) {
auto field = required_field.card_field;
switch (field) {
case UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE:
return fallback_data.cvc;
case UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_MONTH:
if (fallback_data.expiration_month > 0)
return base::StringPrintf("%02d", fallback_data.expiration_month);
break;
case UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_2_DIGIT_YEAR:
if (fallback_data.expiration_year > 0)
return base::StringPrintf("%02d", fallback_data.expiration_year % 100);
break;
case UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_4_DIGIT_YEAR:
if (fallback_data.expiration_year > 0)
return base::NumberToString(fallback_data.expiration_year);
break;
case UseCreditCardProto::RequiredField::CREDIT_CARD_CARD_HOLDER_NAME:
return fallback_data.card_holder_name;
break;
case UseCreditCardProto::RequiredField::CREDIT_CARD_NUMBER:
return fallback_data.card_number;
break;
case UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_MM_YY:
if (fallback_data.expiration_month > 0 &&
fallback_data.expiration_year > 0)
return base::StrCat(
{base::StringPrintf("%02d", fallback_data.expiration_month), "/",
base::StringPrintf("%02d", fallback_data.expiration_year % 100)});
break;
case UseCreditCardProto::RequiredField::UNDEFINED:
NOTREACHED();
return "";
std::unique_ptr<FallbackData> UseCreditCardAction::CreateFallbackData(
const base::string16& cvc,
const autofill::CreditCard& card) {
auto fallback_data = std::make_unique<FallbackData>();
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE,
base::UTF16ToUTF8(cvc));
if (card.expiration_month() > 0) {
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_MONTH,
base::StringPrintf("%02d", card.expiration_month()));
}
return "";
if (card.expiration_year() > 0) {
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_2_DIGIT_YEAR,
base::StringPrintf("%02d", card.expiration_year() % 100));
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_4_DIGIT_YEAR,
base::StringPrintf("%04d", card.expiration_year()));
if (card.expiration_month() > 0) {
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_EXP_MM_YY,
base::StrCat(
{base::StringPrintf("%02d", card.expiration_month()), "/",
base::StringPrintf("%02d", card.expiration_year() % 100)}));
}
}
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_CARD_HOLDER_NAME,
base::UTF16ToUTF8(card.GetRawInfo(autofill::CREDIT_CARD_NAME_FULL)));
fallback_data->field_values.emplace(
(int)UseCreditCardProto::RequiredField::CREDIT_CARD_NUMBER,
base::UTF16ToUTF8(card.GetRawInfo(autofill::CREDIT_CARD_NUMBER)));
return fallback_data;
}
} // namespace autofill_assistant
components/autofill_assistant/browser/actions/use_credit_card_action.h
View file @ ae1c6296
......@@ -52,10 +52,10 @@ class UseCreditCardAction : public Action {
fallback_data,
const ClientStatus& status);
// Gets the fallback value.
std::string GetFallbackValue(
const RequiredFieldsFallbackHandler::RequiredField& required_field,
const RequiredFieldsFallbackHandler::FallbackData& fallback_data);
// Create fallback data.
std::unique_ptr<RequiredFieldsFallbackHandler::FallbackData>
CreateFallbackData(const base::string16& cvc,
const autofill::CreditCard& card);
std::string prompt_;
Selector selector_;
......
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