Skip to content

Commit

Permalink
[iOS] Update params when Payments Bottom Sheet is about to be shown
Browse files Browse the repository at this point in the history
Part 1 of the fix to fill the credit card fields properly with the
payments bottom sheet.

In this CL we are adding AutofillBottomSheetObserver to observe when
the payments bottom sheet is about to be shown. We need to add this
because we encountered an issue where if the auto focus of the page is
on another field, the _lastSeenParams in FormInputAccessoryMediator
still set as that auto focus field. When we tap on another field,
keyboardWillShow will be called before anything else and in that
method, we call retrieveSuggestionsForForm with the _lastSeenParams.

This was causing conflict with our setup call: both
FormInputAccessoryMediator and PaymentsSuggestionBottomSheetMediator
are calling retrieveSuggestionsForForm and the first call to complete
with a success will stop the pipeline. That means the first one to
complete will be setting the query form and query field. We want the
form and field to be the correct one in order to fill the credit card
fields properly.

Bug: 1491904
Change-Id: I7f8b00fbd3ef6265610c21ea8d8ff6ea26293dbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974721
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Veronique Nguyen <veronguyen@google.com>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1217202}
  • Loading branch information
Veronique Nguyen authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 5655160 commit 52d64f5
Show file tree
Hide file tree
Showing 11 changed files with 260 additions and 1 deletion.
14 changes: 14 additions & 0 deletions ios/chrome/browser/autofill/bottom_sheet/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ source_set("bottom_sheet") {
sources = [
"autofill_bottom_sheet_java_script_feature.h",
"autofill_bottom_sheet_java_script_feature.mm",
"autofill_bottom_sheet_observer.h",
"autofill_bottom_sheet_observer_bridge.h",
"autofill_bottom_sheet_observer_bridge.mm",
"autofill_bottom_sheet_tab_helper.h",
"autofill_bottom_sheet_tab_helper.mm",
]
Expand All @@ -32,6 +35,17 @@ source_set("bottom_sheet") {
]
}

source_set("unit_tests") {
testonly = true
sources = [ "autofill_bottom_sheet_observer_bridge_unittest.mm" ]
deps = [
":bottom_sheet",
"//components/autofill/ios/form_util",
"//ios/web/public/test/fakes",
"//testing/gtest",
]
}

optimize_ts("bottom_sheet_ts") {
visibility = [ ":bottom_sheet" ]

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef IOS_CHROME_BROWSER_AUTOFILL_BOTTOM_SHEET_AUTOFILL_BOTTOM_SHEET_OBSERVER_H_
#define IOS_CHROME_BROWSER_AUTOFILL_BOTTOM_SHEET_AUTOFILL_BOTTOM_SHEET_OBSERVER_H_

namespace autofill {

struct FormActivityParams;

// Interface for observing autofill bottom sheet activity.
class AutofillBottomSheetObserver {
public:
AutofillBottomSheetObserver() {}

AutofillBottomSheetObserver(const AutofillBottomSheetObserver&) = delete;
AutofillBottomSheetObserver& operator=(const AutofillBottomSheetObserver&) =
delete;

virtual ~AutofillBottomSheetObserver() {}

// Called when the payments bottom sheet is about to be shown. Sends the
// params used to open the payments bottom sheet.
virtual void WillShowPaymentsBottomSheet(const FormActivityParams& params) {}
};

} // namespace autofill

#endif // IOS_CHROME_BROWSER_AUTOFILL_BOTTOM_SHEET_AUTOFILL_BOTTOM_SHEET_OBSERVER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef IOS_CHROME_BROWSER_AUTOFILL_BOTTOM_SHEET_AUTOFILL_BOTTOM_SHEET_OBSERVER_BRIDGE_H_
#define IOS_CHROME_BROWSER_AUTOFILL_BOTTOM_SHEET_AUTOFILL_BOTTOM_SHEET_OBSERVER_BRIDGE_H_

#import <Foundation/Foundation.h>

#import "base/scoped_observation.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_observer.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_tab_helper.h"

@protocol AutofillBottomSheetObserving <NSObject>
@optional
// Invoked by AutofillBottomSheetObserverBridge::WillShowPaymentsBottomSheet.
- (void)willShowPaymentsBottomSheetWithParams:
(const autofill::FormActivityParams&)params;

@end

namespace autofill {

// Use this class to be notified of the autofill bottom sheet activity in an
// Objective-C class. Implement the AutofillBottomSheetObserving protocol and
// create a AutofillBottomSheetObserverBridge passing self and
// AutofillBottomSheetTabHelper.
class AutofillBottomSheetObserverBridge : public AutofillBottomSheetObserver {
public:
// `owner` will not be retained.
AutofillBottomSheetObserverBridge(id<AutofillBottomSheetObserving> owner,
AutofillBottomSheetTabHelper* helper);
~AutofillBottomSheetObserverBridge() override;

// AutofillBottomSheetObserver overrides:
void WillShowPaymentsBottomSheet(const FormActivityParams& params) override;

private:
__weak id<AutofillBottomSheetObserving> owner_ = nil;
base::ScopedObservation<AutofillBottomSheetTabHelper,
AutofillBottomSheetObserver>
scoped_observation_{this};
};

} // namespace autofill

#endif // IOS_CHROME_BROWSER_AUTOFILL_BOTTOM_SHEET_AUTOFILL_BOTTOM_SHEET_OBSERVER_BRIDGE_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_observer_bridge.h"

namespace autofill {
AutofillBottomSheetObserverBridge::AutofillBottomSheetObserverBridge(
id<AutofillBottomSheetObserving> owner,
AutofillBottomSheetTabHelper* helper)
: owner_(owner) {
scoped_observation_.Observe(helper);
}

AutofillBottomSheetObserverBridge::~AutofillBottomSheetObserverBridge() =
default;

void AutofillBottomSheetObserverBridge::WillShowPaymentsBottomSheet(
const FormActivityParams& params) {
[owner_ willShowPaymentsBottomSheetWithParams:params];
}
} // namespace autofill
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_observer_bridge.h"

#import "components/autofill/ios/form_util/form_activity_params.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_java_script_feature.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_observer.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_tab_helper.h"
#import "ios/web/public/test/fakes/fake_web_frames_manager.h"
#import "ios/web/public/test/fakes/fake_web_state.h"
#import "testing/platform_test.h"

@interface FakeAutofillBottomSheetObserving
: NSObject <AutofillBottomSheetObserving>

- (autofill::FormActivityParams)params;

@end

@implementation FakeAutofillBottomSheetObserving {
autofill::FormActivityParams _params;
}

- (autofill::FormActivityParams)params {
return _params;
}

- (void)willShowPaymentsBottomSheetWithParams:
(const autofill::FormActivityParams&)params {
_params = params;
}

@end

// Test fixture to test AutofillBottomSheetObserverBridge class.
class AutofillBottomSheetObserverBridgeTest : public PlatformTest {
protected:
AutofillBottomSheetObserverBridgeTest() {
observer_ = [[FakeAutofillBottomSheetObserving alloc] init];

auto fake_web_state = std::make_unique<web::FakeWebState>();
auto frames_manager = std::make_unique<web::FakeWebFramesManager>();
web::ContentWorld content_world =
AutofillBottomSheetJavaScriptFeature::GetInstance()
->GetSupportedContentWorld();
fake_web_state->SetWebFramesManager(content_world,
std::move(frames_manager));

AutofillBottomSheetTabHelper::CreateForWebState(fake_web_state.get(), nil);
AutofillBottomSheetTabHelper* helper =
AutofillBottomSheetTabHelper::FromWebState(fake_web_state.get());

observer_bridge_ =
std::make_unique<autofill::AutofillBottomSheetObserverBridge>(observer_,
helper);
}

FakeAutofillBottomSheetObserving* observer_;
std::unique_ptr<autofill::AutofillBottomSheetObserverBridge> observer_bridge_;
};

// Tests willShowPaymentsBottomSheetWithParams: forwarding.
TEST_F(AutofillBottomSheetObserverBridgeTest, TestShowPaymentsBottomSheet) {
// Params values are empty.
EXPECT_EQ("", [observer_ params].form_name);
EXPECT_EQ("", [observer_ params].field_type);
EXPECT_EQ("", [observer_ params].type);

std::string form_name = "form-name";
std::string field_type = "text";
std::string type = "focus";

autofill::FormActivityParams params;
params.form_name = form_name;
params.field_type = field_type;
params.type = type;

observer_bridge_->WillShowPaymentsBottomSheet(params);

// Params values are filled properly.
EXPECT_EQ(form_name, [observer_ params].form_name);
EXPECT_EQ(field_type, [observer_ params].field_type);
EXPECT_EQ(type, [observer_ params].type);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#import "ios/web/public/web_state_user_data.h"

namespace autofill {
class AutofillBottomSheetObserver;
struct FormActivityParams;
} // namespace autofill

Expand Down Expand Up @@ -41,6 +42,10 @@ class AutofillBottomSheetTabHelper

~AutofillBottomSheetTabHelper() override;

// Observer registration methods.
void AddObserver(autofill::AutofillBottomSheetObserver* observer);
void RemoveObserver(autofill::AutofillBottomSheetObserver* observer);

// Handler for JavaScript messages. Dispatch to more specific handler.
void OnFormMessageReceived(const web::ScriptMessage& message);

Expand Down Expand Up @@ -147,6 +152,9 @@ class AutofillBottomSheetTabHelper
std::map<std::string, std::set<autofill::FieldRendererId>>
registered_payments_renderer_ids_;

base::ObserverList<autofill::AutofillBottomSheetObserver>::Unchecked
observers_;

WEB_STATE_USER_DATA_KEY_DECL();
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#import "components/password_manager/ios/password_account_storage_notice_handler.h"
#import "components/prefs/pref_service.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_java_script_feature.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_observer.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/shared/model/prefs/pref_names.h"
#import "ios/chrome/browser/shared/public/commands/autofill_bottom_sheet_commands.h"
Expand Down Expand Up @@ -69,6 +70,16 @@ bool IsPaymentsBottomSheetTriggeringField(autofill::ServerFieldType type) {
commands_handler_ = commands_handler;
}

void AutofillBottomSheetTabHelper::AddObserver(
autofill::AutofillBottomSheetObserver* observer) {
observers_.AddObserver(observer);
}

void AutofillBottomSheetTabHelper::RemoveObserver(
autofill::AutofillBottomSheetObserver* observer) {
observers_.RemoveObserver(observer);
}

void AutofillBottomSheetTabHelper::OnFormMessageReceived(
const web::ScriptMessage& message) {
autofill::FormActivityParams params;
Expand Down Expand Up @@ -108,6 +119,9 @@ bool IsPaymentsBottomSheetTriggeringField(autofill::ServerFieldType type) {

void AutofillBottomSheetTabHelper::ShowPaymentsBottomSheet(
const autofill::FormActivityParams params) {
for (auto& observer : observers_) {
observer.WillShowPaymentsBottomSheet(params);
}
[commands_handler_ showPaymentsBottomSheet:params];
}

Expand Down
2 changes: 2 additions & 0 deletions ios/chrome/browser/ui/autofill/form_input_accessory/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ source_set("form_input_accessory") {
"//ios/chrome/app/strings",
"//ios/chrome/browser/autofill:autofill",
"//ios/chrome/browser/autofill:autofill_shared",
"//ios/chrome/browser/autofill/bottom_sheet",
"//ios/chrome/browser/autofill/manual_fill",
"//ios/chrome/browser/default_browser/model:utils",
"//ios/chrome/browser/feature_engagement/model",
Expand Down Expand Up @@ -83,6 +84,7 @@ source_set("unit_tests") {
"//components/autofill/ios/form_util:form_util_feature",
"//components/autofill/ios/form_util:test_support",
"//ios/chrome/browser/autofill:autofill_shared",
"//ios/chrome/browser/autofill/bottom_sheet",
"//ios/chrome/browser/shared/model/browser/test:test_support",
"//ios/chrome/browser/shared/model/browser_state:test_support",
"//ios/chrome/browser/shared/model/web_state_list",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#import "components/autofill/ios/form_util/form_activity_observer_bridge.h"
#import "components/autofill/ios/form_util/form_activity_params.h"
#import "components/password_manager/core/browser/password_counter.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_observer_bridge.h"
#import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_tab_helper.h"
#import "ios/chrome/browser/autofill/form_input_accessory_view_handler.h"
#import "ios/chrome/browser/autofill/form_input_suggestions_provider.h"
#import "ios/chrome/browser/autofill/form_suggestion_tab_helper.h"
Expand Down Expand Up @@ -74,7 +76,8 @@ void OnPasswordCounterChanged() override {
password_manager::PasswordCounter counter_;
};

@interface FormInputAccessoryMediator () <BooleanObserver,
@interface FormInputAccessoryMediator () <AutofillBottomSheetObserving,
BooleanObserver,
FormActivityObserver,
FormInputAccessoryViewDelegate,
CRWWebStateObserver,
Expand Down Expand Up @@ -145,6 +148,10 @@ @implementation FormInputAccessoryMediator {
std::unique_ptr<autofill::FormActivityObserverBridge>
_formActivityObserverBridge;

// Bridge for the AutofillBottomSheetObserver.
std::unique_ptr<autofill::AutofillBottomSheetObserverBridge>
_autofillBottomSheetObserverBridge;

// Whether suggestions have previously been shown.
BOOL _suggestionsHaveBeenShown;

Expand Down Expand Up @@ -193,6 +200,9 @@ @implementation FormInputAccessoryMediator {
_formActivityObserverBridge =
std::make_unique<autofill::FormActivityObserverBridge>(_webState,
self);
_autofillBottomSheetObserverBridge =
std::make_unique<autofill::AutofillBottomSheetObserverBridge>(
self, AutofillBottomSheetTabHelper::FromWebState(webState));
_webStateObserverBridge =
std::make_unique<web::WebStateObserverBridge>(self);
webState->AddObserver(_webStateObserverBridge.get());
Expand Down Expand Up @@ -251,13 +261,15 @@ @implementation FormInputAccessoryMediator {
- (void)dealloc {
// TODO(crbug.com/1454777)
DUMP_WILL_BE_CHECK(!_formActivityObserverBridge.get());
DUMP_WILL_BE_CHECK(!_autofillBottomSheetObserverBridge.get());
DUMP_WILL_BE_CHECK(!_personalDataManager);
DUMP_WILL_BE_CHECK(!_webState);
DUMP_WILL_BE_CHECK(!_webStateList);
}

- (void)disconnect {
_formActivityObserverBridge.reset();
_autofillBottomSheetObserverBridge.reset();
if (_personalDataManager && _personalDataManagerObserver.get()) {
_personalDataManager->RemoveObserver(_personalDataManagerObserver.get());
_personalDataManagerObserver.reset();
Expand Down Expand Up @@ -285,6 +297,7 @@ - (void)detachFromWebState {
_webStateObserverBridge.reset();
_webState = nullptr;
_formActivityObserverBridge.reset();
_autofillBottomSheetObserverBridge.reset();
}
}

Expand All @@ -298,6 +311,20 @@ - (void)keyboardWillShow:(NSNotification*)notification {
[self updateSuggestionsIfNeeded];
}

#pragma mark - AutofillBottomSheetObserving

- (void)willShowPaymentsBottomSheetWithParams:
(const autofill::FormActivityParams&)params {
// Update params in this mediator because -keyboardWillShow will be called
// before the bottom sheet is being notified to show and that will call
// -retrieveSuggestionsForForm with the last seen params. Depending on what
// the current page is auto focused on, it could be the incorrect params and
// we need to update it.
_lastSeenParams = params;
_hasLastSeenParams = YES;
[self updateSuggestionsIfNeeded];
}

#pragma mark - FormActivityObserver

- (void)webState:(web::WebState*)webState
Expand Down Expand Up @@ -519,6 +546,10 @@ - (void)updateWithNewWebState:(web::WebState*)webState {
webState->AddObserver(_webStateObserverBridge.get());
_formActivityObserverBridge =
std::make_unique<autofill::FormActivityObserverBridge>(webState, self);
_autofillBottomSheetObserverBridge =
std::make_unique<autofill::AutofillBottomSheetObserverBridge>(
self, AutofillBottomSheetTabHelper::FromWebState(webState));

FormSuggestionTabHelper* tabHelper =
FormSuggestionTabHelper::FromWebState(webState);
if (tabHelper) {
Expand Down

0 comments on commit 52d64f5

Please sign in to comment.