[Canary 6063] Add two new password features for iOS and other platforms.

1. Delete undecryptable passwords on Sync start. Currently it is active
for Mac and Linux. It stays active only there.
2. Ignore undecryptable passwords. The feature stays disabled for all.

(cherry picked from commit 32b1445)

Bug: 1489518
Change-Id: Ic7e5d0def20357e8e47d70c72d037a8a15ab6775
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4934710
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1208940}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936851
Reviewed-by: Krishna Govind <govind@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6063@{#4}
Cr-Branched-From: cc84fa1-refs/heads/main@{#1208799}

chrome/browser/flag-metadata.json

@@ -1653,6 +1653,11 @@
     "owners": [ "ckitagawa@google.com", "clank-isochron-team@google.com" ],
     "expiry_milestone": 123
   },
+  {
+    "name": "delete-undecryptable-passwords",
+    "owners": [ "vasilii", "vsemeniuk@google.com"],
+    "expiry_milestone": 122
+  },
   {
     "name": "deprecate-alt-based-six-pack",
     "owners": [ "zentaro@google.com", "jimmyxgong@google.com",
@@ -4904,6 +4909,11 @@
     "owners": ["eddyhsu"],
     "expiry_milestone": 130
   },
+  {
+    "name": "ignore-undecryptable-passwords",
+    "owners": [ "vasilii", "vsemeniuk@google.com"],
+    "expiry_milestone": 122
+  },
   {
     "name": "improved-keyboard-shortcuts",
     "owners": [ "zentaro@google.com", "jimmyxgong@google.com",

components/password_manager/core/browser/features/password_features.cc

@@ -17,6 +17,16 @@ BASE_FEATURE(kBiometricTouchToFill,
              "BiometricTouchToFill",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Delete undecryptable passwords from the store when Sync is active.
+BASE_FEATURE(kClearUndecryptablePasswordsOnSync,
+             "ClearUndecryptablePasswordsInSync",
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+             base::FEATURE_ENABLED_BY_DEFAULT
+#else
+             base::FEATURE_DISABLED_BY_DEFAULT
+#endif
+);
+
 // Disables fallback filling if the server or the autocomplete attribute says it
 // is a credit card field.
 BASE_FEATURE(kDisablePasswordsDropdownForCvcFields,
@@ -86,6 +96,12 @@ BASE_FEATURE(kPasswordManagerLogToTerminal,
              "PasswordManagerLogToTerminal",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Displays at least the decryptable and never saved logins in the password
+// manager
+BASE_FEATURE(kSkipUndecryptablePasswords,
+             "SkipUndecryptablePasswords",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 // Improves PSL matching capabilities by utilizing PSL-extension list from
 // affiliation service. It fixes problem with incorrect password suggestions on
 // websites like slack.com.

components/password_manager/core/browser/features/password_features.h

@@ -17,6 +17,7 @@ namespace password_manager::features {
 BASE_DECLARE_FEATURE(kAutoApproveSharedPasswordUpdatesFromSameSender);
 
 BASE_DECLARE_FEATURE(kBiometricTouchToFill);
+BASE_DECLARE_FEATURE(kClearUndecryptablePasswordsOnSync);
 BASE_DECLARE_FEATURE(kDisablePasswordsDropdownForCvcFields);
 
 BASE_DECLARE_FEATURE(kEnablePasswordsAccountStorage);
@@ -39,6 +40,8 @@ BASE_DECLARE_FEATURE(kPasswordManagerEnableSenderService);
 
 BASE_DECLARE_FEATURE(kPasswordManagerLogToTerminal);
 
+BASE_DECLARE_FEATURE(kSkipUndecryptablePasswords);
+
 BASE_DECLARE_FEATURE(kUseExtensionListForPSLMatching);
 
 }  // namespace password_manager::features

components/password_manager/core/browser/login_database.cc

@@ -990,7 +990,6 @@ std::string GeneratePlaceholders(size_t count) {
   return result;
 }
 
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 // Fills |form| with necessary data required to be removed from the database
 // and returns it.
 PasswordForm GetFormForRemoval(sql::Statement& statement) {
@@ -1002,16 +1001,11 @@ PasswordForm GetFormForRemoval(sql::Statement& statement) {
   form.signon_realm = statement.ColumnString(COLUMN_SIGNON_REALM);
   return form;
 }
-#endif
 
 // Whether we should try to return the decryptable passwords while the
 // encryption service fails for some passwords.
 bool ShouldReturnPartialPasswords() {
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
   return base::FeatureList::IsEnabled(features::kSkipUndecryptablePasswords);
-#else
-  return false;
-#endif
 }
 
 std::unique_ptr<sync_pb::EntityMetadata> DecryptAndParseSyncEntityMetadata(
@@ -1897,7 +1891,6 @@ bool LoginDatabase::DeleteAndRecreateDatabaseFile() {
 }
 
 DatabaseCleanupResult LoginDatabase::DeleteUndecryptableLogins() {
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
   TRACE_EVENT0("passwords", "LoginDatabase::DeleteUndecryptableLogins");
   // If the Keychain in MacOS or the real secret key in Linux is unavailable,
   // don't delete any logins.
@@ -1942,7 +1935,6 @@ DatabaseCleanupResult LoginDatabase::DeleteUndecryptableLogins() {
     metrics_util::LogDeleteUndecryptableLoginsReturnValue(
         metrics_util::DeleteCorruptedPasswordsResult::kSuccessPasswordsDeleted);
   }
-#endif
 
   return DatabaseCleanupResult::kSuccess;
 }

components/password_manager/core/browser/login_database_unittest.cc

@@ -27,6 +27,7 @@
 #include "components/autofill/core/common/unique_ids.h"
 #include "components/os_crypt/sync/os_crypt.h"
 #include "components/os_crypt/sync/os_crypt_mocker.h"
+#include "components/password_manager/core/browser/features/password_features.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/password_store_change.h"
@@ -2099,6 +2100,7 @@ INSTANTIATE_TEST_SUITE_P(MigrationToVCurrent,
                          LoginDatabaseMigrationTestBroken,
                          testing::Values(1, 2, 3, 24));
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_IOS)
 class LoginDatabaseUndecryptableLoginsTest : public testing::Test {
  protected:
   LoginDatabaseUndecryptableLoginsTest() = default;
@@ -2202,9 +2204,11 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, DeleteUndecryptableLoginsTest) {
   base::HistogramTester histogram_tester;
   ASSERT_TRUE(db.Init());
 
-#if BUILDFLAG(IS_MAC) || (BUILDFLAG(IS_LINUX) && !BUILDFLAG(IS_CASTOS))
-  // Make sure that we can't get any logins when database is corrupted.
+#if BUILDFLAG(IS_CASTOS)
   // Disabling the checks in chromecast because encryption is unavailable.
+  EXPECT_EQ(DatabaseCleanupResult::kEncryptionUnavailable,
+            db.DeleteUndecryptableLogins());
+#else
   std::vector<PasswordForm> result;
   EXPECT_FALSE(db.GetAutofillableLogins(&result));
   EXPECT_TRUE(result.empty());
@@ -2219,24 +2223,13 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, DeleteUndecryptableLoginsTest) {
   EXPECT_TRUE(db.GetBlocklistLogins(&result));
   EXPECT_THAT(result, IsEmpty());
 
-  RunUntilIdle();
-#elif BUILDFLAG(IS_CASTOS)
-  EXPECT_EQ(DatabaseCleanupResult::kEncryptionUnavailable,
-            db.DeleteUndecryptableLogins());
-#else
-  EXPECT_EQ(DatabaseCleanupResult::kSuccess, db.DeleteUndecryptableLogins());
-#endif
-
-// Check histograms.
-#if BUILDFLAG(IS_MAC) || (BUILDFLAG(IS_LINUX) && !BUILDFLAG(IS_CASTOS))
   histogram_tester.ExpectUniqueSample(
       "PasswordManager.DeleteUndecryptableLoginsReturnValue",
       metrics_util::DeleteCorruptedPasswordsResult::kSuccessPasswordsDeleted,
       1);
 #endif
 }
 
-#if BUILDFLAG(IS_MAC)
 TEST_F(LoginDatabaseUndecryptableLoginsTest,
        PasswordRecoveryDisabledGetLogins) {
   AddDummyLogin("foo1", GURL("https://foo1.com/"), false,
@@ -2253,6 +2246,7 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest,
   RunUntilIdle();
 }
 
+#if BUILDFLAG(IS_MAC)
 TEST_F(LoginDatabaseUndecryptableLoginsTest, KeychainLockedTest) {
   AddDummyLogin("foo1", GURL("https://foo1.com/"), false,
                 /*blocklisted=*/false);
@@ -2271,7 +2265,6 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, KeychainLockedTest) {
 }
 #endif  // BUILDFLAG(IS_MAC)
 
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 // Test getting auto sign in logins when there are undecryptable ones
 TEST_F(LoginDatabaseUndecryptableLoginsTest, GetAutoSignInLogins) {
   std::vector<PasswordForm> forms;
@@ -2291,8 +2284,8 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, GetAutoSignInLogins) {
 
   EXPECT_FALSE(db.GetAutoSignInLogins(&forms));
 
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kSkipUndecryptablePasswords);
+  base::test::ScopedFeatureList feature_list(
+      features::kSkipUndecryptablePasswords);
 
   EXPECT_TRUE(db.GetAutoSignInLogins(&forms));
   EXPECT_THAT(forms, UnorderedElementsAre(HasPrimaryKeyAndEquals(form1),
@@ -2315,8 +2308,8 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, GetLogins) {
   EXPECT_FALSE(db.GetLogins(PasswordFormDigest(form),
                             /*should_PSL_matching_apply=*/false, &result));
 
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kSkipUndecryptablePasswords);
+  base::test::ScopedFeatureList feature_list(
+      features::kSkipUndecryptablePasswords);
   result.clear();
 
   EXPECT_TRUE(db.GetLogins(PasswordFormDigest(form),
@@ -2343,13 +2336,13 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, GetAutofillableLogins) {
 
   EXPECT_FALSE(db.GetAutofillableLogins(&result));
 
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kSkipUndecryptablePasswords);
+  base::test::ScopedFeatureList feature_list(
+      features::kSkipUndecryptablePasswords);
 
   EXPECT_TRUE(db.GetAutofillableLogins(&result));
   EXPECT_THAT(result, ElementsAre(HasPrimaryKeyAndEquals(form1)));
 }
-#endif
+#endif  // #if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_IOS)
 
 // Test encrypted passwords are present in add change lists.
 TEST_F(LoginDatabaseTest, EncryptedPasswordAdd) {

components/password_manager/core/browser/sync/password_sync_bridge.cc

@@ -18,6 +18,7 @@
 #include "base/strings/escape.h"
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
+#include "components/password_manager/core/browser/features/password_features.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_store_change.h"
@@ -190,12 +191,9 @@ bool IsCredentialPhished(const sync_pb::PasswordSpecificsData& specifics) {
 // the local copy, to be replaced by the remote version coming from Sync during
 // merge.
 bool ShouldRecoverPasswordsDuringMerge() {
-  // Delete the local undecryptable copy when this is MacOS or Linux only.
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
-  return true;
-#else
-  return false;
-#endif
+  // Delete the local undecryptable copy. Launched on MacOS or Linux only.
+  return base::FeatureList::IsEnabled(
+      features::kClearUndecryptablePasswordsOnSync);
 }
 
 bool ShouldCleanSyncMetadataDuringStartupWhenDecryptionFails() {

components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/test/mock_callback.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
+#include "components/password_manager/core/browser/features/password_features.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_store_sync.h"
@@ -1300,7 +1301,6 @@ TEST_F(PasswordSyncBridgeTest,
       syncer::WipeModelUponSyncDisabledBehavior::kNever, base::DoNothing());
 }
 
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 // Tests that in case ReadAllCredentials() during initial merge returns
 // encryption service failure, the bridge would try to do a DB clean up.
 class PasswordSyncBridgeMergeTest
@@ -1328,6 +1328,8 @@ class PasswordSyncBridgeMergeTest
 };
 
 TEST_P(PasswordSyncBridgeMergeTest, ShouldFixWhenDatabaseEncryptionFails) {
+  base::test::ScopedFeatureList feature_list(
+      features::kClearUndecryptablePasswordsOnSync);
   ShouldDeleteUndecryptableLoginsDuringMerge();
 }
 
@@ -1337,7 +1339,6 @@ INSTANTIATE_TEST_SUITE_P(
     testing::Values(
         FormRetrievalResult::kEncryptionServiceFailure,
         FormRetrievalResult::kEncryptionServiceFailureWithPartialData));
-#endif
 
 TEST_F(PasswordSyncBridgeTest,
        ShouldDeleteSyncMetadataWhenApplyDisableSyncChanges) {

components/password_manager/core/common/password_manager_features.cc

@@ -99,14 +99,6 @@ BASE_FEATURE(kRecoverFromNeverSaveAndroid,
              "RecoverFromNeverSaveAndroid_LAUNCHED",
              base::FEATURE_ENABLED_BY_DEFAULT);
 
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
-// Displays at least the decryptable and never saved logins in the password
-// manager
-BASE_FEATURE(kSkipUndecryptablePasswords,
-             "SkipUndecryptablePasswords",
-             base::FEATURE_DISABLED_BY_DEFAULT);
-#endif
-
 #if BUILDFLAG(IS_ANDROID)
 // Use GMS AccountSettings to manage passkeys when UPM is not available.
 BASE_FEATURE(kPasskeyManagementUsingAccountSettingsAndroid,

components/password_manager/core/common/password_manager_features.h

@@ -39,9 +39,7 @@ BASE_DECLARE_FEATURE(kPasswordGenerationExperiment);
 #endif
 BASE_DECLARE_FEATURE(kPasswordsImportM2);
 BASE_DECLARE_FEATURE(kRecoverFromNeverSaveAndroid);
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
-BASE_DECLARE_FEATURE(kSkipUndecryptablePasswords);
-#endif
+
 #if BUILDFLAG(IS_ANDROID)
 BASE_DECLARE_FEATURE(kPasskeyManagementUsingAccountSettingsAndroid);
 BASE_DECLARE_FEATURE(kPasswordEditDialogWithDetails);

ios/chrome/browser/flags/BUILD.gn

@@ -33,6 +33,7 @@ source_set("flags") {
     "//components/omnibox/common",
     "//components/optimization_guide/core",
     "//components/optimization_guide/core:features",
+    "//components/password_manager/core/browser/features:password_features",
     "//components/password_manager/core/common:features",
     "//components/payments/core",
     "//components/policy:generated",

ios/chrome/browser/flags/about_flags.mm

@@ -47,6 +47,7 @@
 #import "components/omnibox/common/omnibox_features.h"
 #import "components/optimization_guide/core/optimization_guide_features.h"
 #import "components/optimization_guide/core/optimization_guide_switches.h"
+#import "components/password_manager/core/browser/features/password_features.h"
 #import "components/password_manager/core/common/password_manager_features.h"
 #import "components/payments/core/features.h"
 #import "components/policy/core/common/features.h"
@@ -1543,6 +1544,18 @@
     {"enable-feed-containment", flag_descriptions::kEnableFeedContainmentName,
      flag_descriptions::kEnableFeedContainmentDescription, flags_ui::kOsIos,
      FEATURE_VALUE_TYPE(kEnableFeedContainment)},
+    {"delete-undecryptable-passwords",
+     flag_descriptions::kClearUndecryptablePasswordsOnSyncName,
+     flag_descriptions::kClearUndecryptablePasswordsOnSyncDescription,
+     flags_ui::kOsIos,
+     FEATURE_VALUE_TYPE(
+         password_manager::features::kClearUndecryptablePasswordsOnSync)},
+    {"ignore-undecryptable-passwords",
+     flag_descriptions::kSkipUndecryptablePasswordsName,
+     flag_descriptions::kSkipUndecryptablePasswordsDescription,
+     flags_ui::kOsIos,
+     FEATURE_VALUE_TYPE(
+         password_manager::features::kSkipUndecryptablePasswords)},
 };
 
 bool SkipConditionalFeatureEntry(const flags_ui::FeatureEntry& entry) {

ios/chrome/browser/flags/ios_chrome_flag_descriptions.cc

@@ -200,6 +200,12 @@ extern const char kAppleCalendarExperienceKitDescription[] =
     "When enabled, long pressing on dates will trigger Experience Kit Apple "
     "Calendar event handling.";
 
+const char kClearUndecryptablePasswordsOnSyncName[] =
+    "Enable cleaning up of password store on initial Sync";
+const char kClearUndecryptablePasswordsOnSyncDescription[] =
+    "Once password sync starts for the first time, the currently undecryptable "
+    "passwords will be silently cleaned up";
+
 const char kContentPushNotificationsName[] = "Content Push Notifications";
 const char kContentPushNotificationsDescription[] =
     "Enables the content push notifications.";
@@ -944,6 +950,12 @@ const char kShowInactiveTabsCountDescription[] =
     "When enabled, the count of Inactive Tabs is shown in the Inactive Tabs "
     "button that appears in the Tab Grid.";
 
+const char kSkipUndecryptablePasswordsName[] =
+    "Enable silent ignoring of undecryptable passwords";
+const char kSkipUndecryptablePasswordsDescription[] =
+    "The password store will silently skip undecryptable passwords when "
+    "reading them";
+
 const char kSmartSortingPriceTrackingDestinationName[] =
     "Price Tracking destination (with Smart Sorting)";
 const char kSmartSortingPriceTrackingDestinationDescription[] =

ios/chrome/browser/flags/ios_chrome_flag_descriptions.h

@@ -166,6 +166,11 @@ extern const char kSpotlightDonateNewIntentsDescription[];
 extern const char kBreakpadNoDelayInitialUploadName[];
 extern const char kBreakpadNoDelayInitialUploadDescription[];
 
+// Title and description for the flag to enable deletion of undecryptable
+// passwords from Sync.
+extern const char kClearUndecryptablePasswordsOnSyncName[];
+extern const char kClearUndecryptablePasswordsOnSyncDescription[];
+
 // Title and description for the flag to enable the content notifications
 // feature.
 extern const char kContentPushNotificationsName[];
@@ -827,6 +832,11 @@ extern const char kShowAutofillTypePredictionsDescription[];
 extern const char kShowInactiveTabsCountName[];
 extern const char kShowInactiveTabsCountDescription[];
 
+// Title and description for the flag to enable ignoring undecryptable passwords
+// in the password storage.
+extern const char kSkipUndecryptablePasswordsName[];
+extern const char kSkipUndecryptablePasswordsDescription[];
+
 // Title and description for the flag to add the Price Tracking destination
 // (with Smart Sorting) to the new overflow menu.
 extern const char kSmartSortingPriceTrackingDestinationName[];