Skip to content

Commit

Permalink
Do not reload on selection
Browse files Browse the repository at this point in the history
The decision when to reload was too broad: Whenever
*any* event happened while we were using the GMS
editors, it would reload the data. This should not
happen for a selection change.

Bug: b:230712105
Change-Id: I496ba6164abcd21108be4d8defca004b32ab8d1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3613365
Auto-Submit: Sandro Maggi <sandromaggi@google.com>
Commit-Queue: Luca Hunkeler <hluca@google.com>
Reviewed-by: Luca Hunkeler <hluca@google.com>
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/main@{#997190}
  • Loading branch information
sandromaggi authored and Chromium LUCI CQ committed Apr 28, 2022
1 parent 31dc37e commit 11fb4ba
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
28 changes: 22 additions & 6 deletions components/autofill_assistant/browser/ui_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,22 @@ bool ShouldShowFeedbackChipForReason(Metrics::DropOutReason reason) {
}
}

bool ShouldReloadData(const CollectUserDataOptions& options,
UserDataEventType event_type) {
if (!options.use_gms_core_edit_dialogs) {
return false;
}
switch (event_type) {
case UserDataEventType::ENTRY_CREATED:
case UserDataEventType::ENTRY_EDITED:
return true;
case UserDataEventType::UNKNOWN:
case UserDataEventType::NO_NOTIFICATION:
case UserDataEventType::SELECTION_CHANGED:
return false;
}
}

} // namespace

UiController::UiController(
Expand Down Expand Up @@ -817,7 +833,7 @@ void UiController::HandleShippingAddressChange(
if (collect_user_data_options_ == nullptr) {
return;
}
if (collect_user_data_options_->use_gms_core_edit_dialogs) {
if (ShouldReloadData(*collect_user_data_options_, event_type)) {
ReloadUserData(UserDataEventField::SHIPPING_EVENT, event_type);
return;
}
Expand All @@ -835,7 +851,7 @@ void UiController::HandleContactInfoChange(
if (collect_user_data_options_ == nullptr) {
return;
}
if (collect_user_data_options_->use_gms_core_edit_dialogs) {
if (ShouldReloadData(*collect_user_data_options_, event_type)) {
ReloadUserData(UserDataEventField::CONTACT_EVENT, event_type);
return;
}
Expand All @@ -853,7 +869,7 @@ void UiController::HandlePhoneNumberChange(
if (collect_user_data_options_ == nullptr) {
return;
}
if (collect_user_data_options_->use_gms_core_edit_dialogs) {
if (ShouldReloadData(*collect_user_data_options_, event_type)) {
ReloadUserData(UserDataEventField::CONTACT_EVENT, event_type);
return;
}
Expand All @@ -872,7 +888,7 @@ void UiController::HandleCreditCardChange(
if (collect_user_data_options_ == nullptr) {
return;
}
if (collect_user_data_options_->use_gms_core_edit_dialogs) {
if (ShouldReloadData(*collect_user_data_options_, event_type)) {
ReloadUserData(UserDataEventField::CREDIT_CARD_EVENT, event_type);
return;
}
Expand Down Expand Up @@ -905,8 +921,8 @@ void UiController::ReloadUserData(UserDataEventField event_field,
collect_user_data_options_->selected_user_data_changed_callback.Run(
event_field, event_type);

auto callback = std::move(collect_user_data_options_->reload_data_callback);
std::move(callback).Run(GetUserData());
std::move(collect_user_data_options_->reload_data_callback)
.Run(GetUserData());
}

void UiController::SetTermsAndConditions(
Expand Down
20 changes: 20 additions & 0 deletions components/autofill_assistant/browser/ui_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,26 @@ TEST_F(UiControllerTest, UserDataFormReloadFromContactChange) {
UserDataEventType::ENTRY_CREATED);
}

TEST_F(UiControllerTest, UserDataFormDoNotReloadFromContactSelectionChange) {
auto options = std::make_unique<FakeCollectUserDataOptions>();
options->contact_details_name = "CONTACT";
base::MockCallback<base::OnceCallback<void(UserData*)>> reload_callback;
options->reload_data_callback = reload_callback.Get();
base::MockCallback<
base::RepeatingCallback<void(UserDataEventField, UserDataEventType)>>
change_callback;
options->selected_user_data_changed_callback = change_callback.Get();
options->use_gms_core_edit_dialogs = true;

ui_controller_->SetCollectUserDataOptions(options.get());

EXPECT_CALL(change_callback, Run(UserDataEventField::CONTACT_EVENT,
UserDataEventType::SELECTION_CHANGED));
EXPECT_CALL(reload_callback, Run).Times(0);
ui_controller_->HandleContactInfoChange(nullptr,
UserDataEventType::SELECTION_CHANGED);
}

TEST_F(UiControllerTest, UserDataFormReloadFromPhoneNumberChange) {
auto options = std::make_unique<FakeCollectUserDataOptions>();
base::MockCallback<base::OnceCallback<void(UserData*)>> reload_callback;
Expand Down

0 comments on commit 11fb4ba

Please sign in to comment.