Implement cancelling card unmask authentication selection on iOS.
Before tapping the cancel button did nothing. Now, the card unmask
authentication selection bottom sheet is dismissed (and metrics are
logged).
Screencap: https://drive.google.com/file/d/1tu5euQcOFQEHD6EMYDQcWRuCklW8P2Qk/view?usp=sharing&resourcekey=0-nE9pkf01wdbFCGSdl3XyiQ
Bug: 40282545, b/324887950
Change-Id: Ia8466af593094236dd5529c11405628cd7f10066
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5402067
Reviewed-by: Siyu An <siyua@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Slobodan Pejic <slobodan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1283376}
diff --git a/ios/chrome/browser/ui/autofill/authentication/BUILD.gn b/ios/chrome/browser/ui/autofill/authentication/BUILD.gn
index d780b2c..bc3ddb6 100644
--- a/ios/chrome/browser/ui/autofill/authentication/BUILD.gn
+++ b/ios/chrome/browser/ui/autofill/authentication/BUILD.gn
@@ -60,6 +60,7 @@
"//ios/chrome/browser/shared/coordinator/chrome_coordinator",
"//ios/chrome/browser/shared/model/browser",
"//ios/chrome/browser/shared/model/web_state_list",
+ "//ios/chrome/browser/shared/public/commands",
]
}
@@ -67,6 +68,7 @@
sources = [
"card_unmask_authentication_selection_mediator.h",
"card_unmask_authentication_selection_mediator.mm",
+ "card_unmask_authentication_selection_mediator_delegate.h",
]
deps = [
":card_unmask_authentication_selection_mutator_bridge",
diff --git a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.h b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.h
index 34058d99..80b85ce 100644
--- a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.h
+++ b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.h
@@ -6,6 +6,7 @@
#define IOS_CHROME_BROWSER_UI_AUTOFILL_AUTHENTICATION_CARD_UNMASK_AUTHENTICATION_SELECTION_COORDINATOR_H_
#import <Foundation/Foundation.h>
+
#import "ios/chrome/browser/shared/coordinator/chrome_coordinator/chrome_coordinator.h"
// This class coordinates the selecting the authentication method for unmasking
diff --git a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.mm b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.mm
index a68fea1..fa1f115 100644
--- a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.mm
+++ b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_coordinator.mm
@@ -8,9 +8,16 @@
#import "ios/chrome/browser/autofill/model/bottom_sheet/autofill_bottom_sheet_tab_helper.h"
#import "ios/chrome/browser/shared/model/browser/browser.h"
#import "ios/chrome/browser/shared/model/web_state_list/web_state_list.h"
+#import "ios/chrome/browser/shared/public/commands/browser_coordinator_commands.h"
+#import "ios/chrome/browser/shared/public/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.h"
+#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_view_controller.h"
+@interface CardUnmaskAuthenticationSelectionCoordinator () <
+ CardUnmaskAuthenticationSelectionMediatorDelegate>
+@end
+
@implementation CardUnmaskAuthenticationSelectionCoordinator {
// A reference to the base view controller with UINavigationController type.
__weak UINavigationController* _baseNavigationController;
@@ -27,6 +34,8 @@
_modelController;
std::unique_ptr<CardUnmaskAuthenticationSelectionMediator> _mediator;
+
+ id<BrowserCoordinatorCommands> _browserCoordinatorCommands;
}
- (instancetype)initWithBaseNavigationController:
@@ -40,6 +49,8 @@
browser->GetWebStateList()->GetActiveWebState());
_modelController =
tabHelper->GetCardUnmaskAuthenticationSelectionDialogController();
+ _browserCoordinatorCommands = HandlerForProtocol(
+ browser->GetCommandDispatcher(), BrowserCoordinatorCommands);
CHECK(_modelController);
}
return self;
@@ -53,6 +64,7 @@
_mediator = std::make_unique<CardUnmaskAuthenticationSelectionMediator>(
_modelController->GetWeakPtr(),
/*consumer=*/selectionViewController);
+ _mediator->set_delegate(self);
selectionViewController.mutator = _mediator->AsMutator();
_selectionViewController = selectionViewController;
@@ -65,4 +77,10 @@
_selectionViewController.mutator = nil;
}
+#pragma mark - CardUnmaskAuthenticationSelectionMediatorDelegate
+
+- (void)dismissAuthenticationSelection {
+ [_browserCoordinatorCommands dismissCardUnmaskAuthentication];
+}
+
@end
diff --git a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.h b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.h
index 2ed3256..45486bd 100644
--- a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.h
+++ b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.h
@@ -8,6 +8,7 @@
#import "base/memory/weak_ptr.h"
#import "components/autofill/core/browser/ui/payments/card_unmask_authentication_selection_dialog.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_consumer.h"
+#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mutator_bridge_target.h"
@protocol CardUnmaskAuthenticationSelectionMutator;
@@ -47,6 +48,12 @@
void Dismiss(bool user_closed_dialog, bool server_success) override;
void UpdateContent() override;
+ // Set the delegate of this mediator.
+ void set_delegate(
+ id<CardUnmaskAuthenticationSelectionMediatorDelegate> delegate) {
+ delegate_ = delegate;
+ }
+
// Returns an implementation of the mutator that forwards to this mediator.
// We need this bridge since this mediator is C++ whereas the ViewController
// expects the Objective-C protocol.
@@ -56,7 +63,12 @@
base::WeakPtr<autofill::CardUnmaskAuthenticationSelectionDialogControllerImpl>
model_controller_;
__weak id<CardUnmaskAuthenticationSelectionConsumer> consumer_;
+ __weak id<CardUnmaskAuthenticationSelectionMediatorDelegate> delegate_;
CardUnmaskAuthenticationSelectionMutatorBridge* mutator_bridge_;
+
+ // Set to true the prompt has been dismissed.
+ bool was_dismissed_ = false;
+
base::WeakPtrFactory<CardUnmaskAuthenticationSelectionMutatorBridgeTarget>
weak_ptr_factory_{this};
diff --git a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.mm b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.mm
index cd8c3f9..d808d6f 100644
--- a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.mm
+++ b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator.mm
@@ -7,6 +7,7 @@
#import "base/memory/weak_ptr.h"
#import "base/strings/sys_string_conversions.h"
#import "components/autofill/core/browser/ui/payments/card_unmask_authentication_selection_dialog_controller_impl.h"
+#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mutator_bridge.h"
namespace {
@@ -59,17 +60,23 @@
}
void CardUnmaskAuthenticationSelectionMediator::DidCancelSelection() {
- // TODO(crbug.com/40282545): Implement cancelling out of the authentication
- // selection.
+ Dismiss(/*user_closed_dialog=*/true, /*server_success=*/false);
}
// Implemention of autofill::CardUnmaskAuthenticationSelectionDialog follows:
void CardUnmaskAuthenticationSelectionMediator::Dismiss(bool user_closed_dialog,
bool server_success) {
- // TODO(crbug.com/40282545): Implement dismissal of the authentication
- // selection view by delegating to the coordinator.
+ DCHECK(!was_dismissed_);
+ was_dismissed_ = true;
model_controller_->OnDialogClosed(user_closed_dialog, server_success);
+ // If the user dismissed the authentication selection, then the dismissal
+ // needs to be delegated up through our delegate (a coordinator).
+ // If Dismiss() was called after a response from the server, then we expect
+ // another prompt to be initiated without dismissal of the overall flow.
+ if (user_closed_dialog) {
+ [delegate_ dismissAuthenticationSelection];
+ }
}
void CardUnmaskAuthenticationSelectionMediator::UpdateContent() {
diff --git a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h
new file mode 100644
index 0000000..ad52bbf
--- /dev/null
+++ b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h
@@ -0,0 +1,16 @@
+// Copyright 2024 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_UI_AUTOFILL_AUTHENTICATION_CARD_UNMASK_AUTHENTICATION_SELECTION_MEDIATOR_DELEGATE_H_
+#define IOS_CHROME_BROWSER_UI_AUTOFILL_AUTHENTICATION_CARD_UNMASK_AUTHENTICATION_SELECTION_MEDIATOR_DELEGATE_H_
+
+// The authentication selection mediator delegates dismissal to the coordinator.
+@protocol CardUnmaskAuthenticationSelectionMediatorDelegate <NSObject>
+
+// Dismiss the authentication selection view.
+- (void)dismissAuthenticationSelection;
+
+@end
+
+#endif // IOS_CHROME_BROWSER_UI_AUTOFILL_AUTHENTICATION_CARD_UNMASK_AUTHENTICATION_SELECTION_MEDIATOR_DELEGATE_H_
diff --git a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_unittest.mm b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_unittest.mm
index 0fcb924..68ecaddd 100644
--- a/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_unittest.mm
+++ b/ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_unittest.mm
@@ -10,6 +10,7 @@
#import "components/autofill/core/browser/ui/payments/card_unmask_authentication_selection_dialog_controller_impl.h"
#import "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_consumer.h"
+#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mediator_delegate.h"
#import "ios/chrome/browser/ui/autofill/authentication/card_unmask_authentication_selection_mutator.h"
#import "testing/gtest/include/gtest/gtest.h"
#import "testing/gtest_mac.h"
@@ -27,12 +28,17 @@
PlatformTest::SetUp();
consumer_ =
OCMProtocolMock(@protocol(CardUnmaskAuthenticationSelectionConsumer));
+ delegate_ = OCMProtocolMock(
+ @protocol(CardUnmaskAuthenticationSelectionMediatorDelegate));
}
void TearDown() override {
if (consumer_) {
EXPECT_OCMOCK_VERIFY((id)consumer_);
}
+ if (delegate_) {
+ EXPECT_OCMOCK_VERIFY((id)delegate_);
+ }
}
CardUnmaskAuthenticationSelectionMediator* InitializeMediator(
@@ -42,14 +48,19 @@
controller_ = std::make_unique<
autofill::CardUnmaskAuthenticationSelectionDialogControllerImpl>(
challenge_options, confirm_unmasking_method_callback_.Get(),
- base::DoNothing());
+ cancel_unmasking_closure_.Get());
mediator_ = std::make_unique<CardUnmaskAuthenticationSelectionMediator>(
controller_->GetWeakPtr(), consumer_);
+ mediator_->set_delegate(delegate_);
return mediator_.get();
}
id<CardUnmaskAuthenticationSelectionConsumer> consumer() { return consumer_; }
+ id<CardUnmaskAuthenticationSelectionMediatorDelegate> delegate() {
+ return delegate_;
+ }
+
autofill::CardUnmaskAuthenticationSelectionDialogControllerImpl*
controller() {
return controller_.get();
@@ -91,9 +102,11 @@
protected:
base::MockOnceCallback<void(const std::string&)>
confirm_unmasking_method_callback_;
+ base::MockOnceClosure cancel_unmasking_closure_;
private:
id<CardUnmaskAuthenticationSelectionConsumer> consumer_;
+ id<CardUnmaskAuthenticationSelectionMediatorDelegate> delegate_;
// Mediator listed first to destruct the controller (which holds a reference
// to the mediator) before the mediator.
std::unique_ptr<CardUnmaskAuthenticationSelectionMediator> mediator_;
@@ -158,3 +171,46 @@
Run(CvcAutofillChallengeOption().id.value()));
[mediator->AsMutator() didAcceptSelection];
}
+
+TEST_F(CardUnmaskAuthenticationSelectionMediatorTest,
+ OnCancelSelection_CallsCancelUnmaskingClosure) {
+ CardUnmaskAuthenticationSelectionMediator* mediator = InitializeMediator(
+ {SmsAutofillChallengeOption(), CvcAutofillChallengeOption()});
+ mediator->DidSelectChallengeOption(CvcIOSChallengeOption());
+
+ EXPECT_CALL(cancel_unmasking_closure_, Run());
+ [mediator->AsMutator() didCancelSelection];
+}
+
+TEST_F(CardUnmaskAuthenticationSelectionMediatorTest,
+ OnCancelSelection_CallsDelegateToDismiss) {
+ CardUnmaskAuthenticationSelectionMediator* mediator =
+ InitializeMediator({SmsAutofillChallengeOption()});
+
+ OCMExpect([delegate() dismissAuthenticationSelection]);
+ [mediator->AsMutator() didCancelSelection];
+}
+
+TEST_F(CardUnmaskAuthenticationSelectionMediatorTest,
+ ServerProcessedAuthentication_DoesNotCallCancelUnmaskingClosure) {
+ InitializeMediator({SmsAutofillChallengeOption()});
+
+ EXPECT_CALL(cancel_unmasking_closure_, Run()).Times(0);
+ controller()->DismissDialogUponServerProcessedAuthenticationMethodRequest(
+ /*server_success=*/true);
+}
+
+TEST_F(CardUnmaskAuthenticationSelectionMediatorTest,
+ ServerProcessedAuthentication_DoesNotDismissAuthenticationSelection) {
+ id<CardUnmaskAuthenticationSelectionMediatorDelegate> delegate =
+ OCMStrictProtocolMock(
+ @protocol(CardUnmaskAuthenticationSelectionMediatorDelegate));
+ CardUnmaskAuthenticationSelectionMediator* mediator =
+ InitializeMediator({SmsAutofillChallengeOption()});
+ mediator->set_delegate(delegate);
+
+ // No calls to delegate() are expected, and OCMStrictProtocolMock will fail
+ // this test if [delegate() dismissAuthenticationSelection] is called.
+ controller()->DismissDialogUponServerProcessedAuthenticationMethodRequest(
+ /*server_success=*/true);
+}
