Convert PasswordStoreSigninNotifierImpl to IdentityManager

Add a new method OnPrimaryAccountSetWithPassword to
IdentityManager::Observer (with comments indicating
it only works for legacy sign-in workflow) and port
PasswordStoreSigninNotifierImpl to it.

Bug: 903870, 887438, 887441
Change-Id: Ic79cff4ffdd341aedb22ee2c8abd40d309ee6fa4
Reviewed-on: https://chromium-review.googlesource.com/c/1344089Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610029}

chrome/browser/password_manager/password_store_signin_notifier_impl.cc

@@ -6,8 +6,8 @@
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/account_tracker_service_factory.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
-#include "components/signin/core/browser/signin_manager.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
+#include "services/identity/public/cpp/identity_manager.h"
 
 namespace password_manager {
 
@@ -22,34 +22,32 @@ PasswordStoreSigninNotifierImpl::~PasswordStoreSigninNotifierImpl() {}
 void PasswordStoreSigninNotifierImpl::SubscribeToSigninEvents(
     PasswordStore* store) {
   set_store(store);
-  SigninManagerFactory::GetForProfile(profile_)->AddObserver(this);
+  IdentityManagerFactory::GetForProfile(profile_)->AddObserver(this);
   AccountTrackerServiceFactory::GetForProfile(profile_)->AddObserver(this);
 }
 
 void PasswordStoreSigninNotifierImpl::UnsubscribeFromSigninEvents() {
-  SigninManagerFactory::GetForProfile(profile_)->RemoveObserver(this);
   AccountTrackerServiceFactory::GetForProfile(profile_)->RemoveObserver(this);
+  IdentityManagerFactory::GetForProfile(profile_)->RemoveObserver(this);
 }
 
-void PasswordStoreSigninNotifierImpl::GoogleSigninSucceededWithPassword(
-    const std::string& account_id,
-    const std::string& username,
+void PasswordStoreSigninNotifierImpl::OnPrimaryAccountSetWithPassword(
+    const AccountInfo& account_info,
     const std::string& password) {
-  NotifySignin(username, password);
+  NotifySignin(account_info.email, password);
 }
 
-void PasswordStoreSigninNotifierImpl::GoogleSignedOut(
-    const std::string& account_id,
-    const std::string& username) {
-  NotifySignedOut(username, /* primary_account= */ true);
+void PasswordStoreSigninNotifierImpl::OnPrimaryAccountCleared(
+    const AccountInfo& account_info) {
+  NotifySignedOut(account_info.email, /* primary_account= */ true);
 }
 
 // AccountTrackerService::Observer implementations.
 void PasswordStoreSigninNotifierImpl::OnAccountRemoved(
     const AccountInfo& info) {
   // Only reacts to content area (non-primary) Gaia account sign-out event.
-  if (info.account_id != SigninManagerFactory::GetForProfile(profile_)
-                             ->GetAuthenticatedAccountId()) {
+  if (info.account_id !=
+      IdentityManagerFactory::GetForProfile(profile_)->GetPrimaryAccountId()) {
     NotifySignedOut(info.email, /* primary_account= */ false);
   }
 }

chrome/browser/password_manager/password_store_signin_notifier_impl.h

@@ -9,7 +9,7 @@
 
 #include "components/password_manager/core/browser/password_store_signin_notifier.h"
 #include "components/signin/core/browser/account_tracker_service.h"
-#include "components/signin/core/browser/signin_manager_base.h"
+#include "services/identity/public/cpp/identity_manager.h"
 
 class Profile;
 
@@ -17,8 +17,9 @@ namespace password_manager {
 
 // Responsible for subscribing to Chrome sign-in events and passing them to
 // PasswordStore.
-class PasswordStoreSigninNotifierImpl : public PasswordStoreSigninNotifier,
-                                        public SigninManagerBase::Observer,
+class PasswordStoreSigninNotifierImpl
+    : public PasswordStoreSigninNotifier,
+      public identity::IdentityManager::Observer,
       public AccountTrackerService::Observer {
  public:
   explicit PasswordStoreSigninNotifierImpl(Profile* profile);
@@ -29,12 +30,9 @@ class PasswordStoreSigninNotifierImpl : public PasswordStoreSigninNotifier,
   void UnsubscribeFromSigninEvents() override;
 
   // SigninManagerBase::Observer implementations.
-  void GoogleSigninSucceededWithPassword(const std::string& account_id,
-                                         const std::string& username,
+  void OnPrimaryAccountSetWithPassword(const AccountInfo& account_info,
                                        const std::string& password) override;
-
-  void GoogleSignedOut(const std::string& account_id,
-                       const std::string& username) override;
+  void OnPrimaryAccountCleared(const AccountInfo& account_info) override;
 
   // AccountTrackerService::Observer implementations.
   void OnAccountRemoved(const AccountInfo& info) override;

components/signin/core/browser/signin_manager.cc

@@ -473,12 +473,11 @@ void SigninManager::OnSignedIn() {
 }
 
 void SigninManager::FireGoogleSigninSucceeded() {
-  std::string account_id = GetAuthenticatedAccountId();
-  std::string email = GetAuthenticatedAccountInfo().email;
+  const AccountInfo account_info = GetAuthenticatedAccountInfo();
   for (auto& observer : observer_list_) {
-    observer.GoogleSigninSucceeded(account_id, email);
-    observer.GoogleSigninSucceeded(GetAuthenticatedAccountInfo());
-    observer.GoogleSigninSucceededWithPassword(account_id, email, password_);
+    observer.GoogleSigninSucceeded(account_info.account_id, account_info.email);
+    observer.GoogleSigninSucceeded(account_info);
+    observer.GoogleSigninSucceededWithPassword(account_info, password_);
   }
 }
 

components/signin/core/browser/signin_manager_base.h

@@ -44,10 +44,6 @@ class PrefService;
 class SigninClient;
 class SigninErrorController;
 
-namespace password_manager {
-class PasswordStoreSigninNotifierImpl;
-}
-
 class SigninManagerBase : public KeyedService {
  public:
   class Observer {
@@ -59,6 +55,22 @@ class SigninManagerBase : public KeyedService {
     // This method is not called during a reauth.
     virtual void GoogleSigninSucceeded(const AccountInfo& account_info) {}
 
+    // Called when a user signs into Google services such as sync. Also passes
+    // the password of the Google account that was used to sign in.
+    // This method is not called during a reauth.
+    //
+    // Observers should override |GoogleSigninSucceeded| if they are not
+    // interested in the password thas was used during the sign-in.
+    //
+    // Note: The password is always empty on mobile as the user signs in to
+    // Chrome with accounts that were added to the device, so Chrome does not
+    // have access to the password.
+    // DEPRECATED: password will be empty if login is using DICE workflow; the
+    // method will be removed once all login is using the DICE workflow.
+    virtual void GoogleSigninSucceededWithPassword(
+        const AccountInfo& account_info,
+        const std::string& password) {}
+
     // DEPRECATED: Use the above method instead.
     virtual void GoogleSigninSucceeded(const std::string& account_id,
                                        const std::string& username) {}
@@ -74,29 +86,10 @@ class SigninManagerBase : public KeyedService {
     virtual ~Observer() {}
 
    private:
-    // Observers that can observer the password of the Google account after a
-    // successful sign-in.
-    friend class PasswordStoreSigninNotifierImpl;
-
     // SigninManagers that fire |GoogleSigninSucceededWithPassword|
     // notifications.
     friend class SigninManager;
     friend class FakeSigninManager;
-
-    // Called when a user signs into Google services such as sync. Also passes
-    // the password of the Google account that was used to sign in.
-    // This method is not called during a reauth.
-    //
-    // Observers should override |GoogleSigninSucceeded| if they are not
-    // interested in the password thas was used during the sign-in.
-    //
-    // Note: The password is always empty on mobile as the user signs in to
-    // Chrome with accounts that were added to the device, so Chrome does not
-    // have access to the password.
-    virtual void GoogleSigninSucceededWithPassword(
-        const std::string& account_id,
-        const std::string& username,
-        const std::string& password) {}
   };
 
 // On non-ChromeOS platforms, SigninManagerBase should only be instantiated

components/signin/core/browser/signin_manager_unittest.cc

@@ -61,8 +61,7 @@ class TestSigninManagerObserver : public SigninManagerBase::Observer {
     num_successful_signins_++;
   }
 
-  void GoogleSigninSucceededWithPassword(const std::string& account_id,
-                                         const std::string& username,
+  void GoogleSigninSucceededWithPassword(const AccountInfo& account_info,
                                          const std::string& password) override {
     num_successful_signins_with_password_++;
   }

services/identity/public/cpp/identity_manager.cc

@@ -281,6 +281,14 @@ void IdentityManager::GoogleSigninSucceeded(const AccountInfo& account_info) {
   }
 }
 
+void IdentityManager::GoogleSigninSucceededWithPassword(
+    const AccountInfo& account_info,
+    const std::string& password) {
+  for (auto& observer : observer_list_) {
+    observer.OnPrimaryAccountSetWithPassword(account_info, password);
+  }
+}
+
 void IdentityManager::GoogleSignedOut(const AccountInfo& account_info) {
   DCHECK(!HasPrimaryAccount());
   for (auto& observer : observer_list_) {

services/identity/public/cpp/identity_manager.h

@@ -68,6 +68,14 @@ class IdentityManager : public SigninManagerBase::Observer,
     // This method is not called during a reauth.
     virtual void OnPrimaryAccountSet(const AccountInfo& primary_account_info) {}
 
+    // Called when an account becomes the user's primary account using the
+    // legacy workflow (non-DICE). If access to the password is not required,
+    // it is preferred to instead override OnPrimaryAccountSet() which will
+    // also be called at the same time.
+    virtual void OnPrimaryAccountSetWithPassword(
+        const AccountInfo& primary_account_info,
+        const std::string& password) {}
+
     // Called when when the user moves from having a primary account to no
     // longer having a primary account.
     virtual void OnPrimaryAccountCleared(
@@ -336,6 +344,8 @@ class IdentityManager : public SigninManagerBase::Observer,
 
   // SigninManagerBase::Observer:
   void GoogleSigninSucceeded(const AccountInfo& account_info) override;
+  void GoogleSigninSucceededWithPassword(const AccountInfo& account_info,
+                                         const std::string& password) override;
   void GoogleSignedOut(const AccountInfo& account_info) override;
   void GoogleSigninFailed(const GoogleServiceAuthError& error) override;