[ios][pbsV2] Payments suggestion bottom sheet V2
Same concept as for the password suggestion bottom sheet
(https://crrev.com/c/5917667) but for the payments one.
Listeners are detached as soon as the sheet is triggered then
the blurred field is refocused if needed after dismissing the sheet.
The sheet is made the first responder so the keyboard cannot pop over
the sheet if there is a focus() event in the WebView while the sheet
is presented, replacing the role of the listeners.
In V2, listeners are only used as a trigger signal then cleaned up.
Change-Id: Idf19b06d3ff7076da0213d2043d54f593a45ac55
Low-Coverage-Reason: Need egtest to cover the new code, follow-up
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5937347
Commit-Queue: Vincent Boisselle <vincb@google.com>
Reviewed-by: Alexis Hétu <sugoi@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372269}
diff --git a/components/autofill/ios/common/features.h b/components/autofill/ios/common/features.h
index 98fc63d8..d4efc94b 100644
--- a/components/autofill/ios/common/features.h
+++ b/components/autofill/ios/common/features.h
@@ -9,6 +9,7 @@
BASE_DECLARE_FEATURE(kAutofillDynamicallyLoadsFieldsForAddressInput);
BASE_DECLARE_FEATURE(kAutofillIsolatedWorldForJavascriptIos);
+BASE_DECLARE_FEATURE(kAutofillPaymentsSheetV2Ios);
BASE_DECLARE_FEATURE(kAutofillStickyInfobarIos);
#endif // COMPONENTS_AUTOFILL_IOS_COMMON_FEATURES_H_
diff --git a/components/autofill/ios/common/features.mm b/components/autofill/ios/common/features.mm
index cc8baa9..e3343727 100644
--- a/components/autofill/ios/common/features.mm
+++ b/components/autofill/ios/common/features.mm
@@ -23,6 +23,13 @@
base::FEATURE_DISABLED_BY_DEFAULT);
// LINT.ThenChange(/components/autofill/ios/form_util/resources/autofill_form_features.ts:autofill_isolated_content_world)
+// Enables the second version of the payments suggestion bottom sheet to prevent
+// bugs that we've seen in production on other transaction sheets (e.g. some
+// fields becoming unresponsive).
+BASE_FEATURE(kAutofillPaymentsSheetV2Ios,
+ "AutofillPaymentsSheetV2Ios",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+
// Makes the autofill and password infobars sticky on iOS. The sticky infobar
// sticks there until navigating from an explicit user gesture (e.g. reload or
// load a new page from the omnibox). This includes the infobar UI and the
diff --git a/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.h b/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.h
index f7b9787..b1f41ea 100644
--- a/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.h
+++ b/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.h
@@ -172,6 +172,13 @@
// This value is moved and should only be retrieved once per bottom sheet.
autofill::VirtualCardEnrollmentCallbacks GetVirtualCardEnrollmentCallbacks();
+ // Attaches the listeners for the payments form corresponding to `form_id`.
+ // Only attaches the listeners on newly discovered renderer ids if `only_new`
+ // is true.
+ void AttachListenersForPaymentsForm(autofill::AutofillManager& manager,
+ autofill::FormGlobalId form_id,
+ bool only_new);
+
private:
friend class web::WebStateUserData<AutofillBottomSheetTabHelper>;
@@ -190,7 +197,8 @@
const std::vector<autofill::FieldRendererId>& renderer_ids,
std::set<autofill::FieldRendererId>& registered_renderer_ids,
const std::string& frame_id,
- bool allow_autofocus);
+ bool allow_autofocus,
+ bool only_new);
// Detach listeners, which will deactivate the associated bottom sheet.
void DetachListenersForFrame(
diff --git a/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.mm b/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.mm
index 5798d88..6410ec6 100644
--- a/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.mm
+++ b/ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.mm
@@ -15,6 +15,7 @@
#import "components/autofill/core/browser/ui/payments/card_unmask_authentication_selection_dialog_controller_impl.h"
#import "components/autofill/core/browser/ui/payments/virtual_card_enroll_ui_model.h"
#import "components/autofill/ios/browser/autofill_driver_ios.h"
+#import "components/autofill/ios/common/features.h"
#import "components/autofill/ios/form_util/form_activity_params.h"
#import "components/password_manager/core/browser/features/password_features.h"
#import "components/password_manager/core/common/password_manager_features.h"
@@ -172,6 +173,13 @@
observer.WillShowPaymentsBottomSheet(params);
}
[commands_handler_ showPaymentsBottomSheet:params];
+ if (base::FeatureList::IsEnabled(kAutofillPaymentsSheetV2Ios)) {
+ // In V2, detach the listeners right away to make sure they're always
+ // cleaned up to avoid issues with rogue listeners, see the documentation in
+ // ShowPasswordBottomSheet() for more details. Postpone refocus for
+ // later once the bottom sheet is dismissed.
+ DetachPaymentsListenersForAllFrames(/*refocus=*/false);
+ }
}
void AutofillBottomSheetTabHelper::ShowProactivePasswordGenerationBottomSheet(
@@ -207,7 +215,7 @@
password_manager::features::kIOSPasswordBottomSheetAutofocus);
AttachListeners(renderer_ids, registered_password_renderer_ids_[frame_id],
- frame_id, allow_autofocus);
+ frame_id, allow_autofocus, /*only_new=*/true);
}
void AutofillBottomSheetTabHelper::AttachPasswordGenerationListeners(
@@ -224,14 +232,15 @@
AttachListeners(renderer_ids,
registered_password_generation_renderer_ids_[frame_id],
- frame_id, /*allow_autofocus=*/true);
+ frame_id, /*allow_autofocus=*/true, /*only_new=*/true);
}
void AutofillBottomSheetTabHelper::AttachListeners(
const std::vector<autofill::FieldRendererId>& renderer_ids,
std::set<autofill::FieldRendererId>& registered_renderer_ids,
const std::string& frame_id,
- bool allow_autofocus) {
+ bool allow_autofocus,
+ bool only_new) {
if (!web_state_) {
return;
}
@@ -254,15 +263,25 @@
std::back_inserter(new_renderer_ids));
if (!new_renderer_ids.empty()) {
- // Enable the bottom sheet on the new renderer IDs.
- AutofillBottomSheetJavaScriptFeature::GetInstance()->AttachListeners(
- new_renderer_ids, frame, allow_autofocus);
-
// Add new renderer IDs to the list of registered renderer IDs.
std::copy(
new_renderer_ids.begin(), new_renderer_ids.end(),
std::inserter(registered_renderer_ids, registered_renderer_ids.end()));
}
+
+ // Only attach the new renderer ids if `only_new` is true, attach all the
+ // `renderer_ids` passed to AttachListeners() otherwise. The renderer will
+ // end up just attaching the listeners to the elements that do not have a
+ // listener yet, which includes the elements that had a listener in the past
+ // but that were detached, so these elements will have a listener attached
+ // again.
+ auto& rendered_ids_to_attach = only_new ? new_renderer_ids : renderer_ids;
+
+ if (!rendered_ids_to_attach.empty()) {
+ // Enable the bottom sheet on the selected renderer ids.
+ AutofillBottomSheetJavaScriptFeature::GetInstance()->AttachListeners(
+ rendered_ids_to_attach, frame, allow_autofocus);
+ }
}
void AutofillBottomSheetTabHelper::DetachPasswordListeners(
@@ -412,10 +431,10 @@
}
}
-void AutofillBottomSheetTabHelper::OnFieldTypesDetermined(
+void AutofillBottomSheetTabHelper::AttachListenersForPaymentsForm(
autofill::AutofillManager& manager,
autofill::FormGlobalId form_id,
- FieldTypeSource source) {
+ bool only_new) {
autofill::FormStructure* form_structure = manager.FindCachedFormById(form_id);
if (!form_structure || !form_structure->IsCompleteCreditCardForm()) {
return;
@@ -442,7 +461,14 @@
}
std::string frame_id = frame->GetFrameId();
AttachListeners(renderer_ids, registered_payments_renderer_ids_[frame_id],
- frame_id, /*allow_autofocus=*/false);
+ frame_id, /*allow_autofocus=*/false, only_new);
+}
+
+void AutofillBottomSheetTabHelper::OnFieldTypesDetermined(
+ autofill::AutofillManager& manager,
+ autofill::FormGlobalId form_id,
+ FieldTypeSource source) {
+ AttachListenersForPaymentsForm(manager, form_id, /*only_new=*/true);
}
std::unique_ptr<autofill::CardUnmaskAuthenticationSelectionDialogControllerImpl>
diff --git a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/BUILD.gn b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/BUILD.gn
index 663bcf96..347d787 100644
--- a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/BUILD.gn
+++ b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/BUILD.gn
@@ -41,6 +41,7 @@
"//base",
"//components/autofill/core/browser",
"//components/autofill/ios/browser",
+ "//components/autofill/ios/common",
"//components/autofill/ios/form_util",
"//components/strings",
"//ios/chrome/app/strings",
@@ -79,6 +80,7 @@
"//build:branding_buildflags",
"//components/autofill/core/browser",
"//components/autofill/ios/browser",
+ "//components/autofill/ios/common",
"//components/resources:components_scaled_resources_grit",
"//components/strings:components_strings_grit",
"//components/url_formatter",
@@ -113,6 +115,7 @@
"//components/autofill/core/browser",
"//components/autofill/core/browser:test_support",
"//components/autofill/core/common",
+ "//components/autofill/ios/common",
"//components/autofill/ios/form_util",
"//components/prefs",
"//components/sync:test_support",
diff --git a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_coordinator.mm b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_coordinator.mm
index 4d8094c5..c96b558 100644
--- a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_coordinator.mm
+++ b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_coordinator.mm
@@ -4,6 +4,8 @@
#import "ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_coordinator.h"
+#import "base/feature_list.h"
+#import "components/autofill/ios/common/features.h"
#import "components/autofill/ios/form_util/form_activity_params.h"
#import "ios/chrome/browser/autofill/model/personal_data_manager_factory.h"
#import "ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_exit_reason.h"
@@ -136,6 +138,7 @@
dismissViewControllerAnimated:NO
completion:^{
[weakSelf.settingsHandler showCreditCardSettings];
+ [weakSelf reattachListenersIfNeeded];
[weakSelf.browserCoordinatorCommandsHandler
dismissPaymentSuggestions];
}];
@@ -154,6 +157,7 @@
[](__weak __typeof(self) weak_self, autofill::CreditCard credit_card) {
[weak_self.settingsHandler showCreditCardDetails:credit_card
inEditMode:NO];
+ [weak_self reattachListenersIfNeeded];
[weak_self
.browserCoordinatorCommandsHandler dismissPaymentSuggestions];
},
@@ -209,4 +213,11 @@
self.viewController.image);
}
+// Reattaches the listeners for the latest focused form if deemed needed.
+- (void)reattachListenersIfNeeded {
+ if (base::FeatureList::IsEnabled(kAutofillPaymentsSheetV2Ios)) {
+ [self.mediator reattachListeners];
+ }
+}
+
@end
diff --git a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.h b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.h
index f9bc03c..16b8d6af 100644
--- a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.h
+++ b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.h
@@ -52,6 +52,12 @@
- (void)didSelectCreditCard:(CreditCardData*)creditCardData
atIndex:(NSInteger)index;
+// Reattaches the listeners on the form that triggered that sheet. This allows
+// showing the sheet again on the same page load when refocusing on a field on
+// that form. Does not reattach listeners on other forms that may have been
+// detached when showing the sheet.
+- (void)reattachListeners;
+
@end
#endif // IOS_CHROME_BROWSER_AUTOFILL_UI_BUNDLED_BOTTOM_SHEET_PAYMENTS_SUGGESTION_BOTTOM_SHEET_MEDIATOR_H_
diff --git a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.mm b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.mm
index d9f3c99..cca1478 100644
--- a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.mm
+++ b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.mm
@@ -4,17 +4,21 @@
#import "ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.h"
+#import "base/feature_list.h"
#import "base/memory/raw_ptr.h"
#import "base/metrics/histogram_functions.h"
#import "base/strings/sys_string_conversions.h"
#import "base/time/time.h"
+#import "components/autofill/core/browser/autofill_manager.h"
#import "components/autofill/core/browser/payments_data_manager.h"
#import "components/autofill/core/browser/personal_data_manager.h"
#import "components/autofill/core/browser/personal_data_manager_observer.h"
#import "components/autofill/core/common/autofill_payments_features.h"
+#import "components/autofill/ios/browser/autofill_driver_ios.h"
#import "components/autofill/ios/browser/credit_card_util.h"
#import "components/autofill/ios/browser/form_suggestion.h"
#import "components/autofill/ios/browser/personal_data_manager_observer_bridge.h"
+#import "components/autofill/ios/common/features.h"
#import "components/autofill/ios/form_util/form_activity_params.h"
#import "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_java_script_feature.h"
@@ -28,6 +32,7 @@
#import "ios/chrome/browser/shared/model/web_state_list/web_state_list_observer_bridge.h"
#import "ios/chrome/grit/ios_strings.h"
#import "ios/web/public/js_messaging/web_frames_manager.h"
+#import "ios/web/public/web_state.h"
#import "ios/web/public/web_state_observer_bridge.h"
#import "ui/base/l10n/l10n_util.h"
#import "ui/base/resource/resource_bundle.h"
@@ -185,6 +190,34 @@
}
}
+- (void)reattachListeners {
+ web::WebState* webState = [self getActiveWebState];
+ if (!webState) {
+ return;
+ }
+
+ web::WebFramesManager* webFramesManager =
+ AutofillBottomSheetJavaScriptFeature::GetInstance()->GetWebFramesManager(
+ webState);
+ web::WebFrame* frame = webFramesManager->GetFrameWithId(_params.frame_id);
+ if (!frame) {
+ return;
+ }
+
+ auto* driver =
+ autofill::AutofillDriverIOS::FromWebStateAndWebFrame(webState, frame);
+ if (!driver) {
+ return;
+ }
+
+ if (AutofillBottomSheetTabHelper* tabHelper = [self tabHelper]) {
+ autofill::FormGlobalId form_global_id(driver->GetFrameToken(),
+ _params.form_renderer_id);
+ tabHelper->AttachListenersForPaymentsForm(
+ driver->GetAutofillManager(), form_global_id, /*only_new=*/false);
+ }
+}
+
#pragma mark - Accessors
- (void)setConsumer:(id<PaymentsSuggestionBottomSheetConsumer>)consumer {
@@ -239,11 +272,7 @@
- (void)didSelectCreditCard:(CreditCardData*)creditCardData
atIndex:(NSInteger)index {
- if (!_webStateList) {
- return;
- }
-
- web::WebState* activeWebState = _webStateList->GetActiveWebState();
+ web::WebState* activeWebState = [self getActiveWebState];
if (!activeWebState) {
return;
}
@@ -292,11 +321,22 @@
[provider didSelectSuggestion:suggestion atIndex:index params:_params];
}
-- (void)disableBottomSheetAndRefocus:(BOOL)refocus {
- if (_webStateList) {
- web::WebState* activeWebState = _webStateList->GetActiveWebState();
- AutofillBottomSheetTabHelper::FromWebState(activeWebState)
- ->DetachPaymentsListenersForAllFrames(refocus);
+- (void)disableBottomSheetAndRefocus:(BOOL)shouldRefocus {
+ bool useV2 = base::FeatureList::IsEnabled(kAutofillPaymentsSheetV2Ios);
+ if (useV2) {
+ // Do not remove the listeners for the bottom sheet (aka disable) in V2
+ // since the listeners were already removed, as soon as the presentation
+ // started. Hence, just refocus if needed.
+ if (shouldRefocus) {
+ [self refocus];
+ }
+ return;
+ }
+
+ CHECK(!useV2);
+
+ if (AutofillBottomSheetTabHelper* tabHelper = [self tabHelper]) {
+ tabHelper->DetachPaymentsListenersForAllFrames(shouldRefocus);
}
}
@@ -368,7 +408,7 @@
// credit card suggestion is selected.
// TODO(crbug.com/40929827): Remove this dependency on suggestions.
- (void)setupSuggestionsProvider {
- web::WebState* activeWebState = _webStateList->GetActiveWebState();
+ web::WebState* activeWebState = [self getActiveWebState];
if (!activeWebState) {
return;
}
@@ -414,4 +454,26 @@
.ToUIImage();
}
+// Returns the AutofillBottomSheetTabHelper for the active webstate or nil if
+// it can't be retrieved.
+- (AutofillBottomSheetTabHelper*)tabHelper {
+ web::WebState* activeWebState = [self getActiveWebState];
+ return activeWebState
+ ? AutofillBottomSheetTabHelper::FromWebState(activeWebState)
+ : nullptr;
+}
+
+// Refocuses the field that was blurred to show the payments suggestion
+// bottom sheet, if deemded needed.
+- (void)refocus {
+ if (AutofillBottomSheetTabHelper* tabHelper = [self tabHelper]) {
+ tabHelper->RefocusElementIfNeeded(_params.frame_id);
+ }
+}
+
+// Returns the currently active WebState. Returns nullptr if there is none.
+- (web::WebState*)getActiveWebState {
+ return _webStateList ? _webStateList->GetActiveWebState() : nullptr;
+}
+
@end
diff --git a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator_unittest.mm b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator_unittest.mm
index 97f6089..f54e592 100644
--- a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator_unittest.mm
+++ b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator_unittest.mm
@@ -5,11 +5,13 @@
#import "ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_mediator.h"
#import "base/test/metrics/histogram_tester.h"
+#import "base/test/scoped_feature_list.h"
#import "base/test/scoped_mock_clock_override.h"
#import "base/time/time.h"
#import "components/autofill/core/browser/autofill_test_utils.h"
#import "components/autofill/core/browser/test_personal_data_manager.h"
#import "components/autofill/core/common/autofill_prefs.h"
+#import "components/autofill/ios/common/features.h"
#import "components/autofill/ios/form_util/form_activity_params.h"
#import "ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_consumer.h"
#import "ios/chrome/browser/shared/model/web_state_list/test/fake_web_state_list_delegate.h"
@@ -33,6 +35,8 @@
protected:
PaymentsSuggestionBottomSheetMediatorTest()
: test_web_state_(std::make_unique<web::FakeWebState>()) {
+ scoped_feature_list_.InitAndDisableFeature(kAutofillPaymentsSheetV2Ios);
+
web_state_list_ = std::make_unique<WebStateList>(&web_state_list_delegate_);
test_web_state_->SetCurrentURL(GURL("http://foo.com"));
@@ -105,6 +109,7 @@
id consumer_;
autofill::TestPersonalDataManager personal_data_manager_;
PaymentsSuggestionBottomSheetMediator* mediator_;
+ base::test::ScopedFeatureList scoped_feature_list_;
};
// Tests PaymentsSuggestionBottomSheetMediator can be initialized.
diff --git a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_view_controller.mm b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_view_controller.mm
index 0bbad0d..400fca15 100644
--- a/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_view_controller.mm
+++ b/ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_view_controller.mm
@@ -4,12 +4,14 @@
#import "ios/chrome/browser/autofill/ui_bundled/bottom_sheet/payments_suggestion_bottom_sheet_view_controller.h"
+#import "base/feature_list.h"
#import "base/metrics/histogram_functions.h"
#import "base/metrics/user_metrics.h"
#import "base/strings/sys_string_conversions.h"
#import "build/branding_buildflags.h"
#import "components/autofill/core/browser/data_model/credit_card.h"
#import "components/autofill/core/common/autofill_payments_features.h"
+#import "components/autofill/ios/common/features.h"
#import "components/grit/components_scaled_resources.h"
#import "components/url_formatter/elide_url.h"
#import "ios/chrome/browser/autofill/model/credit_card/credit_card_data.h"
@@ -293,6 +295,16 @@
return [cell systemLayoutSizeFittingSize:CGSizeMake(tableWidth, 1)].height;
}
+#pragma mark - UIResponder
+
+- (BOOL)canBecomeFirstResponder {
+ // In V2, since the listeners are removed early as soon as the presentation
+ // started, allow the sheet to become a first responder to not allow the
+ // keyboard popping over the sheet when there is a focus event on the WebView
+ // underneath the sheet.
+ return base::FeatureList::IsEnabled(kAutofillPaymentsSheetV2Ios);
+}
+
#pragma mark - Private
// Returns the title logo image that is resized to the correct size for the
