From c30a6232df03e1efbd9f3b226777b07e087a1122 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 12 Oct 2020 14:27:29 +0200 Subject: BASELINE: Update Chromium to 85.0.4183.140 Change-Id: Iaa42f4680837c57725b1344f108c0196741f6057 Reviewed-by: Allan Sandfeld Jensen --- .../components/autofill_assistant/browser/BUILD.gn | 10 +- .../browser/actions/action_delegate.h | 28 +- .../browser/actions/collect_user_data_action.cc | 123 +-- .../actions/collect_user_data_action_unittest.cc | 55 ++ .../actions/fallback_handler/fallback_data.cc | 73 -- .../actions/fallback_handler/fallback_data.h | 40 - .../actions/fallback_handler/required_field.cc | 41 +- .../actions/fallback_handler/required_field.h | 21 +- .../required_fields_fallback_handler.cc | 135 +-- .../required_fields_fallback_handler.h | 33 +- .../required_fields_fallback_handler_unittest.cc | 479 +++++------ ...rate_password_for_form_field_action_unittest.cc | 5 +- .../browser/actions/mock_action_delegate.h | 49 +- .../browser/actions/navigate_action.cc | 7 +- .../browser/actions/prompt_action.cc | 4 +- .../browser/actions/prompt_action_unittest.cc | 30 +- .../browser/actions/set_attribute_action.cc | 1 - .../browser/actions/set_form_field_value_action.cc | 1 - .../set_form_field_value_action_unittest.cc | 5 +- .../browser/actions/show_generic_ui_action.cc | 147 +++- .../browser/actions/show_generic_ui_action.h | 26 +- .../actions/show_generic_ui_action_unittest.cc | 117 ++- .../browser/actions/show_progress_bar_action.cc | 45 +- .../browser/actions/show_progress_bar_action.h | 5 +- .../actions/show_progress_bar_action_unittest.cc | 166 ++++ .../browser/actions/use_address_action.cc | 164 ++-- .../browser/actions/use_address_action.h | 23 +- .../browser/actions/use_address_action_unittest.cc | 229 +++-- .../browser/actions/use_credit_card_action.cc | 145 ++-- .../browser/actions/use_credit_card_action.h | 17 +- .../actions/use_credit_card_action_unittest.cc | 348 ++++---- .../actions/wait_for_document_action_unittest.cc | 4 +- .../actions/wait_for_dom_action_unittest.cc | 22 +- .../browser/basic_interactions.cc | 127 +-- .../browser/basic_interactions.h | 32 +- .../browser/basic_interactions_unittest.cc | 129 ++- .../components/autofill_assistant/browser/client.h | 3 + .../autofill_assistant/browser/client_settings.h | 2 +- .../autofill_assistant/browser/client_status.cc | 3 + .../autofill_assistant/browser/controller.cc | 151 +++- .../autofill_assistant/browser/controller.h | 29 +- .../browser/controller_observer.h | 10 + .../browser/controller_unittest.cc | 123 ++- .../domain_type_conversions_h.template | 2 +- .../browser/devtools/devtools_client.cc | 4 +- .../autofill_assistant/browser/element_area.cc | 3 + .../browser/element_area_unittest.cc | 42 +- .../browser/element_precondition_unittest.cc | 121 ++- .../autofill_assistant/browser/event_handler.cc | 66 ++ .../autofill_assistant/browser/event_handler.h | 4 + .../browser/fake_script_executor_delegate.cc | 17 +- .../browser/fake_script_executor_delegate.h | 13 +- .../autofill_assistant/browser/features.cc | 8 + .../autofill_assistant/browser/features.h | 2 + .../autofill_assistant/browser/field_formatter.cc | 128 +++ .../autofill_assistant/browser/field_formatter.h | 37 + .../browser/field_formatter_unittest.cc | 209 +++++ .../autofill_assistant/browser/generic_ui.proto | 81 +- .../browser/generic_ui_java_generated_enums.h | 20 + .../autofill_assistant/browser/metrics.h | 20 +- .../autofill_assistant/browser/mock_client.h | 1 + .../browser/mock_controller_observer.h | 5 + .../autofill_assistant/browser/model.proto | 12 +- .../browser/radio_button_controller.cc | 44 + .../browser/radio_button_controller.h | 49 ++ .../browser/radio_button_controller_unittest.cc | 77 ++ .../autofill_assistant/browser/script_executor.cc | 52 +- .../autofill_assistant/browser/script_executor.h | 16 +- .../browser/script_executor_delegate.h | 15 +- .../browser/script_executor_unittest.cc | 109 ++- .../browser/script_precondition.cc | 1 + .../browser/script_precondition_unittest.cc | 7 +- .../browser/script_tracker_unittest.cc | 13 +- .../autofill_assistant/browser/selector.cc | 409 ++++++--- .../autofill_assistant/browser/selector.h | 105 ++- .../browser/selector_unittest.cc | 181 +++- .../autofill_assistant/browser/service.proto | 392 ++++++--- .../autofill_assistant/browser/service_impl.cc | 11 + .../autofill_assistant/browser/service_impl.h | 8 +- .../browser/service_impl_unittest.cc | 21 + .../autofill_assistant/browser/trigger_context.cc | 20 + .../autofill_assistant/browser/trigger_context.h | 16 +- .../browser/trigger_context_unittest.cc | 50 +- .../autofill_assistant/browser/ui_delegate.h | 13 + .../autofill_assistant/browser/user_data_util.cc | 101 +++ .../autofill_assistant/browser/user_data_util.h | 12 + .../browser/user_data_util_unittest.cc | 294 +++++-- .../autofill_assistant/browser/user_model.cc | 36 +- .../autofill_assistant/browser/user_model.h | 18 +- .../browser/user_model_unittest.cc | 71 ++ .../autofill_assistant/browser/value_util.cc | 31 +- .../autofill_assistant/browser/value_util.h | 6 - .../browser/value_util_unittest.cc | 37 +- .../autofill_assistant/browser/view_layout.proto | 21 + .../browser/web/element_finder.cc | 929 +++++++++++++++++---- .../browser/web/element_finder.h | 286 ++++++- .../browser/web/element_position_getter.cc | 9 +- .../browser/web/element_rect_getter.cc | 1 + .../browser/web/web_controller.cc | 38 +- .../browser/web/web_controller_browsertest.cc | 759 ++++++++++++----- .../browser/website_login_manager_impl.cc | 17 +- 101 files changed, 5968 insertions(+), 2309 deletions(-) delete mode 100644 chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.cc delete mode 100644 chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h create mode 100644 chromium/components/autofill_assistant/browser/actions/show_progress_bar_action_unittest.cc create mode 100644 chromium/components/autofill_assistant/browser/field_formatter.cc create mode 100644 chromium/components/autofill_assistant/browser/field_formatter.h create mode 100644 chromium/components/autofill_assistant/browser/field_formatter_unittest.cc create mode 100644 chromium/components/autofill_assistant/browser/radio_button_controller.cc create mode 100644 chromium/components/autofill_assistant/browser/radio_button_controller.h create mode 100644 chromium/components/autofill_assistant/browser/radio_button_controller_unittest.cc (limited to 'chromium/components/autofill_assistant/browser') diff --git a/chromium/components/autofill_assistant/browser/BUILD.gn b/chromium/components/autofill_assistant/browser/BUILD.gn index 5d059ccea71..803c0e3f8bc 100644 --- a/chromium/components/autofill_assistant/browser/BUILD.gn +++ b/chromium/components/autofill_assistant/browser/BUILD.gn @@ -32,8 +32,6 @@ jumbo_static_library("browser") { "actions/configure_bottom_sheet_action.h", "actions/expect_navigation_action.cc", "actions/expect_navigation_action.h", - "actions/fallback_handler/fallback_data.cc", - "actions/fallback_handler/fallback_data.h", "actions/fallback_handler/required_field.cc", "actions/fallback_handler/required_field.h", "actions/fallback_handler/required_fields_fallback_handler.cc", @@ -115,6 +113,8 @@ jumbo_static_library("browser") { "event_handler.h", "features.cc", "features.h", + "field_formatter.cc", + "field_formatter.h", "generic_ui_java_generated_enums.h", "info_box.cc", "info_box.h", @@ -123,6 +123,8 @@ jumbo_static_library("browser") { "overlay_state.h", "protocol_utils.cc", "protocol_utils.h", + "radio_button_controller.cc", + "radio_button_controller.h", "rectf.cc", "rectf.h", "retry_timer.cc", @@ -190,6 +192,7 @@ jumbo_static_library("browser") { "//components/autofill_assistant/browser/devtools", "//components/google/core/common:common", "//components/password_manager/core/browser:browser", + "//components/password_manager/core/browser/form_parsing:form_parsing", "//components/signin/public/identity_manager", "//components/strings:components_strings_grit", "//components/version_info", @@ -248,6 +251,7 @@ source_set("unit_tests") { "actions/set_form_field_value_action_unittest.cc", "actions/show_details_action_unittest.cc", "actions/show_generic_ui_action_unittest.cc", + "actions/show_progress_bar_action_unittest.cc", "actions/tell_action_unittest.cc", "actions/use_address_action_unittest.cc", "actions/use_credit_card_action_unittest.cc", @@ -260,7 +264,9 @@ source_set("unit_tests") { "element_area_unittest.cc", "element_precondition_unittest.cc", "event_handler_unittest.cc", + "field_formatter_unittest.cc", "protocol_utils_unittest.cc", + "radio_button_controller_unittest.cc", "retry_timer_unittest.cc", "script_executor_unittest.cc", "script_precondition_unittest.cc", diff --git a/chromium/components/autofill_assistant/browser/actions/action_delegate.h b/chromium/components/autofill_assistant/browser/actions/action_delegate.h index 2998d21d193..2bb9e13af86 100644 --- a/chromium/components/autofill_assistant/browser/actions/action_delegate.h +++ b/chromium/components/autofill_assistant/browser/actions/action_delegate.h @@ -134,17 +134,14 @@ class ActionDelegate { base::OnceCallback write_callback) = 0; - // Executes |write_callback| on the currently stored user_model. - virtual void WriteUserModel( - base::OnceCallback write_callback) = 0; - using GetFullCardCallback = base::OnceCallback card, const base::string16& cvc)>; - // Asks for the full card information for the selected card. Might require the + // Asks for the full card information for |credit_card|. Might require the // user entering CVC. - virtual void GetFullCard(GetFullCardCallback callback) = 0; + virtual void GetFullCard(const autofill::CreditCard* credit_card, + GetFullCardCallback callback) = 0; // Fill the address form given by |selector| with the given address // |profile|. |profile| cannot be nullptr. @@ -316,9 +313,17 @@ class ActionDelegate { // Show the progress bar and set it at |progress|%. virtual void SetProgress(int progress) = 0; + // Show the progress bar and set the |active_step| to active. + virtual void SetProgressActiveStep(int active_step) = 0; + // Shows the progress bar when |visible| is true. Hides it when false. virtual void SetProgressVisible(bool visible) = 0; + // Sets a new step progress bar configuration. + virtual void SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& + configuration) = 0; + // Set the viewport mode. virtual void SetViewportMode(ViewportMode mode) = 0; @@ -363,14 +368,19 @@ class ActionDelegate { // Gets the user data. virtual const UserData* GetUserData() const = 0; + // Access to the user model. + virtual UserModel* GetUserModel() = 0; + // Show |generic_ui| to the user and call |end_action_callback| when done. // Note that this callback needs to be tied to one or multiple interactions // specified in |generic_ui|, as otherwise it will never be called. + // |view_inflation_finished_callback| should be called immediately after + // view inflation, with a status indicating whether view inflation succeeded. virtual void SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback end_action_callback) = 0; + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) = 0; // Clears the generic UI. This will remove all corresponding views from the // view hierarchy and remove all corresponding interactions. Note that diff --git a/chromium/components/autofill_assistant/browser/actions/collect_user_data_action.cc b/chromium/components/autofill_assistant/browser/actions/collect_user_data_action.cc index f8b2add2361..43dc6724748 100644 --- a/chromium/components/autofill_assistant/browser/actions/collect_user_data_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/collect_user_data_action.cc @@ -85,8 +85,6 @@ bool OnlyLoginRequested( collect_user_data_options.additional_appended_sections.end(), find_additional_input_sections) != collect_user_data_options.additional_appended_sections.end(); - LOG(ERROR) << "HAS_INPUT_SECTIONS: " << has_input_sections; - return !has_input_sections && !collect_user_data_options.request_payer_name && !collect_user_data_options.request_payer_email && !collect_user_data_options.request_payer_phone && @@ -98,107 +96,6 @@ bool OnlyLoginRequested( .has_value(); } -bool IsCompleteContact( - const autofill::AutofillProfile* profile, - const CollectUserDataOptions& collect_user_data_options) { - if (!collect_user_data_options.request_payer_name && - !collect_user_data_options.request_payer_email && - !collect_user_data_options.request_payer_phone) { - return true; - } - - if (!profile) { - return false; - } - - if (collect_user_data_options.request_payer_name && - !profile->HasInfo(autofill::NAME_FULL)) { - return false; - } - - if (collect_user_data_options.request_payer_email && - !profile->HasInfo(autofill::EMAIL_ADDRESS)) { - return false; - } - - if (collect_user_data_options.request_payer_phone && - !profile->HasInfo(autofill::PHONE_HOME_WHOLE_NUMBER)) { - return false; - } - return true; -} - -bool IsCompleteAddress(const autofill::AutofillProfile* profile, - bool require_postal_code) { - if (!profile) { - return false; - } - // We use a hard coded locale here since it's not used in the autofill:: code - // anyway. I.e. creating this profile ends up in FormGroup::GetInfoImpl, which - // simply ignores the app_locale. - auto address_data = - autofill::i18n::CreateAddressDataFromAutofillProfile(*profile, "en-US"); - if (!autofill::addressinput::HasAllRequiredFields(*address_data)) { - return false; - } - - if (require_postal_code && address_data->postal_code.empty()) { - return false; - } - - return true; -} - -bool IsCompleteShippingAddress( - const autofill::AutofillProfile* profile, - const CollectUserDataOptions& collect_user_data_options) { - return !collect_user_data_options.request_shipping || - IsCompleteAddress(profile, /* require_postal_code = */ false); -} - -bool IsCompleteCreditCard( - const autofill::CreditCard* credit_card, - const autofill::AutofillProfile* billing_profile, - const CollectUserDataOptions& collect_user_data_options) { - if (!collect_user_data_options.request_payment_method) { - return true; - } - - if (!credit_card || !billing_profile) { - return false; - } - - if (!IsCompleteAddress( - billing_profile, - collect_user_data_options.require_billing_postal_code)) { - return false; - } - - if (credit_card->record_type() != autofill::CreditCard::MASKED_SERVER_CARD && - !credit_card->HasValidCardNumber()) { - // Can't check validity of masked server card numbers because they are - // incomplete until decrypted. - return false; - } - - if (!credit_card->HasValidExpirationDate() || - credit_card->billing_address_id().empty()) { - return false; - } - - std::string basic_card_network = - autofill::data_util::GetPaymentRequestData(credit_card->network()) - .basic_card_issuer_network; - if (!collect_user_data_options.supported_basic_card_networks.empty() && - std::find(collect_user_data_options.supported_basic_card_networks.begin(), - collect_user_data_options.supported_basic_card_networks.end(), - basic_card_network) == - collect_user_data_options.supported_basic_card_networks.end()) { - return false; - } - return true; -} - bool IsValidLoginChoice( const std::string& choice_identifier, const CollectUserDataOptions& collect_user_data_options) { @@ -535,7 +432,6 @@ void CollectUserDataAction::InternalProcessAction( ProcessActionCallback callback) { callback_ = std::move(callback); if (!CreateOptionsFromProto()) { - LOG(ERROR) << "INVALID"; EndAction(ClientStatus(INVALID_ACTION)); return; } @@ -975,17 +871,16 @@ bool CollectUserDataAction::CreateOptionsFromProto() { collect_user_data.additional_model_identifier_to_check(); } - // TODO(crbug.com/806868): Maybe we could refactor this to make the confirm - // chip and direct_action part of the additional_actions. - std::string confirm_text = collect_user_data.confirm_button_text(); - if (confirm_text.empty()) { - confirm_text = - l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_PAYMENT_INFO_CONFIRM); + auto* confirm_chip = + collect_user_data_options_->confirm_action.mutable_chip(); + if (collect_user_data.has_confirm_chip()) { + *confirm_chip = collect_user_data.confirm_chip(); + } else { + confirm_chip->set_text( + l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_PAYMENT_INFO_CONFIRM)); + confirm_chip->set_type(HIGHLIGHTED_ACTION); } - collect_user_data_options_->confirm_action.mutable_chip()->set_text( - confirm_text); - collect_user_data_options_->confirm_action.mutable_chip()->set_type( - HIGHLIGHTED_ACTION); + *collect_user_data_options_->confirm_action.mutable_direct_action() = collect_user_data.confirm_direct_action(); diff --git a/chromium/components/autofill_assistant/browser/actions/collect_user_data_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/collect_user_data_action_unittest.cc index 42f3aec6dd2..894d0f751fe 100644 --- a/chromium/components/autofill_assistant/browser/actions/collect_user_data_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/collect_user_data_action_unittest.cc @@ -2286,5 +2286,60 @@ TEST_F(CollectUserDataActionTest, LinkClickWritesPartialUserData) { action.ProcessAction(callback_.Get()); } +TEST_F(CollectUserDataActionTest, ConfirmButtonChip) { + ActionProto action_proto; + auto* collect_user_data_proto = action_proto.mutable_collect_user_data(); + collect_user_data_proto->set_request_terms_and_conditions(false); + auto* confirm_chip = collect_user_data_proto->mutable_confirm_chip(); + confirm_chip->set_text("Custom text"); + confirm_chip->set_icon(ICON_CLEAR); + confirm_chip->set_sticky(true); + + ON_CALL(mock_action_delegate_, CollectUserData(_)) + .WillByDefault( + Invoke([this](CollectUserDataOptions* collect_user_data_options) { + EXPECT_EQ(collect_user_data_options->confirm_action.chip().text(), + "Custom text"); + EXPECT_EQ(collect_user_data_options->confirm_action.chip().icon(), + ICON_CLEAR); + EXPECT_EQ(collect_user_data_options->confirm_action.chip().sticky(), + true); + user_data_.succeed_ = true; + std::move(collect_user_data_options->confirm_callback) + .Run(&user_data_, &user_model_); + })); + + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + + CollectUserDataAction action(&mock_action_delegate_, action_proto); + action.ProcessAction(callback_.Get()); +} + +TEST_F(CollectUserDataActionTest, ConfirmButtonFallbackText) { + ActionProto action_proto; + auto* collect_user_data_proto = action_proto.mutable_collect_user_data(); + collect_user_data_proto->set_request_terms_and_conditions(false); + + ON_CALL(mock_action_delegate_, CollectUserData(_)) + .WillByDefault( + Invoke([this](CollectUserDataOptions* collect_user_data_options) { + EXPECT_EQ(collect_user_data_options->confirm_action.chip().text(), + l10n_util::GetStringUTF8( + IDS_AUTOFILL_ASSISTANT_PAYMENT_INFO_CONFIRM)); + user_data_.succeed_ = true; + std::move(collect_user_data_options->confirm_callback) + .Run(&user_data_, &user_model_); + })); + + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + + CollectUserDataAction action(&mock_action_delegate_, action_proto); + action.ProcessAction(callback_.Get()); +} + } // namespace } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.cc b/chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.cc deleted file mode 100644 index f0e08b1e03d..00000000000 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.cc +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2020 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" - -#include "base/optional.h" -#include "base/strings/string_number_conversions.h" -#include "base/strings/utf_string_conversions.h" -#include "components/autofill/core/browser/autofill_type.h" -#include "components/autofill/core/browser/data_model/form_group.h" -#include "third_party/re2/src/re2/re2.h" -#include "third_party/re2/src/re2/stringpiece.h" - -namespace autofill_assistant { -namespace { - -// Matches a single $ followed by an opening {, then an inner key and a closing -// }. -const char kKeyPattern[] = R"re(\$\{([^}]+)\})re"; - -} // namespace - -FallbackData::FallbackData() = default; - -FallbackData::~FallbackData() = default; - -void FallbackData::AddFormGroup(const autofill::FormGroup& form_group) { - autofill::ServerFieldTypeSet available_fields; - form_group.GetNonEmptyTypes("en-US", &available_fields); - for (const auto& field : available_fields) { - field_values.emplace(static_cast(field), - base::UTF16ToUTF8(form_group.GetInfo( - autofill::AutofillType(field), "en-US"))); - } -} - -base::Optional FallbackData::GetValue(int key) { - auto it = field_values.find(key); - if (it != field_values.end() && !it->second.empty()) { - return it->second; - } - return base::nullopt; -} - -base::Optional FallbackData::GetValueByExpression( - const std::string& value_expression) { - int key; - if (base::StringToInt(value_expression, &key)) { - return GetValue(key); - } - - std::string out = value_expression; - - re2::StringPiece input(value_expression); - while (re2::RE2::FindAndConsume(&input, kKeyPattern, &key)) { - auto rewrite_value = GetValue(key); - if (!rewrite_value.has_value()) { - DVLOG(3) << "No value for " << key << " in " << value_expression; - return base::nullopt; - } - - re2::RE2::Replace(&out, kKeyPattern, - re2::StringPiece(rewrite_value.value())); - } - - if (out.empty()) { - return base::nullopt; - } - return out; -} - -} // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h b/chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h deleted file mode 100644 index 5deb5111876..00000000000 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2020 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_FALLBACK_HANDLER_FALLBACK_DATA_H_ -#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_FALLBACK_HANDLER_FALLBACK_DATA_H_ - -#include - -#include "base/optional.h" -#include "components/autofill/core/browser/data_model/form_group.h" - -namespace autofill_assistant { - -// Data necessary for filling in the fallback fields. This is kept in a -// separate struct to make sure we don't keep it for longer than strictly -// necessary. -struct FallbackData { - public: - FallbackData(); - ~FallbackData(); - - FallbackData(const FallbackData&) = delete; - FallbackData& operator=(const FallbackData&) = delete; - - // The key of the map. Should be either an entry of field_types.h or an - // enum of Use*Action::AutofillAssistantCustomField. - std::map field_values; - - void AddFormGroup(const autofill::FormGroup& form_group); - - base::Optional GetValueByExpression( - const std::string& value_expression); - - private: - base::Optional GetValue(int key); -}; - -} // namespace autofill_assistant -#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_FALLBACK_HANDLER_FALLBACK_DATA_H_ diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.cc b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.cc index d22104117e8..1a12d5b8696 100644 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.cc +++ b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.cc @@ -12,40 +12,13 @@ RequiredField::~RequiredField() = default; RequiredField::RequiredField(const RequiredField& copy) = default; -void RequiredField::FromProto( - const UseAddressProto::RequiredField& required_field_proto) { - value_expression = required_field_proto.value_expression(); - selector = Selector(required_field_proto.element()); - fill_strategy = required_field_proto.fill_strategy(); - select_strategy = required_field_proto.select_strategy(); - delay_in_millisecond = required_field_proto.delay_in_millisecond(); - if (required_field_proto.has_option_element_to_click()) { - fallback_click_element = - Selector(required_field_proto.option_element_to_click()); - click_type = required_field_proto.click_type(); - } - forced = required_field_proto.forced(); -} - -void RequiredField::FromProto( - const UseCreditCardProto::RequiredField& required_field_proto) { - value_expression = required_field_proto.value_expression(); - selector = Selector(required_field_proto.element()); - fill_strategy = required_field_proto.fill_strategy(); - select_strategy = required_field_proto.select_strategy(); - delay_in_millisecond = required_field_proto.delay_in_millisecond(); - if (required_field_proto.has_option_element_to_click()) { - fallback_click_element = - Selector(required_field_proto.option_element_to_click()); - click_type = required_field_proto.click_type(); - } - forced = required_field_proto.forced(); -} - -bool RequiredField::ShouldFallback(bool has_fallback_data) const { - return (status == EMPTY && !fallback_click_element.has_value()) || - (forced && has_fallback_data) || - (fallback_click_element.has_value() && has_fallback_data); +bool RequiredField::ShouldFallback(bool apply_fallback) const { + return (status == EMPTY && !value_expression.empty() && + !fallback_click_element.has_value()) || + (status != EMPTY && value_expression.empty() && + !fallback_click_element.has_value()) || + (forced && apply_fallback) || + (fallback_click_element.has_value() && apply_fallback); } } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.h b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.h index 97026813ae4..f5713c2ff03 100644 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.h +++ b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_field.h @@ -16,12 +16,25 @@ struct RequiredField { public: enum FieldValueStatus { UNKNOWN, EMPTY, NOT_EMPTY }; - RequiredField(); ~RequiredField(); + RequiredField(); RequiredField(const RequiredField& copy); - void FromProto(const UseAddressProto::RequiredField& proto); - void FromProto(const UseCreditCardProto::RequiredField& proto); + template + void FromProto(const T& required_field_proto) { + selector = Selector(required_field_proto.element()); + value_expression = required_field_proto.value_expression(); + forced = required_field_proto.forced(); + fill_strategy = required_field_proto.fill_strategy(); + delay_in_millisecond = required_field_proto.delay_in_millisecond(); + select_strategy = required_field_proto.select_strategy(); + + if (required_field_proto.has_option_element_to_click()) { + fallback_click_element = + Selector(required_field_proto.option_element_to_click()); + click_type = required_field_proto.click_type(); + } + } // The selector of the field that must be filled. Selector selector; @@ -60,7 +73,7 @@ struct RequiredField { ClickType click_type = ClickType::NOT_SET; // Returns true if fallback is required for this field. - bool ShouldFallback(bool has_fallback_data) const; + bool ShouldFallback(bool apply_fallback) const; }; } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.cc b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.cc index bf8eeaab150..1b127e12dd1 100644 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.cc +++ b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.cc @@ -13,6 +13,7 @@ #include "components/autofill_assistant/browser/actions/action_delegate.h" #include "components/autofill_assistant/browser/batch_element_checker.h" #include "components/autofill_assistant/browser/client_status.h" +#include "components/autofill_assistant/browser/field_formatter.h" namespace autofill_assistant { namespace { @@ -22,11 +23,12 @@ const char kSelectElementTag[] = "SELECT"; AutofillErrorInfoProto::AutofillFieldError* AddAutofillError( const RequiredField& required_field, ClientStatus* client_status) { + client_status->set_proto_status( + ProcessedActionStatusProto::AUTOFILL_INCOMPLETE); auto* field_error = client_status->mutable_details() ->mutable_autofill_error_info() ->add_autofill_field_error(); - *field_error->mutable_field() = - required_field.selector.ToElementReferenceProto(); + *field_error->mutable_field() = required_field.selector.proto; field_error->set_value_expression(required_field.value_expression); return field_error; } @@ -45,25 +47,35 @@ void FillStatusDetailsWithError(const RequiredField& required_field, field_error->set_status(error_status); } -} // namespace +void FillStatusDetailsWithEmptyField(const RequiredField& required_field, + ClientStatus* client_status) { + auto* field_error = AddAutofillError(required_field, client_status); + field_error->set_empty_after_fallback(true); +} -RequiredFieldsFallbackHandler::RequiredFieldsFallbackHandler( - const std::vector& required_fields, - ActionDelegate* action_delegate) { - required_fields_.assign(required_fields.begin(), required_fields.end()); - action_delegate_ = action_delegate; +void FillStatusDetailsWithNotClearedField(const RequiredField& required_field, + ClientStatus* client_status) { + auto* field_error = AddAutofillError(required_field, client_status); + field_error->set_filled_after_clear(true); } +} // namespace + RequiredFieldsFallbackHandler::~RequiredFieldsFallbackHandler() = default; +RequiredFieldsFallbackHandler::RequiredFieldsFallbackHandler( + const std::vector& required_fields, + const std::map& fallback_values, + ActionDelegate* delegate) + : required_fields_(required_fields), + fallback_values_(fallback_values), + action_delegate_(delegate) {} + void RequiredFieldsFallbackHandler::CheckAndFallbackRequiredFields( const ClientStatus& initial_autofill_status, - std::unique_ptr fallback_data, base::OnceCallback&)> status_update_callback) { - DCHECK(fallback_data != nullptr); - client_status_ = initial_autofill_status; status_update_callback_ = std::move(status_update_callback); @@ -78,18 +90,18 @@ void RequiredFieldsFallbackHandler::CheckAndFallbackRequiredFields( return; } - CheckAllRequiredFields(std::move(fallback_data)); + CheckAllRequiredFields(/* apply_fallback = */ true); } void RequiredFieldsFallbackHandler::CheckAllRequiredFields( - std::unique_ptr fallback_data) { + bool apply_fallback) { DCHECK(!batch_element_checker_); batch_element_checker_ = std::make_unique(); for (size_t i = 0; i < required_fields_.size(); i++) { - // First run (with fallback data) we skip checking forced fields, since - // we overwrite them anyway. Second run (without fallback data) forced - // fields should be checked. - if (required_fields_[i].forced && fallback_data != nullptr) { + // First run (with fallback) we skip checking forced fields, since we + // overwrite them anyway. Second run (without fallback) forced fields should + // be checked. + if (required_fields_[i].forced && apply_fallback) { continue; } @@ -109,7 +121,7 @@ void RequiredFieldsFallbackHandler::CheckAllRequiredFields( batch_element_checker_->AddAllDoneCallback( base::BindOnce(&RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone, - weak_ptr_factory_.GetWeakPtr(), std::move(fallback_data))); + weak_ptr_factory_.GetWeakPtr(), apply_fallback)); action_delegate_->RunElementChecks(batch_element_checker_.get()); } @@ -122,15 +134,26 @@ void RequiredFieldsFallbackHandler::OnGetRequiredFieldValue( } void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone( - std::unique_ptr fallback_data) { + bool apply_fallback) { batch_element_checker_.reset(); // We process all fields with an empty value in order to perform the fallback // on all those fields, if any. bool should_fallback = false; for (const RequiredField& required_field : required_fields_) { - if (required_field.ShouldFallback(fallback_data != nullptr)) { + if (required_field.ShouldFallback(apply_fallback)) { should_fallback = true; + if (!apply_fallback) { + if (required_field.value_expression.empty()) { + VLOG(1) << "Field was filled after attempting to clear it: " + << required_field.selector; + FillStatusDetailsWithNotClearedField(required_field, &client_status_); + } else { + VLOG(1) << "Field was empty after applying fallback: " + << required_field.selector; + FillStatusDetailsWithEmptyField(required_field, &client_status_); + } + } break; } } @@ -141,7 +164,7 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone( return; } - if (fallback_data == nullptr) { + if (!apply_fallback) { // Validation failed and we don't want to try the fallback. std::move(status_update_callback_) .Run(ClientStatus(AUTOFILL_INCOMPLETE), client_status_); @@ -152,14 +175,19 @@ 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(/* apply_fallback= */ true)) { continue; } - if (fallback_data->GetValueByExpression(required_field.value_expression) - .has_value()) { + if (required_field.value_expression.empty()) { + has_fallbacks = true; + } else if (field_formatter::FormatString(required_field.value_expression, + fallback_values_) + .has_value()) { has_fallbacks = true; } else { + VLOG(3) << "Field has no fallback data: " << required_field.selector + << " " << required_field.value_expression; FillStatusDetailsWithMissingFallbackData(required_field, &client_status_); } } @@ -170,16 +198,15 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone( } // Set the fallback values and check again. - SetFallbackFieldValuesSequentially(0, std::move(fallback_data)); + SetFallbackFieldValuesSequentially(0); } void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially( - size_t required_fields_index, - std::unique_ptr fallback_data) { + size_t required_fields_index) { // Skip non-empty fields. while (required_fields_index < required_fields_.size() && !required_fields_[required_fields_index].ShouldFallback( - fallback_data != nullptr)) { + /* apply_fallback= */ true)) { required_fields_index++; } @@ -188,18 +215,29 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially( if (required_fields_index >= required_fields_.size()) { DCHECK_EQ(required_fields_index, required_fields_.size()); - return CheckAllRequiredFields(/* fallback_data= */ nullptr); + CheckAllRequiredFields(/* apply_fallback= */ false); + return; } - // Set the next field to its fallback value. + // Treat the next field. const RequiredField& required_field = required_fields_[required_fields_index]; - auto fallback_value = - fallback_data->GetValueByExpression(required_field.value_expression); + + if (required_field.value_expression.empty()) { + action_delegate_->SetFieldValue( + required_field.selector, "", required_field.fill_strategy, + required_field.delay_in_millisecond, + base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue, + weak_ptr_factory_.GetWeakPtr(), required_fields_index)); + return; + } + + auto fallback_value = field_formatter::FormatString( + required_field.value_expression, fallback_values_); if (!fallback_value.has_value()) { VLOG(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)); + SetFallbackFieldValuesSequentially(++required_fields_index); + return; } if (required_field.fallback_click_element.has_value()) { @@ -214,7 +252,7 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially( base::BindOnce( &RequiredFieldsFallbackHandler::OnClickOrTapFallbackElement, weak_ptr_factory_.GetWeakPtr(), fallback_value.value(), - required_fields_index, std::move(fallback_data))); + required_fields_index)); return; } @@ -223,13 +261,12 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially( required_field.selector, base::BindOnce(&RequiredFieldsFallbackHandler::OnGetFallbackFieldTag, weak_ptr_factory_.GetWeakPtr(), fallback_value.value(), - required_fields_index, std::move(fallback_data))); + required_fields_index)); } void RequiredFieldsFallbackHandler::OnGetFallbackFieldTag( const std::string& value, size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& element_tag_status, const std::string& element_tag) { if (!element_tag_status.ok()) { @@ -252,8 +289,7 @@ void RequiredFieldsFallbackHandler::OnGetFallbackFieldTag( action_delegate_->SelectOption( required_field.selector, value, select_strategy, base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue, - weak_ptr_factory_.GetWeakPtr(), required_fields_index, - std::move(fallback_data))); + weak_ptr_factory_.GetWeakPtr(), required_fields_index)); return; } @@ -261,14 +297,12 @@ void RequiredFieldsFallbackHandler::OnGetFallbackFieldTag( required_field.selector, value, required_field.fill_strategy, required_field.delay_in_millisecond, base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue, - weak_ptr_factory_.GetWeakPtr(), required_fields_index, - std::move(fallback_data))); + weak_ptr_factory_.GetWeakPtr(), required_fields_index)); } void RequiredFieldsFallbackHandler::OnClickOrTapFallbackElement( const std::string& value, size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& element_click_status) { const RequiredField& required_field = required_fields_[required_fields_index]; if (!element_click_status.ok()) { @@ -283,21 +317,20 @@ void RequiredFieldsFallbackHandler::OnClickOrTapFallbackElement( DCHECK(required_field.fallback_click_element.has_value()); Selector value_selector = required_field.fallback_click_element.value(); - value_selector.inner_text_pattern = value; - value_selector.must_be_visible = true; + value_selector.MatchingInnerText(value).MustBeVisible(); DVLOG(3) << "Finding option for " << required_field.selector; action_delegate_->ShortWaitForElement( value_selector, base::BindOnce(&RequiredFieldsFallbackHandler::OnShortWaitForElement, weak_ptr_factory_.GetWeakPtr(), value_selector, - required_fields_index, std::move(fallback_data))); + required_fields_index)); } void RequiredFieldsFallbackHandler::OnShortWaitForElement( const Selector& selector_to_click, size_t required_fields_index, - std::unique_ptr fallback_data, + const ClientStatus& find_element_status) { const RequiredField& required_field = required_fields_[required_fields_index]; if (!find_element_status.ok()) { @@ -319,15 +352,16 @@ void RequiredFieldsFallbackHandler::OnShortWaitForElement( action_delegate_->ClickOrTapElement( selector_to_click, click_type, base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue, - weak_ptr_factory_.GetWeakPtr(), required_fields_index, - std::move(fallback_data))); + weak_ptr_factory_.GetWeakPtr(), required_fields_index)); } void RequiredFieldsFallbackHandler::OnSetFallbackFieldValue( size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& set_field_status) { if (!set_field_status.ok()) { + VLOG(1) << "Error setting value for required_field: " + << required_fields_[required_fields_index].selector << " " + << set_field_status; FillStatusDetailsWithError(required_fields_[required_fields_index], set_field_status.proto_status(), &client_status_); @@ -338,8 +372,7 @@ void RequiredFieldsFallbackHandler::OnSetFallbackFieldValue( return; } - SetFallbackFieldValuesSequentially(++required_fields_index, - std::move(fallback_data)); + SetFallbackFieldValuesSequentially(++required_fields_index); } } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h index 9707612510d..23637adc736 100644 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h +++ b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h @@ -16,20 +16,21 @@ #include "base/optional.h" #include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill_assistant/browser/actions/action.h" -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_field.h" #include "components/autofill_assistant/browser/batch_element_checker.h" namespace autofill_assistant { class ClientStatus; -// A handler for required fields and fallback values, used for example in -// UseAddressAction. +// A handler for required fields and fallback values, used by UseAddressAction +// and UseCreditCardAction. class RequiredFieldsFallbackHandler { public: explicit RequiredFieldsFallbackHandler( const std::vector& required_fields, + const std::map& fallback_values, ActionDelegate* delegate); + ~RequiredFieldsFallbackHandler(); // Check if there are required fields. If so, verify them and fallback if @@ -37,23 +38,20 @@ class RequiredFieldsFallbackHandler { // action. void CheckAndFallbackRequiredFields( const ClientStatus& initial_autofill_status, - std::unique_ptr fallback_data, base::OnceCallback&)> status_update_callback); private: // Check whether all required fields have a non-empty value. If it is the - // case, update the status to success. If it's not and |fallback_data| - // is null, update the status to failure. If |fallback_data| is non-null, use - // it to attempt to fill the failed fields without Autofill. - void CheckAllRequiredFields(std::unique_ptr fallback_data); + // case, update the status to success. If it's not and |apply_fallback| + // is false, update the status to failure. If |apply_fallback| is true, + // attempt to fill the failed fields without Autofill using fallback values. + void CheckAllRequiredFields(bool apply_fallback); // Triggers the check for a specific field. - void CheckRequiredFieldsSequentially( - bool allow_fallback, - size_t required_fields_index, - std::unique_ptr fallback_data); + void CheckRequiredFieldsSequentially(bool allow_fallback, + size_t required_fields_index); // Updates the status of the required field. void OnGetRequiredFieldValue(size_t required_fields_index, @@ -61,40 +59,35 @@ class RequiredFieldsFallbackHandler { const std::string& value); // Called when all required fields have been checked. - void OnCheckRequiredFieldsDone(std::unique_ptr fallback_data); + void OnCheckRequiredFieldsDone(bool apply_fallback); // Sets fallback field values for empty fields. - void SetFallbackFieldValuesSequentially( - size_t required_fields_index, - std::unique_ptr fallback_data); + void SetFallbackFieldValuesSequentially(size_t required_fields_index); // Called after retrieving tag name from a field. void OnGetFallbackFieldTag(const std::string& value, size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& element_tag_status, const std::string& element_tag); // Called after clicking a fallback element. void OnClickOrTapFallbackElement(const std::string& value, size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& element_click_status); // Called after waiting for option element to appear before clicking it. void OnShortWaitForElement(const Selector& selector_to_click, size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& find_element_status); // Called after trying to set form values without Autofill in case of // fallback after failed validation. void OnSetFallbackFieldValue(size_t required_fields_index, - std::unique_ptr fallback_data, const ClientStatus& status); ClientStatus client_status_; std::vector required_fields_; + std::map fallback_values_; base::OnceCallback&)> status_update_callback_; diff --git a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc index 62c9c65a8e4..863135de8b9 100644 --- a/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc @@ -12,10 +12,10 @@ #include "base/test/mock_callback.h" #include "components/autofill/core/browser/autofill_test_utils.h" #include "components/autofill/core/browser/data_model/autofill_profile.h" -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_field.h" #include "components/autofill_assistant/browser/actions/mock_action_delegate.h" #include "components/autofill_assistant/browser/client_status.h" +#include "components/autofill_assistant/browser/service.pb.h" #include "components/autofill_assistant/browser/web/mock_web_controller.h" #include "testing/gmock/include/gmock/gmock.h" @@ -57,12 +57,9 @@ class RequiredFieldsFallbackHandlerTest : public testing::Test { TEST_F(RequiredFieldsFallbackHandlerTest, AutofillFailureExitsEarlyForEmptyRequiredFields) { - std::vector fields; - - auto fallback_data = std::make_unique(); - - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + /* required_fields = */ {}, + /* fallback_values = */ {}, &mock_action_delegate_); base::OnceCallback&)> @@ -74,30 +71,30 @@ TEST_F(RequiredFieldsFallbackHandlerTest, }); fallback_handler.CheckAndFallbackRequiredFields( - ClientStatus(OTHER_ACTION_STATUS), std::move(fallback_data), - std::move(callback)); + ClientStatus(OTHER_ACTION_STATUS), std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, - AddsMissingOrEmptyFallbackFieldToError) { + AddsMissingOrEmptyFallbackValuesToError) { ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "")); - std::vector fields; - fields.emplace_back(CreateRequiredField("51", {"#card_name"})); - fields.emplace_back(CreateRequiredField("52", {"#card_number"})); - fields.emplace_back(CreateRequiredField("-3", {"#card_network"})); + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"}), + CreateRequiredField("${52}", {"#card_number"}), + CreateRequiredField("${-3}", {"#card_network"})}; - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL), + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), "John Doe"); - fallback_data->field_values.emplace( - static_cast(UseCreditCardProto::RequiredField::CREDIT_CARD_NETWORK), - ""); + fallback_values.emplace(base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_NETWORK)), + ""); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -106,17 +103,19 @@ TEST_F(RequiredFieldsFallbackHandlerTest, const base::Optional& detail_status) { EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); ASSERT_TRUE(detail_status.has_value()); + ASSERT_EQ(detail_status.value().proto_status(), + AUTOFILL_INCOMPLETE); ASSERT_EQ(detail_status.value() .details() .autofill_error_info() .autofill_field_error_size(), - 2); + 3); EXPECT_EQ(detail_status.value() .details() .autofill_error_info() .autofill_field_error(0) .value_expression(), - "52"); + "${52}"); EXPECT_TRUE(detail_status.value() .details() .autofill_error_info() @@ -127,16 +126,27 @@ TEST_F(RequiredFieldsFallbackHandlerTest, .autofill_error_info() .autofill_field_error(1) .value_expression(), - "-3"); + "${-3}"); EXPECT_TRUE(detail_status.value() .details() .autofill_error_info() .autofill_field_error(1) .no_fallback_value()); + EXPECT_EQ(detail_status.value() + .details() + .autofill_error_info() + .autofill_field_error(2) + .value_expression(), + "${51}"); + EXPECT_TRUE(detail_status.value() + .details() + .autofill_error_info() + .autofill_field_error(2) + .empty_after_fallback()); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) { @@ -145,20 +155,21 @@ TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) { ON_CALL(mock_action_delegate_, OnSetFieldValue(_, _, _)) .WillByDefault(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS))); - std::vector fields; - fields.emplace_back(CreateRequiredField("51", {"#card_name"})); - fields.emplace_back(CreateRequiredField("52", {"#card_number"})); + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"}), + CreateRequiredField("${52}", {"#card_number"})}; - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL), + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), "John Doe"); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_NUMBER), - "4111111111111111"); + fallback_values.emplace(base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_NUMBER)), + "4111111111111111"); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -167,6 +178,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) { const base::Optional& detail_status) { EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); ASSERT_TRUE(detail_status.has_value()); + ASSERT_EQ(detail_status.value().proto_status(), + AUTOFILL_INCOMPLETE); ASSERT_EQ(detail_status.value() .details() .autofill_error_info() @@ -177,7 +190,7 @@ TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) { .autofill_error_info() .autofill_field_error(0) .value_expression(), - "51"); + "${51}"); EXPECT_EQ(detail_status.value() .details() .autofill_error_info() @@ -186,8 +199,60 @@ TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) { OTHER_ACTION_STATUS); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); +} + +TEST_F(RequiredFieldsFallbackHandlerTest, + AddsFirstEmptyFieldAfterFillingToError) { + ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "")); + + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"}), + CreateRequiredField("${52}", {"#card_number"})}; + + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), + "John Doe"); + fallback_values.emplace(base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_NUMBER)), + "4111111111111111"); + + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); + + base::OnceCallback&)> + callback = + base::BindOnce([](const ClientStatus& status, + const base::Optional& detail_status) { + EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); + ASSERT_TRUE(detail_status.has_value()); + ASSERT_EQ(detail_status.value().proto_status(), + AUTOFILL_INCOMPLETE); + ASSERT_EQ(detail_status.value() + .details() + .autofill_error_info() + .autofill_field_error_size(), + 1); + EXPECT_EQ(detail_status.value() + .details() + .autofill_error_info() + .autofill_field_error(0) + .value_expression(), + "${51}"); + EXPECT_TRUE(detail_status.value() + .details() + .autofill_error_info() + .autofill_field_error(0) + .empty_after_fallback()); + }); + + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, DoesNotFallbackIfFieldsAreFilled) { @@ -195,12 +260,10 @@ TEST_F(RequiredFieldsFallbackHandlerTest, DoesNotFallbackIfFieldsAreFilled) { .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "value")); EXPECT_CALL(mock_action_delegate_, OnSetFieldValue(_, _, _)).Times(0); - std::vector fields; - fields.emplace_back(CreateRequiredField("51", {"#card_name"})); - - auto fallback_data = std::make_unique(); + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"})}; - RequiredFieldsFallbackHandler fallback_handler(fields, + RequiredFieldsFallbackHandler fallback_handler(required_fields, {}, &mock_action_delegate_); base::OnceCallback(OkClientStatus(), "John Doe")); - std::vector fields; - fields.emplace_back(CreateRequiredField("51", {"#card_name"})); + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"})}; - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL), + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), "John Doe"); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -245,8 +309,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FillsEmptyRequiredField) { EXPECT_EQ(status.proto_status(), ACTION_APPLIED); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, FallsBackForForcedFilledField) { @@ -256,18 +320,18 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FallsBackForForcedFilledField) { OnSetFieldValue(Eq(Selector({"#card_name"})), "John Doe", _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); - std::vector fields; - auto field = CreateRequiredField("51", {"#card_name"}); - field.forced = true; - fields.emplace_back(field); + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"})}; + required_fields[0].forced = true; - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL), + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), "John Doe"); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -277,8 +341,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FallsBackForForcedFilledField) { EXPECT_EQ(status.proto_status(), ACTION_APPLIED); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, FailsIfForcedFieldDidNotGetFilled) { @@ -286,14 +350,11 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FailsIfForcedFieldDidNotGetFilled) { .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "value")); EXPECT_CALL(mock_action_delegate_, OnSetFieldValue(_, _, _)).Times(0); - std::vector fields; - auto field = CreateRequiredField("51", {"#card_name"}); - field.forced = true; - fields.emplace_back(field); + std::vector required_fields = { + CreateRequiredField("${51}", {"#card_name"})}; + required_fields[0].forced = true; - auto fallback_data = std::make_unique(); - - RequiredFieldsFallbackHandler fallback_handler(fields, + RequiredFieldsFallbackHandler fallback_handler(required_fields, {}, &mock_action_delegate_); base::OnceCallback& detail_status) { EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); ASSERT_TRUE(detail_status.has_value()); + ASSERT_EQ(detail_status.value().proto_status(), + AUTOFILL_INCOMPLETE); ASSERT_EQ(detail_status.value() .details() .autofill_error_info() @@ -313,7 +376,7 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FailsIfForcedFieldDidNotGetFilled) { .autofill_error_info() .autofill_field_error(0) .value_expression(), - "51"); + "${51}"); EXPECT_TRUE(detail_status.value() .details() .autofill_error_info() @@ -321,8 +384,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FailsIfForcedFieldDidNotGetFilled) { .no_fallback_value()); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, FillsFieldWithPattern) { @@ -336,18 +399,21 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FillsFieldWithPattern) { .After(set_value) .WillOnce(RunOnceCallback<1>(OkClientStatus(), "not empty")); - std::vector fields; - fields.emplace_back(CreateRequiredField("${53}/${55}", {"#card_expiry"})); - - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH), "08"); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_4_DIGIT_YEAR), + std::vector required_fields = { + CreateRequiredField("${53}/${55}", {"#card_expiry"})}; + + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH)), + "08"); + fallback_values.emplace( + base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_EXP_4_DIGIT_YEAR)), "2050"); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -357,8 +423,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FillsFieldWithPattern) { EXPECT_EQ(status.proto_status(), ACTION_APPLIED); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, @@ -368,17 +434,17 @@ TEST_F(RequiredFieldsFallbackHandlerTest, .WillRepeatedly(RunOnceCallback<1>(OkClientStatus(), "")); EXPECT_CALL(mock_action_delegate_, OnSetFieldValue(_, _, _)).Times(0); - std::vector fields; - fields.emplace_back(CreateRequiredField("53", {"#card_expiry"})); - fields.emplace_back(CreateRequiredField("-3", {"#card_network"})); + std::vector required_fields = { + CreateRequiredField("${53}", {"#card_expiry"}), + CreateRequiredField("${-3}", {"#card_network"})}; - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(UseCreditCardProto::RequiredField::CREDIT_CARD_NETWORK), - ""); + std::map fallback_values; + fallback_values.emplace(base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_NETWORK)), + ""); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -387,6 +453,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, const base::Optional& detail_status) { EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); ASSERT_TRUE(detail_status.has_value()); + ASSERT_EQ(detail_status.value().proto_status(), + AUTOFILL_INCOMPLETE); ASSERT_EQ(detail_status.value() .details() .autofill_error_info() @@ -397,7 +465,7 @@ TEST_F(RequiredFieldsFallbackHandlerTest, .autofill_error_info() .autofill_field_error(0) .value_expression(), - "53"); + "${53}"); EXPECT_TRUE(detail_status.value() .details() .autofill_error_info() @@ -408,7 +476,7 @@ TEST_F(RequiredFieldsFallbackHandlerTest, .autofill_error_info() .autofill_field_error(1) .value_expression(), - "-3"); + "${-3}"); EXPECT_TRUE(detail_status.value() .details() .autofill_error_info() @@ -416,114 +484,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, .no_fallback_value()); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); -} - -TEST_F(RequiredFieldsFallbackHandlerTest, IgnoresNonIntegerKeys) { - EXPECT_CALL(mock_web_controller_, OnGetFieldValue(_, _)) - .WillOnce(RunOnceCallback<1>(OkClientStatus(), "")); - Expectation set_value = - EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Eq(Selector({"#card_expiry"})), "${KEY}", _)) - .WillOnce(RunOnceCallback<2>(OkClientStatus())); - EXPECT_CALL(mock_web_controller_, OnGetFieldValue(_, _)) - .After(set_value) - .WillOnce(RunOnceCallback<1>(OkClientStatus(), "not empty")); - - std::vector fields; - fields.emplace_back(CreateRequiredField("${KEY}", {"#card_expiry"})); - - auto fallback_data = std::make_unique(); - - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); - - base::OnceCallback&)> - callback = - base::BindOnce([](const ClientStatus& status, - const base::Optional& detail_status) { - EXPECT_EQ(status.proto_status(), ACTION_APPLIED); - }); - - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); -} - -TEST_F(RequiredFieldsFallbackHandlerTest, AddsAllProfileFields) { - std::unordered_map expected_values = { - {3, "Alpha"}, - {4, "Beta"}, - {5, "Gamma"}, - {6, "B"}, - {7, "Alpha Beta Gamma"}, - {9, "alpha@google.com"}, - {10, "1234567"}, - {11, "79"}, - {12, "41"}, - {13, "0791234567"}, - {14, "+41791234567"}, - {30, "Brandschenkestrasse 110"}, - {31, "Google Building 110"}, - {33, "Zurich"}, - {34, "Canton Zurich"}, - {35, "8002"}, - {36, "Switzerland"}, - {60, "Google"}, - {77, "Brandschenkestrasse 110\nGoogle Building 110"}}; - - auto profile = std::make_unique( - base::GenerateGUID(), autofill::test::kEmptyOrigin); - autofill::test::SetProfileInfo( - profile.get(), "Alpha", "Beta", "Gamma", "alpha@google.com", "Google", - "Brandschenkestrasse 110", "Google Building 110", "Zurich", - "Canton Zurich", "8002", "CH", "+41791234567"); - FallbackData fallback_data; - fallback_data.AddFormGroup(*profile); - - for (auto iter = expected_values.begin(); iter != expected_values.end(); - ++iter) { - ASSERT_TRUE( - fallback_data.GetValueByExpression(base::NumberToString(iter->first)) - .has_value()); - EXPECT_EQ( - fallback_data.GetValueByExpression(base::NumberToString(iter->first)) - .value(), - iter->second); - } -} - -TEST_F(RequiredFieldsFallbackHandlerTest, AddsAllCreditCardFields) { - std::unordered_map expected_values = { - {51, "Alpha Beta Gamma"}, - {52, "4111111111111111"}, - {53, "08"}, - {54, "50"}, - {55, "2050"}, - {56, "08/50"}, - {57, "08/2050"}, - {58, "Visa"}, - {91, "Alpha"}, - {92, "Gamma"}}; - - auto card = std::make_unique( - base::GenerateGUID(), autofill::test::kEmptyOrigin); - autofill::test::SetCreditCardInfo(card.get(), "Alpha Beta Gamma", - "4111111111111111", "8", "2050", ""); - FallbackData fallback_data; - fallback_data.AddFormGroup(*card); - - for (auto iter = expected_values.begin(); iter != expected_values.end(); - ++iter) { - ASSERT_TRUE( - fallback_data.GetValueByExpression(base::NumberToString(iter->first)) - .has_value()); - EXPECT_EQ( - fallback_data.GetValueByExpression(base::NumberToString(iter->first)) - .value(), - iter->second); - } + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, ClicksOnCustomDropdown) { @@ -534,8 +496,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, ClicksOnCustomDropdown) { ClickOrTapElement(Eq(Selector({"#card_expiry"})), ClickType::TAP, _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); Selector expected_selector({".option"}); - expected_selector.must_be_visible = true; - expected_selector.inner_text_pattern = "08"; + expected_selector.MatchingInnerText("08"); + expected_selector.MustBeVisible(); EXPECT_CALL(mock_action_delegate_, OnShortWaitForElement(Eq(expected_selector), _)) .WillOnce(RunOnceCallback<1>(OkClientStatus())); @@ -543,17 +505,18 @@ TEST_F(RequiredFieldsFallbackHandlerTest, ClicksOnCustomDropdown) { ClickOrTapElement(Eq(expected_selector), ClickType::TAP, _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); - std::vector fields; - RequiredField required_field = CreateRequiredField("53", {"#card_expiry"}); - required_field.fallback_click_element = Selector({".option"}); - fields.emplace_back(required_field); + std::vector required_fields = { + CreateRequiredField("${53}", {"#card_expiry"})}; + required_fields[0].fallback_click_element = Selector({".option"}); - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH), "08"); + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH)), + "08"); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -563,8 +526,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, ClicksOnCustomDropdown) { EXPECT_EQ(status.proto_status(), ACTION_APPLIED); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } TEST_F(RequiredFieldsFallbackHandlerTest, CustomDropdownClicksStopOnError) { @@ -575,8 +538,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, CustomDropdownClicksStopOnError) { ClickOrTapElement(Eq(Selector({"#card_expiry"})), ClickType::TAP, _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); Selector expected_selector({".option"}); - expected_selector.must_be_visible = true; - expected_selector.inner_text_pattern = "08"; + expected_selector.MatchingInnerText("08"); + expected_selector.MustBeVisible(); EXPECT_CALL(mock_action_delegate_, OnShortWaitForElement(Eq(expected_selector), _)) .WillOnce(RunOnceCallback<1>(ClientStatus(ELEMENT_RESOLUTION_FAILED))); @@ -584,17 +547,18 @@ TEST_F(RequiredFieldsFallbackHandlerTest, CustomDropdownClicksStopOnError) { ClickOrTapElement(Eq(expected_selector), _, _)) .Times(0); - std::vector fields; - RequiredField required_field = CreateRequiredField("53", {"#card_expiry"}); - required_field.fallback_click_element = Selector({".option"}); - fields.emplace_back(required_field); + std::vector required_fields = { + CreateRequiredField("${53}", {"#card_expiry"})}; + required_fields[0].fallback_click_element = Selector({".option"}); - auto fallback_data = std::make_unique(); - fallback_data->field_values.emplace( - static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH), "08"); + std::map fallback_values; + fallback_values.emplace( + base::NumberToString( + static_cast(autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH)), + "08"); - RequiredFieldsFallbackHandler fallback_handler(fields, - &mock_action_delegate_); + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); base::OnceCallback&)> @@ -604,8 +568,53 @@ TEST_F(RequiredFieldsFallbackHandlerTest, CustomDropdownClicksStopOnError) { EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); }); - fallback_handler.CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), std::move(callback)); + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); +} + +TEST_F(RequiredFieldsFallbackHandlerTest, ClearsFilledFields) { + Selector full_field_selector({"#full_field"}); + Selector empty_field_selector({"#empty_field"}); + EXPECT_CALL(mock_web_controller_, OnGetFieldValue(full_field_selector, _)) + .WillOnce(RunOnceCallback<1>(OkClientStatus(), "value")); + EXPECT_CALL(mock_web_controller_, OnGetFieldValue(empty_field_selector, _)) + .Times(0); + + Expectation clear_full_value = + EXPECT_CALL(mock_action_delegate_, + OnSetFieldValue(full_field_selector, "", _)) + .WillOnce(RunOnceCallback<2>(OkClientStatus())); + EXPECT_CALL(mock_web_controller_, OnGetFieldValue(full_field_selector, _)) + .After(clear_full_value) + .WillOnce(RunOnceCallback<1>(OkClientStatus(), "")); + Expectation clear_empty_value = + EXPECT_CALL(mock_action_delegate_, + OnSetFieldValue(empty_field_selector, "", _)) + .WillOnce(RunOnceCallback<2>(OkClientStatus())); + EXPECT_CALL(mock_web_controller_, OnGetFieldValue(empty_field_selector, _)) + .After(clear_empty_value) + .WillOnce(RunOnceCallback<1>(OkClientStatus(), "")); + + auto non_forced_field = CreateRequiredField("", {"#full_field"}); + auto forced_field = CreateRequiredField("", {"#empty_field"}); + forced_field.forced = true; + std::vector required_fields = {non_forced_field, forced_field}; + + std::map fallback_values; + + RequiredFieldsFallbackHandler fallback_handler( + required_fields, fallback_values, &mock_action_delegate_); + + base::OnceCallback&)> + callback = + base::BindOnce([](const ClientStatus& status, + const base::Optional& detail_status) { + EXPECT_EQ(status.proto_status(), ACTION_APPLIED); + }); + + fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(), + std::move(callback)); } } // namespace diff --git a/chromium/components/autofill_assistant/browser/actions/generate_password_for_form_field_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/generate_password_for_form_field_action_unittest.cc index dacb0c3fc80..df974c8433e 100644 --- a/chromium/components/autofill_assistant/browser/actions/generate_password_for_form_field_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/generate_password_for_form_field_action_unittest.cc @@ -62,9 +62,8 @@ class GeneratePasswordForFormFieldActionTest : public testing::Test { TEST_F(GeneratePasswordForFormFieldActionTest, GeneratedPassword) { GeneratePasswordForFormFieldProto* generate_password_proto = proto_.mutable_generate_password_for_form_field(); - generate_password_proto->mutable_element()->add_selectors(kFakeSelector); - generate_password_proto->mutable_element()->set_visibility_requirement( - MUST_BE_VISIBLE); + *generate_password_proto->mutable_element() = + Selector({kFakeSelector}).MustBeVisible().proto; generate_password_proto->set_memory_key(kMemoryKeyForGeneratedPassword); Selector fake_selector = Selector({kFakeSelector}).MustBeVisible(); diff --git a/chromium/components/autofill_assistant/browser/actions/mock_action_delegate.h b/chromium/components/autofill_assistant/browser/actions/mock_action_delegate.h index 22d50591d0d..808d29d18a7 100644 --- a/chromium/components/autofill_assistant/browser/actions/mock_action_delegate.h +++ b/chromium/components/autofill_assistant/browser/actions/mock_action_delegate.h @@ -135,26 +135,18 @@ class MockActionDelegate : public ActionDelegate { MOCK_METHOD1( WriteUserData, void(base::OnceCallback)); - MOCK_METHOD1(WriteUserModel, void(base::OnceCallback)); - - MOCK_METHOD1(OnGetFullCard, - void(base::OnceCallback&)); - - void GetFullCard(GetFullCardCallback callback) override { - // A local variable is necessary to allow OnGetFullCard to get a reference. - base::OnceCallback - transformed_callback = base::BindOnce( - [](GetFullCardCallback real_callback, - const autofill::CreditCard& card, const base::string16& cvc) { - std::move(real_callback) - .Run(std::make_unique(card), cvc); - }, - std::move(callback)); - OnGetFullCard(transformed_callback); + + void GetFullCard(const autofill::CreditCard* credit_card, + ActionDelegate::GetFullCardCallback callback) override { + OnGetFullCard(credit_card, callback); } + MOCK_METHOD2( + OnGetFullCard, + void(const autofill::CreditCard* credit_card, + base::OnceCallback card, + const base::string16& cvc)>& callback)); + void GetFieldValue(const Selector& selector, base::OnceCallback callback) { @@ -238,7 +230,11 @@ class MockActionDelegate : public ActionDelegate { MOCK_METHOD1(SetInfoBox, void(const InfoBox& info_box)); MOCK_METHOD0(ClearInfoBox, void()); MOCK_METHOD1(SetProgress, void(int progress)); + MOCK_METHOD1(SetProgressActiveStep, void(int active_step)); MOCK_METHOD1(SetProgressVisible, void(bool visible)); + MOCK_METHOD1(SetStepProgressBarConfiguration, + void(const ShowProgressBarProto::StepProgressBarConfiguration& + configuration)); MOCK_METHOD1(SetUserActions, void(std::unique_ptr> user_action)); MOCK_METHOD1(SetViewportMode, void(ViewportMode mode)); @@ -294,19 +290,20 @@ class MockActionDelegate : public ActionDelegate { MOCK_METHOD0(RequireUI, void()); MOCK_METHOD0(SetExpandSheetForPromptAction, bool()); - MOCK_METHOD2( + MOCK_METHOD3( OnSetGenericUi, void(std::unique_ptr generic_ui, - base::OnceCallback& end_action_callback)); + base::OnceCallback& end_action_callback, + base::OnceCallback& + view_inflation_finished_callback)); void SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback end_action_callback) override { - OnSetGenericUi(std::move(generic_ui), end_action_callback); + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) override { + OnSetGenericUi(std::move(generic_ui), end_action_callback, + view_inflation_finished_callback); } MOCK_METHOD0(ClearGenericUi, void()); diff --git a/chromium/components/autofill_assistant/browser/actions/navigate_action.cc b/chromium/components/autofill_assistant/browser/actions/navigate_action.cc index 684c694dd34..21773c5dca2 100644 --- a/chromium/components/autofill_assistant/browser/actions/navigate_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/navigate_action.cc @@ -26,16 +26,20 @@ void NavigateAction::InternalProcessAction(ProcessActionCallback callback) { // We know to expect navigation to happen, since we're about to cause it. This // allows scripts to put wait_for_navigation just after navigate, if needed, // without having to add an expect_navigation first. - delegate_->ExpectNavigation(); + // The navigations are not declared as renderer initiated, they can cause + // failures. Setting the navigation to expected will prevent this from + // happening. auto& proto = proto_.navigate(); if (!proto.url().empty()) { + delegate_->ExpectNavigation(); GURL url(proto_.navigate().url()); delegate_->LoadURL(url); UpdateProcessedAction(ACTION_APPLIED); } else if (proto.go_backward()) { auto& controller = delegate_->GetWebContents()->GetController(); if (controller.CanGoBack()) { + delegate_->ExpectNavigation(); controller.GoBack(); UpdateProcessedAction(ACTION_APPLIED); } else { @@ -44,6 +48,7 @@ void NavigateAction::InternalProcessAction(ProcessActionCallback callback) { } else if (proto.go_forward()) { auto& controller = delegate_->GetWebContents()->GetController(); if (controller.CanGoForward()) { + delegate_->ExpectNavigation(); controller.GoForward(); UpdateProcessedAction(ACTION_APPLIED); } else { diff --git a/chromium/components/autofill_assistant/browser/actions/prompt_action.cc b/chromium/components/autofill_assistant/browser/actions/prompt_action.cc index c6f3fde6429..9f1cd9cd746 100644 --- a/chromium/components/autofill_assistant/browser/actions/prompt_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/prompt_action.cc @@ -221,8 +221,8 @@ void PromptAction::OnSuggestionChosen(int choice_index) { } DCHECK(choice_index >= 0 && choice_index <= proto_.prompt().choices_size()); - *processed_action_proto_->mutable_prompt_choice() = - proto_.prompt().choices(choice_index); + processed_action_proto_->mutable_prompt_choice()->set_server_payload( + proto_.prompt().choices(choice_index).server_payload()); EndAction(ClientStatus(ACTION_APPLIED)); } diff --git a/chromium/components/autofill_assistant/browser/actions/prompt_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/prompt_action_unittest.cc index d5455c5bdce..afb7d7a6336 100644 --- a/chromium/components/autofill_assistant/browser/actions/prompt_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/prompt_action_unittest.cc @@ -172,9 +172,9 @@ TEST_F(PromptActionTest, SelectButtons) { Run(Pointee(AllOf( Property(&ProcessedActionProto::status, ACTION_APPLIED), Property(&ProcessedActionProto::prompt_choice, - Property(&PromptProto::Choice::navigation_ended, false)), + Property(&PromptProto::Result::navigation_ended, false)), Property(&ProcessedActionProto::prompt_choice, - Property(&PromptProto::Choice::server_payload, "ok")))))); + Property(&PromptProto::Result::server_payload, "ok")))))); EXPECT_TRUE((*user_actions_)[0].HasCallback()); (*user_actions_)[0].Call(TriggerContext::CreateEmpty()); } @@ -209,7 +209,8 @@ TEST_F(PromptActionTest, ShowOnlyIfElementExists) { ok_proto->mutable_chip()->set_text("Ok"); ok_proto->mutable_chip()->set_type(HIGHLIGHTED_ACTION); ok_proto->set_server_payload("ok"); - ok_proto->mutable_show_only_when()->mutable_match()->add_selectors("element"); + *ok_proto->mutable_show_only_when()->mutable_match() = + ToSelectorProto("element"); PromptAction action(&mock_action_delegate_, proto_); action.ProcessAction(callback_.Get()); @@ -235,7 +236,8 @@ TEST_F(PromptActionTest, DisabledUnlessElementExists) { ok_proto->mutable_chip()->set_type(HIGHLIGHTED_ACTION); ok_proto->set_server_payload("ok"); ok_proto->set_allow_disabling(true); - ok_proto->mutable_show_only_when()->mutable_match()->add_selectors("element"); + *ok_proto->mutable_show_only_when()->mutable_match() = + ToSelectorProto("element"); PromptAction action(&mock_action_delegate_, proto_); action.ProcessAction(callback_.Get()); @@ -259,8 +261,8 @@ TEST_F(PromptActionTest, DisabledUnlessElementExists) { TEST_F(PromptActionTest, AutoSelectWhenElementExists) { auto* choice_proto = prompt_proto_->add_choices(); choice_proto->set_server_payload("auto-select"); - choice_proto->mutable_auto_select_when()->mutable_match()->add_selectors( - "element"); + *choice_proto->mutable_auto_select_when()->mutable_match() = + ToSelectorProto("element"); PromptAction action(&mock_action_delegate_, proto_); action.ProcessAction(callback_.Get()); @@ -275,7 +277,7 @@ TEST_F(PromptActionTest, AutoSelectWhenElementExists) { callback_, Run(Pointee(AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED), Property(&ProcessedActionProto::prompt_choice, - Property(&PromptProto::Choice::server_payload, + Property(&PromptProto::Result::server_payload, "auto-select")))))); task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1)); } @@ -288,8 +290,8 @@ TEST_F(PromptActionTest, AutoSelectWithButton) { auto* choice_proto = prompt_proto_->add_choices(); choice_proto->set_server_payload("auto-select"); - choice_proto->mutable_auto_select_when()->mutable_match()->add_selectors( - "element"); + *choice_proto->mutable_auto_select_when()->mutable_match() = + ToSelectorProto("element"); PromptAction action(&mock_action_delegate_, proto_); action.ProcessAction(callback_.Get()); @@ -303,7 +305,7 @@ TEST_F(PromptActionTest, AutoSelectWithButton) { callback_, Run(Pointee(AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED), Property(&ProcessedActionProto::prompt_choice, - Property(&PromptProto::Choice::server_payload, + Property(&PromptProto::Result::server_payload, "auto-select")))))); task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1)); } @@ -401,8 +403,8 @@ TEST_F(PromptActionTest, ForwardInterruptFailure) { prompt_proto_->set_allow_interrupt(true); auto* choice_proto = prompt_proto_->add_choices(); choice_proto->set_server_payload("auto-select"); - choice_proto->mutable_auto_select_when()->mutable_match()->add_selectors( - "element"); + *choice_proto->mutable_auto_select_when()->mutable_match() = + ToSelectorProto("element"); PromptAction action(&mock_action_delegate_, proto_); action.ProcessAction(callback_.Get()); @@ -419,7 +421,7 @@ TEST_F(PromptActionTest, ForwardInterruptFailure) { Pointee(Property(&ProcessedActionProto::status, INTERRUPT_FAILED)), Pointee( Property(&ProcessedActionProto::prompt_choice, - Property(&PromptProto::Choice::server_payload, "")))))); + Property(&PromptProto::Result::server_payload, "")))))); ASSERT_TRUE(fake_wait_for_dom_done_); std::move(fake_wait_for_dom_done_).Run(ClientStatus(INTERRUPT_FAILED)); } @@ -446,7 +448,7 @@ TEST_F(PromptActionTest, EndActionOnNavigation) { Run(Pointee(AllOf( Property(&ProcessedActionProto::status, ACTION_APPLIED), Property(&ProcessedActionProto::prompt_choice, - Property(&PromptProto::Choice::navigation_ended, true)))))); + Property(&PromptProto::Result::navigation_ended, true)))))); action.ProcessAction(callback_.Get()); } diff --git a/chromium/components/autofill_assistant/browser/actions/set_attribute_action.cc b/chromium/components/autofill_assistant/browser/actions/set_attribute_action.cc index a1a8ceb4088..b49213d0b5e 100644 --- a/chromium/components/autofill_assistant/browser/actions/set_attribute_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/set_attribute_action.cc @@ -16,7 +16,6 @@ namespace autofill_assistant { SetAttributeAction::SetAttributeAction(ActionDelegate* delegate, const ActionProto& proto) : Action(delegate, proto) { - DCHECK_GT(proto_.set_attribute().element().selectors_size(), 0); DCHECK_GT(proto_.set_attribute().attribute_size(), 0); } diff --git a/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action.cc b/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action.cc index c628e837b8f..4e2464531d5 100644 --- a/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action.cc @@ -40,7 +40,6 @@ SetFormFieldValueAction::SetFormFieldValueAction(ActionDelegate* delegate, const ActionProto& proto) : Action(delegate, proto) { DCHECK(proto_.has_set_form_value()); - DCHECK_GT(proto_.set_form_value().element().selectors_size(), 0); DCHECK_GT(proto_.set_form_value().value_size(), 0); } diff --git a/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc index a397b7d5182..d90f43e9cfc 100644 --- a/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc @@ -36,9 +36,8 @@ class SetFormFieldValueActionTest : public testing::Test { public: void SetUp() override { set_form_field_proto_ = proto_.mutable_set_form_value(); - set_form_field_proto_->mutable_element()->add_selectors(kFakeSelector); - set_form_field_proto_->mutable_element()->set_visibility_requirement( - MUST_BE_VISIBLE); + *set_form_field_proto_->mutable_element() = + Selector({kFakeSelector}).MustBeVisible().proto; ON_CALL(mock_action_delegate_, GetUserData) .WillByDefault(Return(&user_data_)); ON_CALL(mock_action_delegate_, WriteUserData) diff --git a/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.cc b/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.cc index 1d239707e18..6f66a26ded7 100644 --- a/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.cc @@ -53,8 +53,8 @@ void WriteProfilesToUserModel( void WriteLoginOptionsToUserModel( const ShowGenericUiProto::RequestLoginOptions& proto, - std::vector logins, - UserModel* user_model) { + UserModel* user_model, + std::vector logins) { DCHECK(user_model); ValueProto model_value; model_value.set_is_client_side_only(true); @@ -83,7 +83,6 @@ void WriteLoginOptionsToUserModel( } user_model->SetValue(proto.model_identifier(), model_value); } - } // namespace ShowGenericUiAction::ShowGenericUiAction(ActionDelegate* delegate, @@ -107,17 +106,34 @@ void ShowGenericUiAction::InternalProcessAction( /* force_notifications = */ false); if (!temp_model.GetValues(proto_.show_generic_ui().output_model_identifiers()) .has_value()) { - EndAction(false, INVALID_ACTION, nullptr); + EndAction(ClientStatus(INVALID_ACTION)); return; } + for (const auto& element_check : + proto_.show_generic_ui().periodic_element_checks().element_checks()) { + if (element_check.model_identifier().empty()) { + VLOG(1) << "Invalid action: ElementCheck with empty model_identifier"; + EndAction(ClientStatus(INVALID_ACTION)); + return; + } + } delegate_->Prompt(/* user_actions = */ nullptr, /* disable_force_expand_sheet = */ false); delegate_->SetGenericUi( std::make_unique( proto_.show_generic_ui().generic_user_interface()), - base::BindOnce(&ShowGenericUiAction::EndAction, + base::BindOnce(&ShowGenericUiAction::OnEndActionInteraction, + weak_ptr_factory_.GetWeakPtr()), + base::BindOnce(&ShowGenericUiAction::OnViewInflationFinished, weak_ptr_factory_.GetWeakPtr())); +} + +void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) { + if (!status.ok()) { + EndAction(status); + return; + } // Note: it is important to write autofill profiles etc. to the model AFTER // the UI has been inflated, otherwise the UI won't get change notifications @@ -133,31 +149,108 @@ void ShowGenericUiAction::InternalProcessAction( }) != login_options.end()) { delegate_->GetWebsiteLoginManager()->GetLoginsForUrl( delegate_->GetWebContents()->GetLastCommittedURL(), - base::BindOnce(&ShowGenericUiAction::OnGetLogins, - weak_ptr_factory_.GetWeakPtr(), - proto_.show_generic_ui().request_login_options())); + base::BindOnce(&WriteLoginOptionsToUserModel, + proto_.show_generic_ui().request_login_options(), + delegate_->GetUserModel())); } else { - delegate_->WriteUserModel(base::BindOnce( - &WriteLoginOptionsToUserModel, + WriteLoginOptionsToUserModel( proto_.show_generic_ui().request_login_options(), - /* logins = */ std::vector())); + delegate_->GetUserModel(), + /* logins = */ std::vector()); } } delegate_->GetPersonalDataManager()->AddObserver(this); OnPersonalDataChanged(); + for (const auto& element_check : + proto_.show_generic_ui().periodic_element_checks().element_checks()) { + preconditions_.emplace_back(std::make_unique( + element_check.element_condition())); + } + if (std::any_of( + preconditions_.begin(), preconditions_.end(), + [&](const auto& precondition) { return !precondition->empty(); })) { + has_pending_wait_for_dom_ = true; + delegate_->WaitForDom( + base::TimeDelta::Max(), false, + base::BindRepeating(&ShowGenericUiAction::RegisterChecks, + weak_ptr_factory_.GetWeakPtr()), + base::BindOnce(&ShowGenericUiAction::OnDoneWaitForDom, + weak_ptr_factory_.GetWeakPtr())); + } } -void ShowGenericUiAction::EndAction(bool view_inflation_successful, - ProcessedActionStatusProto status, - const UserModel* user_model) { +void ShowGenericUiAction::RegisterChecks( + BatchElementChecker* checker, + base::OnceCallback wait_for_dom_callback) { + if (!callback_) { + // Action is done; checks aren't necessary anymore. + std::move(wait_for_dom_callback).Run(OkClientStatus()); + return; + } + + for (size_t i = 0; i < preconditions_.size(); i++) { + preconditions_[i]->Check( + checker, base::BindOnce(&ShowGenericUiAction::OnPreconditionResult, + weak_ptr_factory_.GetWeakPtr(), i)); + } + // Let WaitForDom know we're still waiting for elements. + checker->AddAllDoneCallback(base::BindOnce( + &ShowGenericUiAction::OnElementChecksDone, weak_ptr_factory_.GetWeakPtr(), + std::move(wait_for_dom_callback))); +} + +void ShowGenericUiAction::OnPreconditionResult( + size_t precondition_index, + const ClientStatus& status, + const std::vector& ignored_payloads) { + delegate_->GetUserModel()->SetValue(proto_.show_generic_ui() + .periodic_element_checks() + .element_checks(precondition_index) + .model_identifier(), + SimpleValue(status.ok())); +} + +void ShowGenericUiAction::OnElementChecksDone( + base::OnceCallback wait_for_dom_callback) { + // Calling wait_for_dom_callback with successful status is a way of asking the + // WaitForDom to end gracefully and call OnDoneWaitForDom with the status. + // Note that it is possible for WaitForDom to decide not to call + // OnDoneWaitForDom, if an interrupt triggers at the same time, so we cannot + // cancel the prompt and choose the suggestion just yet. + if (should_end_action_) { + std::move(wait_for_dom_callback).Run(OkClientStatus()); + return; + } + // Let WaitForDom know we're still waiting for an element. + std::move(wait_for_dom_callback).Run(ClientStatus(ELEMENT_RESOLUTION_FAILED)); +} + +void ShowGenericUiAction::OnDoneWaitForDom(const ClientStatus& status) { + if (!callback_) { + return; + } + EndAction(status); +} + +void ShowGenericUiAction::OnEndActionInteraction(const ClientStatus& status) { + // If WaitForDom was called, we end the action the next time the callback + // is called in order to end WaitForDom gracefully. + if (has_pending_wait_for_dom_) { + should_end_action_ = true; + return; + } + EndAction(status); +} + +void ShowGenericUiAction::EndAction(const ClientStatus& status) { delegate_->ClearGenericUi(); delegate_->CleanUpAfterPrompt(); UpdateProcessedAction(status); - if (view_inflation_successful) { - DCHECK(user_model); + if (status.ok()) { const auto& output_model_identifiers = proto_.show_generic_ui().output_model_identifiers(); - auto values = user_model->GetValues(output_model_identifiers); + auto values = + delegate_->GetUserModel()->GetValues(output_model_identifiers); // This should always be the case since there is no way to erase a value // from the model. DCHECK(values.has_value()); @@ -185,9 +278,9 @@ void ShowGenericUiAction::OnPersonalDataChanged() { profiles->emplace_back( std::make_unique(*profile)); } - delegate_->WriteUserModel( - base::BindOnce(&WriteProfilesToUserModel, std::move(profiles), - proto_.show_generic_ui().request_profiles())); + WriteProfilesToUserModel(std::move(profiles), + proto_.show_generic_ui().request_profiles(), + delegate_->GetUserModel()); } if (proto_.show_generic_ui().has_request_credit_cards()) { @@ -198,18 +291,10 @@ void ShowGenericUiAction::OnPersonalDataChanged() { credit_cards->emplace_back( std::make_unique(*credit_card)); } - delegate_->WriteUserModel( - base::BindOnce(&WriteCreditCardsToUserModel, std::move(credit_cards), - proto_.show_generic_ui().request_credit_cards())); + WriteCreditCardsToUserModel(std::move(credit_cards), + proto_.show_generic_ui().request_credit_cards(), + delegate_->GetUserModel()); } } -void ShowGenericUiAction::OnGetLogins( - const ShowGenericUiProto::RequestLoginOptions& proto, - std::vector logins) { - LOG(ERROR) << "Retrieved " << logins.size() << " logins"; - delegate_->WriteUserModel( - base::BindOnce(&WriteLoginOptionsToUserModel, proto, logins)); -} - } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.h b/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.h index 091191ade32..5cdca28c679 100644 --- a/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.h +++ b/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action.h @@ -11,10 +11,10 @@ #include "components/autofill/core/browser/personal_data_manager.h" #include "components/autofill/core/browser/personal_data_manager_observer.h" #include "components/autofill_assistant/browser/actions/action.h" +#include "components/autofill_assistant/browser/element_precondition.h" #include "components/autofill_assistant/browser/website_login_manager.h" namespace autofill_assistant { -class UserModel; // Action to show generic UI in the sheet. class ShowGenericUiAction : public Action, @@ -31,16 +31,28 @@ class ShowGenericUiAction : public Action, // Overrides Action: void InternalProcessAction(ProcessActionCallback callback) override; - void EndAction(bool view_inflation_successful, - ProcessedActionStatusProto status, - const UserModel* user_model); + void RegisterChecks( + BatchElementChecker* checker, + base::OnceCallback wait_for_dom_callback); + void OnPreconditionResult(size_t choice_index, + const ClientStatus& status, + const std::vector& ignored_payloads); + void OnElementChecksDone( + base::OnceCallback wait_for_dom_callback); + void OnDoneWaitForDom(const ClientStatus& status); + // If there is an active WaitForDom this method ends it before calling + // EndAction, otherwise it just calls EndAction. + void OnEndActionInteraction(const ClientStatus& status); + void EndAction(const ClientStatus& status); + + void OnViewInflationFinished(const ClientStatus& status); // From autofill::PersonalDataManagerObserver. void OnPersonalDataChanged() override; - void OnGetLogins(const ShowGenericUiProto::RequestLoginOptions& proto, - std::vector logins); - + bool has_pending_wait_for_dom_ = false; + bool should_end_action_ = false; + std::vector> preconditions_; ProcessActionCallback callback_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action_unittest.cc index 8464d22ef0b..d0405c87cff 100644 --- a/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/show_generic_ui_action_unittest.cc @@ -43,21 +43,20 @@ class ShowGenericUiActionTest : public content::RenderViewHostTestHarness { void SetUp() override { RenderViewHostTestHarness::SetUp(); - ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _)) - .WillByDefault(Invoke( - [this](std::unique_ptr generic_ui, - base::OnceCallback& - end_action_callback) { - std::move(end_action_callback) - .Run(true, UNKNOWN_ACTION_STATUS, &user_model_); - })); - ON_CALL(mock_action_delegate_, ClearGenericUi()).WillByDefault(Return()); - ON_CALL(mock_action_delegate_, WriteUserModel(_)) + ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _)) .WillByDefault( - Invoke([this](base::OnceCallback write_callback) { - std::move(write_callback).Run(&user_model_); + Invoke([&](std::unique_ptr generic_ui, + base::OnceCallback& + end_action_callback, + base::OnceCallback& + view_inflation_finished_callback) { + std::move(view_inflation_finished_callback) + .Run(ClientStatus(ACTION_APPLIED)); + std::move(end_action_callback).Run(ClientStatus(ACTION_APPLIED)); })); + ON_CALL(mock_action_delegate_, ClearGenericUi()).WillByDefault(Return()); + ON_CALL(mock_action_delegate_, GetUserModel()) + .WillByDefault(Return(&user_model_)); ON_CALL(mock_action_delegate_, GetPersonalDataManager) .WillByDefault(Return(&mock_personal_data_manager_)); ON_CALL(mock_action_delegate_, GetWebsiteLoginManager) @@ -92,15 +91,16 @@ class ShowGenericUiActionTest : public content::RenderViewHostTestHarness { ShowGenericUiProto proto_; }; -TEST_F(ShowGenericUiActionTest, SmokeTest) { - ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _)) - .WillByDefault(Invoke( - [this]( - std::unique_ptr generic_ui, - base::OnceCallback& end_action_callback) { - std::move(end_action_callback) - .Run(false, INVALID_ACTION, &user_model_); +TEST_F(ShowGenericUiActionTest, FailedViewInflationEndsAction) { + ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _)) + .WillByDefault( + Invoke([&](std::unique_ptr generic_ui, + base::OnceCallback& + end_action_callback, + base::OnceCallback& + view_inflation_finished_callback) { + std::move(view_inflation_finished_callback) + .Run(ClientStatus(INVALID_ACTION)); })); EXPECT_CALL(mock_action_delegate_, ClearGenericUi()).Times(1); @@ -112,19 +112,9 @@ TEST_F(ShowGenericUiActionTest, SmokeTest) { } TEST_F(ShowGenericUiActionTest, GoesIntoPromptState) { - ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _)) - .WillByDefault(Invoke( - [this]( - std::unique_ptr generic_ui, - base::OnceCallback& end_action_callback) { - std::move(end_action_callback) - .Run(true, ACTION_APPLIED, &user_model_); - })); - InSequence seq; EXPECT_CALL(mock_action_delegate_, Prompt(_, _, _, _)).Times(1); - EXPECT_CALL(mock_action_delegate_, OnSetGenericUi(_, _)).Times(1); + EXPECT_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _)).Times(1); EXPECT_CALL(mock_action_delegate_, ClearGenericUi()).Times(1); EXPECT_CALL(mock_action_delegate_, CleanUpAfterPrompt()).Times(1); EXPECT_CALL( @@ -140,16 +130,6 @@ TEST_F(ShowGenericUiActionTest, EmptyOutputModel) { input_value->set_identifier("input"); *input_value->mutable_value() = SimpleValue(std::string("InputValue")); - ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _)) - .WillByDefault(Invoke( - [this]( - std::unique_ptr generic_ui, - base::OnceCallback& end_action_callback) { - std::move(end_action_callback) - .Run(true, ACTION_APPLIED, &user_model_); - })); - EXPECT_CALL(mock_action_delegate_, ClearGenericUi()).Times(1); EXPECT_CALL( callback_, @@ -174,15 +154,17 @@ TEST_F(ShowGenericUiActionTest, NonEmptyOutputModel) { proto_.add_output_model_identifiers("value_2"); - ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _)) - .WillByDefault(Invoke( - [this]( - std::unique_ptr generic_ui, - base::OnceCallback& end_action_callback) { + ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _)) + .WillByDefault( + Invoke([this](std::unique_ptr generic_ui, + base::OnceCallback& + end_action_callback, + base::OnceCallback& + view_inflation_finished_callback) { + std::move(view_inflation_finished_callback) + .Run(ClientStatus(ACTION_APPLIED)); user_model_.SetValue("value_2", SimpleValue(std::string("change"))); - std::move(end_action_callback) - .Run(true, ACTION_APPLIED, &user_model_); + std::move(end_action_callback).Run(ClientStatus(ACTION_APPLIED)); })); EXPECT_CALL(mock_action_delegate_, ClearGenericUi()).Times(1); @@ -215,7 +197,7 @@ TEST_F(ShowGenericUiActionTest, OutputModelNotSubsetOfInputModel) { proto_.add_output_model_identifiers("value_2"); proto_.add_output_model_identifiers("value_3"); - EXPECT_CALL(mock_action_delegate_, OnSetGenericUi(_, _)).Times(0); + EXPECT_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _)).Times(0); EXPECT_CALL(mock_action_delegate_, ClearGenericUi()).Times(1); EXPECT_CALL( callback_, @@ -242,16 +224,6 @@ TEST_F(ShowGenericUiActionTest, ClientOnlyValuesDoNotLeaveDevice) { proto_.add_output_model_identifiers("regular_value"); proto_.add_output_model_identifiers("sensitive_value"); - ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _)) - .WillByDefault(Invoke( - [this]( - std::unique_ptr generic_ui, - base::OnceCallback& end_action_callback) { - std::move(end_action_callback) - .Run(true, ACTION_APPLIED, &user_model_); - })); - EXPECT_CALL( callback_, Run(Pointee(AllOf( @@ -462,5 +434,26 @@ TEST_F(ShowGenericUiActionTest, RequestLogins) { EXPECT_EQ(*user_model_.GetValue("login_options"), expected_value); } +TEST_F(ShowGenericUiActionTest, ElementPreconditionMissesIdentifier) { + auto* element_check = + proto_.mutable_periodic_element_checks()->add_element_checks(); + element_check->mutable_element_condition() + ->mutable_match() + ->add_filters() + ->set_css_selector("selector"); + + EXPECT_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _)).Times(0); + EXPECT_CALL(mock_action_delegate_, ClearGenericUi()).Times(1); + EXPECT_CALL( + callback_, + Run(Pointee(AllOf( + Property(&ProcessedActionProto::status, INVALID_ACTION), + Property(&ProcessedActionProto::show_generic_ui_result, + Property(&ShowGenericUiProto::Result::model, + Property(&ModelProto::values, SizeIs(0)))))))); + + Run(); +} + } // namespace } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.cc b/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.cc index e23d511cc44..d99c9298f61 100644 --- a/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.cc @@ -20,7 +20,7 @@ ShowProgressBarAction::ShowProgressBarAction(ActionDelegate* delegate, DCHECK(proto_.has_show_progress_bar()); } -ShowProgressBarAction::~ShowProgressBarAction() {} +ShowProgressBarAction::~ShowProgressBarAction() = default; void ShowProgressBarAction::InternalProcessAction( ProcessActionCallback callback) { @@ -29,14 +29,49 @@ void ShowProgressBarAction::InternalProcessAction( // use tell instead. delegate_->SetStatusMessage(proto_.show_progress_bar().message()); } - int progress = - base::ClampToRange(proto_.show_progress_bar().progress(), 0, 100); - delegate_->SetProgress(progress); + if (proto_.show_progress_bar().has_hide()) { delegate_->SetProgressVisible(!proto_.show_progress_bar().hide()); } + if (proto_.show_progress_bar().has_step_progress_bar_configuration()) { + const auto& configuration = + proto_.show_progress_bar().step_progress_bar_configuration(); + if (!configuration.step_icons().empty() && + configuration.step_icons().size() < 2) { + EndAction(std::move(callback), INVALID_ACTION); + return; + } + delegate_->SetStepProgressBarConfiguration(configuration); + } + + switch (proto_.show_progress_bar().progress_indicator_case()) { + case ShowProgressBarProto::ProgressIndicatorCase::kProgress: { + int progress = + base::ClampToRange(proto_.show_progress_bar().progress(), 0, 100); + delegate_->SetProgress(progress); + break; + } + case ShowProgressBarProto::ProgressIndicatorCase::kActiveStep: { + int active_step = proto_.show_progress_bar().active_step(); + if (active_step < 0) { + EndAction(std::move(callback), INVALID_ACTION); + return; + } + + delegate_->SetProgressActiveStep(active_step); + break; + } + default: + // Ignore. + break; + } + + EndAction(std::move(callback), ACTION_APPLIED); +} - UpdateProcessedAction(ACTION_APPLIED); +void ShowProgressBarAction::EndAction(ProcessActionCallback callback, + ProcessedActionStatusProto status) { + UpdateProcessedAction(status); std::move(callback).Run(std::move(processed_action_proto_)); } diff --git a/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.h b/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.h index e6832adfa31..395fca951b1 100644 --- a/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.h +++ b/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action.h @@ -8,7 +8,7 @@ #include "components/autofill_assistant/browser/actions/action.h" #include "base/macros.h" -#include "base/memory/weak_ptr.h" +#include "components/autofill_assistant/browser/service.pb.h" namespace autofill_assistant { // An action to show the current progress. @@ -22,6 +22,9 @@ class ShowProgressBarAction : public Action { // Overrides Action: void InternalProcessAction(ProcessActionCallback callback) override; + void EndAction(ProcessActionCallback callback, + ProcessedActionStatusProto status); + DISALLOW_COPY_AND_ASSIGN(ShowProgressBarAction); }; diff --git a/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action_unittest.cc new file mode 100644 index 00000000000..571a9c99577 --- /dev/null +++ b/chromium/components/autofill_assistant/browser/actions/show_progress_bar_action_unittest.cc @@ -0,0 +1,166 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/autofill_assistant/browser/actions/show_progress_bar_action.h" + +#include "base/test/gmock_callback_support.h" +#include "base/test/mock_callback.h" +#include "components/autofill_assistant/browser/actions/mock_action_delegate.h" +#include "components/autofill_assistant/browser/service.pb.h" +#include "components/autofill_assistant/browser/view_layout.pb.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace autofill_assistant { +namespace { + +using ::testing::_; +using ::testing::Invoke; +using ::testing::Property; +using ::testing::Return; +using ::testing::StrEq; + +class ShowProgressBarActionTest : public testing::Test { + public: + ShowProgressBarActionTest() {} + + void SetUp() override {} + + protected: + void Run() { + ActionProto action_proto; + *action_proto.mutable_show_progress_bar() = proto_; + ShowProgressBarAction action(&mock_action_delegate_, action_proto); + action.ProcessAction(callback_.Get()); + } + + MockActionDelegate mock_action_delegate_; + base::MockCallback callback_; + ShowProgressBarProto proto_; +}; + +TEST_F(ShowProgressBarActionTest, EmptyProgressBarDoesNothing) { + EXPECT_CALL(mock_action_delegate_, SetStatusMessage(_)).Times(0); + EXPECT_CALL(mock_action_delegate_, SetProgressVisible(_)).Times(0); + EXPECT_CALL(mock_action_delegate_, SetStepProgressBarConfiguration(_)) + .Times(0); + EXPECT_CALL(mock_action_delegate_, SetProgress(_)).Times(0); + EXPECT_CALL(mock_action_delegate_, SetProgressActiveStep(_)).Times(0); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, SpecifiedMessageGetsSet) { + proto_.set_message("Message"); + + EXPECT_CALL(mock_action_delegate_, SetStatusMessage("Message")); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, HidesProgressBar) { + proto_.set_hide(true); + + EXPECT_CALL(mock_action_delegate_, SetProgressVisible(false)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, ShowsProgressBar) { + proto_.set_hide(false); + + EXPECT_CALL(mock_action_delegate_, SetProgressVisible(true)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, FewerThanTwoStepsProgressBarFailsAction) { + auto* config = proto_.mutable_step_progress_bar_configuration(); + config->set_use_step_progress_bar(true); + config->add_step_icons()->set_icon( + DrawableProto::PROGRESSBAR_DEFAULT_INITIAL_STEP); + + EXPECT_CALL(mock_action_delegate_, SetStepProgressBarConfiguration(_)) + .Times(0); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, INVALID_ACTION)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, UpdateStepProgressBarConfiguration) { + auto* config = proto_.mutable_step_progress_bar_configuration(); + config->set_use_step_progress_bar(true); + config->add_step_icons()->set_icon( + DrawableProto::PROGRESSBAR_DEFAULT_INITIAL_STEP); + config->add_step_icons()->set_icon( + DrawableProto::PROGRESSBAR_DEFAULT_FINAL_STEP); + + EXPECT_CALL(mock_action_delegate_, SetStepProgressBarConfiguration(_)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, DeactivateStepProgressBar) { + auto* config = proto_.mutable_step_progress_bar_configuration(); + config->set_use_step_progress_bar(false); + + EXPECT_CALL(mock_action_delegate_, SetStepProgressBarConfiguration(_)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, SetProgress) { + proto_.set_progress(50); + + EXPECT_CALL(mock_action_delegate_, SetProgress(50)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, ClampsProgress) { + proto_.set_progress(150); + + EXPECT_CALL(mock_action_delegate_, SetProgress(100)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, SetActiveStep) { + proto_.set_active_step(2); + + EXPECT_CALL(mock_action_delegate_, SetProgressActiveStep(2)); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); + Run(); +} + +TEST_F(ShowProgressBarActionTest, OutOfBoundsActiveStepFailsAction) { + proto_.set_active_step(-1); + + EXPECT_CALL(mock_action_delegate_, SetProgressActiveStep(_)).Times(0); + EXPECT_CALL( + callback_, + Run(Pointee(Property(&ProcessedActionProto::status, INVALID_ACTION)))); + Run(); +} + +} // namespace +} // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/use_address_action.cc b/chromium/components/autofill_assistant/browser/actions/use_address_action.cc index ad2c1078bf2..938cab87309 100644 --- a/chromium/components/autofill_assistant/browser/actions/use_address_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/use_address_action.cc @@ -12,12 +12,13 @@ #include "base/optional.h" #include "base/time/time.h" #include "components/autofill/core/browser/autofill_data_util.h" -#include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill_assistant/browser/actions/action_delegate.h" -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_field.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h" #include "components/autofill_assistant/browser/client_status.h" +#include "components/autofill_assistant/browser/field_formatter.h" +#include "components/autofill_assistant/browser/user_model.h" +#include "components/autofill_assistant/browser/value_util.h" namespace autofill_assistant { @@ -25,23 +26,6 @@ UseAddressAction::UseAddressAction(ActionDelegate* delegate, const ActionProto& proto) : Action(delegate, proto) { DCHECK(proto.has_use_address()); - prompt_ = proto.use_address().prompt(); - name_ = proto.use_address().name(); - std::vector required_fields; - for (const auto& required_field_proto : - proto_.use_address().required_fields()) { - if (required_field_proto.value_expression().empty()) { - DVLOG(3) << "No fallback filling information provided, skipping field"; - continue; - } - - required_fields.emplace_back(); - required_fields.back().FromProto(required_field_proto); - } - - required_fields_fallback_handler_ = - std::make_unique(required_fields, - delegate); selector_ = Selector(proto.use_address().form_field_element()); selector_.MustBeVisible(); } @@ -65,19 +49,68 @@ void UseAddressAction::InternalProcessAction( } // Ensure data already selected in a previous action. - auto* user_data = delegate_->GetUserData(); - if (!user_data->has_selected_address(name_)) { - auto* error_info = processed_action_proto_->mutable_status_details() - ->mutable_autofill_error_info(); - error_info->set_address_key_requested(name_); - error_info->set_client_memory_address_key_names( - user_data->GetAllAddressKeyNames()); - error_info->set_address_pointee_was_null( - !user_data->has_selected_address(name_) || - !user_data->selected_address(name_)); - EndAction(ClientStatus(PRECONDITION_FAILED)); - return; + switch (proto_.use_address().address_source_case()) { + case UseAddressProto::kName: { + if (proto_.use_address().name().empty()) { + VLOG(1) << "UseAddress failed: |name| specified but empty"; + EndAction(ClientStatus(INVALID_ACTION)); + return; + } + auto* profile = delegate_->GetUserData()->selected_address( + proto_.use_address().name()); + if (profile == nullptr) { + auto* error_info = processed_action_proto_->mutable_status_details() + ->mutable_autofill_error_info(); + error_info->set_address_key_requested(proto_.use_address().name()); + error_info->set_client_memory_address_key_names( + delegate_->GetUserData()->GetAllAddressKeyNames()); + error_info->set_address_pointee_was_null(true); + VLOG(1) << "UseAddress failed: no profile found under " + << proto_.use_address().name(); + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + profile_ = std::make_unique(*profile); + break; + } + case UseAddressProto::kModelIdentifier: { + if (proto_.use_address().model_identifier().empty()) { + VLOG(1) << "UseAddress failed: |model_identifier| set but empty"; + EndAction(ClientStatus(INVALID_ACTION)); + return; + } + auto profile_value = delegate_->GetUserModel()->GetValue( + proto_.use_address().model_identifier()); + if (!profile_value.has_value()) { + VLOG(1) << "UseAddress failed: " + << proto_.use_address().model_identifier() + << " not found in user model"; + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + if (profile_value->profiles().values().size() != 1) { + VLOG(1) << "UseAddress failed: expected single profile for " + << proto_.use_address().model_identifier() << ", but got " + << *profile_value; + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + auto* profile = delegate_->GetUserModel()->GetProfile( + profile_value->profiles().values(0).guid()); + if (profile == nullptr) { + VLOG(1) << "UseAddress failed: profile not found for guid " + << *profile_value; + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + profile_ = std::make_unique(*profile); + break; + } + case UseAddressProto::ADDRESS_SOURCE_NOT_SET: + EndAction(ClientStatus(INVALID_ACTION)); + return; } + DCHECK(profile_ != nullptr); FillFormWithData(); } @@ -94,21 +127,12 @@ void UseAddressAction::EndAction( } void UseAddressAction::FillFormWithData() { - if (proto_.use_address().skip_autofill()) { - VLOG(3) << "Retrieving address from client memory under '" << name_ << "'."; - const autofill::AutofillProfile* profile = - delegate_->GetUserData()->selected_address(name_); - DCHECK(profile); - auto fallback_data = CreateFallbackData(*profile); - - required_fields_fallback_handler_->CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), - base::BindOnce(&UseAddressAction::EndAction, - weak_ptr_factory_.GetWeakPtr())); + if (selector_.empty()) { + DCHECK(proto_.use_address().skip_autofill()); + OnWaitForElement(OkClientStatus()); return; } - DCHECK(!selector_.empty()); delegate_->ShortWaitForElement( selector_, base::BindOnce(&UseAddressAction::OnWaitForElement, weak_ptr_factory_.GetWeakPtr())); @@ -116,36 +140,46 @@ void UseAddressAction::FillFormWithData() { void UseAddressAction::OnWaitForElement(const ClientStatus& element_status) { if (!element_status.ok()) { - EndAction(ClientStatus(element_status.proto_status())); + EndAction(element_status); return; } - VLOG(3) << "Retrieving address from client memory under '" << name_ << "'."; - const autofill::AutofillProfile* profile = - delegate_->GetUserData()->selected_address(name_); - DCHECK(profile); - auto fallback_data = CreateFallbackData(*profile); + if (proto_.use_address().skip_autofill()) { + ExecuteFallback(OkClientStatus()); + return; + } - delegate_->FillAddressForm( - profile, selector_, - base::BindOnce(&UseAddressAction::OnFormFilled, - weak_ptr_factory_.GetWeakPtr(), std::move(fallback_data))); + DCHECK(!selector_.empty()); + DCHECK(profile_ != nullptr); + delegate_->FillAddressForm(profile_.get(), selector_, + base::BindOnce(&UseAddressAction::ExecuteFallback, + weak_ptr_factory_.GetWeakPtr())); } -void UseAddressAction::OnFormFilled(std::unique_ptr fallback_data, - const ClientStatus& status) { - required_fields_fallback_handler_->CheckAndFallbackRequiredFields( - status, std::move(fallback_data), - base::BindOnce(&UseAddressAction::EndAction, - weak_ptr_factory_.GetWeakPtr())); -} +void UseAddressAction::ExecuteFallback(const ClientStatus& status) { + DCHECK(profile_ != nullptr); + std::vector required_fields; + for (const auto& required_field_proto : + proto_.use_address().required_fields()) { + if (!required_field_proto.has_value_expression()) { + continue; + } + + RequiredField required_field; + required_field.FromProto(required_field_proto); + required_fields.emplace_back(required_field); + } -std::unique_ptr UseAddressAction::CreateFallbackData( - const autofill::AutofillProfile& profile) { - auto fallback_data = std::make_unique(); + DCHECK(fallback_handler_ == nullptr); + fallback_handler_ = std::make_unique( + required_fields, + field_formatter::CreateAutofillMappings(*profile_, + /* locale = */ "en-US"), + delegate_); - fallback_data->AddFormGroup(profile); - return fallback_data; + fallback_handler_->CheckAndFallbackRequiredFields( + status, base::BindOnce(&UseAddressAction::EndAction, + weak_ptr_factory_.GetWeakPtr())); } } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/use_address_action.h b/chromium/components/autofill_assistant/browser/actions/use_address_action.h index 6366e7420fb..01c5c53c705 100644 --- a/chromium/components/autofill_assistant/browser/actions/use_address_action.h +++ b/chromium/components/autofill_assistant/browser/actions/use_address_action.h @@ -13,8 +13,8 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/optional.h" +#include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill_assistant/browser/actions/action.h" -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h" namespace autofill { @@ -38,27 +38,20 @@ class UseAddressAction : public Action { const base::Optional& optional_details_status = base::nullopt); - // Fill the form using data in client memory. Return whether filling succeeded - // or not through OnFormFilled. + // Fill the form using |profile_|. Return whether filling succeeded or not + // through OnFormFilled. void FillFormWithData(); void OnWaitForElement(const ClientStatus& element_status); // Called when the address has been filled. - void OnFormFilled(std::unique_ptr fallback_data, - const ClientStatus& status); + void ExecuteFallback(const ClientStatus& status); - // Create fallback data. - std::unique_ptr CreateFallbackData( - const autofill::AutofillProfile& profile); - - // Usage of the autofilled address. - std::string name_; - std::string prompt_; + // Note: |fallback_handler_| must be a member, because checking for fallbacks + // is asynchronous and the existence of the handler must be ensured. + std::unique_ptr fallback_handler_; + std::unique_ptr profile_; Selector selector_; - std::unique_ptr - required_fields_fallback_handler_; - ProcessActionCallback process_action_callback_; base::WeakPtrFactory weak_ptr_factory_{this}; diff --git a/chromium/components/autofill_assistant/browser/actions/use_address_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/use_address_action_unittest.cc index 5017c774a1d..3827d03443c 100644 --- a/chromium/components/autofill_assistant/browser/actions/use_address_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/use_address_action_unittest.cc @@ -7,6 +7,7 @@ #include #include "base/guid.h" +#include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/test/gmock_callback_support.h" @@ -16,12 +17,20 @@ #include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill_assistant/browser/actions/mock_action_delegate.h" #include "components/autofill_assistant/browser/mock_personal_data_manager.h" +#include "components/autofill_assistant/browser/user_model.h" #include "components/autofill_assistant/browser/web/mock_web_controller.h" #include "components/autofill_assistant/browser/web/web_controller_util.h" #include "testing/gmock/include/gmock/gmock.h" namespace autofill_assistant { namespace { +const char kAddressName[] = "billing"; +const char kFakeSelector[] = "#selector"; +const char kFirstName[] = "FirstName"; +const char kLastName[] = "LastName"; +const char kEmail[] = "foobar@gmail.com"; +const char kPhoneNumber[] = "+41791234567"; +const char kModelIdentifier[] = "identifier"; using ::base::test::RunOnceCallback; using ::testing::_; @@ -30,25 +39,31 @@ using ::testing::Expectation; using ::testing::InSequence; using ::testing::Invoke; using ::testing::NotNull; +using ::testing::Pointee; using ::testing::Return; using ::testing::SaveArgPointee; class UseAddressActionTest : public testing::Test { public: void SetUp() override { - // Build two identical autofill profiles. One for the memory, one for the - // mock. - auto autofill_profile = std::make_unique( - base::GenerateGUID(), autofill::test::kEmptyOrigin); - autofill::test::SetProfileInfo(autofill_profile.get(), kFirstName, "", - kLastName, kEmail, "", "", "", "", "", "", - "", kPhoneNumber); - user_data_.selected_addresses_[kAddressName] = std::move(autofill_profile); - - ON_CALL(mock_personal_data_manager_, GetProfileByGUID) - .WillByDefault(Return(&autofill_profile_)); + autofill::test::SetProfileInfo(&profile_, kFirstName, "", kLastName, kEmail, + "", "", "", "", "", "", "", kPhoneNumber); + // Store copies of |profile_| in |user_data_| and |user_model_|. + user_data_.selected_addresses_[kAddressName] = + std::make_unique(profile_); + auto profiles = std::make_unique< + std::vector>>(); + profiles->emplace_back( + std::make_unique(profile_)); + user_model_.SetAutofillProfiles(std::move(profiles)); + ValueProto profile_value; + profile_value.mutable_profiles()->add_values()->set_guid(profile_.guid()); + user_model_.SetValue(kModelIdentifier, profile_value); + ON_CALL(mock_action_delegate_, GetUserData) .WillByDefault(Return(&user_data_)); + ON_CALL(mock_action_delegate_, GetUserModel) + .WillByDefault(Return(&user_model_)); ON_CALL(mock_action_delegate_, GetPersonalDataManager) .WillByDefault(Return(&mock_personal_data_manager_)); ON_CALL(mock_action_delegate_, RunElementChecks) @@ -60,19 +75,11 @@ class UseAddressActionTest : public testing::Test { } protected: - const char* const kAddressName = "billing"; - const char* const kFakeSelector = "#selector"; - const char* const kSelectionPrompt = "prompt"; - const char* const kFirstName = "FirstName"; - const char* const kLastName = "LastName"; - const char* const kEmail = "foobar@gmail.com"; - const char* const kPhoneNumber = "+41791234567"; - ActionProto CreateUseAddressAction() { ActionProto action; UseAddressProto* use_address = action.mutable_use_address(); use_address->set_name(kAddressName); - use_address->mutable_form_field_element()->add_selectors(kFakeSelector); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); return action; } @@ -81,7 +88,7 @@ class UseAddressActionTest : public testing::Test { std::string selector) { auto* required_field = action->mutable_use_address()->add_required_fields(); required_field->set_value_expression(value_expression); - required_field->mutable_element()->add_selectors(selector); + *required_field->mutable_element() = ToSelectorProto(selector); return required_field; } @@ -98,8 +105,9 @@ class UseAddressActionTest : public testing::Test { MockActionDelegate mock_action_delegate_; MockWebController mock_web_controller_; UserData user_data_; - - autofill::AutofillProfile autofill_profile_; + UserModel user_model_; + autofill::AutofillProfile profile_ = {base::GenerateGUID(), + autofill::test::kEmptyOrigin}; }; #if !defined(OS_ANDROID) @@ -111,37 +119,97 @@ TEST_F(UseAddressActionTest, MAYBE_FillManually) { InSequence seq; ActionProto action_proto = CreateUseAddressAction(); - action_proto.mutable_use_address()->set_prompt(kSelectionPrompt); EXPECT_EQ(ProcessedActionStatusProto::MANUAL_FALLBACK, ProcessAction(action_proto)); } -TEST_F(UseAddressActionTest, NoSelectedAddress) { - InSequence seq; +TEST_F(UseAddressActionTest, InvalidActionNoSelectorSet) { + ActionProto action; + action.mutable_use_address(); + EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); +} - ActionProto action_proto = CreateUseAddressAction(); - action_proto.mutable_use_address()->set_prompt(kSelectionPrompt); +TEST_F(UseAddressActionTest, InvalidActionNameSetButEmpty) { + ActionProto action; + UseAddressProto* use_address = action.mutable_use_address(); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_address->set_name(""); + EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); +} - user_data_.selected_addresses_[kAddressName] = nullptr; +TEST_F(UseAddressActionTest, InvalidActionSkipAutofillWithoutRequiredFields) { + ActionProto action; + UseAddressProto* use_address = action.mutable_use_address(); + use_address->set_name(kAddressName); + use_address->set_skip_autofill(true); + EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); +} +TEST_F(UseAddressActionTest, PreconditionFailedNoProfileForName) { + ActionProto action; + UseAddressProto* use_address = action.mutable_use_address(); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_address->set_name("invalid"); EXPECT_EQ(ProcessedActionStatusProto::PRECONDITION_FAILED, - ProcessAction(action_proto)); + ProcessAction(action)); } -TEST_F(UseAddressActionTest, InvalidActionSkipAutofillWithoutRequiredFields) { +TEST_F(UseAddressActionTest, ResolveProfileByNameSucceeds) { + ON_CALL(mock_action_delegate_, + OnShortWaitForElement(Selector({kFakeSelector}).MustBeVisible(), _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus())); + ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "not empty")); + ActionProto action; UseAddressProto* use_address = action.mutable_use_address(); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); use_address->set_name(kAddressName); - use_address->set_skip_autofill(true); + EXPECT_CALL(mock_action_delegate_, + OnFillAddressForm(Pointee(Eq(profile_)), _, _)) + .WillOnce(RunOnceCallback<2>(OkClientStatus())); + EXPECT_EQ(ProcessedActionStatusProto::ACTION_APPLIED, ProcessAction(action)); +} + +TEST_F(UseAddressActionTest, InvalidActionModelIdentifierSetButEmpty) { + ActionProto action; + UseAddressProto* use_address = action.mutable_use_address(); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_address->set_model_identifier(""); EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); } +TEST_F(UseAddressActionTest, PreconditionFailedNoProfileForModelIdentifier) { + ActionProto action; + UseAddressProto* use_address = action.mutable_use_address(); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_address->set_model_identifier("invalid"); + EXPECT_EQ(ProcessedActionStatusProto::PRECONDITION_FAILED, + ProcessAction(action)); +} + +TEST_F(UseAddressActionTest, ResolveProfileByModelIdentifierSucceeds) { + ON_CALL(mock_action_delegate_, + OnShortWaitForElement(Selector({kFakeSelector}).MustBeVisible(), _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus())); + ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "not empty")); + + ActionProto action; + UseAddressProto* use_address = action.mutable_use_address(); + *use_address->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_address->set_model_identifier(kModelIdentifier); + EXPECT_CALL(mock_action_delegate_, + OnFillAddressForm(Pointee(Eq(profile_)), _, _)) + .WillOnce(RunOnceCallback<2>(OkClientStatus())); + EXPECT_EQ(ProcessedActionStatusProto::ACTION_APPLIED, ProcessAction(action)); +} + TEST_F(UseAddressActionTest, PreconditionFailedPopulatesUnexpectedErrorInfo) { InSequence seq; ActionProto action_proto = CreateUseAddressAction(); - action_proto.mutable_use_address()->set_prompt(kSelectionPrompt); user_data_.selected_addresses_[kAddressName] = nullptr; user_data_.selected_addresses_["one_more"] = nullptr; @@ -185,16 +253,22 @@ TEST_F(UseAddressActionTest, ValidationSucceeds) { ActionProto action_proto = CreateUseAddressAction(); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_FIRST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_FIRST)), + "}"}), "#first_name"); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_LAST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_LAST)), + "}"}), "#last_name"); AddRequiredField(&action_proto, - base::NumberToString(static_cast( - autofill::ServerFieldType::EMAIL_ADDRESS)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::EMAIL_ADDRESS)), + "}"}), "#email"); // Autofill succeeds. @@ -217,16 +291,22 @@ TEST_F(UseAddressActionTest, FallbackFails) { ActionProto action_proto = CreateUseAddressAction(); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_FIRST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_FIRST)), + "}"}), "#first_name"); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_LAST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_LAST)), + "}"}), "#last_name"); AddRequiredField(&action_proto, - base::NumberToString(static_cast( - autofill::ServerFieldType::EMAIL_ADDRESS)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::EMAIL_ADDRESS)), + "}"}), "#email"); // Autofill succeeds. @@ -251,8 +331,23 @@ TEST_F(UseAddressActionTest, FallbackFails) { OnSetFieldValue(Eq(Selector({"#first_name"})), kFirstName, _)) .WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS))); - EXPECT_EQ(ProcessedActionStatusProto::AUTOFILL_INCOMPLETE, - ProcessAction(action_proto)); + ProcessedActionProto processed_action; + EXPECT_CALL(callback_, Run(_)).WillOnce(SaveArgPointee<0>(&processed_action)); + + UseAddressAction action(&mock_action_delegate_, action_proto); + action.ProcessAction(callback_.Get()); + + EXPECT_EQ(processed_action.status(), + ProcessedActionStatusProto::AUTOFILL_INCOMPLETE); + EXPECT_TRUE(processed_action.has_status_details()); + EXPECT_EQ(processed_action.status_details() + .autofill_error_info() + .autofill_field_error_size(), + 1); + EXPECT_EQ(OTHER_ACTION_STATUS, processed_action.status_details() + .autofill_error_info() + .autofill_field_error(0) + .status()); } TEST_F(UseAddressActionTest, FallbackSucceeds) { @@ -261,16 +356,22 @@ TEST_F(UseAddressActionTest, FallbackSucceeds) { ActionProto action_proto = CreateUseAddressAction(); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_FIRST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_FIRST)), + "}"}), "#first_name"); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_LAST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_LAST)), + "}"}), "#last_name"); AddRequiredField(&action_proto, - base::NumberToString(static_cast( - autofill::ServerFieldType::EMAIL_ADDRESS)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::EMAIL_ADDRESS)), + "}"}), "#email"); // Autofill succeeds. @@ -331,8 +432,10 @@ TEST_F(UseAddressActionTest, ActionProto action_proto = CreateUseAddressAction(); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_FIRST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_FIRST)), + "}"}), "#first_name"); EXPECT_CALL(mock_action_delegate_, @@ -411,8 +514,10 @@ TEST_F(UseAddressActionTest, ForcedFallbackWithKeystrokes) { ActionProto action_proto = CreateUseAddressAction(); auto* name_required = AddRequiredField( &action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_FIRST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_FIRST)), + "}"}), "#first_name"); name_required->set_forced(true); name_required->set_fill_strategy(SIMULATE_KEY_PRESSES); @@ -428,7 +533,8 @@ TEST_F(UseAddressActionTest, ForcedFallbackWithKeystrokes) { ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "not empty")); - // But we still want the first name filled, with simulated keypresses. + // But we still want the first name filled, with + // simulated keypresses. EXPECT_CALL(mock_action_delegate_, OnSetFieldValue(Selector({"#first_name"}), kFirstName, true, 1000, _)) .WillOnce(RunOnceCallback<4>(OkClientStatus())); @@ -444,11 +550,14 @@ TEST_F(UseAddressActionTest, SkippingAutofill) { ActionProto action_proto; action_proto.mutable_use_address()->set_name(kAddressName); AddRequiredField(&action_proto, - base::NumberToString( - static_cast(autofill::ServerFieldType::NAME_FIRST)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::NAME_FIRST)), + "}"}), "#first_name"); action_proto.mutable_use_address()->set_skip_autofill(true); + EXPECT_CALL(mock_action_delegate_, OnShortWaitForElement(_, _)).Times(0); EXPECT_CALL(mock_action_delegate_, OnFillAddressForm(_, _, _)).Times(0); // First validation fails. diff --git a/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.cc b/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.cc index 655533eddeb..717f02e68a2 100644 --- a/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.cc +++ b/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.cc @@ -4,12 +4,14 @@ #include "components/autofill_assistant/browser/actions/use_credit_card_action.h" +#include #include #include #include "base/bind.h" #include "base/callback.h" #include "base/optional.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" @@ -17,10 +19,11 @@ #include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill/core/browser/data_model/credit_card.h" #include "components/autofill_assistant/browser/actions/action_delegate.h" -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_field.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h" #include "components/autofill_assistant/browser/client_status.h" +#include "components/autofill_assistant/browser/field_formatter.h" +#include "components/autofill_assistant/browser/user_model.h" namespace autofill_assistant { @@ -28,21 +31,6 @@ UseCreditCardAction::UseCreditCardAction(ActionDelegate* delegate, const ActionProto& proto) : Action(delegate, proto) { DCHECK(proto.has_use_card()); - prompt_ = proto.use_card().prompt(); - std::vector required_fields; - for (const auto& required_field_proto : proto_.use_card().required_fields()) { - if (required_field_proto.value_expression().empty()) { - DVLOG(3) << "No fallback filling information provided, skipping field"; - continue; - } - - required_fields.emplace_back(); - required_fields.back().FromProto(required_field_proto); - } - - required_fields_fallback_handler_ = - std::make_unique(required_fields, - delegate); selector_ = Selector(proto.use_card().form_field_element()); selector_.MustBeVisible(); } @@ -66,11 +54,45 @@ void UseCreditCardAction::InternalProcessAction( } // Ensure data already selected in a previous action. - auto* user_data = delegate_->GetUserData(); - if (user_data->selected_card_.get() == nullptr) { - EndAction(ClientStatus(PRECONDITION_FAILED)); - return; + if (proto_.use_card().has_model_identifier()) { + if (proto_.use_card().model_identifier().empty()) { + VLOG(1) << "UseCreditCard failed: |model_identifier| set but empty"; + EndAction(ClientStatus(INVALID_ACTION)); + return; + } + auto credit_card_value = delegate_->GetUserModel()->GetValue( + proto_.use_card().model_identifier()); + if (!credit_card_value.has_value()) { + VLOG(1) << "UseCreditCard failed: " + << proto_.use_card().model_identifier() + << " not found in user model"; + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + if (credit_card_value->credit_cards().values().size() != 1) { + VLOG(1) << "UseCreditCard failed: expected single card for " + << proto_.use_card().model_identifier() << ", but got " + << *credit_card_value; + } + auto* credit_card = delegate_->GetUserModel()->GetCreditCard( + credit_card_value->credit_cards().values(0).guid()); + if (credit_card == nullptr) { + VLOG(1) << "UseCreditCard failed: card not found for guid " + << *credit_card_value; + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + credit_card_ = std::make_unique(*credit_card); + } else { + auto* credit_card = delegate_->GetUserData()->selected_card_.get(); + if (credit_card == nullptr) { + VLOG(1) << "UseCreditCard failed: card not found in user_data"; + EndAction(ClientStatus(PRECONDITION_FAILED)); + return; + } + credit_card_ = std::make_unique(*credit_card); } + DCHECK(credit_card_ != nullptr); FillFormWithData(); } @@ -87,13 +109,12 @@ void UseCreditCardAction::EndAction( } void UseCreditCardAction::FillFormWithData() { - if (proto_.use_card().skip_autofill()) { - delegate_->GetFullCard(base::BindOnce(&UseCreditCardAction::OnGetFullCard, - weak_ptr_factory_.GetWeakPtr())); + if (selector_.empty()) { + DCHECK(proto_.use_card().skip_autofill()); + OnWaitForElement(OkClientStatus()); return; } - DCHECK(!selector_.empty()); delegate_->ShortWaitForElement( selector_, base::BindOnce(&UseCreditCardAction::OnWaitForElement, weak_ptr_factory_.GetWeakPtr())); @@ -105,7 +126,9 @@ void UseCreditCardAction::OnWaitForElement(const ClientStatus& element_status) { return; } - delegate_->GetFullCard(base::BindOnce(&UseCreditCardAction::OnGetFullCard, + DCHECK(credit_card_ != nullptr); + delegate_->GetFullCard(credit_card_.get(), + base::BindOnce(&UseCreditCardAction::OnGetFullCard, weak_ptr_factory_.GetWeakPtr())); } @@ -117,49 +140,49 @@ void UseCreditCardAction::OnGetFullCard( return; } - auto fallback_data = CreateFallbackData(cvc, *card); + std::vector required_fields; + for (const auto& required_field_proto : proto_.use_card().required_fields()) { + if (!required_field_proto.has_value_expression()) { + continue; + } + + RequiredField required_field; + required_field.FromProto(required_field_proto); + required_fields.emplace_back(required_field); + } + + std::map fallback_values = + field_formatter::CreateAutofillMappings(*card, + /* locale = */ "en-US"); + fallback_values.emplace( + base::NumberToString( + static_cast(AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)), + base::UTF16ToUTF8(cvc)); + fallback_values.emplace( + base::NumberToString( + static_cast(AutofillFormatProto::CREDIT_CARD_RAW_NUMBER)), + base::UTF16ToUTF8(card->GetRawInfo(autofill::CREDIT_CARD_NUMBER))); + + DCHECK(fallback_handler_ == nullptr); + fallback_handler_ = std::make_unique( + required_fields, fallback_values, delegate_); if (proto_.use_card().skip_autofill()) { - required_fields_fallback_handler_->CheckAndFallbackRequiredFields( - OkClientStatus(), std::move(fallback_data), - base::BindOnce(&UseCreditCardAction::EndAction, - weak_ptr_factory_.GetWeakPtr())); + ExecuteFallback(OkClientStatus()); return; } - delegate_->FillCardForm( - std::move(card), cvc, selector_, - base::BindOnce(&UseCreditCardAction::OnFormFilled, - weak_ptr_factory_.GetWeakPtr(), std::move(fallback_data))); + DCHECK(!selector_.empty()); + delegate_->FillCardForm(std::move(card), cvc, selector_, + base::BindOnce(&UseCreditCardAction::ExecuteFallback, + weak_ptr_factory_.GetWeakPtr())); } -void UseCreditCardAction::OnFormFilled( - std::unique_ptr fallback_data, - const ClientStatus& status) { - required_fields_fallback_handler_->CheckAndFallbackRequiredFields( - status, std::move(fallback_data), - base::BindOnce(&UseCreditCardAction::EndAction, - weak_ptr_factory_.GetWeakPtr())); +void UseCreditCardAction::ExecuteFallback(const ClientStatus& status) { + DCHECK(fallback_handler_ != nullptr); + fallback_handler_->CheckAndFallbackRequiredFields( + status, base::BindOnce(&UseCreditCardAction::EndAction, + weak_ptr_factory_.GetWeakPtr())); } -std::unique_ptr UseCreditCardAction::CreateFallbackData( - const base::string16& cvc, - const autofill::CreditCard& card) { - auto fallback_data = std::make_unique(); - - fallback_data->field_values.emplace( - static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE), - base::UTF16ToUTF8(cvc)); - fallback_data->field_values.emplace( - (int)UseCreditCardProto::RequiredField::CREDIT_CARD_RAW_NUMBER, - base::UTF16ToUTF8(card.GetRawInfo(autofill::CREDIT_CARD_NUMBER))); - fallback_data->field_values.emplace( - static_cast(UseCreditCardProto::RequiredField::CREDIT_CARD_NETWORK), - autofill::data_util::GetPaymentRequestData(card.network()) - .basic_card_issuer_network); - - fallback_data->AddFormGroup(card); - return fallback_data; -} } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.h b/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.h index 99191230fec..7bf36ff9eda 100644 --- a/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.h +++ b/chromium/components/autofill_assistant/browser/actions/use_credit_card_action.h @@ -14,7 +14,6 @@ #include "base/memory/weak_ptr.h" #include "base/optional.h" #include "components/autofill_assistant/browser/actions/action.h" -#include "components/autofill_assistant/browser/actions/fallback_handler/fallback_data.h" #include "components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h" namespace autofill { @@ -49,20 +48,14 @@ class UseCreditCardAction : public Action { const base::string16& cvc); // Called when the form credit card has been filled. - void OnFormFilled(std::unique_ptr fallback_data, - const ClientStatus& status); + void ExecuteFallback(const ClientStatus& status); - // Create fallback data. - std::unique_ptr CreateFallbackData( - const base::string16& cvc, - const autofill::CreditCard& card); - - std::string prompt_; + // Note: |fallback_handler_| must be a member, because checking for fallbacks + // is asynchronous and the existence of the handler must be ensured. + std::unique_ptr fallback_handler_; + std::unique_ptr credit_card_; Selector selector_; - std::unique_ptr - required_fields_fallback_handler_; - ProcessActionCallback process_action_callback_; base::WeakPtrFactory weak_ptr_factory_{this}; diff --git a/chromium/components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc index 7d737b8b98b..a81d311d961 100644 --- a/chromium/components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc @@ -7,45 +7,63 @@ #include #include "base/guid.h" +#include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/test/gmock_callback_support.h" #include "base/test/mock_callback.h" #include "components/autofill/core/browser/autofill_test_utils.h" -#include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill_assistant/browser/actions/mock_action_delegate.h" #include "components/autofill_assistant/browser/mock_personal_data_manager.h" +#include "components/autofill_assistant/browser/user_model.h" #include "components/autofill_assistant/browser/web/mock_web_controller.h" #include "components/autofill_assistant/browser/web/web_controller_util.h" #include "testing/gmock/include/gmock/gmock.h" namespace autofill_assistant { namespace { +const char kFakeSelector[] = "#selector"; +const char kFakeCvc[] = "123"; +const char kModelIdentifier[] = "identifier"; +const char kCardName[] = "Adam West"; +const char kCardNumber[] = "4111111111111111"; +const char kExpirationMonth[] = "9"; +const char kExpirationYear[] = "2050"; using ::base::test::RunOnceCallback; using ::testing::_; using ::testing::Eq; using ::testing::Expectation; +using ::testing::InSequence; using ::testing::Invoke; using ::testing::NotNull; +using ::testing::Pointee; using ::testing::Return; using ::testing::SaveArgPointee; class UseCreditCardActionTest : public testing::Test { public: void SetUp() override { - // Build two identical autofill profiles. One for the memory, one for the - // mock. - auto autofill_profile = std::make_unique( - base::GenerateGUID(), autofill::test::kEmptyOrigin); - autofill::test::SetProfileInfo(autofill_profile.get(), kFirstName, "", - kLastName, kEmail, "", "", "", "", "", "", - "", ""); - user_data_.selected_addresses_[kAddressName] = std::move(autofill_profile); - ON_CALL(mock_personal_data_manager_, GetProfileByGUID) - .WillByDefault(Return(&autofill_profile_)); + autofill::test::SetCreditCardInfo(&credit_card_, kCardName, kCardNumber, + kExpirationMonth, kExpirationYear, + /* billing_address_id= */ ""); + + // Store copies of |credit_card_| in |user_data_| and |user_model_|. + user_data_.selected_card_ = + std::make_unique(credit_card_); + auto cards = + std::make_unique>>(); + cards->emplace_back(std::make_unique(credit_card_)); + user_model_.SetAutofillCreditCards(std::move(cards)); + ValueProto card_value; + card_value.mutable_credit_cards()->add_values()->set_guid( + credit_card_.guid()); + user_model_.SetValue(kModelIdentifier, card_value); + ON_CALL(mock_action_delegate_, GetUserData) .WillByDefault(Return(&user_data_)); + ON_CALL(mock_action_delegate_, GetUserModel) + .WillByDefault(Return(&user_model_)); ON_CALL(mock_action_delegate_, GetPersonalDataManager) .WillByDefault(Return(&mock_personal_data_manager_)); ON_CALL(mock_action_delegate_, RunElementChecks) @@ -54,20 +72,23 @@ class UseCreditCardActionTest : public testing::Test { })); ON_CALL(mock_action_delegate_, OnShortWaitForElement(_, _)) .WillByDefault(RunOnceCallback<1>(OkClientStatus())); + ON_CALL(mock_action_delegate_, OnGetFullCard) + .WillByDefault(Invoke([](const autofill::CreditCard* credit_card, + base::OnceCallback card, + const base::string16& cvc)>& callback) { + std::move(callback).Run( + credit_card ? std::make_unique(*credit_card) + : nullptr, + base::UTF8ToUTF16(kFakeCvc)); + })); } protected: - const char* const kAddressName = "billing"; - const char* const kFakeSelector = "#selector"; - const char* const kSelectionPrompt = "prompt"; - const char* const kFirstName = "FirstName"; - const char* const kLastName = "LastName"; - const char* const kEmail = "foobar@gmail.com"; - ActionProto CreateUseCreditCardAction() { ActionProto action; - action.mutable_use_card()->mutable_form_field_element()->add_selectors( - kFakeSelector); + *action.mutable_use_card()->mutable_form_field_element() = + ToSelectorProto(kFakeSelector); return action; } @@ -77,14 +98,14 @@ class UseCreditCardActionTest : public testing::Test { std::string selector) { auto* required_field = action->mutable_use_card()->add_required_fields(); required_field->set_value_expression(value_expression); - required_field->mutable_element()->add_selectors(selector); + *required_field->mutable_element() = ToSelectorProto(selector); return required_field; } ActionProto CreateUseCardAction() { ActionProto action; UseCreditCardProto* use_card = action.mutable_use_card(); - use_card->mutable_form_field_element()->add_selectors(kFakeSelector); + *use_card->mutable_form_field_element() = ToSelectorProto(kFakeSelector); return action; } @@ -101,14 +122,15 @@ class UseCreditCardActionTest : public testing::Test { MockActionDelegate mock_action_delegate_; MockWebController mock_web_controller_; UserData user_data_; - - autofill::AutofillProfile autofill_profile_; + UserModel user_model_; + autofill::CreditCard credit_card_ = {base::GenerateGUID(), + autofill::test::kEmptyOrigin}; }; -TEST_F(UseCreditCardActionTest, FillCreditCardNoCardSelected) { - ActionProto action = CreateUseCreditCardAction(); - EXPECT_EQ(ProcessedActionStatusProto::PRECONDITION_FAILED, - ProcessAction(action)); +TEST_F(UseCreditCardActionTest, InvalidActionNoSelectorSet) { + ActionProto action; + action.mutable_use_card(); + EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); } TEST_F(UseCreditCardActionTest, @@ -119,16 +141,74 @@ TEST_F(UseCreditCardActionTest, EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); } +TEST_F(UseCreditCardActionTest, PreconditionFailedNoCreditCardInUserData) { + ActionProto action; + auto* use_card = action.mutable_use_card(); + *use_card->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + user_data_.selected_card_.reset(); + EXPECT_EQ(ProcessedActionStatusProto::PRECONDITION_FAILED, + ProcessAction(action)); +} + +TEST_F(UseCreditCardActionTest, CreditCardInUserDataSucceeds) { + ON_CALL(mock_action_delegate_, + OnShortWaitForElement(Selector({kFakeSelector}).MustBeVisible(), _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus())); + ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "not empty")); + ActionProto action; + auto* use_card = action.mutable_use_card(); + *use_card->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + EXPECT_CALL( + mock_action_delegate_, + OnFillCardForm(Pointee(Eq(credit_card_)), base::UTF8ToUTF16(kFakeCvc), + Selector({kFakeSelector}).MustBeVisible(), _)) + .WillOnce(RunOnceCallback<3>(OkClientStatus())); + EXPECT_EQ(ProcessedActionStatusProto::ACTION_APPLIED, ProcessAction(action)); +} + +TEST_F(UseCreditCardActionTest, InvalidActionModelIdentifierSetButEmpty) { + ActionProto action; + auto* use_card = action.mutable_use_card(); + *use_card->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_card->set_model_identifier(""); + EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action)); +} + +TEST_F(UseCreditCardActionTest, + PreconditionFailedNoCreditCardForModelIdentifier) { + ActionProto action; + auto* use_card = action.mutable_use_card(); + *use_card->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_card->set_model_identifier("invalid"); + EXPECT_EQ(ProcessedActionStatusProto::PRECONDITION_FAILED, + ProcessAction(action)); +} + +TEST_F(UseCreditCardActionTest, CreditCardInUserModelSucceeds) { + ON_CALL(mock_action_delegate_, + OnShortWaitForElement(Selector({kFakeSelector}).MustBeVisible(), _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus())); + ON_CALL(mock_web_controller_, OnGetFieldValue(_, _)) + .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "not empty")); + ActionProto action; + auto* use_card = action.mutable_use_card(); + *use_card->mutable_form_field_element() = ToSelectorProto(kFakeSelector); + use_card->set_model_identifier(kModelIdentifier); + EXPECT_CALL( + mock_action_delegate_, + OnFillCardForm(Pointee(Eq(credit_card_)), base::UTF8ToUTF16(kFakeCvc), + Selector({kFakeSelector}).MustBeVisible(), _)) + .WillOnce(RunOnceCallback<3>(OkClientStatus())); + EXPECT_EQ(ProcessedActionStatusProto::ACTION_APPLIED, ProcessAction(action)); +} + TEST_F(UseCreditCardActionTest, FillCreditCard) { ActionProto action = CreateUseCreditCardAction(); - autofill::CreditCard credit_card; - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); + user_data_.selected_card_ = std::make_unique(); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(OkClientStatus())); @@ -141,23 +221,18 @@ TEST_F(UseCreditCardActionTest, FillCreditCardRequiredFieldsFilled) { .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "not empty")); ActionProto action = CreateUseCreditCardAction(); - AddRequiredField( - &action, - base::NumberToString(static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE)), - "#cvc"); + AddRequiredField(&action, + base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)), + "#cvc"); AddRequiredField(&action, base::NumberToString(static_cast( autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH)), "#expmonth"); - autofill::CreditCard credit_card; - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); + user_data_.selected_card_ = std::make_unique(); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(OkClientStatus())); @@ -171,34 +246,53 @@ TEST_F(UseCreditCardActionTest, FillCreditCardWithFallback) { ActionProto action = CreateUseCreditCardAction(); AddRequiredField( &action, - base::NumberToString(static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE)), + base::StrCat({"${", + base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)), + "}"}), "#cvc"); - AddRequiredField(&action, - base::NumberToString(static_cast( - autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH)), - "#expmonth"); AddRequiredField( &action, - base::NumberToString(static_cast( - autofill::ServerFieldType::CREDIT_CARD_EXP_2_DIGIT_YEAR)), + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_EXP_MONTH)), + "}"}), + "#expmonth"); + AddRequiredField( + &action, + base::StrCat( + {"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_EXP_2_DIGIT_YEAR)), + "}"}), "#expyear2"); AddRequiredField( &action, - base::NumberToString(static_cast( - autofill::ServerFieldType::CREDIT_CARD_EXP_4_DIGIT_YEAR)), + base::StrCat( + {"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_EXP_4_DIGIT_YEAR)), + "}"}), "#expyear4"); + AddRequiredField( + &action, + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), + "}"}), + "#card_name"); + AddRequiredField( + &action, + base::StrCat({"${", + base::NumberToString(static_cast( + autofill::ServerFieldType::CREDIT_CARD_NUMBER)), + "}"}), + "#card_number"); AddRequiredField(&action, - base::NumberToString(static_cast( - autofill::ServerFieldType::CREDIT_CARD_NAME_FULL)), - "#card_name"); - AddRequiredField(&action, - base::NumberToString(static_cast( - autofill::ServerFieldType::CREDIT_CARD_NUMBER)), - "#card_number"); - AddRequiredField(&action, - base::NumberToString(static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_NETWORK)), + base::StrCat({"${", + base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_NETWORK)), + "}"}), "#network"); // First validation fails. @@ -222,7 +316,7 @@ TEST_F(UseCreditCardActionTest, FillCreditCardWithFallback) { // Expect fields to be filled Expectation set_cvc = EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#cvc"}), "123", _)) + OnSetFieldValue(Selector({"#cvc"}), kFakeCvc, _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); Expectation set_expmonth = EXPECT_CALL(mock_action_delegate_, @@ -230,15 +324,15 @@ TEST_F(UseCreditCardActionTest, FillCreditCardWithFallback) { .WillOnce(RunOnceCallback<2>(OkClientStatus())); Expectation set_expyear2 = EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#expyear2"}), "24", _)) + OnSetFieldValue(Selector({"#expyear2"}), "50", _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); Expectation set_expyear4 = EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#expyear4"}), "2024", _)) + OnSetFieldValue(Selector({"#expyear4"}), "2050", _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); Expectation set_cardholder_name = EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#card_name"}), "Jon Doe", _)) + OnSetFieldValue(Selector({"#card_name"}), "Adam West", _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); Expectation set_card_number = EXPECT_CALL( @@ -275,19 +369,8 @@ TEST_F(UseCreditCardActionTest, FillCreditCardWithFallback) { .After(set_card_network) .WillOnce(RunOnceCallback<1>(OkClientStatus(), "not empty")); - autofill::CreditCard credit_card; - credit_card.SetExpirationMonth(9); - credit_card.SetExpirationYear(2024); - credit_card.SetRawInfo(autofill::CREDIT_CARD_NAME_FULL, - base::UTF8ToUTF16("Jon Doe")); - credit_card.SetRawInfo(autofill::CREDIT_CARD_NUMBER, - base::UTF8ToUTF16("4111111111111111")); - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(OkClientStatus())); @@ -301,8 +384,10 @@ TEST_F(UseCreditCardActionTest, ForcedFallbackWithKeystrokes) { ActionProto action = CreateUseCreditCardAction(); auto* cvc_required = AddRequiredField( &action, - base::NumberToString(static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE)), + base::StrCat({"${", + base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)), + "}"}), "#cvc"); cvc_required->set_forced(true); cvc_required->set_fill_strategy(SIMULATE_KEY_PRESSES); @@ -314,16 +399,12 @@ TEST_F(UseCreditCardActionTest, ForcedFallbackWithKeystrokes) { // But we still want the CVC filled, with simulated keypresses. EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#cvc"}), "123", true, 1000, _)) + OnSetFieldValue(Selector({"#cvc"}), kFakeCvc, true, 1000, _)) .WillOnce(RunOnceCallback<4>(OkClientStatus())); - autofill::CreditCard credit_card; - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); + user_data_.selected_card_ = std::make_unique(); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(OkClientStatus())); @@ -334,19 +415,17 @@ TEST_F(UseCreditCardActionTest, SkippingAutofill) { ON_CALL(mock_action_delegate_, GetElementTag(_, _)) .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "INPUT")); - ActionProto action = CreateUseCreditCardAction(); + ActionProto action; AddRequiredField( &action, - base::NumberToString(static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE)), + base::StrCat({"${", + base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)), + "}"}), "#cvc"); action.mutable_use_card()->set_skip_autofill(true); - autofill::CreditCard credit_card; - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); + EXPECT_CALL(mock_action_delegate_, OnShortWaitForElement(_, _)).Times(0); EXPECT_CALL(mock_action_delegate_, OnFillCardForm(_, _, _, _)).Times(0); // First validation fails. @@ -355,7 +434,7 @@ TEST_F(UseCreditCardActionTest, SkippingAutofill) { // Fill cvc. Expectation set_cvc = EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#cvc"}), "123", _)) + OnSetFieldValue(Selector({"#cvc"}), kFakeCvc, _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); // Second validation succeeds. EXPECT_CALL(mock_web_controller_, OnGetFieldValue(Selector({"#cvc"}), _)) @@ -368,13 +447,9 @@ TEST_F(UseCreditCardActionTest, SkippingAutofill) { TEST_F(UseCreditCardActionTest, AutofillFailureWithoutRequiredFieldsIsFatal) { ActionProto action_proto = CreateUseCreditCardAction(); - autofill::CreditCard credit_card; - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); + user_data_.selected_card_ = std::make_unique(); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(ClientStatus(OTHER_ACTION_STATUS))); @@ -397,17 +472,15 @@ TEST_F(UseCreditCardActionTest, ActionProto action_proto = CreateUseCreditCardAction(); AddRequiredField( &action_proto, - base::NumberToString(static_cast( - UseCreditCardProto::RequiredField::CREDIT_CARD_VERIFICATION_CODE)), + base::StrCat({"${", + base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)), + "}"}), "#cvc"); - autofill::CreditCard credit_card; - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); + user_data_.selected_card_ = std::make_unique(); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>( FillAutofillErrorStatus(ClientStatus(OTHER_ACTION_STATUS)))); @@ -418,7 +491,7 @@ TEST_F(UseCreditCardActionTest, // Fill CVC. Expectation set_cvc = EXPECT_CALL(mock_action_delegate_, - OnSetFieldValue(Selector({"#cvc"}), "123", _)) + OnSetFieldValue(Selector({"#cvc"}), kFakeCvc, _)) .WillOnce(RunOnceCallback<2>(OkClientStatus())); // Second validation succeeds. EXPECT_CALL(mock_web_controller_, OnGetFieldValue(Selector({"#cvc"}), _)) @@ -447,19 +520,8 @@ TEST_F(UseCreditCardActionTest, FallbackForCardExpirationSucceeds) { AddRequiredField(&action_proto, "${53} - ${55}", "#expiration_date"); // Autofill succeeds. - autofill::CreditCard credit_card; - credit_card.SetExpirationMonth(9); - credit_card.SetExpirationYear(2050); - credit_card.SetRawInfo(autofill::CREDIT_CARD_NAME_FULL, - base::UTF8ToUTF16("Jon Doe")); - credit_card.SetRawInfo(autofill::CREDIT_CARD_NUMBER, - base::UTF8ToUTF16("4111111111111111")); - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(OkClientStatus())); @@ -489,22 +551,11 @@ TEST_F(UseCreditCardActionTest, FallbackFails) { .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "INPUT")); ActionProto action_proto = CreateUseCreditCardAction(); - AddRequiredField(&action_proto, "57", "#expiration_date"); + AddRequiredField(&action_proto, "${57}", "#expiration_date"); // Autofill succeeds. - autofill::CreditCard credit_card; - credit_card.SetExpirationMonth(9); - credit_card.SetExpirationYear(2050); - credit_card.SetRawInfo(autofill::CREDIT_CARD_NAME_FULL, - base::UTF8ToUTF16("Jon Doe")); - credit_card.SetRawInfo(autofill::CREDIT_CARD_NUMBER, - base::UTF8ToUTF16("4111111111111111")); - user_data_.selected_card_ = - std::make_unique(credit_card); - EXPECT_CALL(mock_action_delegate_, OnGetFullCard(_)) - .WillOnce(RunOnceCallback<0>(credit_card, base::UTF8ToUTF16("123"))); EXPECT_CALL(mock_action_delegate_, - OnFillCardForm(_, base::UTF8ToUTF16("123"), + OnFillCardForm(_, base::UTF8ToUTF16(kFakeCvc), Selector({kFakeSelector}).MustBeVisible(), _)) .WillOnce(RunOnceCallback<3>(OkClientStatus())); @@ -518,8 +569,23 @@ TEST_F(UseCreditCardActionTest, FallbackFails) { OnSetFieldValue(Eq(Selector({"#expiration_date"})), "09/2050", _)) .WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS))); - EXPECT_EQ(ProcessedActionStatusProto::AUTOFILL_INCOMPLETE, - ProcessAction(action_proto)); + ProcessedActionProto processed_action; + EXPECT_CALL(callback_, Run(_)).WillOnce(SaveArgPointee<0>(&processed_action)); + + UseCreditCardAction action(&mock_action_delegate_, action_proto); + action.ProcessAction(callback_.Get()); + + EXPECT_EQ(processed_action.status(), + ProcessedActionStatusProto::AUTOFILL_INCOMPLETE); + EXPECT_TRUE(processed_action.has_status_details()); + EXPECT_EQ(processed_action.status_details() + .autofill_error_info() + .autofill_field_error_size(), + 1); + EXPECT_EQ(OTHER_ACTION_STATUS, processed_action.status_details() + .autofill_error_info() + .autofill_field_error(0) + .status()); } } // namespace diff --git a/chromium/components/autofill_assistant/browser/actions/wait_for_document_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/wait_for_document_action_unittest.cc index 7cf07a0faad..3ae75c0a587 100644 --- a/chromium/components/autofill_assistant/browser/actions/wait_for_document_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/wait_for_document_action_unittest.cc @@ -192,7 +192,7 @@ TEST_F(WaitForDocumentActionTest, CheckDocumentInFrame) { .WillOnce(RunOnceCallback<1>(OkClientStatus(), DOCUMENT_COMPLETE)); proto_.set_timeout_ms(0); - proto_.mutable_frame()->add_selectors("#frame"); + *proto_.mutable_frame() = ToSelectorProto("#frame"); Run(); EXPECT_EQ(ACTION_APPLIED, processed_action_.status()); } @@ -203,7 +203,7 @@ TEST_F(WaitForDocumentActionTest, CheckFrameElementNotFound) { RunOnceCallback<1>(ClientStatus(ELEMENT_RESOLUTION_FAILED))); proto_.set_timeout_ms(0); - proto_.mutable_frame()->add_selectors("#frame"); + *proto_.mutable_frame() = ToSelectorProto("#frame"); Run(); EXPECT_EQ(ELEMENT_RESOLUTION_FAILED, processed_action_.status()); } diff --git a/chromium/components/autofill_assistant/browser/actions/wait_for_dom_action_unittest.cc b/chromium/components/autofill_assistant/browser/actions/wait_for_dom_action_unittest.cc index ecedb4531a9..4f496d18a68 100644 --- a/chromium/components/autofill_assistant/browser/actions/wait_for_dom_action_unittest.cc +++ b/chromium/components/autofill_assistant/browser/actions/wait_for_dom_action_unittest.cc @@ -102,7 +102,8 @@ TEST_F(WaitForDomActionTest, ConditionMet) { EXPECT_CALL(mock_web_controller_, OnElementCheck(Selector({"#element"}), _)) .WillRepeatedly(RunOnceCallback<1>(OkClientStatus())); - proto_.mutable_wait_condition()->mutable_match()->add_selectors("#element"); + *proto_.mutable_wait_condition()->mutable_match() = + ToSelectorProto("#element"); EXPECT_CALL( callback_, Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED)))); @@ -110,7 +111,8 @@ TEST_F(WaitForDomActionTest, ConditionMet) { } TEST_F(WaitForDomActionTest, ConditionNotMet) { - proto_.mutable_wait_condition()->mutable_match()->add_selectors("#element"); + *proto_.mutable_wait_condition()->mutable_match() = + ToSelectorProto("#element"); EXPECT_CALL(callback_, Run(Pointee(Property(&ProcessedActionProto::status, ELEMENT_RESOLUTION_FAILED)))); Run(); @@ -128,25 +130,21 @@ TEST_F(WaitForDomActionTest, ReportMatchesToServer) { auto* any_of = proto_.mutable_wait_condition()->mutable_any_of(); auto* condition1 = any_of->add_conditions(); - condition1->mutable_match()->add_selectors("#element1"); + *condition1->mutable_match() = ToSelectorProto("#element1"); condition1->set_payload("1"); auto* condition2 = any_of->add_conditions(); - condition2->mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("#element2"); + *condition2->mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("#element2"); condition2->set_payload("2"); auto* condition3 = any_of->add_conditions(); - condition3->mutable_match()->add_selectors("#element3"); + *condition3->mutable_match() = ToSelectorProto("#element3"); condition3->set_payload("3"); auto* condition4 = any_of->add_conditions(); - condition4->mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("#element4"); + *condition4->mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("#element4"); condition4->set_payload("4"); // Condition 1 and 2 are met, conditions 3 and 4 are not. diff --git a/chromium/components/autofill_assistant/browser/basic_interactions.cc b/chromium/components/autofill_assistant/browser/basic_interactions.cc index 149429ede1d..e11808288fb 100644 --- a/chromium/components/autofill_assistant/browser/basic_interactions.cc +++ b/chromium/components/autofill_assistant/browser/basic_interactions.cc @@ -9,6 +9,7 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "components/autofill/core/browser/autofill_data_util.h" +#include "components/autofill_assistant/browser/field_formatter.h" #include "components/autofill_assistant/browser/script_executor_delegate.h" #include "components/autofill_assistant/browser/trigger_context.h" #include "components/autofill_assistant/browser/user_model.h" @@ -103,10 +104,8 @@ bool ValueToString(UserModel* user_model, switch (value->kind_case()) { case ValueProto::kUserActions: case ValueProto::kLoginOptions: - case ValueProto::kCreditCards: - case ValueProto::kProfiles: case ValueProto::kCreditCardResponse: - case ValueProto::kLoginOptionResponse: + case ValueProto::kServerPayload: DVLOG(2) << "Error evaluating " << __func__ << ": does not support values of type " << value->kind_case(); return false; @@ -156,12 +155,59 @@ bool ValueToString(UserModel* user_model, time, proto.date_format().date_format().c_str()))); break; } + case ValueProto::kCreditCards: { + if (proto.autofill_format().pattern().empty()) { + DVLOG(2) << "Error evaluating " << __func__ << ": pattern not set"; + return false; + } + auto* credit_card = + user_model->GetCreditCard(value->credit_cards().values(i).guid()); + if (!credit_card) { + DVLOG(2) << "Error evaluating " << __func__ + << ": credit card not found"; + return false; + } + auto formatted_string = field_formatter::FormatString( + proto.autofill_format().pattern(), + field_formatter::CreateAutofillMappings( + *credit_card, proto.autofill_format().locale())); + if (!formatted_string.has_value()) { + DVLOG(2) << "Error evaluating " << __func__ + << ": error formatting pattern '" + << proto.autofill_format().pattern() << "'"; + return false; + } + result.mutable_strings()->add_values(*formatted_string); + break; + } + case ValueProto::kProfiles: { + if (proto.autofill_format().pattern().empty()) { + DVLOG(2) << "Error evaluating " << __func__ << ": pattern not set"; + return false; + } + auto* profile = + user_model->GetProfile(value->profiles().values(i).guid()); + if (!profile) { + DVLOG(2) << "Error evaluating " << __func__ << ": profile not found"; + return false; + } + auto formatted_string = field_formatter::FormatString( + proto.autofill_format().pattern(), + field_formatter::CreateAutofillMappings( + *profile, proto.autofill_format().locale())); + if (!formatted_string.has_value()) { + DVLOG(2) << "Error evaluating " << __func__ + << ": error formatting pattern '" + << proto.autofill_format().pattern() << "'"; + return false; + } + result.mutable_strings()->add_values(*formatted_string); + break; + } case ValueProto::kUserActions: case ValueProto::kLoginOptions: - case ValueProto::kCreditCards: - case ValueProto::kProfiles: case ValueProto::kCreditCardResponse: - case ValueProto::kLoginOptionResponse: + case ValueProto::kServerPayload: case ValueProto::KIND_NOT_SET: NOTREACHED(); return false; @@ -326,8 +372,7 @@ bool CreateLoginOptionResponse(UserModel* user_model, // The result is intentionally not client_side_only, irrespective of input. ValueProto result; - result.mutable_login_option_response()->set_payload( - value->login_options().values(0).payload()); + result.set_server_payload(value->login_options().values(0).payload()); user_model->SetValue(result_model_identifier, result); return true; } @@ -515,28 +560,46 @@ bool BasicInteractions::ToggleUserAction(const ToggleUserActionProto& proto) { return true; } -bool BasicInteractions::EndAction(bool view_inflation_successful, - const EndActionProto& proto) { +bool BasicInteractions::EndAction(const ClientStatus& status) { if (!end_action_callback_) { DVLOG(2) << "Failed to EndAction: no callback set"; return false; } - std::move(end_action_callback_) - .Run(view_inflation_successful, proto.status(), - delegate_->GetUserModel()); + + // It is possible for the action to end before view inflation was finished. + // In that case, the action can end directly and does not need to receive this + // callback. + view_inflation_finished_callback_.Reset(); + std::move(end_action_callback_).Run(status); return true; } -void BasicInteractions::ClearEndActionCallback() { +bool BasicInteractions::NotifyViewInflationFinished( + const ClientStatus& status) { + if (!view_inflation_finished_callback_) { + return false; + } + std::move(view_inflation_finished_callback_).Run(status); + return true; +} + +void BasicInteractions::ClearCallbacks() { end_action_callback_.Reset(); + view_inflation_finished_callback_.Reset(); } void BasicInteractions::SetEndActionCallback( - base::OnceCallback - end_action_callback) { + base::OnceCallback end_action_callback) { end_action_callback_ = std::move(end_action_callback); } +void BasicInteractions::SetViewInflationFinishedCallback( + base::OnceCallback + view_inflation_finished_callback) { + view_inflation_finished_callback_ = + std::move(view_inflation_finished_callback); +} + bool BasicInteractions::RunConditionalCallback( const std::string& condition_identifier, base::RepeatingCallback callback) { @@ -559,36 +622,4 @@ bool BasicInteractions::RunConditionalCallback( return true; } -bool BasicInteractions::UpdateRadioButtonGroup( - const std::vector& model_identifiers, - const std::string& selected_model_identifier) { - auto selected_iterator = - std::find(model_identifiers.begin(), model_identifiers.end(), - selected_model_identifier); - if (selected_iterator == model_identifiers.end()) { - return false; - } - - auto values = delegate_->GetUserModel()->GetValues(model_identifiers); - if (!values.has_value()) { - return false; - } - - if (!AreAllValuesOfType(*values, ValueProto::kBooleans)) { - return false; - } - - if (!AreAllValuesOfSize(*values, 1)) { - return false; - } - - for (const auto& model_identifier : model_identifiers) { - if (model_identifier == selected_model_identifier) { - continue; - } - delegate_->GetUserModel()->SetValue(model_identifier, SimpleValue(false)); - } - return true; -} - } // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/basic_interactions.h b/chromium/components/autofill_assistant/browser/basic_interactions.h index 0a265fad541..69e256e15db 100644 --- a/chromium/components/autofill_assistant/browser/basic_interactions.h +++ b/chromium/components/autofill_assistant/browser/basic_interactions.h @@ -7,11 +7,11 @@ #include "base/bind_helpers.h" #include "base/memory/weak_ptr.h" +#include "components/autofill_assistant/browser/client_status.h" #include "components/autofill_assistant/browser/generic_ui.pb.h" namespace autofill_assistant { class ScriptExecutorDelegate; -class UserModel; // Provides basic interactions for use by the generic UI framework. These // methods are intended to be bound to by the corresponding interaction @@ -43,16 +43,23 @@ class BasicInteractions { bool ToggleUserAction(const ToggleUserActionProto& proto); // Ends the current action. Can only be called during a ShowGenericUiAction. - bool EndAction(bool view_inflation_successful, const EndActionProto& proto); + bool EndAction(const ClientStatus& status); + + // Runs |view_inflation_finished_callback_| to notify its owner that view + // inflation has finished. Can only be called during a ShowGenericUiAction. + bool NotifyViewInflationFinished(const ClientStatus& status); // Sets the callback to end the current ShowGenericUiAction. void SetEndActionCallback( - base::OnceCallback end_action_callback); + base::OnceCallback end_action_callback); + + // Sets the callback to indicate whether view inflation was successful or not. + void SetViewInflationFinishedCallback( + base::OnceCallback + view_inflation_finished_callback); - // Clears the |end_action_callback_|. - void ClearEndActionCallback(); + // Clears all callbacks associated with the current ShowGenericUi action. + void ClearCallbacks(); // Runs |callback| if |condition_identifier| points to a single boolean set to // 'true'. Returns true on success (i.e., condition was evaluated @@ -60,17 +67,12 @@ class BasicInteractions { bool RunConditionalCallback(const std::string& condition_identifier, base::RepeatingCallback callback); - // Disables all radio buttons in |model_identifiers| except - // |selected_model_identifier|. Fails if one or more model identifiers are not - // found. - bool UpdateRadioButtonGroup(const std::vector& model_identifiers, - const std::string& selected_model_identifier); - private: ScriptExecutorDelegate* delegate_; // Only valid during a ShowGenericUiAction. - base::OnceCallback - end_action_callback_; + base::OnceCallback end_action_callback_; + base::OnceCallback + view_inflation_finished_callback_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/chromium/components/autofill_assistant/browser/basic_interactions_unittest.cc b/chromium/components/autofill_assistant/browser/basic_interactions_unittest.cc index fb1316eba9e..1eb98bdb3cc 100644 --- a/chromium/components/autofill_assistant/browser/basic_interactions_unittest.cc +++ b/chromium/components/autofill_assistant/browser/basic_interactions_unittest.cc @@ -16,6 +16,7 @@ namespace autofill_assistant { +using ::testing::_; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::InSequence; @@ -219,6 +220,91 @@ TEST_F(BasicInteractionsTest, ComputeValueToString) { /* is_client_side_only = */ true)); } + // Credit cards + autofill::CreditCard credit_card_a(base::GenerateGUID(), + "https://www.example.com"); + autofill::test::SetCreditCardInfo(&credit_card_a, "Marion Mitchell", + "4111 1111 1111 1111", "01", "2050", ""); + autofill::CreditCard credit_card_b(base::GenerateGUID(), + "https://www.example.com"); + autofill::test::SetCreditCardInfo(&credit_card_b, "John Doe", + "4111 1111 1111 2222", "02", "2051", ""); + auto credit_cards = + std::make_unique>>(); + credit_cards->emplace_back( + std::make_unique(credit_card_a)); + credit_cards->emplace_back( + std::make_unique(credit_card_b)); + user_model_.SetAutofillCreditCards(std::move(credit_cards)); + ValueProto credit_cards_value; + credit_cards_value.mutable_credit_cards()->add_values()->set_guid( + credit_card_a.guid()); + credit_cards_value.mutable_credit_cards()->add_values()->set_guid( + credit_card_b.guid()); + user_model_.SetValue("value", credit_cards_value); + // Formatting credit cards fails if pattern or locale are not set. + proto.mutable_to_string()->mutable_autofill_format()->set_locale("en-US"); + EXPECT_FALSE(basic_interactions_.ComputeValue(proto)); + // {name} {network} **** {last-4-digits} ({month/year}) + proto.mutable_to_string()->mutable_autofill_format()->set_pattern( + "${51}. ${-5} **** ${-4} (${53}/${54})"); + EXPECT_TRUE(basic_interactions_.ComputeValue(proto)); + ValueProto expected_result; + expected_result.mutable_strings()->add_values( + "Marion Mitchell. Visa **** 1111 (01/50)"); + expected_result.mutable_strings()->add_values( + "John Doe. Visa **** 2222 (02/51)"); + EXPECT_EQ(*user_model_.GetValue("output"), expected_result); + + // Profiles + autofill::AutofillProfile profile_a(base::GenerateGUID(), + "https://www.example.com"); + autofill::test::SetProfileInfo( + &profile_a, "Marion", "Mitchell", "Morrison", "marion@me.xyz", "Fox", + "123 Zoo St.", "unit 5", "Hollywood", "CA", "91601", "US", "16505678910"); + autofill::AutofillProfile profile_b(base::GenerateGUID(), + "https://www.example.com"); + autofill::test::SetProfileInfo(&profile_b, "John", "", "Doe", + "editor@gmail.com", "", "203 Barfield Lane", + "apt A", "Mountain View", "CA", "94043", "US", + "+12345678901"); + auto profiles = std::make_unique< + std::vector>>(); + profiles->emplace_back( + std::make_unique(profile_a)); + profiles->emplace_back( + std::make_unique(profile_b)); + user_model_.SetAutofillProfiles(std::move(profiles)); + ValueProto profiles_value; + profiles_value.mutable_profiles()->add_values()->set_guid(profile_a.guid()); + profiles_value.mutable_profiles()->add_values()->set_guid(profile_b.guid()); + user_model_.SetValue("value", profiles_value); + // Formatting profiles fails if pattern is not set. + EXPECT_FALSE(basic_interactions_.ComputeValue(proto)); + // {name_full}, {address_line_1} {address_line_2} {zip code} {city} {country} + proto.mutable_to_string()->mutable_autofill_format()->set_pattern( + "${7} ${30} ${31} ${35} ${33} ${36}"); + EXPECT_TRUE(basic_interactions_.ComputeValue(proto)); + expected_result.Clear(); + expected_result.mutable_strings()->add_values( + "Marion Mitchell Morrison 123 Zoo St. unit 5 91601 Hollywood United " + "States"); + expected_result.mutable_strings()->add_values( + "John Doe 203 Barfield Lane apt A 94043 Mountain View United States"); + EXPECT_EQ(*user_model_.GetValue("output"), expected_result); + + // Different locale. + proto.mutable_to_string()->mutable_autofill_format()->set_locale("de-DE"); + EXPECT_TRUE(basic_interactions_.ComputeValue(proto)); + expected_result.Clear(); + expected_result.mutable_strings()->add_values( + "Marion Mitchell Morrison 123 Zoo St. unit 5 91601 Hollywood Vereinigte " + "Staaten"); + expected_result.mutable_strings()->add_values( + "John Doe 203 Barfield Lane apt A 94043 Mountain View Vereinigte " + "Staaten"); + EXPECT_EQ(*user_model_.GetValue("output"), expected_result); + // Empty value fails. user_model_.SetValue("value", ValueProto()); EXPECT_FALSE(basic_interactions_.ComputeValue(proto)); @@ -229,7 +315,7 @@ TEST_F(BasicInteractionsTest, ComputeValueToString) { multi_value.mutable_booleans()->add_values(false); user_model_.SetValue("value", multi_value); EXPECT_TRUE(basic_interactions_.ComputeValue(proto)); - ValueProto expected_result; + expected_result.Clear(); expected_result.mutable_strings()->add_values("true"); expected_result.mutable_strings()->add_values("false"); EXPECT_EQ(*user_model_.GetValue("output"), expected_result); @@ -294,20 +380,42 @@ TEST_F(BasicInteractionsTest, ComputeValueIntegerSum) { TEST_F(BasicInteractionsTest, EndActionWithoutCallbackFails) { EndActionProto proto; - EXPECT_FALSE(basic_interactions_.EndAction(true, proto)); + EXPECT_FALSE(basic_interactions_.EndAction(ClientStatus(INVALID_ACTION))); } TEST_F(BasicInteractionsTest, EndActionWithCallbackSucceeds) { - base::MockCallback> - callback; + base::MockCallback> callback; basic_interactions_.SetEndActionCallback(callback.Get()); - EndActionProto proto; - proto.set_status(ACTION_APPLIED); + EXPECT_CALL(callback, + Run(Property(&ClientStatus::proto_status, ACTION_APPLIED))); + EXPECT_TRUE(basic_interactions_.EndAction(ClientStatus(ACTION_APPLIED))); +} + +TEST_F(BasicInteractionsTest, NotifyViewInflationFinishedRunsCallback) { + base::MockCallback> callback; + basic_interactions_.SetViewInflationFinishedCallback(callback.Get()); + + EXPECT_CALL(callback, + Run(Property(&ClientStatus::proto_status, ACTION_APPLIED))); + basic_interactions_.NotifyViewInflationFinished(ClientStatus(ACTION_APPLIED)); +} - EXPECT_CALL(callback, Run(true, ACTION_APPLIED, &user_model_)); - EXPECT_TRUE(basic_interactions_.EndAction(true, proto)); +TEST_F(BasicInteractionsTest, EndActionResetsViewInflationCallback) { + base::MockCallback> + view_inflation_finished_callback; + base::MockCallback> + end_action_callback; + basic_interactions_.SetViewInflationFinishedCallback( + view_inflation_finished_callback.Get()); + basic_interactions_.SetEndActionCallback(end_action_callback.Get()); + + EXPECT_CALL(end_action_callback, + Run(Property(&ClientStatus::proto_status, INVALID_ACTION))); + EXPECT_CALL(view_inflation_finished_callback, Run(_)).Times(0); + EXPECT_TRUE(basic_interactions_.EndAction(ClientStatus(INVALID_ACTION))); + EXPECT_FALSE(basic_interactions_.NotifyViewInflationFinished( + ClientStatus(ACTION_APPLIED))); } TEST_F(BasicInteractionsTest, ComputeValueCompare) { @@ -493,8 +601,7 @@ TEST_F(BasicInteractionsTest, ComputeValueCreateLoginOptionResponse) { // LoginOptionResponseProto is allowed to extract the payload from // client-only values. ValueProto expected_response_value; - expected_response_value.mutable_login_option_response()->set_payload( - "payload"); + expected_response_value.set_server_payload("payload"); expected_response_value.set_is_client_side_only(false); EXPECT_EQ(user_model_.GetValue("result"), expected_response_value); } diff --git a/chromium/components/autofill_assistant/browser/client.h b/chromium/components/autofill_assistant/browser/client.h index 2b44e689581..bbcba879b07 100644 --- a/chromium/components/autofill_assistant/browser/client.h +++ b/chromium/components/autofill_assistant/browser/client.h @@ -76,6 +76,9 @@ class Client { // Returns details about the device. virtual DeviceContext GetDeviceContext() const = 0; + // Returns whether a11y (talkback and touch exploration) is enabled or not. + virtual bool IsAccessibilityEnabled() const = 0; + // Returns current WebContents. virtual content::WebContents* GetWebContents() const = 0; diff --git a/chromium/components/autofill_assistant/browser/client_settings.h b/chromium/components/autofill_assistant/browser/client_settings.h index 94674066739..330aac1a2d8 100644 --- a/chromium/components/autofill_assistant/browser/client_settings.h +++ b/chromium/components/autofill_assistant/browser/client_settings.h @@ -82,7 +82,7 @@ struct ClientSettings { base::Optional overlay_image; // Optional settings intended for integration tests. - base::Optional + base::Optional integration_test_settings; void UpdateFromProto(const ClientSettingsProto& proto); diff --git a/chromium/components/autofill_assistant/browser/client_status.cc b/chromium/components/autofill_assistant/browser/client_status.cc index d99fa0dfb91..c3b4ad9cd54 100644 --- a/chromium/components/autofill_assistant/browser/client_status.cc +++ b/chromium/components/autofill_assistant/browser/client_status.cc @@ -119,6 +119,9 @@ std::ostream& operator<<(std::ostream& out, case ProcessedActionStatusProto::AUTOFILL_INCOMPLETE: out << "AUTOFILL_INCOMPLETE"; break; + case ProcessedActionStatusProto::TOO_MANY_CANDIDATES: + out << "TOO_MANY_CANDIDATES"; + break; // Intentionally no default case to make compilation fail if a new value // was added to the enum but not to this list. diff --git a/chromium/components/autofill_assistant/browser/controller.cc b/chromium/components/autofill_assistant/browser/controller.cc index f980f810704..db966cfd225 100644 --- a/chromium/components/autofill_assistant/browser/controller.cc +++ b/chromium/components/autofill_assistant/browser/controller.cc @@ -12,7 +12,6 @@ #include "base/no_destructor.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" -#include "base/task/post_task.h" #include "base/time/tick_clock.h" #include "base/values.h" #include "components/autofill_assistant/browser/actions/collect_user_data_action.h" @@ -23,6 +22,7 @@ #include "components/autofill_assistant/browser/service_impl.h" #include "components/autofill_assistant/browser/trigger_context.h" #include "components/autofill_assistant/browser/user_data.h" +#include "components/autofill_assistant/browser/view_layout.pb.h" #include "components/google/core/common/google_util.h" #include "components/password_manager/core/browser/password_manager_client.h" #include "components/strings/grit/components_strings.h" @@ -43,14 +43,16 @@ namespace { static constexpr int kAutostartInitialProgress = 5; // Parameter that allows setting the color of the overlay. -static const char* const kOverlayColorParameterName = "OVERLAY_COLORS"; +const char kOverlayColorParameterName[] = "OVERLAY_COLORS"; // Parameter that contains the current session username. Should be synced with // |SESSION_USERNAME_PARAMETER| from // .../password_manager/PasswordChangeLauncher.java // TODO(b/151401974): Eliminate duplicate parameter definitions. -static const char* const kPasswordChangeUsernameParameterName = - "PASSWORD_CHANGE_USERNAME"; +const char kPasswordChangeUsernameParameterName[] = "PASSWORD_CHANGE_USERNAME"; + +// Experiment for toggling the new progress bar. +const char kProgressBarExperiment[] = "4400697"; // Returns true if the state requires a UI to be shown. // @@ -241,6 +243,15 @@ int Controller::GetProgress() const { return progress_; } +base::Optional Controller::GetProgressActiveStep() const { + return progress_active_step_; +} + +base::Optional +Controller::GetStepProgressBarConfiguration() const { + return step_progress_bar_configuration_; +} + void Controller::SetInfoBox(const InfoBox& info_box) { if (!info_box_) { info_box_ = std::make_unique(); @@ -273,6 +284,18 @@ void Controller::SetProgress(int progress) { } } +void Controller::SetProgressActiveStep(int active_step) { + // Step can only increase. + if (progress_active_step_ >= active_step) { + return; + } + + progress_active_step_ = active_step; + for (ControllerObserver& observer : observers_) { + observer.OnProgressActiveStepChanged(active_step); + } +} + void Controller::SetProgressVisible(bool visible) { if (progress_visible_ == visible) return; @@ -287,6 +310,38 @@ bool Controller::GetProgressVisible() const { return progress_visible_; } +void Controller::SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& configuration) { + step_progress_bar_configuration_ = configuration; + if (!configuration.step_icons().empty() && + progress_active_step_.has_value() && + configuration.step_icons().size() < *progress_active_step_) { + progress_active_step_ = configuration.step_icons().size(); + } + for (ControllerObserver& observer : observers_) { + observer.OnStepProgressBarConfigurationChanged(configuration); + if (progress_active_step_.has_value()) { + observer.OnProgressActiveStepChanged(*progress_active_step_); + } + observer.OnProgressBarErrorStateChanged(progress_bar_error_state_); + } +} + +void Controller::SetProgressBarErrorState(bool error) { + if (progress_bar_error_state_ == error) { + return; + } + + progress_bar_error_state_ = error; + for (ControllerObserver& observer : observers_) { + observer.OnProgressBarErrorStateChanged(error); + } +} + +bool Controller::GetProgressBarErrorState() const { + return progress_bar_error_state_; +} + const std::vector& Controller::GetUserActions() const { static const base::NoDestructor> no_user_actions_; return user_actions_ ? *user_actions_ : *no_user_actions_; @@ -321,10 +376,13 @@ void Controller::RequireUI() { void Controller::SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback - end_action_callback) { + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) { generic_user_interface_ = std::move(generic_ui); basic_interactions_.SetEndActionCallback(std::move(end_action_callback)); + basic_interactions_.SetViewInflationFinishedCallback( + std::move(view_inflation_finished_callback)); for (ControllerObserver& observer : observers_) { observer.OnGenericUserInterfaceChanged(generic_user_interface_.get()); } @@ -332,7 +390,7 @@ void Controller::SetGenericUi( void Controller::ClearGenericUi() { generic_user_interface_.reset(); - basic_interactions_.ClearEndActionCallback(); + basic_interactions_.ClearCallbacks(); for (ControllerObserver& observer : observers_) { observer.OnGenericUserInterfaceChanged(nullptr); } @@ -699,10 +757,11 @@ void Controller::StartPeriodicScriptChecks() { if (periodic_script_check_scheduled_) return; periodic_script_check_scheduled_ = true; - base::PostDelayedTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(&Controller::OnPeriodicScriptCheck, - weak_ptr_factory_.GetWeakPtr()), - settings_.periodic_script_check_interval); + content::GetUIThreadTaskRunner({})->PostDelayedTask( + FROM_HERE, + base::BindOnce(&Controller::OnPeriodicScriptCheck, + weak_ptr_factory_.GetWeakPtr()), + settings_.periodic_script_check_interval); } void Controller::StopPeriodicScriptChecks() { @@ -732,10 +791,11 @@ void Controller::OnPeriodicScriptCheck() { } script_tracker()->CheckScripts(); - base::PostDelayedTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(&Controller::OnPeriodicScriptCheck, - weak_ptr_factory_.GetWeakPtr()), - settings_.periodic_script_check_interval); + content::GetUIThreadTaskRunner({})->PostDelayedTask( + FROM_HERE, + base::BindOnce(&Controller::OnPeriodicScriptCheck, + weak_ptr_factory_.GetWeakPtr()), + settings_.periodic_script_check_interval); } void Controller::OnGetScripts(const GURL& url, @@ -980,6 +1040,12 @@ void Controller::InitFromParameters() { user_data_->selected_login_.emplace(web_contents()->GetLastCommittedURL(), *password_change_username); } + + if (trigger_context_->HasExperimentId(kProgressBarExperiment)) { + ShowProgressBarProto::StepProgressBarConfiguration mock_configuration; + mock_configuration.set_use_step_progress_bar(true); + SetStepProgressBarConfiguration(mock_configuration); + } } void Controller::Track(std::unique_ptr trigger_context, @@ -1043,7 +1109,12 @@ void Controller::ShowFirstMessageAndStart() { SetStatusMessage( l10n_util::GetStringFUTF8(IDS_AUTOFILL_ASSISTANT_LOADING, base::UTF8ToUTF16(GetCurrentURL().host()))); - SetProgress(kAutostartInitialProgress); + if (step_progress_bar_configuration_.has_value() && + step_progress_bar_configuration_->use_step_progress_bar()) { + SetProgressActiveStep(0); + } else { + SetProgress(kAutostartInitialProgress); + } EnterState(AutofillAssistantState::STARTING); } @@ -1051,6 +1122,10 @@ AutofillAssistantState Controller::GetState() { return state_; } +int64_t Controller::GetErrorCausingNavigationId() const { + return error_causing_navigation_id_; +} + void Controller::OnScriptSelected(const ScriptHandle& handle, std::unique_ptr context) { ExecuteScript(handle.path, handle.start_message, handle.needs_ui, @@ -1435,6 +1510,7 @@ void Controller::OnScriptError(const std::string& error_message, RequireUI(); SetStatusMessage(error_message); + SetProgressBarErrorState(true); EnterStoppedState(); if (tracking_) { @@ -1454,6 +1530,7 @@ void Controller::OnFatalError(const std::string& error_message, return; SetStatusMessage(error_message); + SetProgressBarErrorState(true); EnterStoppedState(); // If we haven't managed to check the set of scripts yet at this point, we @@ -1588,10 +1665,6 @@ void Controller::OnRunnableScriptsChanged( SetUserActions(std::move(user_actions)); } -void Controller::DidAttachInterstitialPage() { - client_->Shutdown(Metrics::DropOutReason::INTERSTITIAL_PAGE); -} - void Controller::DidFinishLoad(content::RenderFrameHost* render_frame_host, const GURL& validated_url) { // validated_url might not be the page URL. Ignore it and always check the @@ -1599,6 +1672,10 @@ void Controller::DidFinishLoad(content::RenderFrameHost* render_frame_host, OnUrlChange(); } +void Controller::ExpectNavigation() { + expect_navigation_ = true; +} + void Controller::DidStartNavigation( content::NavigationHandle* navigation_handle) { if (!navigation_handle->IsInMainFrame() || @@ -1611,6 +1688,12 @@ void Controller::DidStartNavigation( ReportNavigationStateChanged(); } + // The navigation is expected, do not check for errors below. + if (expect_navigation_) { + expect_navigation_ = false; + return; + } + // The following types of navigations are allowed for the main frame, when // in PROMPT state: // - first-time URL load @@ -1627,19 +1710,38 @@ void Controller::DidStartNavigation( // Everything else, such as going back to a previous page, or refreshing the // page is considered an end condition. If going back to a previous page is // required, consider using the BROWSE state instead. - // Note that BROWSE state end conditions are in DidFinishNavigation, in order - // to be able to properly evaluate the committed url. if (state_ == AutofillAssistantState::PROMPT && web_contents()->GetLastCommittedURL().is_valid() && !navigation_handle->WasServerRedirect() && !navigation_handle->IsRendererInitiated()) { + error_causing_navigation_id_ = navigation_handle->GetNavigationId(); OnScriptError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP), Metrics::DropOutReason::NAVIGATION); + return; } + + if (base::FeatureList::IsEnabled( + features::kAutofillAssistantBreakOnRunningNavigation)) { + // When in RUNNING state, all renderer initiated navigation is allowed, + // user initiated navigation will cause an error. + if (state_ == AutofillAssistantState::RUNNING && + !navigation_handle->WasServerRedirect() && + !navigation_handle->IsRendererInitiated()) { + error_causing_navigation_id_ = navigation_handle->GetNavigationId(); + OnScriptError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP), + Metrics::DropOutReason::NAVIGATION_WHILE_RUNNING); + return; + } + } + + // Note that BROWSE state end conditions are in DidFinishNavigation, in order + // to be able to properly evaluate the committed url. } void Controller::DidFinishNavigation( content::NavigationHandle* navigation_handle) { + // TODO(b/159871774): Rethink how we handle navigation events. The early + // return here may prevent us from updating |navigating_to_new_document_|. if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSameDocument() || !navigation_handle->HasCommitted() || !IsNavigatingToNewDocument()) { @@ -1758,11 +1860,6 @@ void Controller::WriteUserData( } } -void Controller::WriteUserModel( - base::OnceCallback write_callback) { - std::move(write_callback).Run(&user_model_); -} - ElementArea* Controller::touchable_element_area() { if (!touchable_element_area_) { touchable_element_area_ = std::make_unique(this); diff --git a/chromium/components/autofill_assistant/browser/controller.h b/chromium/components/autofill_assistant/browser/controller.h index 998f8f84e7d..e35235d7bd4 100644 --- a/chromium/components/autofill_assistant/browser/controller.h +++ b/chromium/components/autofill_assistant/browser/controller.h @@ -119,7 +119,11 @@ class Controller : public ScriptExecutorDelegate, void SetInfoBox(const InfoBox& info_box) override; void ClearInfoBox() override; void SetProgress(int progress) override; + void SetProgressActiveStep(int active_step) override; void SetProgressVisible(bool visible) override; + void SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& configuration) + override; void SetUserActions( std::unique_ptr> user_actions) override; void SetViewportMode(ViewportMode mode) override; @@ -130,13 +134,14 @@ class Controller : public ScriptExecutorDelegate, std::unique_ptr form, base::RepeatingCallback changed_callback, base::OnceCallback cancel_callback) override; + void ExpectNavigation() override; bool IsNavigatingToNewDocument() override; bool HasNavigationError() override; void SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback end_action_callback) override; + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) override; void ClearGenericUi() override; // Show the UI if it's not already shown. This is only meaningful while in @@ -153,18 +158,21 @@ class Controller : public ScriptExecutorDelegate, void SetCollectUserDataOptions(CollectUserDataOptions* options) override; void WriteUserData( base::OnceCallback) override; - void WriteUserModel( - base::OnceCallback write_callback) override; void OnScriptError(const std::string& error_message, Metrics::DropOutReason reason); // Overrides autofill_assistant::UiDelegate: AutofillAssistantState GetState() override; + int64_t GetErrorCausingNavigationId() const override; void OnUserInteractionInsideTouchableArea() override; const Details* GetDetails() const override; const InfoBox* GetInfoBox() const override; int GetProgress() const override; + base::Optional GetProgressActiveStep() const override; bool GetProgressVisible() const override; + bool GetProgressBarErrorState() const override; + base::Optional + GetStepProgressBarConfiguration() const override; const std::vector& GetUserActions() const override; bool PerformUserActionWithContext( int index, @@ -279,7 +287,6 @@ class Controller : public ScriptExecutorDelegate, const std::vector& runnable_scripts) override; // Overrides content::WebContentsObserver: - void DidAttachInterstitialPage() override; void DidFinishLoad(content::RenderFrameHost* render_frame_host, const GURL& validated_url) override; void DidStartNavigation( @@ -311,6 +318,8 @@ class Controller : public ScriptExecutorDelegate, // Clear out visible state and enter the stopped state. void EnterStoppedState(); + void SetProgressBarErrorState(bool error); + ElementArea* touchable_element_area(); ScriptTracker* script_tracker(); bool allow_autostart() { return state_ == AutofillAssistantState::STARTING; } @@ -327,6 +336,7 @@ class Controller : public ScriptExecutorDelegate, std::unique_ptr trigger_context_; AutofillAssistantState state_ = AutofillAssistantState::INACTIVE; + int64_t error_causing_navigation_id_ = -1; // The URL passed to Start(). Used only as long as there's no committed URL. // Note that this is the deeplink passed by a caller. @@ -373,9 +383,13 @@ class Controller : public ScriptExecutorDelegate, // Current progress. int progress_ = 0; + base::Optional progress_active_step_; // Current visibility of the progress bar. It is initially visible. bool progress_visible_ = true; + bool progress_bar_error_state_ = false; + base::Optional + step_progress_bar_configuration_; // Current set of user actions. May be null, but never empty. std::unique_ptr> user_actions_; @@ -411,6 +425,9 @@ class Controller : public ScriptExecutorDelegate, bool navigation_error_ = false; base::ObserverList navigation_listeners_; + // The next DidStartNavigation will not cause an error. + bool expect_navigation_ = false; + // Tracks scripts and script execution. It's kept at the end, as it tend to // depend on everything the controller support, through script and script // actions. diff --git a/chromium/components/autofill_assistant/browser/controller_observer.h b/chromium/components/autofill_assistant/browser/controller_observer.h index 6cc7d6a6cd4..eeb525f47fe 100644 --- a/chromium/components/autofill_assistant/browser/controller_observer.h +++ b/chromium/components/autofill_assistant/browser/controller_observer.h @@ -67,10 +67,20 @@ class ControllerObserver : public base::CheckedObserver { // percentage. virtual void OnProgressChanged(int progress) = 0; + // Called when the currently active progress step has changed. + virtual void OnProgressActiveStepChanged(int active_step) = 0; + // Called when the current progress bar visibility has changed. If |visible| // is true, then the bar is now shown. virtual void OnProgressVisibilityChanged(bool visible) = 0; + virtual void OnStepProgressBarConfigurationChanged( + const ShowProgressBarProto::StepProgressBarConfiguration& + configuration) = 0; + + // Called when the progress bar error state changes. + virtual void OnProgressBarErrorStateChanged(bool error) = 0; + // Updates the area of the visible viewport that is accessible when the // overlay state is OverlayState::PARTIAL. // diff --git a/chromium/components/autofill_assistant/browser/controller_unittest.cc b/chromium/components/autofill_assistant/browser/controller_unittest.cc index eb8dbeed3c6..9b16df336cf 100644 --- a/chromium/components/autofill_assistant/browser/controller_unittest.cc +++ b/chromium/components/autofill_assistant/browser/controller_unittest.cc @@ -416,12 +416,11 @@ TEST_F(ControllerTest, NoRelevantScripts) { TEST_F(ControllerTest, NoRelevantScriptYet) { SupportsScriptResponseProto script_response; - AddRunnableScript(&script_response, "no_match_yet") - ->mutable_presentation() - ->mutable_precondition() - ->mutable_element_condition() - ->mutable_match() - ->add_selectors("#element"); + *AddRunnableScript(&script_response, "no_match_yet") + ->mutable_presentation() + ->mutable_precondition() + ->mutable_element_condition() + ->mutable_match() = ToSelectorProto("#element"); SetNextScriptResponse(script_response); Start("http://a.example.com/path"); @@ -714,12 +713,11 @@ TEST_F(ControllerTest, AttachUIWhenContentsFocused) { TEST_F(ControllerTest, KeepCheckingForElement) { SupportsScriptResponseProto script_response; - AddRunnableScript(&script_response, "no_match_yet") - ->mutable_presentation() - ->mutable_precondition() - ->mutable_element_condition() - ->mutable_match() - ->add_selectors("#element"); + *AddRunnableScript(&script_response, "no_match_yet") + ->mutable_presentation() + ->mutable_precondition() + ->mutable_element_condition() + ->mutable_match() = ToSelectorProto("#element"); SetNextScriptResponse(script_response); Start("http://a.example.com/path"); @@ -743,12 +741,11 @@ TEST_F(ControllerTest, ScriptTimeoutError) { // Wait for #element to show up for will_never_match. After 25s, execute the // script on_timeout_error. SupportsScriptResponseProto script_response; - AddRunnableScript(&script_response, "will_never_match") - ->mutable_presentation() - ->mutable_precondition() - ->mutable_element_condition() - ->mutable_match() - ->add_selectors("#element"); + *AddRunnableScript(&script_response, "will_never_match") + ->mutable_presentation() + ->mutable_precondition() + ->mutable_element_condition() + ->mutable_match() = ToSelectorProto("#element"); script_response.mutable_script_timeout_error()->set_timeout_ms(30000); script_response.mutable_script_timeout_error()->set_script_path( "on_timeout_error"); @@ -777,12 +774,11 @@ TEST_F(ControllerTest, ScriptTimeoutWarning) { // Wait for #element to show up for will_never_match. After 10s, execute the // script on_timeout_error. SupportsScriptResponseProto script_response; - AddRunnableScript(&script_response, "will_never_match") - ->mutable_presentation() - ->mutable_precondition() - ->mutable_element_condition() - ->mutable_match() - ->add_selectors("#element"); + *AddRunnableScript(&script_response, "will_never_match") + ->mutable_presentation() + ->mutable_precondition() + ->mutable_element_condition() + ->mutable_match() = ToSelectorProto("#element"); script_response.mutable_script_timeout_error()->set_timeout_ms(4000); script_response.mutable_script_timeout_error()->set_script_path( "on_timeout_error"); @@ -1249,12 +1245,11 @@ TEST_F(ControllerTest, TrackReportsNoScripts) { TEST_F(ControllerTest, TrackReportsNoScriptsForNow) { SupportsScriptResponseProto script_response; - AddRunnableScript(&script_response, "no_match_yet") - ->mutable_presentation() - ->mutable_precondition() - ->mutable_element_condition() - ->mutable_match() - ->add_selectors("#element"); + *AddRunnableScript(&script_response, "no_match_yet") + ->mutable_presentation() + ->mutable_precondition() + ->mutable_element_condition() + ->mutable_match() = ToSelectorProto("#element"); SetNextScriptResponse(script_response); SetLastCommittedUrl(GURL("http://example.com/")); @@ -1574,10 +1569,23 @@ TEST_F(ControllerTest, UnexpectedNavigationDuringPromptAction) { EXPECT_CALL(mock_observer_, OnStatusMessageChanged(testing::Not(never_shown))) .Times(testing::AnyNumber()); - EXPECT_CALL(mock_client_, Shutdown(Metrics::DropOutReason::NAVIGATION)); + // Renderer (Document) initiated navigation is allowed. + EXPECT_CALL(mock_client_, Shutdown(_)).Times(0); + content::NavigationSimulator::NavigateAndCommitFromDocument( + GURL("http://a.example.com/page"), web_contents()->GetMainFrame()); + EXPECT_EQ(AutofillAssistantState::PROMPT, controller_->GetState()); + + // Expected browser initiated navigation is allowed. + EXPECT_CALL(mock_client_, Shutdown(_)).Times(0); + controller_->ExpectNavigation(); content::NavigationSimulator::NavigateAndCommitFromBrowser( - web_contents(), GURL("http://example.com/otherpage")); + web_contents(), GURL("http://b.example.com/page")); + EXPECT_EQ(AutofillAssistantState::PROMPT, controller_->GetState()); + // Unexpected browser initiated navigation will cause an error. + EXPECT_CALL(mock_client_, Shutdown(Metrics::DropOutReason::NAVIGATION)); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + web_contents(), GURL("http://c.example.com/page")); EXPECT_EQ(AutofillAssistantState::STOPPED, controller_->GetState()); // Full history of state transitions. @@ -1587,6 +1595,55 @@ TEST_F(ControllerTest, UnexpectedNavigationDuringPromptAction) { AutofillAssistantState::STOPPED)); } +TEST_F(ControllerTest, UnexpectedNavigationInRunningState) { + SupportsScriptResponseProto script_response; + AddRunnableScript(&script_response, "autostart") + ->mutable_presentation() + ->set_autostart(true); + SetNextScriptResponse(script_response); + + ActionsResponseProto autostart_script; + auto* wait_for_dom = autostart_script.add_actions()->mutable_wait_for_dom(); + wait_for_dom->set_timeout_ms(10000); + wait_for_dom->mutable_wait_condition() + ->mutable_match() + ->add_filters() + ->set_css_selector("#some-element"); + SetupActionsForScript("autostart", autostart_script); + + Start(); + EXPECT_EQ(AutofillAssistantState::RUNNING, controller_->GetState()); + + // Document (not user) initiated navigation while in RUNNING state: + // The controller keeps going. + EXPECT_CALL(mock_client_, Shutdown(_)).Times(0); + content::NavigationSimulator::NavigateAndCommitFromDocument( + GURL("http://a.example.com/page"), web_contents()->GetMainFrame()); + EXPECT_EQ(AutofillAssistantState::RUNNING, controller_->GetState()); + + // Expected browser initiated navigation while in RUNNING state: + // The controller keeps going. + EXPECT_CALL(mock_client_, Shutdown(_)).Times(0); + controller_->ExpectNavigation(); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + web_contents(), GURL("http://b.example.com/page")); + EXPECT_EQ(AutofillAssistantState::RUNNING, controller_->GetState()); + + // Unexpected browser initiated navigation while in RUNNING state: + // The controller stops the scripts, shows an error and shuts down. + EXPECT_CALL(mock_client_, + Shutdown(Metrics::DropOutReason::NAVIGATION_WHILE_RUNNING)); + EXPECT_CALL(mock_observer_, OnStatusMessageChanged(_)); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + web_contents(), GURL("http://c.example.com/page")); + EXPECT_EQ(AutofillAssistantState::STOPPED, controller_->GetState()); + + // Full history of state transitions. + EXPECT_THAT(states_, ElementsAre(AutofillAssistantState::STARTING, + AutofillAssistantState::RUNNING, + AutofillAssistantState::STOPPED)); +} + TEST_F(ControllerTest, NavigationToGooglePropertyDestroysUI) { SupportsScriptResponseProto script_response; AddRunnableScript(&script_response, "autostart") @@ -2271,7 +2328,7 @@ TEST_F(ControllerTest, SetGenericUi) { } controller_->SetGenericUi( std::make_unique(GenericUserInterfaceProto()), - base::DoNothing()); + base::DoNothing(), base::DoNothing()); controller_->ClearGenericUi(); } diff --git a/chromium/components/autofill_assistant/browser/devtools/devtools_api/domain_type_conversions_h.template b/chromium/components/autofill_assistant/browser/devtools/devtools_api/domain_type_conversions_h.template index bbc506c3ed8..681e555e16d 100644 --- a/chromium/components/autofill_assistant/browser/devtools/devtools_api/domain_type_conversions_h.template +++ b/chromium/components/autofill_assistant/browser/devtools/devtools_api/domain_type_conversions_h.template @@ -9,7 +9,7 @@ #ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_DEVTOOLS_DEVTOOLS_INTERNAL_TYPE_CONVERSIONS_{{domain.domain | camelcase_to_hacker_style | upper}}_H_ #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_DEVTOOLS_DEVTOOLS_INTERNAL_TYPE_CONVERSIONS_{{domain.domain | camelcase_to_hacker_style | upper}}_H_ -#include "base/logging.h" +#include "base/notreached.h" #include "components/autofill_assistant/browser/devtools/devtools/domains/types_{{domain.domain | camelcase_to_hacker_style}}.h" #include "components/autofill_assistant/browser/devtools/value_conversions.h" diff --git a/chromium/components/autofill_assistant/browser/devtools/devtools_client.cc b/chromium/components/autofill_assistant/browser/devtools/devtools_client.cc index 3c213373bba..8f7af508b55 100644 --- a/chromium/components/autofill_assistant/browser/devtools/devtools_client.cc +++ b/chromium/components/autofill_assistant/browser/devtools/devtools_client.cc @@ -13,7 +13,6 @@ #include "base/callback_forward.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" -#include "base/task/post_task.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" @@ -30,8 +29,7 @@ DevtoolsClient::DevtoolsClient( renderer_crashed_(false), next_message_id_(0), frame_tracker_(this) { - browser_main_thread_ = - base::CreateSingleThreadTaskRunner({content::BrowserThread::UI}); + browser_main_thread_ = content::GetUIThreadTaskRunner({}); agent_host_->AttachClient(this); frame_tracker_.Start(); } diff --git a/chromium/components/autofill_assistant/browser/element_area.cc b/chromium/components/autofill_assistant/browser/element_area.cc index 6136a6e62e7..0b513492506 100644 --- a/chromium/components/autofill_assistant/browser/element_area.cc +++ b/chromium/components/autofill_assistant/browser/element_area.cc @@ -50,6 +50,9 @@ void ElementArea::Clear() { void ElementArea::SetFromProto(const ElementAreaProto& proto) { rectangles_.clear(); + last_visual_viewport_ = RectF(); + last_rectangles_.clear(); + AddRectangles(proto.touchable(), /* restricted= */ false); AddRectangles(proto.restricted(), /* restricted= */ true); diff --git a/chromium/components/autofill_assistant/browser/element_area_unittest.cc b/chromium/components/autofill_assistant/browser/element_area_unittest.cc index 5da55e62e68..3ccde28d42f 100644 --- a/chromium/components/autofill_assistant/browser/element_area_unittest.cc +++ b/chromium/components/autofill_assistant/browser/element_area_unittest.cc @@ -86,7 +86,7 @@ class ElementAreaTest : public testing::Test { void SetElement(const std::string& selector, bool restricted) { ElementAreaProto area; auto* rectangle = restricted ? area.add_restricted() : area.add_touchable(); - rectangle->add_elements()->add_selectors(selector); + *rectangle->add_elements() = ToSelectorProto(selector); element_area_.SetFromProto(area); } @@ -164,6 +164,17 @@ TEST_F(ElementAreaTest, CallOnUpdate) { EXPECT_THAT(reported_area_, ElementsAre(MatchingRectF(25, 25, 75, 75))); } +TEST_F(ElementAreaTest, CallOnUpdateAfterSetFromProto) { + EXPECT_CALL(mock_web_controller_, + OnGetElementPosition(Eq(Selector({"#found"}).MustBeVisible()), _)) + .WillRepeatedly(RunOnceCallback<1>(true, RectF(25, 25, 75, 75))); + + SetElement("#found"); + EXPECT_EQ(on_update_call_count_, 1); + SetElement("#found"); + EXPECT_EQ(on_update_call_count_, 2); +} + TEST_F(ElementAreaTest, DontCallOnUpdateWhenViewportMissing) { // Swallowing calls to OnGetVisualViewport guarantees that the viewport // position will never be known. @@ -203,8 +214,9 @@ TEST_F(ElementAreaTest, TwoRectangles) { .WillOnce(RunOnceCallback<1>(true, RectF(25, 25, 100, 100))); ElementAreaProto area_proto; - area_proto.add_touchable()->add_elements()->add_selectors("#top_left"); - area_proto.add_touchable()->add_elements()->add_selectors("#bottom_right"); + *area_proto.add_touchable()->add_elements() = ToSelectorProto("#top_left"); + *area_proto.add_touchable()->add_elements() = + ToSelectorProto("#bottom_right"); element_area_.SetFromProto(area_proto); std::vector rectangles; @@ -225,8 +237,8 @@ TEST_F(ElementAreaTest, OneRectangleTwoElements) { ElementAreaProto area_proto; auto* rectangle_proto = area_proto.add_touchable(); - rectangle_proto->add_elements()->add_selectors("#element1"); - rectangle_proto->add_elements()->add_selectors("#element2"); + *rectangle_proto->add_elements() = ToSelectorProto("#element1"); + *rectangle_proto->add_elements() = ToSelectorProto("#element2"); element_area_.SetFromProto(area_proto); std::vector rectangles; @@ -249,8 +261,8 @@ TEST_F(ElementAreaTest, DoNotReportIncompleteRectangles) { ElementAreaProto area_proto; auto* rectangle_proto = area_proto.add_touchable(); - rectangle_proto->add_elements()->add_selectors("#element1"); - rectangle_proto->add_elements()->add_selectors("#element2"); + *rectangle_proto->add_elements() = ToSelectorProto("#element1"); + *rectangle_proto->add_elements() = ToSelectorProto("#element2"); element_area_.SetFromProto(area_proto); EXPECT_THAT(reported_area_, IsEmpty()); @@ -280,10 +292,10 @@ TEST_F(ElementAreaTest, OneRectangleFourElements) { ElementAreaProto area_proto; auto* rectangle_proto = area_proto.add_touchable(); - rectangle_proto->add_elements()->add_selectors("#element1"); - rectangle_proto->add_elements()->add_selectors("#element2"); - rectangle_proto->add_elements()->add_selectors("#element3"); - rectangle_proto->add_elements()->add_selectors("#element4"); + *rectangle_proto->add_elements() = ToSelectorProto("#element1"); + *rectangle_proto->add_elements() = ToSelectorProto("#element2"); + *rectangle_proto->add_elements() = ToSelectorProto("#element3"); + *rectangle_proto->add_elements() = ToSelectorProto("#element4"); element_area_.SetFromProto(area_proto); std::vector rectangles; @@ -303,8 +315,8 @@ TEST_F(ElementAreaTest, OneRectangleMissingElementsReported) { ElementAreaProto area_proto; auto* rectangle_proto = area_proto.add_touchable(); - rectangle_proto->add_elements()->add_selectors("#element1"); - rectangle_proto->add_elements()->add_selectors("#element2"); + *rectangle_proto->add_elements() = ToSelectorProto("#element1"); + *rectangle_proto->add_elements() = ToSelectorProto("#element2"); element_area_.SetFromProto(area_proto); std::vector rectangles; @@ -328,8 +340,8 @@ TEST_F(ElementAreaTest, FullWidthRectangle) { ElementAreaProto area_proto; auto* rectangle_proto = area_proto.add_touchable(); - rectangle_proto->add_elements()->add_selectors("#element1"); - rectangle_proto->add_elements()->add_selectors("#element2"); + *rectangle_proto->add_elements() = ToSelectorProto("#element1"); + *rectangle_proto->add_elements() = ToSelectorProto("#element2"); rectangle_proto->set_full_width(true); element_area_.SetFromProto(area_proto); diff --git a/chromium/components/autofill_assistant/browser/element_precondition_unittest.cc b/chromium/components/autofill_assistant/browser/element_precondition_unittest.cc index 7756d8ccdb1..b9f1058c92a 100644 --- a/chromium/components/autofill_assistant/browser/element_precondition_unittest.cc +++ b/chromium/components/autofill_assistant/browser/element_precondition_unittest.cc @@ -67,7 +67,7 @@ TEST_F(ElementPreconditionTest, Empty) { } TEST_F(ElementPreconditionTest, NonEmpty) { - condition_.mutable_match()->add_selectors("exists"); + *condition_.mutable_match() = ToSelectorProto("exists"); EXPECT_FALSE(ElementPrecondition(condition_).empty()); } @@ -87,15 +87,15 @@ TEST_F(ElementPreconditionTest, EmptySelector) { } TEST_F(ElementPreconditionTest, ElementExists) { - condition_.mutable_match()->add_selectors("exists"); + *condition_.mutable_match() = ToSelectorProto("exists"); EXPECT_CALL(mock_callback_, Run(Property(&ClientStatus::proto_status, ACTION_APPLIED), _)); Check(mock_callback_.Get()); } -TEST_F(ElementPreconditionTest, ElementDoesNotExist) { - condition_.mutable_match()->add_selectors("does_not_exist"); +TEST_F(ElementPreconditionTest, ElementDoes_Not_Exist) { + *condition_.mutable_match() = ToSelectorProto("does_not_exist"); EXPECT_CALL( mock_callback_, @@ -113,10 +113,10 @@ TEST_F(ElementPreconditionTest, AnyOf_Empty) { } TEST_F(ElementPreconditionTest, AnyOf_NoneMatch) { - condition_.mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist"); - condition_.mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist_either"); + *condition_.mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); + *condition_.mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist_either"); EXPECT_CALL( mock_callback_, @@ -125,10 +125,10 @@ TEST_F(ElementPreconditionTest, AnyOf_NoneMatch) { } TEST_F(ElementPreconditionTest, AnyOf_SomeMatch) { - condition_.mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "exists"); - condition_.mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist"); + *condition_.mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); EXPECT_CALL(mock_callback_, Run(Property(&ClientStatus::proto_status, ACTION_APPLIED), _)); @@ -136,10 +136,10 @@ TEST_F(ElementPreconditionTest, AnyOf_SomeMatch) { } TEST_F(ElementPreconditionTest, AnyOf_AllMatch) { - condition_.mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "exists"); - condition_.mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "exists_too"); + *condition_.mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists_too"); EXPECT_CALL(mock_callback_, Run(Property(&ClientStatus::proto_status, ACTION_APPLIED), _)); @@ -155,10 +155,10 @@ TEST_F(ElementPreconditionTest, AllOf_Empty) { } TEST_F(ElementPreconditionTest, AllOf_NoneMatch) { - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist"); - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist_either"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist_either"); EXPECT_CALL( mock_callback_, @@ -167,10 +167,10 @@ TEST_F(ElementPreconditionTest, AllOf_NoneMatch) { } TEST_F(ElementPreconditionTest, AllOf_SomeMatch) { - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "exists"); - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); EXPECT_CALL( mock_callback_, @@ -179,10 +179,10 @@ TEST_F(ElementPreconditionTest, AllOf_SomeMatch) { } TEST_F(ElementPreconditionTest, AllOf_AllMatch) { - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "exists"); - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "exists_too"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists_too"); EXPECT_CALL(mock_callback_, Run(Property(&ClientStatus::proto_status, ACTION_APPLIED), _)); @@ -198,14 +198,10 @@ TEST_F(ElementPreconditionTest, NoneOf_Empty) { } TEST_F(ElementPreconditionTest, NoneOf_NoneMatch) { - condition_.mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("does_not_exist"); - condition_.mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("does_not_exist_either"); + *condition_.mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); + *condition_.mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist_either"); EXPECT_CALL(mock_callback_, Run(Property(&ClientStatus::proto_status, ACTION_APPLIED), _)); @@ -213,14 +209,10 @@ TEST_F(ElementPreconditionTest, NoneOf_NoneMatch) { } TEST_F(ElementPreconditionTest, NoneOf_SomeMatch) { - condition_.mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("exists"); - condition_.mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("does_not_exist"); + *condition_.mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); EXPECT_CALL( mock_callback_, @@ -229,14 +221,10 @@ TEST_F(ElementPreconditionTest, NoneOf_SomeMatch) { } TEST_F(ElementPreconditionTest, NoneOf_AllMatch) { - condition_.mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("exists"); - condition_.mutable_none_of() - ->add_conditions() - ->mutable_match() - ->add_selectors("exists_too"); + *condition_.mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists_too"); EXPECT_CALL( mock_callback_, @@ -246,11 +234,11 @@ TEST_F(ElementPreconditionTest, NoneOf_AllMatch) { TEST_F(ElementPreconditionTest, Payload_ConditionMet) { auto* exists = condition_.mutable_any_of()->add_conditions(); - exists->mutable_match()->add_selectors("exists"); + *exists->mutable_match() = ToSelectorProto("exists"); exists->set_payload("exists"); auto* exists_too = condition_.mutable_any_of()->add_conditions(); - exists_too->mutable_match()->add_selectors("exists_too"); + *exists_too->mutable_match() = ToSelectorProto("exists_too"); exists_too->set_payload("exists_too"); condition_.set_payload("any_of"); @@ -263,11 +251,11 @@ TEST_F(ElementPreconditionTest, Payload_ConditionMet) { TEST_F(ElementPreconditionTest, Payload_ConditionNotMet) { auto* exists = condition_.mutable_none_of()->add_conditions(); - exists->mutable_match()->add_selectors("exists"); + *exists->mutable_match() = ToSelectorProto("exists"); exists->set_payload("exists"); auto* exists_too = condition_.mutable_none_of()->add_conditions(); - exists_too->mutable_match()->add_selectors("exists_too"); + *exists_too->mutable_match() = ToSelectorProto("exists_too"); exists_too->set_payload("exists_too"); condition_.set_payload("none_of"); @@ -279,27 +267,28 @@ TEST_F(ElementPreconditionTest, Payload_ConditionNotMet) { } TEST_F(ElementPreconditionTest, Complex) { - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "exists"); - condition_.mutable_all_of()->add_conditions()->mutable_match()->add_selectors( - "exists_too"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists"); + *condition_.mutable_all_of()->add_conditions()->mutable_match() = + ToSelectorProto("exists_too"); auto* none_of = condition_.mutable_all_of()->add_conditions(); none_of->set_payload("none_of"); auto* does_not_exist_in_none_of = none_of->mutable_none_of()->add_conditions(); - does_not_exist_in_none_of->mutable_match()->add_selectors("does_not_exist"); + *does_not_exist_in_none_of->mutable_match() = + ToSelectorProto("does_not_exist"); does_not_exist_in_none_of->set_payload("does_not_exist in none_of"); - none_of->mutable_none_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist_either"); + *none_of->mutable_none_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist_either"); auto* any_of = condition_.mutable_all_of()->add_conditions(); any_of->set_payload("any_of"); auto* exists_in_any_of = any_of->mutable_any_of()->add_conditions(); - exists_in_any_of->mutable_match()->add_selectors("exists"); + *exists_in_any_of->mutable_match() = ToSelectorProto("exists"); exists_in_any_of->set_payload("exists in any_of"); - any_of->mutable_any_of()->add_conditions()->mutable_match()->add_selectors( - "does_not_exist"); + *any_of->mutable_any_of()->add_conditions()->mutable_match() = + ToSelectorProto("does_not_exist"); EXPECT_CALL(mock_callback_, Run(Property(&ClientStatus::proto_status, ACTION_APPLIED), diff --git a/chromium/components/autofill_assistant/browser/event_handler.cc b/chromium/components/autofill_assistant/browser/event_handler.cc index 886f8f96c4c..543d22d176b 100644 --- a/chromium/components/autofill_assistant/browser/event_handler.cc +++ b/chromium/components/autofill_assistant/browser/event_handler.cc @@ -3,6 +3,9 @@ // found in the LICENSE file. #include "components/autofill_assistant/browser/event_handler.h" +#include "base/strings/string_number_conversions.h" + +#include "base/logging.h" namespace autofill_assistant { @@ -24,6 +27,66 @@ void EventHandler::RemoveObserver(const Observer* observer) { observers_.RemoveObserver(observer); } +// static +base::Optional EventHandler::CreateEventKeyFromProto( + const EventProto& proto) { + switch (proto.kind_case()) { + case EventProto::kOnValueChanged: + if (proto.on_value_changed().model_identifier().empty()) { + VLOG(1) << "Invalid OnValueChangedEventProto: no model_identifier " + "specified"; + return base::nullopt; + } + return base::Optional( + {proto.kind_case(), proto.on_value_changed().model_identifier()}); + case EventProto::kOnViewClicked: + if (proto.on_view_clicked().view_identifier().empty()) { + VLOG(1) << "Invalid OnViewClickedEventProto: no view_identifier " + "specified"; + return base::nullopt; + } + return base::Optional( + {proto.kind_case(), proto.on_view_clicked().view_identifier()}); + case EventProto::kOnUserActionCalled: + if (proto.on_user_action_called().user_action_identifier().empty()) { + VLOG(1) << "Invalid OnUserActionCalled: no user_action_identifier " + "specified"; + return base::nullopt; + } + return base::Optional( + {proto.kind_case(), + proto.on_user_action_called().user_action_identifier()}); + case EventProto::kOnTextLinkClicked: + if (!proto.on_text_link_clicked().has_text_link()) { + VLOG(1) << "Invalid OnTextLinkClickedProto: no text_link specified"; + return base::nullopt; + } + return base::Optional( + {proto.kind_case(), + base::NumberToString(proto.on_text_link_clicked().text_link())}); + case EventProto::kOnPopupDismissed: + if (proto.on_popup_dismissed().popup_identifier().empty()) { + VLOG(1) + << "Invalid OnPopupDismissedProto: no popup_identifier specified"; + return base::nullopt; + } + return base::Optional( + {proto.kind_case(), proto.on_popup_dismissed().popup_identifier()}); + case EventProto::kOnViewContainerCleared: + if (proto.on_view_container_cleared().view_identifier().empty()) { + VLOG(1) << "Invalid OnViewContainerClearedProto: no view_identifier " + "specified"; + return base::nullopt; + } + return base::Optional( + {proto.kind_case(), + proto.on_view_container_cleared().view_identifier()}); + case EventProto::KIND_NOT_SET: + VLOG(1) << "Error creating event: kind not set"; + return base::nullopt; + } +} + std::ostream& operator<<(std::ostream& out, const EventProto::KindCase& event_case) { #ifdef NDEBUG @@ -46,6 +109,9 @@ std::ostream& operator<<(std::ostream& out, case EventProto::kOnPopupDismissed: out << "kOnPopupDismissed"; break; + case EventProto::kOnViewContainerCleared: + out << "kOnViewContainerCleared"; + break; case EventProto::KIND_NOT_SET: break; } diff --git a/chromium/components/autofill_assistant/browser/event_handler.h b/chromium/components/autofill_assistant/browser/event_handler.h index e4718433008..92ac4b96344 100644 --- a/chromium/components/autofill_assistant/browser/event_handler.h +++ b/chromium/components/autofill_assistant/browser/event_handler.h @@ -10,6 +10,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" +#include "base/optional.h" #include "components/autofill_assistant/browser/service.pb.h" #include "components/autofill_assistant/browser/user_model.h" @@ -37,6 +38,9 @@ class EventHandler { void DispatchEvent(const EventKey& key); + static base::Optional CreateEventKeyFromProto( + const EventProto& proto); + void AddObserver(Observer* observer); void RemoveObserver(const Observer* observer); diff --git a/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.cc b/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.cc index 3c8b0febdc0..616f918d7e9 100644 --- a/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.cc +++ b/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.cc @@ -103,8 +103,13 @@ void FakeScriptExecutorDelegate::ClearInfoBox() { void FakeScriptExecutorDelegate::SetProgress(int progress) {} +void FakeScriptExecutorDelegate::SetProgressActiveStep(int active_step) {} + void FakeScriptExecutorDelegate::SetProgressVisible(bool visible) {} +void FakeScriptExecutorDelegate::SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& configuration) {} + void FakeScriptExecutorDelegate::SetUserActions( std::unique_ptr> user_actions) { user_actions_ = std::move(user_actions); @@ -126,11 +131,6 @@ void FakeScriptExecutorDelegate::WriteUserData( std::move(write_callback).Run(payment_request_info_.get(), &field_change); } -void FakeScriptExecutorDelegate::WriteUserModel( - base::OnceCallback write_callback) { - std::move(write_callback).Run(user_model_); -} - void FakeScriptExecutorDelegate::SetViewportMode(ViewportMode mode) { viewport_mode_ = mode; } @@ -158,6 +158,8 @@ void FakeScriptExecutorDelegate::CollapseBottomSheet() { expand_or_collapse_value_ = false; } +void FakeScriptExecutorDelegate::ExpectNavigation() {} + bool FakeScriptExecutorDelegate::HasNavigationError() { return navigation_error_; } @@ -204,8 +206,9 @@ EventHandler* FakeScriptExecutorDelegate::GetEventHandler() { void FakeScriptExecutorDelegate::SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback - end_action_callback) {} + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) {} void FakeScriptExecutorDelegate::ClearGenericUi() {} diff --git a/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.h b/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.h index 4594334faac..3948c00cc2a 100644 --- a/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.h +++ b/chromium/components/autofill_assistant/browser/fake_script_executor_delegate.h @@ -46,14 +46,16 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate { void SetInfoBox(const InfoBox& info_box) override; void ClearInfoBox() override; void SetProgress(int progress) override; + void SetProgressActiveStep(int active_step) override; void SetProgressVisible(bool visible) override; + void SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& configuration) + override; void SetUserActions( std::unique_ptr> user_actions) override; void SetCollectUserDataOptions(CollectUserDataOptions* options) override; void WriteUserData( base::OnceCallback) override; - void WriteUserModel( - base::OnceCallback write_callback) override; void SetViewportMode(ViewportMode mode) override; ViewportMode GetViewportMode() override; void SetPeekMode(ConfigureBottomSheetProto::PeekMode peek_mode) override; @@ -66,6 +68,7 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate { base::OnceCallback cancel_callback) override; UserModel* GetUserModel() override; EventHandler* GetEventHandler() override; + void ExpectNavigation() override; bool HasNavigationError() override; bool IsNavigatingToNewDocument() override; void RequireUI() override; @@ -76,9 +79,9 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate { void SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback end_action_callback) override; + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) override; void ClearGenericUi() override; ClientSettings* GetMutableSettings() { return &client_settings_; } diff --git a/chromium/components/autofill_assistant/browser/features.cc b/chromium/components/autofill_assistant/browser/features.cc index 943ab632ad6..6f42c54b476 100644 --- a/chromium/components/autofill_assistant/browser/features.cc +++ b/chromium/components/autofill_assistant/browser/features.cc @@ -12,6 +12,14 @@ namespace features { const base::Feature kAutofillAssistant{"AutofillAssistant", base::FEATURE_ENABLED_BY_DEFAULT}; +// Guard for the end condition when a non-renderer-initiated navigation occurs +// while the AutofillAssistant is in RUNNING state. +// TODO(b/159309621): Remove this if the end condition shows no unwanted side +// effects. +const base::Feature kAutofillAssistantBreakOnRunningNavigation{ + "AutofillAssistantBreakOnRunningNavigation", + base::FEATURE_ENABLED_BY_DEFAULT}; + // Controls whether to enable Assistant Autofill in a normal Chrome tab. const base::Feature kAutofillAssistantChromeEntry{ "AutofillAssistantChromeEntry", base::FEATURE_ENABLED_BY_DEFAULT}; diff --git a/chromium/components/autofill_assistant/browser/features.h b/chromium/components/autofill_assistant/browser/features.h index 689c466b23c..0478342a905 100644 --- a/chromium/components/autofill_assistant/browser/features.h +++ b/chromium/components/autofill_assistant/browser/features.h @@ -11,8 +11,10 @@ struct Feature; namespace autofill_assistant { namespace features { + // All features in alphabetical order. extern const base::Feature kAutofillAssistant; +extern const base::Feature kAutofillAssistantBreakOnRunningNavigation; extern const base::Feature kAutofillAssistantChromeEntry; extern const base::Feature kAutofillAssistantDirectActions; diff --git a/chromium/components/autofill_assistant/browser/field_formatter.cc b/chromium/components/autofill_assistant/browser/field_formatter.cc new file mode 100644 index 00000000000..8045c89530e --- /dev/null +++ b/chromium/components/autofill_assistant/browser/field_formatter.cc @@ -0,0 +1,128 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/autofill_assistant/browser/field_formatter.h" + +#include "base/logging.h" +#include "base/strings/string_number_conversions.h" +#include "base/strings/utf_string_conversions.h" +#include "components/autofill/core/browser/autofill_data_util.h" +#include "components/autofill/core/browser/autofill_type.h" +#include "components/autofill/core/browser/field_types.h" +#include "components/autofill/core/browser/geo/state_names.h" +#include "components/autofill_assistant/browser/generic_ui.pb.h" +#include "third_party/re2/src/re2/re2.h" +#include "third_party/re2/src/re2/stringpiece.h" + +namespace { +// Regex to find placeholders of the form ${key}, where key is an arbitrary +// string that does not contain curly braces. +const char kPlaceholderExtractor[] = R"re(\$\{([^{}]+)\})re"; + +base::Optional GetFieldValue( + const std::map& mappings, + const std::string& key) { + auto it = mappings.find(key); + if (it == mappings.end()) { + return base::nullopt; + } + return it->second; +} + +std::map CreateFormGroupMappings( + const autofill::FormGroup& form_group, + const std::string& locale) { + std::map mappings; + autofill::ServerFieldTypeSet available_fields; + form_group.GetNonEmptyTypes(locale, &available_fields); + for (const auto& field : available_fields) { + mappings.emplace(base::NumberToString(static_cast(field)), + base::UTF16ToUTF8(form_group.GetInfo( + autofill::AutofillType(field), locale))); + } + return mappings; +} + +} // namespace + +namespace autofill_assistant { +namespace field_formatter { + +base::Optional FormatString( + const std::string& pattern, + const std::map& mappings) { + if (pattern.empty()) { + return std::string(); + } + + std::string key; + std::string out = pattern; + re2::StringPiece input(pattern); + while (re2::RE2::FindAndConsume(&input, kPlaceholderExtractor, &key)) { + auto rewrite_value = GetFieldValue(mappings, key); + if (!rewrite_value.has_value()) { + VLOG(2) << "No value for " << key << " in " << pattern; + return base::nullopt; + } + + re2::RE2::Replace(&out, kPlaceholderExtractor, + re2::StringPiece(rewrite_value.value())); + } + + return out; +} + +template <> +std::map +CreateAutofillMappings( + const autofill::AutofillProfile& profile, + const std::string& locale) { + auto mappings = CreateFormGroupMappings(profile, locale); + + auto state = profile.GetInfo( + autofill::AutofillType(autofill::ADDRESS_HOME_STATE), locale); + if (!state.empty()) { + // TODO(b/159309560): Capitalize first letter of the state name. + auto state_name = + base::UTF16ToUTF8(autofill::state_names::GetNameForAbbreviation(state)); + if (!state_name.empty()) { + mappings[base::NumberToString(static_cast( + AutofillFormatProto::ADDRESS_HOME_STATE_NAME))] = state_name; + } + } + + return mappings; +} + +template <> +std::map CreateAutofillMappings( + const autofill::CreditCard& credit_card, + const std::string& locale) { + auto mappings = CreateFormGroupMappings(credit_card, locale); + + auto network = std::string( + autofill::data_util::GetPaymentRequestData(credit_card.network()) + .basic_card_issuer_network); + if (!network.empty()) { + mappings[base::NumberToString( + static_cast(AutofillFormatProto::CREDIT_CARD_NETWORK))] = network; + } + auto network_for_display = base::UTF16ToUTF8(credit_card.NetworkForDisplay()); + if (!network_for_display.empty()) { + mappings[base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_NETWORK_FOR_DISPLAY))] = + network_for_display; + } + auto last_four_digits = base::UTF16ToUTF8(credit_card.LastFourDigits()); + if (!last_four_digits.empty()) { + mappings[base::NumberToString(static_cast( + AutofillFormatProto::CREDIT_CARD_NUMBER_LAST_FOUR_DIGITS))] = + last_four_digits; + } + + return mappings; +} + +} // namespace field_formatter +} // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/field_formatter.h b/chromium/components/autofill_assistant/browser/field_formatter.h new file mode 100644 index 00000000000..ce934e9132a --- /dev/null +++ b/chromium/components/autofill_assistant/browser/field_formatter.h @@ -0,0 +1,37 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_FIELD_FORMATTER_H_ +#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_FIELD_FORMATTER_H_ + +#include +#include +#include "base/optional.h" +#include "components/autofill/core/browser/data_model/autofill_profile.h" +#include "components/autofill/core/browser/data_model/credit_card.h" + +namespace autofill_assistant { +namespace field_formatter { + +// Replaces all placeholder occurrences of the form ${key} in |input| with the +// corresponding value in |mappings|, where |key| is an arbitrary string that +// does not contain curly braces. Fails if any of the found placeholders is not +// in |mappings|. +base::Optional FormatString( + const std::string& input, + const std::map& mappings); + +// Creates a lookup map for all non-empty autofill and custom +// AutofillFormatProto::AutofillAssistantCustomField field types in +// |autofill_data_model|. +// |locale| should be a locale string such as "en-US". +template +std::map CreateAutofillMappings( + const T& autofill_data_model, + const std::string& locale); + +} // namespace field_formatter +} // namespace autofill_assistant + +#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_FIELD_FORMATTER_H_ diff --git a/chromium/components/autofill_assistant/browser/field_formatter_unittest.cc b/chromium/components/autofill_assistant/browser/field_formatter_unittest.cc new file mode 100644 index 00000000000..f7bda3bdfbf --- /dev/null +++ b/chromium/components/autofill_assistant/browser/field_formatter_unittest.cc @@ -0,0 +1,209 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/autofill_assistant/browser/field_formatter.h" + +#include "base/guid.h" +#include "base/strings/string_number_conversions.h" +#include "components/autofill/core/browser/autofill_test_utils.h" +#include "components/autofill/core/browser/data_model/autofill_profile.h" +#include "components/autofill/core/browser/data_model/credit_card.h" + +#include "testing/gmock/include/gmock/gmock.h" + +namespace autofill_assistant { +namespace field_formatter { +namespace { +const char kFakeUrl[] = "https://www.example.com"; + +using ::testing::_; +using ::testing::Eq; +using ::testing::UnorderedElementsAreArray; + +TEST(FieldFormatterTest, FormatString) { + std::map mappings = { + {"keyA", "valueA"}, {"keyB", "valueB"}, {"keyC", "valueC"}}; + + EXPECT_EQ(*FormatString("", mappings), ""); + EXPECT_EQ(*FormatString("input", mappings), "input"); + EXPECT_EQ(*FormatString("prefix ${keyA}", mappings), "prefix valueA"); + EXPECT_EQ(*FormatString("prefix ${keyA}${keyB}${keyC} suffix", mappings), + "prefix valueAvalueBvalueC suffix"); + EXPECT_EQ(*FormatString("keyA = ${keyA}", mappings), "keyA = valueA"); + EXPECT_EQ(FormatString("${keyD}", mappings), base::nullopt); + EXPECT_EQ(FormatString("${keyA}${keyD}", mappings), base::nullopt); +} + +TEST(FieldFormatterTest, AutofillProfile) { + autofill::AutofillProfile profile(base::GenerateGUID(), kFakeUrl); + autofill::test::SetProfileInfo( + &profile, "John", "", "Doe", "editor@gmail.com", "", "203 Barfield Lane", + "", "Mountain View", "CA", "94043", "US", "+12345678901"); + + // NAME_FIRST NAME_LAST + EXPECT_EQ( + *FormatString("${3} ${5}", CreateAutofillMappings(profile, "en-US")), + "John Doe"); + + // PHONE_HOME_COUNTRY_CODE, PHONE_HOME_CITY_CODE, PHONE_HOME_NUMBER + EXPECT_EQ(*FormatString("(+${12}) (${11}) ${10}", + CreateAutofillMappings(profile, "en-US")), + "(+1) (234) 5678901"); + + // ADDRESS_HOME_STATE, ADDRESS_HOME_STATE_NAME + EXPECT_EQ( + *FormatString("${34} - ${-6}", CreateAutofillMappings(profile, "en-US")), + "CA - california"); + + // Unknown state. + autofill::AutofillProfile unknown_state_profile(base::GenerateGUID(), + kFakeUrl); + autofill::test::SetProfileInfo(&unknown_state_profile, "John", "", "Doe", "", + "", "", "", "", "XY", "", "US", ""); + EXPECT_EQ(FormatString("${34}", CreateAutofillMappings(unknown_state_profile, + "en-US")), + "XY"); + EXPECT_EQ(FormatString("${-6}", CreateAutofillMappings(unknown_state_profile, + "en-US")), + base::nullopt); + + // UNKNOWN_TYPE + EXPECT_EQ(FormatString("${1}", CreateAutofillMappings(profile, "en-US")), + base::nullopt); +} + +TEST(FieldFormatterTest, CreditCard) { + autofill::CreditCard credit_card(base::GenerateGUID(), kFakeUrl); + autofill::test::SetCreditCardInfo(&credit_card, "John Doe", + "4111 1111 1111 1111", "01", "2050", ""); + + // CREDIT_CARD_NAME_FULL + EXPECT_EQ( + *FormatString("${51}", CreateAutofillMappings(credit_card, "en-US")), + "John Doe"); + + // CREDIT_CARD_NUMBER + EXPECT_EQ( + *FormatString("${52}", CreateAutofillMappings(credit_card, "en-US")), + "4111111111111111"); + + // CREDIT_CARD_NUMBER_LAST_FOUR_DIGITS + EXPECT_EQ( + *FormatString("**** ${-4}", CreateAutofillMappings(credit_card, "en-US")), + "**** 1111"); + + // CREDIT_CARD_EXP_MONTH, CREDIT_CARD_EXP_2_DIGIT_YEAR + EXPECT_EQ(*FormatString("${53}/${54}", + CreateAutofillMappings(credit_card, "en-US")), + "01/50"); + + // CREDIT_CARD_NETWORK, CREDIT_CARD_NETWORK_FOR_DISPLAY + EXPECT_EQ(*FormatString("${-2} ${-5}", + CreateAutofillMappings(credit_card, "en-US")), + "visa Visa"); +} + +TEST(FieldFormatterTest, SpecialCases) { + autofill::AutofillProfile profile(base::GenerateGUID(), kFakeUrl); + autofill::test::SetProfileInfo( + &profile, "John", "", "Doe", "editor@gmail.com", "", "203 Barfield Lane", + "", "Mountain View", "CA", "94043", "US", "+12345678901"); + + EXPECT_EQ(*FormatString("", CreateAutofillMappings(profile, "en-US")), + std::string()); + EXPECT_EQ(*FormatString("${3}", CreateAutofillMappings(profile, "en-US")), + "John"); + EXPECT_EQ(FormatString("${-1}", CreateAutofillMappings(profile, "en-US")), + base::nullopt); + EXPECT_EQ( + FormatString( + "${" + base::NumberToString(autofill::MAX_VALID_FIELD_TYPE) + "}", + CreateAutofillMappings(profile, "en-US")), + base::nullopt); + + // Second {} is not prefixed with $. + EXPECT_EQ( + *FormatString("${3} {10}", CreateAutofillMappings(profile, "en-US")), + "John {10}"); +} + +TEST(FieldFormatterTest, DifferentLocales) { + autofill::AutofillProfile profile(base::GenerateGUID(), kFakeUrl); + autofill::test::SetProfileInfo( + &profile, "John", "", "Doe", "editor@gmail.com", "", "203 Barfield Lane", + "", "Mountain View", "CA", "94043", "US", "+12345678901"); + auto mappings = CreateAutofillMappings(profile, "de-DE"); + + // 36 == ADDRESS_HOME_COUNTRY + EXPECT_EQ(*FormatString("${36}", CreateAutofillMappings(profile, "en-US")), + "United States"); + EXPECT_EQ(*FormatString("${36}", CreateAutofillMappings(profile, "de-DE")), + "Vereinigte Staaten"); + + // Invalid locales default to "en-US". + EXPECT_EQ(*FormatString("${36}", CreateAutofillMappings(profile, "")), + "United States"); + EXPECT_EQ(*FormatString("${36}", CreateAutofillMappings(profile, "invalid")), + "United States"); +} + +TEST(FieldFormatterTest, AddsAllProfileFields) { + std::map expected_values = { + {"3", "Alpha"}, + {"4", "Beta"}, + {"5", "Gamma"}, + {"6", "B"}, + {"7", "Alpha Beta Gamma"}, + {"9", "alpha@google.com"}, + {"10", "1234567"}, + {"11", "79"}, + {"12", "41"}, + {"13", "0791234567"}, + {"14", "+41791234567"}, + {"30", "Brandschenkestrasse 110"}, + {"31", "Google Building 110"}, + {"33", "Zurich"}, + {"34", "Canton Zurich"}, + {"35", "8002"}, + {"36", "Switzerland"}, + {"60", "Google"}, + {"77", "Brandschenkestrasse 110\nGoogle Building 110"}}; + + autofill::AutofillProfile profile(base::GenerateGUID(), kFakeUrl); + autofill::test::SetProfileInfo( + &profile, "Alpha", "Beta", "Gamma", "alpha@google.com", "Google", + "Brandschenkestrasse 110", "Google Building 110", "Zurich", + "Canton Zurich", "8002", "CH", "+41791234567"); + + EXPECT_THAT(CreateAutofillMappings(profile, "en-US"), + UnorderedElementsAreArray(expected_values)); +} + +TEST(FieldFormatterTest, AddsAllCreditCardFields) { + std::map expected_values = { + {"-5", "Visa"}, + {"-4", "1111"}, + {"-2", "visa"}, + {"51", "Alpha Beta Gamma"}, + {"52", "4111111111111111"}, + {"53", "08"}, + {"54", "50"}, + {"55", "2050"}, + {"56", "08/50"}, + {"57", "08/2050"}, + {"58", "Visa"}, + {"91", "Alpha"}, + {"92", "Gamma"}}; + + autofill::CreditCard credit_card(base::GenerateGUID(), kFakeUrl); + autofill::test::SetCreditCardInfo(&credit_card, "Alpha Beta Gamma", + "4111111111111111", "8", "2050", ""); + + EXPECT_THAT(CreateAutofillMappings(credit_card, "en-US"), + UnorderedElementsAreArray(expected_values)); +} + +} // namespace +} // namespace field_formatter +} // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/generic_ui.proto b/chromium/components/autofill_assistant/browser/generic_ui.proto index d87228400a7..e4031ba99c7 100644 --- a/chromium/components/autofill_assistant/browser/generic_ui.proto +++ b/chromium/components/autofill_assistant/browser/generic_ui.proto @@ -49,6 +49,9 @@ message CallbackProto { SetViewVisibilityProto set_view_visibility = 11; SetViewEnabledProto set_view_enabled = 12; ShowGenericUiPopupProto show_generic_popup = 13; + CreateNestedGenericUiProto create_nested_ui = 14; + ClearViewContainerProto clear_view_container = 15; + ForEachProto for_each = 16; } // Optional model identifier pointing to a single boolean. If set, the // callback will only be invoked if the condition is true. @@ -62,6 +65,7 @@ message EventProto { OnUserActionCalled on_user_action_called = 3; OnTextLinkClickedProto on_text_link_clicked = 4; OnPopupDismissedProto on_popup_dismissed = 5; + OnViewContainerClearedProto on_view_container_cleared = 6; } } @@ -96,6 +100,13 @@ message OnPopupDismissedProto { optional string popup_identifier = 1; } +// Event which is called whenever |view_identifier| was cleared with a +// |ClearViewContainer| interaction. +message OnViewContainerClearedProto { + // The view container that was cleared. + optional string view_identifier = 1; +} + // Callback that writes the specified value to |model_identifier|. message SetModelValueProto { // The model identifier to write to. @@ -158,7 +169,10 @@ message ToStringProto { optional ValueReferenceProto value = 3; // Optional format options. - oneof format_options { DateFormatProto date_format = 2; } + oneof format_options { + DateFormatProto date_format = 2; + AutofillFormatProto autofill_format = 4; + } reserved 1; } @@ -168,6 +182,32 @@ message DateFormatProto { optional string date_format = 1; } +// Format options for autofill profiles and credit cards. +message AutofillFormatProto { + // Fields we add that are not mapped to field_types.h. The values must be + // negative to avoid overlap with field_types.h. + // Note: CREDIT_CARD_VERIFICATION_CODE and CREDIT_CARD_RAW_NUMBER are only + // available in UseCreditCardAction, as they require the CVC dialog to be + // shown. + enum AutofillAssistantCustomField { + UNDEFINED = 0; + CREDIT_CARD_VERIFICATION_CODE = -1; + CREDIT_CARD_NETWORK = -2; + CREDIT_CARD_RAW_NUMBER = -3; + CREDIT_CARD_NUMBER_LAST_FOUR_DIGITS = -4; + CREDIT_CARD_NETWORK_FOR_DISPLAY = -5; + // Currently only US states are supported. The state name is in lower case. + ADDRESS_HOME_STATE_NAME = -6; + } + // The format string to use. May contain one or multiple "${key}" + // placeholders, where the key is an integer corresponding to + // entries from field_types.h or AutofillAssistantCustomField. + optional string pattern = 1; + // The locale to use when stringifying data. Invalid locales will + // automatically default to "en-US". + optional string locale = 2; +} + // A comparison of two values in the form |value_a| |value_b|. EQUAL is // supported for all values. All other comparison modes are only supported for // single integers, strings, and dates. @@ -331,3 +371,42 @@ message SetViewEnabledProto { // Whether the view should be enabled or not. Must point to a single boolean. optional ValueReferenceProto enabled = 2; } + +// Creates new UI and interactions, i.e., a nested instance and attaches it to +// |parent_view_identifier|. Nested UI instances have their own view layout and +// interactions, but share the same model with their parent. All interactions +// between ancestor and descendant UIs must pass through the user model, as +// neither can directly reference views or interactions of the other. +// +// Use |ClearViewContainerProto| on a nested UI's view parent to detach and +// destroy the nested UI. +message CreateNestedGenericUiProto { + // Identifier for the newly created instance. + optional string generic_ui_identifier = 1; + // The UI to create. + optional GenericUserInterfaceProto generic_ui = 2; + // The view identifier of the parent to attach (=append) this UI to. + optional string parent_view_identifier = 3; +} + +// Removes all views from the target view container. Nested UI instances +// attached to |view_identifier| will be destroyed. +message ClearViewContainerProto { + // The view container to clear. + optional string view_identifier = 1; +} + +// Invokes |callbacks| for each item in the loop value. Automatically replaces +// instances of "${i}" in model and view identifiers with the loop counter. +message ForEachProto { + // The loop counter, usually "i", "j", etc. Callbacks may use this counter + // in view and model identifiers by using "${i}"" placeholders, e.g., + // "profiles[${i}]" or "my_view_${i}". + optional string loop_counter = 1; + + // The value list to loop over. + optional string loop_value_model_identifier = 2; + + // The callbacks to invoke for every iteration of the loop. + repeated CallbackProto callbacks = 3; +} diff --git a/chromium/components/autofill_assistant/browser/generic_ui_java_generated_enums.h b/chromium/components/autofill_assistant/browser/generic_ui_java_generated_enums.h index 29539e5368a..23d29a1a570 100644 --- a/chromium/components/autofill_assistant/browser/generic_ui_java_generated_enums.h +++ b/chromium/components/autofill_assistant/browser/generic_ui_java_generated_enums.h @@ -19,6 +19,26 @@ enum class VerticalExpanderChevronStyle { NEVER = 2, }; +// GENERATED_JAVA_ENUM_PACKAGE: ( +// org.chromium.chrome.browser.autofill_assistant.drawable) +// GENERATED_JAVA_CLASS_NAME_OVERRIDE: AssistantDrawableIcon +enum class DrawableIcon { + DRAWABLE_ICON_UNDEFINED = 0, + PROGRESSBAR_DEFAULT_INITIAL_STEP = 1, + PROGRESSBAR_DEFAULT_DATA_COLLECTION = 2, + PROGRESSBAR_DEFAULT_PAYMENT = 3, + PROGRESSBAR_DEFAULT_FINAL_STEP = 4, + SITTING_PERSON = 5, + TICKET_STUB = 6, + SHOPPING_BASKET = 7, + FAST_FOOD = 8, + LOCAL_DINING = 9, + COGWHEEL = 10, + KEY = 11, + CAR = 12, + GROCERY = 13, +}; + } // namespace autofill_assistant #endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_GENERIC_UI_JAVA_GENERATED_ENUMS_H_ diff --git a/chromium/components/autofill_assistant/browser/metrics.h b/chromium/components/autofill_assistant/browser/metrics.h index 8eabecca360..ff8a84c225a 100644 --- a/chromium/components/autofill_assistant/browser/metrics.h +++ b/chromium/components/autofill_assistant/browser/metrics.h @@ -45,8 +45,11 @@ class Metrics { NO_INITIAL_SCRIPTS = 19, DFM_INSTALL_FAILED = 20, DOMAIN_CHANGE_DURING_BROWSE_MODE = 21, + BACK_BUTTON_CLICKED = 22, + ONBOARDING_BACK_BUTTON_CLICKED = 23, + NAVIGATION_WHILE_RUNNING = 24, - kMaxValue = DOMAIN_CHANGE_DURING_BROWSE_MODE + kMaxValue = NAVIGATION_WHILE_RUNNING }; // The different ways that autofill assistant can stop. @@ -64,8 +67,9 @@ class Metrics { OB_NOT_SHOWN = 1, OB_ACCEPTED = 2, OB_CANCELLED = 3, + OB_NO_ANSWER = 4, - kMaxValue = OB_CANCELLED + kMaxValue = OB_NO_ANSWER }; // The different ways for payment request to succeed or fail, broken down by @@ -222,6 +226,15 @@ class Metrics { case DropOutReason::DOMAIN_CHANGE_DURING_BROWSE_MODE: out << "DOMAIN_CHANGE_DURING_BROWSE_MODE"; break; + case DropOutReason::BACK_BUTTON_CLICKED: + out << "BACK_BUTTON_CLICKED"; + break; + case DropOutReason::ONBOARDING_BACK_BUTTON_CLICKED: + out << "ONBOARDING_BACK_BUTTON_CLICKED"; + break; + case DropOutReason::NAVIGATION_WHILE_RUNNING: + out << "NAVIGATION_WHILE_RUNNING"; + break; // Do not add default case to force compilation error for new values. } return out; @@ -249,6 +262,9 @@ class Metrics { case OnBoarding::OB_CANCELLED: out << "OB_CANCELLED"; break; + case OnBoarding::OB_NO_ANSWER: + out << "OB_NO_ANSWER"; + break; // Do not add default case to force compilation error for new values. } return out; diff --git a/chromium/components/autofill_assistant/browser/mock_client.h b/chromium/components/autofill_assistant/browser/mock_client.h index b96f0c72eb9..a83fe7ab273 100644 --- a/chromium/components/autofill_assistant/browser/mock_client.h +++ b/chromium/components/autofill_assistant/browser/mock_client.h @@ -25,6 +25,7 @@ class MockClient : public Client { MOCK_CONST_METHOD0(GetLocale, std::string()); MOCK_CONST_METHOD0(GetCountryCode, std::string()); MOCK_CONST_METHOD0(GetDeviceContext, DeviceContext()); + MOCK_CONST_METHOD0(IsAccessibilityEnabled, bool()); MOCK_CONST_METHOD0(GetEmailAddressForAccessTokenAccount, std::string()); MOCK_CONST_METHOD0(GetChromeSignedInEmailAddress, std::string()); MOCK_CONST_METHOD0(GetWebContents, content::WebContents*()); diff --git a/chromium/components/autofill_assistant/browser/mock_controller_observer.h b/chromium/components/autofill_assistant/browser/mock_controller_observer.h index be6ae4d2d51..8cd6fc4f8d7 100644 --- a/chromium/components/autofill_assistant/browser/mock_controller_observer.h +++ b/chromium/components/autofill_assistant/browser/mock_controller_observer.h @@ -37,7 +37,12 @@ class MockControllerObserver : public ControllerObserver { MOCK_METHOD1(OnDetailsChanged, void(const Details* details)); MOCK_METHOD1(OnInfoBoxChanged, void(const InfoBox* info_box)); MOCK_METHOD1(OnProgressChanged, void(int progress)); + MOCK_METHOD1(OnProgressActiveStepChanged, void(int active_step)); MOCK_METHOD1(OnProgressVisibilityChanged, void(bool visible)); + MOCK_METHOD1(OnStepProgressBarConfigurationChanged, + void(const ShowProgressBarProto::StepProgressBarConfiguration& + configuration)); + MOCK_METHOD1(OnProgressBarErrorStateChanged, void(bool error)); MOCK_METHOD3(OnTouchableAreaChanged, void(const RectF&, const std::vector& touchable_areas, diff --git a/chromium/components/autofill_assistant/browser/model.proto b/chromium/components/autofill_assistant/browser/model.proto index 607825be75d..7e3547ed105 100644 --- a/chromium/components/autofill_assistant/browser/model.proto +++ b/chromium/components/autofill_assistant/browser/model.proto @@ -33,13 +33,15 @@ message ValueProto { ProfileList profiles = 8; LoginOptionList login_options = 9; CreditCardResponseProto credit_card_response = 10; - LoginOptionResponseProto login_option_response = 11; + bytes server_payload = 12; } // If set to true, this value will not be sent to the backend. This flag is // mostly used internally, to prevent certain values from leaving the device. // Note that values derived from client_side_only values will inherit this // property. optional bool is_client_side_only = 6; + + reserved 11; } message ValueReferenceProto { @@ -192,6 +194,10 @@ enum ProcessedActionStatusProto { // ProcessedActionStatusProto.AutofillErrorInfoProto contains more details. AUTOFILL_INCOMPLETE = 24; + // Evaluating a selector required considering too many candidates. Please + // generate more specific selectors. See SelectorProto::ProximityFilter. + TOO_MANY_CANDIDATES = 25; + reserved 15, 23; } @@ -337,7 +343,3 @@ message LoginOptionProto { optional string sublabel = 2; optional bytes payload = 3; } - -message LoginOptionResponseProto { - optional bytes payload = 1; -} diff --git a/chromium/components/autofill_assistant/browser/radio_button_controller.cc b/chromium/components/autofill_assistant/browser/radio_button_controller.cc new file mode 100644 index 00000000000..b905bb39e7a --- /dev/null +++ b/chromium/components/autofill_assistant/browser/radio_button_controller.cc @@ -0,0 +1,44 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/autofill_assistant/browser/radio_button_controller.h" + +namespace autofill_assistant { + +RadioButtonController::RadioButtonController(UserModel* user_model) + : user_model_(user_model) {} +RadioButtonController::~RadioButtonController() = default; + +base::WeakPtr RadioButtonController::GetWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); +} + +void RadioButtonController::AddRadioButtonToGroup( + const std::string& radio_group, + const std::string& model_identifier) { + radio_groups_[radio_group].insert(model_identifier); +} + +bool RadioButtonController::UpdateRadioButtonGroup( + const std::string& radio_group, + const std::string& selected_model_identifier) { + auto radio_group_it = radio_groups_.find(radio_group); + if (radio_group_it == radio_groups_.end()) { + return false; + } + + if (radio_group_it->second.find(selected_model_identifier) == + radio_group_it->second.end()) { + return false; + } + + for (const auto& model_identifier : radio_group_it->second) { + user_model_->SetValue( + model_identifier, + SimpleValue(model_identifier == selected_model_identifier)); + } + return true; +} + +} // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/radio_button_controller.h b/chromium/components/autofill_assistant/browser/radio_button_controller.h new file mode 100644 index 00000000000..b9287ea91dd --- /dev/null +++ b/chromium/components/autofill_assistant/browser/radio_button_controller.h @@ -0,0 +1,49 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_RADIO_BUTTON_CONTROLLER_H_ +#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_RADIO_BUTTON_CONTROLLER_H_ + +#include +#include +#include + +#include "base/memory/weak_ptr.h" +#include "components/autofill_assistant/browser/user_model.h" + +namespace autofill_assistant { + +class RadioButtonController { + public: + RadioButtonController(UserModel* user_model); + ~RadioButtonController(); + RadioButtonController(const RadioButtonController&) = delete; + RadioButtonController& operator=(const RadioButtonController&) = delete; + + base::WeakPtr GetWeakPtr(); + + // Adds |model_identifier| to the list of model identifiers belonging to + // |radio_group|. + void AddRadioButtonToGroup(const std::string& radio_group, + const std::string& model_identifier); + + // Ensures that only |selected_model_identifier| is set to true in + // |radio_group|. + bool UpdateRadioButtonGroup(const std::string& radio_group, + const std::string& selected_model_identifier); + + private: + // Maps radiogroup identifiers to the list of corresponding model identifiers. + std::map> radio_groups_; + + private: + friend class RadioButtonControllerTest; + + UserModel* user_model_; + base::WeakPtrFactory weak_ptr_factory_{this}; +}; + +} // namespace autofill_assistant + +#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_RADIO_BUTTON_CONTROLLER_H_ diff --git a/chromium/components/autofill_assistant/browser/radio_button_controller_unittest.cc b/chromium/components/autofill_assistant/browser/radio_button_controller_unittest.cc new file mode 100644 index 00000000000..5de656a82c4 --- /dev/null +++ b/chromium/components/autofill_assistant/browser/radio_button_controller_unittest.cc @@ -0,0 +1,77 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/autofill_assistant/browser/radio_button_controller.h" +#include "components/autofill_assistant/browser/user_model.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace autofill_assistant { + +using ::testing::Pair; +using ::testing::SizeIs; +using ::testing::UnorderedElementsAre; + +class RadioButtonControllerTest : public testing::Test { + protected: + RadioButtonControllerTest() : controller_(&user_model_) {} + ~RadioButtonControllerTest() override {} + + std::map> GetRadioGroups() { + return controller_.radio_groups_; + } + + UserModel user_model_; + RadioButtonController controller_; +}; + +TEST_F(RadioButtonControllerTest, AddRadioButtonToGroup) { + EXPECT_THAT(GetRadioGroups(), SizeIs(0)); + controller_.AddRadioButtonToGroup("group_1", "id_1"); + EXPECT_THAT(GetRadioGroups(), UnorderedElementsAre(Pair( + "group_1", std::set{"id_1"}))); + + controller_.AddRadioButtonToGroup("group_1", "id_1"); + EXPECT_THAT(GetRadioGroups(), UnorderedElementsAre(Pair( + "group_1", std::set{"id_1"}))); + + controller_.AddRadioButtonToGroup("group_1", "id_2"); + EXPECT_THAT(GetRadioGroups(), + UnorderedElementsAre( + Pair("group_1", std::set{"id_1", "id_2"}))); + + controller_.AddRadioButtonToGroup("group_2", "id_3"); + EXPECT_THAT(GetRadioGroups(), + UnorderedElementsAre( + Pair("group_1", std::set{"id_1", "id_2"}), + Pair("group_2", std::set{"id_3"}))); +} + +TEST_F(RadioButtonControllerTest, UpdateRadioButtonGroup) { + controller_.AddRadioButtonToGroup("group_1", "id_1"); + controller_.AddRadioButtonToGroup("group_1", "id_2"); + controller_.AddRadioButtonToGroup("group_1", "id_3"); + controller_.AddRadioButtonToGroup("group_2", "id_4"); + controller_.AddRadioButtonToGroup("group_2", "id_5"); + + EXPECT_FALSE(controller_.UpdateRadioButtonGroup("does_not_exist", "id_1")); + + EXPECT_TRUE(controller_.UpdateRadioButtonGroup("group_1", "id_1")); + EXPECT_EQ(user_model_.GetValue("id_1"), SimpleValue(true)); + EXPECT_EQ(user_model_.GetValue("id_2"), SimpleValue(false)); + EXPECT_EQ(user_model_.GetValue("id_3"), SimpleValue(false)); + + EXPECT_FALSE(controller_.UpdateRadioButtonGroup("group_1", "does_not_exist")); + EXPECT_EQ(user_model_.GetValue("id_1"), SimpleValue(true)); + EXPECT_EQ(user_model_.GetValue("id_2"), SimpleValue(false)); + EXPECT_EQ(user_model_.GetValue("id_3"), SimpleValue(false)); + + EXPECT_TRUE(controller_.UpdateRadioButtonGroup("group_2", "id_5")); + EXPECT_EQ(user_model_.GetValue("id_1"), SimpleValue(true)); + EXPECT_EQ(user_model_.GetValue("id_2"), SimpleValue(false)); + EXPECT_EQ(user_model_.GetValue("id_3"), SimpleValue(false)); + EXPECT_EQ(user_model_.GetValue("id_4"), SimpleValue(false)); + EXPECT_EQ(user_model_.GetValue("id_5"), SimpleValue(true)); +} + +} // namespace autofill_assistant diff --git a/chromium/components/autofill_assistant/browser/script_executor.cc b/chromium/components/autofill_assistant/browser/script_executor.cc index a0c76ad340e..5125c48d81b 100644 --- a/chromium/components/autofill_assistant/browser/script_executor.cc +++ b/chromium/components/autofill_assistant/browser/script_executor.cc @@ -104,7 +104,12 @@ void ScriptExecutor::Run(const UserData* user_data, callback_ = std::move(callback); DCHECK(delegate_->GetService()); +#ifdef NDEBUG + VLOG(2) << "GetActions for (redacted)"; +#else VLOG(2) << "GetActions for " << delegate_->GetCurrentURL().host(); +#endif + delegate_->GetService()->GetActions( script_path_, delegate_->GetScriptURL(), MergedTriggerContext( @@ -119,6 +124,10 @@ const UserData* ScriptExecutor::GetUserData() const { return user_data_; } +UserModel* ScriptExecutor::GetUserModel() { + return delegate_->GetUserModel(); +} + void ScriptExecutor::OnNavigationStateChanged() { NavigationInfoProto& navigation_info = current_action_data_.navigation_info; if (delegate_->IsNavigatingToNewDocument()) { @@ -150,7 +159,9 @@ void ScriptExecutor::OnNavigationStateChanged() { std::move(on_expected_navigation_done_) .Run(!delegate_->HasNavigationError()); } - break; + // Early return since current_action_data_ is no longer valid at this + // point. + return; case ExpectedNavigationStep::DONE: // nothing to do @@ -242,11 +253,6 @@ void ScriptExecutor::WriteUserData( delegate_->WriteUserData(std::move(write_callback)); } -void ScriptExecutor::WriteUserModel( - base::OnceCallback write_callback) { - delegate_->WriteUserModel(std::move(write_callback)); -} - void ScriptExecutor::OnGetUserData( base::OnceCallback callback, UserData* user_data, @@ -274,8 +280,11 @@ void ScriptExecutor::OnTermsAndConditionsLinkClicked( std::move(callback).Run(link, user_data, user_model); } -void ScriptExecutor::GetFullCard(GetFullCardCallback callback) { - DCHECK(GetUserData()->selected_card_.get()); +void ScriptExecutor::GetFullCard( + const autofill::CreditCard* credit_card, + base::OnceCallback card, + const base::string16& cvc)> callback) { + DCHECK(credit_card); // User might be asked to provide the cvc. delegate_->EnterState(AutofillAssistantState::MODAL_DIALOG); @@ -284,7 +293,7 @@ void ScriptExecutor::GetFullCard(GetFullCardCallback callback) { // so as to unit test it. (new SelfDeleteFullCardRequester()) ->GetFullCard( - GetWebContents(), GetUserData()->selected_card_.get(), + GetWebContents(), credit_card, base::BindOnce(&ScriptExecutor::OnGetFullCard, weak_ptr_factory_.GetWeakPtr(), std::move(callback))); } @@ -422,10 +431,19 @@ void ScriptExecutor::SetProgress(int progress) { delegate_->SetProgress(progress); } +void ScriptExecutor::SetProgressActiveStep(int active_step) { + delegate_->SetProgressActiveStep(active_step); +} + void ScriptExecutor::SetProgressVisible(bool visible) { delegate_->SetProgressVisible(visible); } +void ScriptExecutor::SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& configuration) { + delegate_->SetStepProgressBarConfiguration(configuration); +} + void ScriptExecutor::GetFieldValue( const Selector& selector, base::OnceCallback @@ -478,6 +496,9 @@ void ScriptExecutor::GetElementTag( } void ScriptExecutor::ExpectNavigation() { + // TODO(b/160948417): Clean this up such that the logic is not required in + // both |ScriptExecutor| and |Controller|. + delegate_->ExpectNavigation(); expected_navigation_step_ = ExpectedNavigationStep::EXPECTED; } @@ -622,10 +643,11 @@ void ScriptExecutor::RequireUI() { void ScriptExecutor::SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback - end_action_callback) { - delegate_->SetGenericUi(std::move(generic_ui), - std::move(end_action_callback)); + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) { + delegate_->SetGenericUi(std::move(generic_ui), std::move(end_action_callback), + std::move(view_inflation_finished_callback)); } void ScriptExecutor::ClearGenericUi() { @@ -771,6 +793,7 @@ void ScriptExecutor::OnProcessedAction( processed_action.set_direct_action(current_action_data_.direct_action); *processed_action.mutable_navigation_info() = current_action_data_.navigation_info; + if (processed_action.status() != ProcessedActionStatusProto::ACTION_APPLIED) { if (delegate_->HasNavigationError()) { // Overwrite the original error, as the root cause is most likely a @@ -848,9 +871,6 @@ ScriptExecutor::WaitForDomOperation::~WaitForDomOperation() { void ScriptExecutor::WaitForDomOperation::Run() { delegate_->AddListener(this); - if (delegate_->IsNavigatingToNewDocument()) - return; // start paused - Start(); } diff --git a/chromium/components/autofill_assistant/browser/script_executor.h b/chromium/components/autofill_assistant/browser/script_executor.h index ea4c6f4b6a7..42fe159a5fc 100644 --- a/chromium/components/autofill_assistant/browser/script_executor.h +++ b/chromium/components/autofill_assistant/browser/script_executor.h @@ -95,6 +95,7 @@ class ScriptExecutor : public ActionDelegate, void Run(const UserData* user_data, RunScriptCallback callback); const UserData* GetUserData() const override; + UserModel* GetUserModel() override; // Override ScriptExecutorDelegate::Listener void OnNavigationStateChanged() override; @@ -123,9 +124,8 @@ class ScriptExecutor : public ActionDelegate, CollectUserDataOptions* collect_user_data_options) override; void WriteUserData( base::OnceCallback) override; - void WriteUserModel( - base::OnceCallback write_callback) override; - void GetFullCard(GetFullCardCallback callback) override; + void GetFullCard(const autofill::CreditCard* credit_card, + GetFullCardCallback callback) override; void Prompt(std::unique_ptr> user_actions, bool disable_force_expand_sheet, base::OnceCallback end_on_navigation_callback, @@ -214,7 +214,11 @@ class ScriptExecutor : public ActionDelegate, void ClearInfoBox() override; void SetInfoBox(const InfoBox& info_box) override; void SetProgress(int progress) override; + void SetProgressActiveStep(int active_step) override; void SetProgressVisible(bool visible) override; + void SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& configuration) + override; void SetViewportMode(ViewportMode mode) override; ViewportMode GetViewportMode() override; void SetPeekMode(ConfigureBottomSheetProto::PeekMode peek_mode) override; @@ -231,9 +235,9 @@ class ScriptExecutor : public ActionDelegate, void RequireUI() override; void SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback end_action_callback) override; + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) override; void ClearGenericUi() override; private: diff --git a/chromium/components/autofill_assistant/browser/script_executor_delegate.h b/chromium/components/autofill_assistant/browser/script_executor_delegate.h index 0cf42b6253b..7b646ed3b3b 100644 --- a/chromium/components/autofill_assistant/browser/script_executor_delegate.h +++ b/chromium/components/autofill_assistant/browser/script_executor_delegate.h @@ -78,10 +78,12 @@ class ScriptExecutorDelegate { virtual void WriteUserData( base::OnceCallback write_callback) = 0; - virtual void WriteUserModel( - base::OnceCallback write_callback) = 0; virtual void SetProgress(int progress) = 0; + virtual void SetProgressActiveStep(int active_step) = 0; virtual void SetProgressVisible(bool visible) = 0; + virtual void SetStepProgressBarConfiguration( + const ShowProgressBarProto::StepProgressBarConfiguration& + configuration) = 0; virtual void SetUserActions( std::unique_ptr> user_action) = 0; virtual ViewportMode GetViewportMode() = 0; @@ -102,6 +104,9 @@ class ScriptExecutorDelegate { SetTouchableElementArea(ElementAreaProto::default_instance()); } + // The next navigation is expected and will not cause an error. + virtual void ExpectNavigation() = 0; + // Returns true if a new document is being fetched for the main frame. // // Navigation ends once a response, with its associated URL has been @@ -147,9 +152,9 @@ class ScriptExecutorDelegate { // Sets the generic UI to show to the user. virtual void SetGenericUi( std::unique_ptr generic_ui, - base::OnceCallback end_action_callback) = 0; + base::OnceCallback end_action_callback, + base::OnceCallback + view_inflation_finished_callback) = 0; // Clears the generic UI. virtual void ClearGenericUi() = 0; diff --git a/chromium/components/autofill_assistant/browser/script_executor_unittest.cc b/chromium/components/autofill_assistant/browser/script_executor_unittest.cc index 6a7eca439e7..571d73aadc9 100644 --- a/chromium/components/autofill_assistant/browser/script_executor_unittest.cc +++ b/chromium/components/autofill_assistant/browser/script_executor_unittest.cc @@ -105,8 +105,8 @@ class ScriptExecutorTest : public testing::Test, interruptible.set_global_payload("main script global payload"); interruptible.set_script_payload("main script payload"); auto* wait_action = interruptible.add_actions()->mutable_wait_for_dom(); - wait_action->mutable_wait_condition()->mutable_match()->add_selectors( - element); + *wait_action->mutable_wait_condition()->mutable_match() = + ToSelectorProto(element); wait_action->set_allow_interrupt(true); interruptible.add_actions()->mutable_tell()->set_message(path); EXPECT_CALL(mock_service_, OnGetActions(StrEq(path), _, _, _, _, _)) @@ -136,9 +136,8 @@ class ScriptExecutorTest : public testing::Test, auto interrupt = std::make_unique