Stop showing move bubble if non-opted-in user refused it too many times

This CL adds a field trial parameter to the passwords account storage
feature. It is used to limit the number of times Chrome offers a user
that hasn't gone through the opt-in flow yet to move a password to their
account. The methods from crrev.com/c/2340966 are used to query the
number of times the user has already refused the move.

No behavior is actually changed however, since the refusal counter is
not yet incremented (future CLs).

TBR=vasilii@chromium.org

Bug: 1082152
Change-Id: I248f0964e1b406026016288d5e41fb64b2a6e10d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2345309
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Victor Vianna <victorvianna@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796871}

components/password_manager/core/browser/mock_password_feature_manager.h

@@ -31,6 +31,9 @@ class MockPasswordFeatureManager : public PasswordFeatureManager {
 
   MOCK_CONST_METHOD0(ComputePasswordAccountStorageUsageLevel,
                      metrics_util::PasswordAccountStorageUsageLevel());
+
+  MOCK_METHOD0(IncrementMoveToAccountRefusedCount, void());
+  MOCK_CONST_METHOD0(GetMoveToAccountRefusedCount, int());
 };
 
 }  // namespace password_manager

components/password_manager/core/browser/password_feature_manager.h

@@ -71,6 +71,18 @@ class PasswordFeatureManager {
   virtual metrics_util::PasswordAccountStorageUsageLevel
   ComputePasswordAccountStorageUsageLevel() const = 0;
 
+  // Increases the count of how many times Chrome automatically offered to move
+  // a password to the account and the user refused this, e.g. by ignoring the
+  // bubble that's shown after successful sign-in with a device password. Should
+  // only be called if the user is signed-in and not opted-in.
+  virtual void IncrementMoveToAccountRefusedCount() = 0;
+
+  // Gets the count of how many times Chrome automatically offered to move a
+  // password to the account and the user refused this, e.g. by ignoring the
+  // bubble that's shown after successful sign-in with a device password. Should
+  // only be called if the user is signed-in and not opted-in.
+  virtual int GetMoveToAccountRefusedCount() const = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(PasswordFeatureManager);
 };

components/password_manager/core/browser/password_feature_manager_impl.cc

@@ -77,4 +77,14 @@ PasswordFeatureManagerImpl::ComputePasswordAccountStorageUsageLevel() const {
                                                                 sync_service_);
 }
 
+void PasswordFeatureManagerImpl::IncrementMoveToAccountRefusedCount() {
+  features_util::IncrementMoveToAccountRefusedCount(pref_service_,
+                                                    sync_service_);
+}
+
+int PasswordFeatureManagerImpl::GetMoveToAccountRefusedCount() const {
+  return features_util::GetMoveToAccountRefusedCount(pref_service_,
+                                                     sync_service_);
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_feature_manager_impl.h

@@ -41,6 +41,9 @@ class PasswordFeatureManagerImpl : public PasswordFeatureManager {
   metrics_util::PasswordAccountStorageUsageLevel
   ComputePasswordAccountStorageUsageLevel() const override;
 
+  void IncrementMoveToAccountRefusedCount() override;
+  int GetMoveToAccountRefusedCount() const override;
+
  private:
   PrefService* const pref_service_;
   const syncer::SyncService* const sync_service_;

components/password_manager/core/browser/password_manager_client_helper.cc

@@ -4,6 +4,7 @@
 
 #include "components/password_manager/core/browser/password_manager_client_helper.h"
 
+#include "base/metrics/field_trial_params.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/password_manager/core/browser/password_bubble_experiment.h"
 #include "components/password_manager/core/browser/password_feature_manager.h"
@@ -105,11 +106,11 @@ bool PasswordManagerClientHelper::ShouldPromptToEnableAutoSignIn() const {
 
 bool PasswordManagerClientHelper::ShouldPromptToMovePasswordToAccount(
     const PasswordFormManagerForUI& submitted_manager) const {
-  if (!delegate_->GetPasswordFeatureManager()
-           ->ShouldShowAccountStorageBubbleUi()) {
+  PasswordFeatureManager* feature_manager =
+      delegate_->GetPasswordFeatureManager();
+  if (!feature_manager->ShouldShowAccountStorageBubbleUi())
     return false;
-  }
-  if (delegate_->GetPasswordFeatureManager()->GetDefaultPasswordStore() ==
+  if (feature_manager->GetDefaultPasswordStore() ==
       autofill::PasswordForm::Store::kProfileStore) {
     return false;
   }
@@ -125,7 +126,14 @@ bool PasswordManagerClientHelper::ShouldPromptToMovePasswordToAccount(
           submitted_manager.GetPendingCredentials().signon_realm)) {
     return false;
   }
-  return true;
+  int max_move_to_account_offers_for_non_opted_in_user =
+      base::GetFieldTrialParamByFeatureAsInt(
+          features::kEnablePasswordsAccountStorage,
+          features::kMaxMoveToAccountOffersForNonOptedInUser,
+          features::kMaxMoveToAccountOffersForNonOptedInUserDefaultValue);
+  return feature_manager->IsOptedInForAccountStorage() ||
+         feature_manager->GetMoveToAccountRefusedCount() <
+             max_move_to_account_offers_for_non_opted_in_user;
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/password_manager_client_helper_unittest.cc

@@ -172,7 +172,7 @@ TEST_F(PasswordManagerClientHelperTest,
 TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveWithoutFeature) {
   base::test::ScopedFeatureList account_storage_feature;
   account_storage_feature.InitAndDisableFeature(
-      password_manager::features::kEnablePasswordsAccountStorage);
+      features::kEnablePasswordsAccountStorage);
   EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount).Times(0);
   EXPECT_CALL(*client(), PromptUserToEnableAutosignin).Times(0);
 
@@ -186,7 +186,7 @@ TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveWithoutFeature) {
 TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForUnmovableForm) {
   base::test::ScopedFeatureList account_storage_feature;
   account_storage_feature.InitAndEnableFeature(
-      password_manager::features::kEnablePasswordsAccountStorage);
+      features::kEnablePasswordsAccountStorage);
   EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount).Times(0);
   EXPECT_CALL(*client(), PromptUserToEnableAutosignin).Times(0);
 
@@ -200,7 +200,7 @@ TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForUnmovableForm) {
 TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForGaiaAccountForm) {
   base::test::ScopedFeatureList account_storage_feature;
   account_storage_feature.InitAndEnableFeature(
-      password_manager::features::kEnablePasswordsAccountStorage);
+      features::kEnablePasswordsAccountStorage);
   EXPECT_CALL(*client()->GetPasswordFeatureManager(),
               ShouldShowAccountStorageBubbleUi)
       .WillOnce(Return(true));
@@ -215,4 +215,35 @@ TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForGaiaAccountForm) {
       CreateFormManager(&gaia_account_form, /*is_movable=*/true));
 }
 
+TEST_F(PasswordManagerClientHelperTest,
+       NoPromptToMoveForNonOptedInUserIfRefusedTooManyTimes) {
+  base::test::ScopedFeatureList account_storage_feature;
+  account_storage_feature.InitAndEnableFeatureWithParameters(
+      features::kEnablePasswordsAccountStorage,
+      {{features::kMaxMoveToAccountOffersForNonOptedInUser, "1"}});
+  ON_CALL(*client()->GetPasswordFeatureManager(),
+          ShouldShowAccountStorageBubbleUi)
+      .WillByDefault(Return(true));
+  ON_CALL(*client()->GetPasswordFeatureManager(), GetDefaultPasswordStore)
+      .WillByDefault(Return(autofill::PasswordForm::Store::kAccountStore));
+
+  // Simulate that no refusals happened so far. Moving should be offered.
+  EXPECT_CALL(*client()->GetPasswordFeatureManager(),
+              GetMoveToAccountRefusedCount)
+      .WillOnce(Return(0));
+  EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount);
+  const PasswordForm form =
+      CreateForm(kTestUsername, kTestPassword, GURL(kTestOrigin));
+  helper()->NotifySuccessfulLoginWithExistingPassword(
+      CreateFormManager(&form, /*is_movable=*/true));
+
+  // If the previous move was refused and the max is 1, shouldn't offer anymore.
+  EXPECT_CALL(*client()->GetPasswordFeatureManager(),
+              GetMoveToAccountRefusedCount)
+      .WillOnce(Return(1));
+  EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount).Times(0);
+  helper()->NotifySuccessfulLoginWithExistingPassword(
+      CreateFormManager(&form, /*is_movable=*/true));
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_manager_features_util.h

@@ -132,7 +132,8 @@ ComputePasswordAccountStorageUsageLevel(
 // password to the account and the user refused this, e.g. by ignoring the
 // bubble that's shown after successful sign-in with a device password. Should
 // only be called if the user is signed-in and not opted-in. |pref_service| and
-// |sync_service| should be non-null.
+// |sync_service| must be non-null.
+// See PasswordFeatureManager::IncrementMoveToAccountRefusedCount().
 void IncrementMoveToAccountRefusedCount(
     PrefService* pref_service,
     const syncer::SyncService* sync_service);
@@ -141,7 +142,8 @@ void IncrementMoveToAccountRefusedCount(
 // password to the account and the user refused this, e.g. by ignoring the
 // bubble that's shown after successful sign-in with a device password. Should
 // only be called if the user is signed-in and not opted-in. |pref_service| and
-// |sync_service| should be non-null.
+// |sync_service| must be non-null.
+// See PasswordFeatureManager::GetMoveToAccountRefusedCount().
 int GetMoveToAccountRefusedCount(const PrefService* pref_service,
                                  const syncer::SyncService* sync_service);
 

components/password_manager/core/common/password_manager_features.cc

@@ -133,6 +133,14 @@ const char kPasswordChangeWithForcedDialogAfterEverySuccessfulSubmission[] =
 const char kPasswordChangeInSettingsWithForcedWarningForEverySite[] =
     "should_force_warning_for_every_site_in_settings";
 
+// Number of times the user can refuse an offer to move a password to the
+// account before Chrome stops offering this flow. Only applies to users who
+// haven't gone through the opt-in flow for passwords account storage.
+const char kMaxMoveToAccountOffersForNonOptedInUser[] =
+    "max_move_to_account_offers_for_non_opted_in_user";
+
+const int kMaxMoveToAccountOffersForNonOptedInUserDefaultValue = 5;
+
 }  // namespace features
 
 }  // namespace password_manager

components/password_manager/core/common/password_manager_features.h

@@ -54,6 +54,10 @@ extern const char
     kPasswordChangeWithForcedDialogAfterEverySuccessfulSubmission[];
 extern const char kPasswordChangeInSettingsWithForcedWarningForEverySite[];
 
+// |kEnablePasswordAccountStorage| variations.
+extern const char kMaxMoveToAccountOffersForNonOptedInUser[];
+extern const int kMaxMoveToAccountOffersForNonOptedInUserDefaultValue;
+
 }  // namespace features
 
 }  // namespace password_manager

ios/web_view/internal/passwords/web_view_password_feature_manager.h

@@ -41,6 +41,9 @@ class WebViewPasswordFeatureManager
   password_manager::metrics_util::PasswordAccountStorageUsageLevel
   ComputePasswordAccountStorageUsageLevel() const override;
 
+  void IncrementMoveToAccountRefusedCount() override;
+  int GetMoveToAccountRefusedCount() const override;
+
  private:
   PrefService* const pref_service_;
   const syncer::SyncService* const sync_service_;

ios/web_view/internal/passwords/web_view_password_feature_manager.mm

@@ -71,4 +71,13 @@ WebViewPasswordFeatureManager::ComputePasswordAccountStorageUsageLevel() const {
       kUsingAccountStorage;
 }
 
+void WebViewPasswordFeatureManager::IncrementMoveToAccountRefusedCount() {
+  NOTREACHED();
+}
+
+int WebViewPasswordFeatureManager::GetMoveToAccountRefusedCount() const {
+  NOTREACHED();
+  return 0;
+}
+
 }  // namespace ios_web_view