[iOS] Hide passkey upgrades toggle on saving disabled
Modifies the logic of settings VC slightly to store the value of saving
passkeys pref instead of whether it's managed (the latter doesn't really
matter, the behaviour of the toggle should only be affected by whether
saving passkeys is disabled).
The toggle is now visible when both saving (passkeys, passwords) prefs
are enabled and the automatic passkey upgrades feature is enabled;
otherwise it's hidden.
Bug: 358343061
Change-Id: I5e5bca44a17f63a1d7d7c483951a24afbfed5551
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6298119
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Rafał Godlewski <rgod@google.com>
Cr-Commit-Position: refs/heads/main@{#1425643}
diff --git a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_consumer.h b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_consumer.h
index 612f2d9..8726578 100644
--- a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_consumer.h
+++ b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_consumer.h
@@ -28,17 +28,17 @@
// export is already in progress, and YES when idle.
- (void)setCanExportPasswords:(BOOL)canExportPasswords;
-// Indicates whether automatic passkey upgrades setting is `enabled` and
-// `managed` by enterprise policy. If managed, the view should not display the
-// switch for it.
-- (void)setAutomaticPasskeyUpgradesEnabled:(BOOL)enabled
- managedByPolicy:(BOOL)managed;
+// Indicates whether automatic passkey upgrades setting is `enabled`.
+- (void)setAutomaticPasskeyUpgradesEnabled:(BOOL)enabled;
// Indicates whether or not the "Offer to save passwords and passkeys" feature
// is `enabled` and if it is managed by an enterprise policy.
- (void)setSavePasswordsEnabled:(BOOL)enabled
managedByPolicy:(BOOL)managedByPolicy;
+// Indicates whether saving passkeys is `enabled`.
+- (void)setSavePasskeysEnabled:(BOOL)enabled;
+
// The count of local passwords passed along with the user eligibility to see
// the move passwords to account section.
- (void)setLocalPasswordsCount:(int)count withUserEligibility:(BOOL)eligibility;
diff --git a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_mediator.mm b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_mediator.mm
index 05da8814..f46fb5d1 100644
--- a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_mediator.mm
+++ b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_mediator.mm
@@ -178,21 +178,20 @@
self.exporterIsReady = self.passwordExporter.exportState == ExportState::IDLE;
[self savedPasswordsDidChange];
- bool savingCredentialsManagedByPolicy =
- _prefService->IsManagedPreference(kCredentialsEnableService);
[self.consumer setSavePasswordsEnabled:_prefService->GetBoolean(
kCredentialsEnableService)
- managedByPolicy:savingCredentialsManagedByPolicy];
+ managedByPolicy:_prefService->IsManagedPreference(
+ kCredentialsEnableService)];
[self.consumer setSignedInAccount:base::SysUTF8ToNSString(
_syncService->GetAccountInfo().email)];
[self.consumer
setAutomaticPasskeyUpgradesEnabled:_prefService->GetBoolean(
- kAutomaticPasskeyUpgrades)
- managedByPolicy:savingCredentialsManagedByPolicy ||
- _prefService->IsManagedPreference(
- kCredentialsEnablePasskeys)];
+ kAutomaticPasskeyUpgrades)];
+
+ [self.consumer setSavePasskeysEnabled:_prefService->GetBoolean(
+ kCredentialsEnablePasskeys)];
[self passwordAutoFillStatusDidChange];
@@ -370,20 +369,22 @@
preferenceName == kCredentialsEnableService)
<< "Unsupported preference: " << preferenceName;
- bool savingCredentialsManagedByPolicy =
- _prefService->IsManagedPreference(kCredentialsEnableService);
- bool savingPasskeysManagedByPolicy =
- _prefService->IsManagedPreference(kCredentialsEnablePasskeys);
[self.consumer
setAutomaticPasskeyUpgradesEnabled:_prefService->GetBoolean(
- kAutomaticPasskeyUpgrades)
- managedByPolicy:savingCredentialsManagedByPolicy ||
- savingPasskeysManagedByPolicy];
+ kAutomaticPasskeyUpgrades)];
- if (preferenceName == kCredentialsEnableService) {
+ if (preferenceName == kAutomaticPasskeyUpgrades) {
+ [self.consumer
+ setAutomaticPasskeyUpgradesEnabled:_prefService->GetBoolean(
+ kAutomaticPasskeyUpgrades)];
+ } else if (preferenceName == kCredentialsEnablePasskeys) {
+ [self.consumer setSavePasskeysEnabled:_prefService->GetBoolean(
+ kCredentialsEnablePasskeys)];
+ } else {
[self.consumer setSavePasswordsEnabled:_prefService->GetBoolean(
kCredentialsEnableService)
- managedByPolicy:savingCredentialsManagedByPolicy];
+ managedByPolicy:_prefService->IsManagedPreference(
+ kCredentialsEnableService)];
}
}
diff --git a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller.mm b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller.mm
index e4510216..96310b13 100644
--- a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller.mm
+++ b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller.mm
@@ -146,10 +146,6 @@
// Whether or not the Password Manager is managed by enterprise policy.
@property(nonatomic, assign, getter=isManagedByPolicy) BOOL managedByPolicy;
-// Whether automatic passkey upgrades setting is managed by enterprise policy.
-// If managed, the switch controlling this settings should not be displayed.
-@property(nonatomic, assign) BOOL automaticPasskeyUpgradesManagedByPolicy;
-
// Whether automatic passkey upgrades are enabled.
@property(nonatomic, assign) BOOL automaticPasskeyUpgradesEnabled;
@@ -157,6 +153,9 @@
@property(nonatomic, assign, getter=isSavePasswordsEnabled)
BOOL savePasswordsEnabled;
+// Whether saving passkeys is enabled.
+@property(nonatomic, assign) BOOL savePasskeysEnabled;
+
// The amount of local passwords present on device.
@property(nonatomic, assign) int localPasswordsCount;
@@ -706,15 +705,12 @@
// The `setCanExportPasswords` method required for the PasswordSettingsConsumer
// protocol is provided by property synthesis.
-- (void)setAutomaticPasskeyUpgradesEnabled:(BOOL)enabled
- managedByPolicy:(BOOL)managed {
- if (_automaticPasskeyUpgradesEnabled == enabled &&
- _automaticPasskeyUpgradesManagedByPolicy == managed) {
+- (void)setAutomaticPasskeyUpgradesEnabled:(BOOL)enabled {
+ if (_automaticPasskeyUpgradesEnabled == enabled) {
return;
}
_automaticPasskeyUpgradesEnabled = enabled;
- _automaticPasskeyUpgradesManagedByPolicy = managed;
if (self.modelLoadStatus == ModelNotLoaded) {
return;
@@ -756,6 +752,22 @@
} else {
[self updateSavePasswordsSwitch];
}
+
+ [self updateAutomaticPasskeyUpgradesSwitch];
+}
+
+- (void)setSavePasskeysEnabled:(BOOL)enabled {
+ if (_savePasskeysEnabled == enabled) {
+ return;
+ }
+
+ _savePasskeysEnabled = enabled;
+
+ if (self.modelLoadStatus == ModelNotLoaded) {
+ return;
+ }
+
+ [self updateAutomaticPasskeyUpgradesSwitch];
}
- (void)setLocalPasswordsCount:(int)count
@@ -1180,9 +1192,8 @@
}
// Updates the view to by either adding or removing the automatic passkey
-// upgrades switch section based on whether password manager or saving passkeys
-// is controlled by enterprise policy (in those cases there is no point in
-// displaying the switch).
+// upgrades toggle section. The toggle should be visible if saving passkeys and
+// passwords is enabled.
- (void)updateAutomaticPasskeyUpgradesSwitch {
if (self.modelLoadStatus != ModelLoadComplete) {
return;
@@ -1229,12 +1240,10 @@
}
// Automatic passkey upgrades switch should be displayed if the feature is
-// enabled and is not managed by an enterprise policy.
-// TODO(crbug.com/358343061): Consult with UX how this should relate to
-// `_savePasswordsEnabled`.
+// enabled and both saving passkeys and password setting is enabled.
- (BOOL)shouldDisplayPasskeyUpgradesSwitch {
- return AutomaticPasskeyUpgradeFeatureEnabled() &&
- !self.automaticPasskeyUpgradesManagedByPolicy;
+ return AutomaticPasskeyUpgradeFeatureEnabled() && _savePasswordsEnabled &&
+ _savePasskeysEnabled;
}
@end
diff --git a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller_unittest.mm b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller_unittest.mm
index 49741c7b..d8ce732c 100644
--- a/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller_unittest.mm
+++ b/ios/chrome/browser/settings/ui_bundled/password/password_settings/password_settings_view_controller_unittest.mm
@@ -78,9 +78,7 @@
TEST_F(PasswordSettingsViewControllerTest,
DisplaysOfferToSavePasswordsManagedByPolicy) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setSavePasswordsEnabled:NO managedByPolicy:YES];
+ [controller() setSavePasswordsEnabled:NO managedByPolicy:YES];
TableViewInfoButtonItem* managedSavePasswordsItem =
static_cast<TableViewInfoButtonItem*>(GetTableViewItem(/*section=*/0, 0));
EXPECT_NSEQ(managedSavePasswordsItem.text,
@@ -89,9 +87,7 @@
TEST_F(PasswordSettingsViewControllerTest,
DisplaysMovePasswordsToAccountButtonWithLocalPasswords) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setLocalPasswordsCount:2 withUserEligibility:YES];
+ [controller() setLocalPasswordsCount:2 withUserEligibility:YES];
TableViewDetailTextItem* movePasswordsToAccountDescriptionItem =
static_cast<TableViewDetailTextItem*>(
@@ -118,9 +114,7 @@
base::test::ScopedFeatureList feature_list(kIOSPasskeysM2);
CreateController();
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setPasswordsInOtherAppsEnabled:NO];
+ [controller() setPasswordsInOtherAppsEnabled:NO];
TableViewMultiDetailTextItem* passwords_in_other_apps_item =
static_cast<TableViewMultiDetailTextItem*>(
@@ -148,10 +142,7 @@
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kIOSPasskeysM2);
CreateController();
-
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setPasswordsInOtherAppsEnabled:NO];
+ [controller() setPasswordsInOtherAppsEnabled:NO];
TableViewMultiDetailTextItem* passwords_in_other_apps_item =
static_cast<TableViewMultiDetailTextItem*>(
@@ -168,9 +159,7 @@
}
TEST_F(PasswordSettingsViewControllerTest, DisplaysPasswordInOtherAppsEnabled) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setPasswordsInOtherAppsEnabled:YES];
+ [controller() setPasswordsInOtherAppsEnabled:YES];
TableViewMultiDetailTextItem* passwords_in_other_apps_item =
static_cast<TableViewMultiDetailTextItem*>(
@@ -198,6 +187,8 @@
// Re-create the controller so that the enabled flag is picked up.
CreateController();
+ [controller() setSavePasswordsEnabled:YES managedByPolicy:NO];
+ [controller() setSavePasskeysEnabled:YES];
TableViewSwitchItem* automaticPasskeyUpgradesSwitch =
static_cast<TableViewSwitchItem*>(
@@ -211,9 +202,7 @@
TEST_F(PasswordSettingsViewControllerTest,
DisplaysChangeGPMPinButtonForEligibleUser) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setupChangeGPMPinButton];
+ [controller() setupChangeGPMPinButton];
TableViewImageItem* changeGPMPinDescription =
static_cast<TableViewImageItem*>(GetTableViewItem(
@@ -235,9 +224,7 @@
TEST_F(PasswordSettingsViewControllerTest,
CallsPresentationDelegateOnGPMPinButtonTap) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setupChangeGPMPinButton];
+ [controller() setupChangeGPMPinButton];
id mockPresentationDelegate =
OCMProtocolMock(@protocol(PasswordSettingsPresentationDelegate));
@@ -254,10 +241,8 @@
TEST_F(PasswordSettingsViewControllerTest,
DisplaysEncryptionOptedInForOptedInState) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setOnDeviceEncryptionState:
- PasswordSettingsOnDeviceEncryptionStateOptedIn];
+ [controller() setOnDeviceEncryptionState:
+ PasswordSettingsOnDeviceEncryptionStateOptedIn];
TableViewImageItem* onDeviceEncryptionOptedInDescription =
static_cast<TableViewImageItem*>(GetTableViewItem(
@@ -280,10 +265,8 @@
TEST_F(PasswordSettingsViewControllerTest,
DisplaysEncryptionOptInButtonInOfferOptInState) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setOnDeviceEncryptionState:
- PasswordSettingsOnDeviceEncryptionStateOfferOptIn];
+ [controller() setOnDeviceEncryptionState:
+ PasswordSettingsOnDeviceEncryptionStateOfferOptIn];
TableViewImageItem* onDeviceEncryptionOptInDescription =
static_cast<TableViewImageItem*>(GetTableViewItem(
@@ -305,10 +288,8 @@
TEST_F(PasswordSettingsViewControllerTest,
ExportButtonDisabledWhenUserNotEligible) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setCanExportPasswords:NO];
- [consumer updateExportPasswordsButton];
+ [controller() setCanExportPasswords:NO];
+ [controller() updateExportPasswordsButton];
EXPECT_TRUE(GetTableViewItem(ExpectedSectionAfterAlwaysVisibleTopSections(),
/*item=*/0)
.accessibilityTraits &
@@ -317,10 +298,8 @@
TEST_F(PasswordSettingsViewControllerTest,
ExportButtonEnabledWhenUserEligible) {
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setCanExportPasswords:YES];
- [consumer updateExportPasswordsButton];
+ [controller() setCanExportPasswords:YES];
+ [controller() updateExportPasswordsButton];
EXPECT_FALSE(GetTableViewItem(ExpectedSectionAfterAlwaysVisibleTopSections(),
/*item=*/0)
.accessibilityTraits &
@@ -335,11 +314,8 @@
// Re-create the controller so that the enabled flag is picked up.
CreateController();
-
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setCanDeleteAllCredentials:NO];
- [consumer updateDeleteAllCredentialsSection];
+ [controller() setCanDeleteAllCredentials:NO];
+ [controller() updateDeleteAllCredentialsSection];
EXPECT_TRUE(
GetTableViewItem(ExpectedSectionAfterAlwaysVisibleTopSections() + 1,
/*item=*/0)
@@ -355,11 +331,8 @@
// Re-create the controller so that the enabled flag is picked up.
CreateController();
-
- id<PasswordSettingsConsumer> consumer =
- base::apple::ObjCCast<PasswordSettingsViewController>(controller());
- [consumer setCanDeleteAllCredentials:YES];
- [consumer updateDeleteAllCredentialsSection];
+ [controller() setCanDeleteAllCredentials:YES];
+ [controller() updateDeleteAllCredentialsSection];
EXPECT_FALSE(
GetTableViewItem(ExpectedSectionAfterAlwaysVisibleTopSections() + 1,
/*item=*/0)
