B4P: Don't offer saving the password for the primary account

Saving the password for a given account *within* that account doesn't
make sense, so stop offering it.

Bug: 1105846
Change-Id: Ibfee6beadbbbc983fde42b36cd5518d462bdca2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2323522
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792711}

components/password_manager/core/browser/sync_credentials_filter.cc

@@ -6,16 +6,16 @@
 
 #include <algorithm>
 
+#include "base/feature_list.h"
 #include "base/macros.h"
-#include "base/metrics/field_trial.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "components/password_manager/core/browser/password_form_manager.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/password_sync_util.h"
 #include "components/password_manager/core/common/password_manager_features.h"
-#include "google_apis/gaia/gaia_urls.h"
-#include "net/base/url_util.h"
+#include "components/signin/public/identity_manager/account_info.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 
 using autofill::PasswordForm;
 
@@ -32,11 +32,41 @@ SyncCredentialsFilter::~SyncCredentialsFilter() = default;
 
 bool SyncCredentialsFilter::ShouldSave(
     const autofill::PasswordForm& form) const {
-  return !client_->IsIncognito() &&
-         !form.form_data.is_gaia_with_skip_save_password_form &&
-         !sync_util::IsSyncAccountCredential(
-             form, sync_service_factory_function_.Run(),
-             client_->GetIdentityManager());
+  if (client_->IsIncognito())
+    return false;
+
+  if (form.form_data.is_gaia_with_skip_save_password_form)
+    return false;
+
+  const syncer::SyncService* sync_service =
+      sync_service_factory_function_.Run();
+  const signin::IdentityManager* identity_manager =
+      client_->GetIdentityManager();
+
+  if (base::FeatureList::IsEnabled(features::kEnablePasswordsAccountStorage)) {
+    // If kEnablePasswordsAccountStorage is enabled, then don't allow saving the
+    // password if it corresponds to the primary account. Note that if the user
+    // is just signing in to the first Gaia account, then IdentityManager might
+    // not know about the account yet.
+    if (sync_util::IsGaiaCredentialPage(form.signon_realm)) {
+      CoreAccountInfo primary_account = identity_manager->GetPrimaryAccountInfo(
+          signin::ConsentLevel::kNotRequired);
+      if (primary_account.IsEmpty() ||
+          gaia::AreEmailsSame(base::UTF16ToUTF8(form.username_value),
+                              primary_account.email)) {
+        return false;
+      }
+    }
+  } else {
+    // If kEnablePasswordsAccountStorage is NOT enabled, then don't allow saving
+    // the password for the sync account specifically.
+    if (sync_util::IsSyncAccountCredential(form, sync_service,
+                                           identity_manager)) {
+      return false;
+    }
+  }
+
+  return true;
 }
 
 bool SyncCredentialsFilter::ShouldSaveGaiaPasswordHash(

components/password_manager/core/browser/sync_credentials_filter_unittest.cc

@@ -216,11 +216,25 @@ TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_NotSyncing) {
   EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName));
 }
 
+TEST_P(CredentialsFilterTest, ShouldSave_NotSignedIn) {
+  PasswordForm form = SimpleGaiaForm("user@example.org");
+
+  ASSERT_TRUE(identity_manager()->GetPrimaryAccountInfo().IsEmpty());
+  SetSyncingPasswords(false);
+  // If kEnablePasswordsAccountStorage is enabled, then Chrome shouldn't offer
+  // to save the password for the primary account. If there is no primary
+  // account yet, then the just-signed-in account will *become* the primary
+  // account immediately, so it shouldn't be saved either.
+  if (base::FeatureList::IsEnabled(features::kEnablePasswordsAccountStorage))
+    EXPECT_FALSE(filter_.ShouldSave(form));
+  else
+    EXPECT_TRUE(filter_.ShouldSave(form));
+}
+
 TEST_P(CredentialsFilterTest, ShouldSave_NotSyncCredential) {
   PasswordForm form = SimpleGaiaForm("user@example.org");
 
-  ASSERT_NE("user@example.org",
-            identity_manager()->GetPrimaryAccountInfo().email);
+  FakeSigninAs("different_user@example.org");
   SetSyncingPasswords(true);
   EXPECT_TRUE(filter_.ShouldSave(form));
 }
@@ -246,7 +260,13 @@ TEST_P(CredentialsFilterTest, ShouldSave_SyncCredential_NotSyncingPasswords) {
 
   FakeSigninAs("user@example.org");
   SetSyncingPasswords(false);
-  EXPECT_TRUE(filter_.ShouldSave(form));
+  // If kEnablePasswordsAccountStorage is enabled, then Chrome shouldn't offer
+  // to save the password for the primary account - doesn't matter if passwords
+  // are being synced or not.
+  if (base::FeatureList::IsEnabled(features::kEnablePasswordsAccountStorage))
+    EXPECT_FALSE(filter_.ShouldSave(form));
+  else
+    EXPECT_TRUE(filter_.ShouldSave(form));
 }
 
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)