Inject preferences in SigninManagerAndroid

Inject PrefServices to SigninManagerAndroid via
SigninManagerAndroidWrapperFactory. This tackles the last dependency
preventing the componentization of SigninManager.java.

This change is discussed and described within "componentization of
SigninManager.java":
https://docs.google.com/document/d/18887XeZNJ9pmoTdJducssk5_yVU2CQ3EqaRFoMqWCCk/edit?usp=sharing

Bug: 963408
Change-Id: Ib0afa4e818b545f325f79e689ff6882e80fb0735
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1688959
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Auto-Submit: Pâris Meuleman <pmeuleman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676424}

chrome/browser/android/signin/signin_manager_android.cc

@@ -11,12 +11,11 @@
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "chrome/android/chrome_jni_headers/SigninManager_jni.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/primary_account_manager.h"
 #include "components/signin/public/base/account_consistency_method.h"
+#include "components/signin/public/base/signin_client.h"
 #include "components/signin/public/base/signin_pref_names.h"
 #include "components/signin/public/identity_manager/primary_account_mutator.h"
 #include "google_apis/gaia/gaia_auth_util.h"
@@ -25,28 +24,34 @@ using base::android::JavaParamRef;
 
 namespace {
 // Clears the information about the last signed-in user from |profile|.
-void ClearLastSignedInUserForProfile(Profile* profile) {
-  profile->GetPrefs()->ClearPref(prefs::kGoogleServicesLastAccountId);
-  profile->GetPrefs()->ClearPref(prefs::kGoogleServicesLastUsername);
+void ClearLastSignedInUserForProfile(SigninClient* signin_client) {
+  signin_client->GetPrefs()->ClearPref(prefs::kGoogleServicesLastAccountId);
+  signin_client->GetPrefs()->ClearPref(prefs::kGoogleServicesLastUsername);
 }
 }  // namespace
 
 SigninManagerAndroid::SigninManagerAndroid(
-    Profile* profile,
+    SigninClient* signin_client,
+    PrefService* local_state_pref_service,
     identity::IdentityManager* identity_manager,
     std::unique_ptr<SigninManagerDelegate> signin_manager_delegate)
-    : profile_(profile),
+    : signin_client_(signin_client),
       identity_manager_(identity_manager),
       signin_manager_delegate_(std::move(signin_manager_delegate)) {
-  DCHECK(profile_);
+  DCHECK(signin_client_);
+  DCHECK(local_state_pref_service);
   DCHECK(identity_manager_);
   DCHECK(signin_manager_delegate_);
   identity_manager_->AddObserver(this);
-  pref_change_registrar_.Init(profile_->GetPrefs());
-  pref_change_registrar_.Add(
-      prefs::kSigninAllowed,
+
+  signin_allowed_.Init(
+      prefs::kSigninAllowed, signin_client_->GetPrefs(),
       base::Bind(&SigninManagerAndroid::OnSigninAllowedPrefChanged,
                  base::Unretained(this)));
+
+  force_browser_signin_.Init(prefs::kForceBrowserSignin,
+                             local_state_pref_service);
+
   java_signin_manager_ = Java_SigninManager_create(
       base::android::AttachCurrentThread(), reinterpret_cast<intptr_t>(this),
       signin_manager_delegate_->GetJavaObject(),
@@ -98,7 +103,7 @@ void SigninManagerAndroid::SignOut(JNIEnv* env,
 void SigninManagerAndroid::ClearLastSignedInUser(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
-  ClearLastSignedInUserForProfile(profile_);
+  ClearLastSignedInUserForProfile(signin_client_);
 }
 
 void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env,
@@ -109,18 +114,20 @@ void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env,
   identity_manager_->LegacyReloadAccountsFromSystem();
 }
 
+bool SigninManagerAndroid::IsSigninAllowed() const {
+  return signin_allowed_.GetValue();
+}
+
 jboolean SigninManagerAndroid::IsSigninAllowedByPolicy(
     JNIEnv* env,
-    const JavaParamRef<jobject>& obj) {
-  return profile_->GetPrefs()->GetBoolean(prefs::kSigninAllowed);
+    const JavaParamRef<jobject>& obj) const {
+  return IsSigninAllowed();
 }
 
 jboolean SigninManagerAndroid::IsForceSigninEnabled(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
-  // prefs::kForceBrowserSignin is set in Local State, not in user prefs.
-  PrefService* prefs = g_browser_process->local_state();
-  return prefs->GetBoolean(prefs::kForceBrowserSignin);
+  return force_browser_signin_.GetValue();
 }
 
 jboolean SigninManagerAndroid::IsSignedInOnNative(
@@ -136,10 +143,10 @@ void SigninManagerAndroid::OnPrimaryAccountCleared(
                                      java_signin_manager_);
 }
 
-void SigninManagerAndroid::OnSigninAllowedPrefChanged() {
+void SigninManagerAndroid::OnSigninAllowedPrefChanged() const {
   Java_SigninManager_onSigninAllowedByPolicyChanged(
       base::android::AttachCurrentThread(), java_signin_manager_,
-      profile_->GetPrefs()->GetBoolean(prefs::kSigninAllowed));
+      IsSigninAllowed());
 }
 
 base::android::ScopedJavaLocalRef<jstring> JNI_SigninManager_ExtractDomainName(

chrome/browser/android/signin/signin_manager_android.h

@@ -13,9 +13,10 @@
 #include "base/threading/thread_checker.h"
 #include "chrome/browser/android/signin/signin_manager_delegate.h"
 #include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_member.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 
-class Profile;
+class SigninClient;
 
 // Android wrapper of Chrome's C++ identity management code which provides
 // access from the Java layer. Note that on Android, there's only a single
@@ -28,7 +29,8 @@ class Profile;
 class SigninManagerAndroid : public identity::IdentityManager::Observer {
  public:
   SigninManagerAndroid(
-      Profile* profile,
+      SigninClient* signin_client,
+      PrefService* local_state_prefs_service,
       identity::IdentityManager* identity_manager,
       std::unique_ptr<SigninManagerDelegate> signin_manager_delegate);
 
@@ -56,7 +58,7 @@ class SigninManagerAndroid : public identity::IdentityManager::Observer {
 
   jboolean IsSigninAllowedByPolicy(
       JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj);
+      const base::android::JavaParamRef<jobject>& obj) const;
 
   jboolean IsForceSigninEnabled(
       JNIEnv* env,
@@ -70,9 +72,17 @@ class SigninManagerAndroid : public identity::IdentityManager::Observer {
       const CoreAccountInfo& previous_primary_account_info) override;
 
  private:
-  void OnSigninAllowedPrefChanged();
+  void OnSigninAllowedPrefChanged() const;
+  bool IsSigninAllowed() const;
+
+  SigninClient* signin_client_;
 
-  Profile* profile_;
+  // Handler for prefs::kSigninAllowed set in user's profile.
+  BooleanPrefMember signin_allowed_;
+
+  // Handler for prefs::kForceBrowserSignin. This preference is set in Local
+  // State, not in user prefs.
+  BooleanPrefMember force_browser_signin_;
 
   identity::IdentityManager* identity_manager_;
 
@@ -81,8 +91,6 @@ class SigninManagerAndroid : public identity::IdentityManager::Observer {
   // Java-side SigninManager object.
   base::android::ScopedJavaGlobalRef<jobject> java_signin_manager_;
 
-  PrefChangeRegistrar pref_change_registrar_;
-
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(SigninManagerAndroid);

chrome/browser/signin/signin_manager_android_wrapper.cc

@@ -5,10 +5,12 @@
 #include "chrome/browser/signin/signin_manager_android_wrapper.h"
 
 SigninManagerAndroidWrapper::SigninManagerAndroidWrapper(
-    Profile* profile,
+    SigninClient* signin_client,
+    PrefService* local_state_prefs_service,
     identity::IdentityManager* identity_manager,
     std::unique_ptr<SigninManagerDelegate> signin_manager_delegate)
-    : signin_manager_android_(profile,
+    : signin_manager_android_(signin_client,
+                              local_state_prefs_service,
                               identity_manager,
                               std::move(signin_manager_delegate)) {}
 

chrome/browser/signin/signin_manager_android_wrapper.h

@@ -10,13 +10,12 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 
-class Profile;
-
 class SigninManagerAndroidWrapper : public KeyedService {
  public:
   // initializes the member signin_manager_android_ and keeps ownership.
   SigninManagerAndroidWrapper(
-      Profile* profile,
+      SigninClient* signin_client,
+      PrefService* local_state_prefs_service,
       identity::IdentityManager* identity_manager,
       std::unique_ptr<SigninManagerDelegate> signin_manager_delegate);
 

chrome/browser/signin/signin_manager_android_wrapper_factory.cc

@@ -6,14 +6,18 @@
 
 #include "chrome/browser/android/signin/chrome_signin_manager_delegate.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 
+#include "chrome/browser/browser_process.h"
+
 SigninManagerAndroidWrapperFactory::SigninManagerAndroidWrapperFactory()
     : BrowserContextKeyedServiceFactory(
           "SigninManagerAndroidWrapper",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(IdentityManagerFactory::GetInstance());
+  DependsOn(ChromeSigninClientFactory::GetInstance());
 }
 
 SigninManagerAndroidWrapperFactory::~SigninManagerAndroidWrapperFactory() {}
@@ -36,10 +40,12 @@ SigninManagerAndroidWrapperFactory::GetInstance() {
 KeyedService* SigninManagerAndroidWrapperFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
+  auto* signin_client = ChromeSigninClientFactory::GetForProfile(profile);
   auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
   auto signin_manager_delegate =
       std::make_unique<ChromeSigninManagerDelegate>();
 
-  return new SigninManagerAndroidWrapper(profile, identity_manager,
-                                         std::move(signin_manager_delegate));
+  return new SigninManagerAndroidWrapper(
+      signin_client, g_browser_process->local_state(), identity_manager,
+      std::move(signin_manager_delegate));
 }