iOS factories for SigninManager and GaiaCookieManagerService

ios/chrome/browser/signin/signin_manager_factory.cc

Index: ios/chrome/browser/signin/signin_manager_factory.cc
diff --git a/ios/chrome/browser/signin/signin_manager_factory.cc b/ios/chrome/browser/signin/signin_manager_factory.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1e6acac5bdc4da5740cf0be88277eeca8217d9a2
--- /dev/null
+++ b/ios/chrome/browser/signin/signin_manager_factory.cc
@@ -0,0 +1,104 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ios/chrome/browser/signin/signin_manager_factory.h"
+
+#include "base/memory/singleton.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/time/time.h"
+#include "components/keyed_service/ios/browser_state_dependency_manager.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/signin/core/browser/signin_manager.h"
+#include "components/signin/core/common/signin_pref_names.h"
+#include "ios/chrome/browser/application_context.h"
+#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
+#include "ios/chrome/browser/signin/gaia_cookie_manager_service_factory.h"
+#include "ios/chrome/browser/signin/oauth2_token_service_factory.h"
+#include "ios/chrome/browser/signin/signin_client_factory.h"
+#include "ios/public/provider/chrome/browser/browser_state/chrome_browser_state.h"
+
+namespace ios {
+
+SigninManagerFactory::SigninManagerFactory()
+    : BrowserStateKeyedServiceFactory(
+          "SigninManager",
+          BrowserStateDependencyManager::GetInstance()) {
+  DependsOn(SigninClientFactory::GetInstance());
+  DependsOn(ios::GaiaCookieManagerServiceFactory::GetInstance());
+  DependsOn(OAuth2TokenServiceFactory::GetInstance());
+  DependsOn(ios::AccountTrackerServiceFactory::GetInstance());
+}
+
+SigninManagerFactory::~SigninManagerFactory() {}
+
+// static
+SigninManager* SigninManagerFactory::GetForBrowserState(
+    ios::ChromeBrowserState* browser_state) {
+  return static_cast<SigninManager*>(
+      GetInstance()->GetServiceForBrowserState(browser_state, true));
+}
+
+// static
+SigninManager* SigninManagerFactory::GetForBrowserStateIfExists(
+    ios::ChromeBrowserState* browser_state) {
+  return static_cast<SigninManager*>(
+      GetInstance()->GetServiceForBrowserState(browser_state, false));
+}
+
+// static
+SigninManagerFactory* SigninManagerFactory::GetInstance() {
+  return Singleton<SigninManagerFactory>::get();
+}
+
+void SigninManagerFactory::RegisterBrowserStatePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  SigninManagerBase::RegisterProfilePrefs(registry);
+}
+
+// static
+void SigninManagerFactory::RegisterPrefs(PrefRegistrySimple* registry) {
+  SigninManagerBase::RegisterPrefs(registry);
+}
+
+void SigninManagerFactory::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void SigninManagerFactory::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void SigninManagerFactory::NotifyObserversOfSigninManagerCreationForTesting(
+    SigninManagerBase* manager) {
+  FOR_EACH_OBSERVER(Observer, observer_list_, SigninManagerCreated(manager));
+}
+
+scoped_ptr<KeyedService> SigninManagerFactory::BuildServiceInstanceFor(
+    web::BrowserState* context) const {
+  SigninManagerBase* service = nullptr;

[sdefresne, 2015/07/24 12:38:46]

I think it would be better to use "scoped_ptr<SigninManager> service" and
"return service.Pass()" later.

+  ios::ChromeBrowserState* chrome_browser_state =
+      ios::ChromeBrowserState::FromBrowserState(context);
+  SigninClient* client = SigninClientFactory::GetInstance()->GetForBrowserState(

[sdefresne, 2015/07/24 12:38:46]

SigninClient::GetForBrowserState() is a static method, so no need to call
GetInstance() here.

+      chrome_browser_state);
+  service = new SigninManager(
+      client,

[sdefresne, 2015/07/24 12:38:46]

nit: why use a variable for SigninClient? why not directly:

service = new
SigninManager(SigninClientfactory::GetForBrowserState(chrome_browser_state), ...

+      OAuth2TokenServiceFactory::GetForBrowserState(chrome_browser_state),
+      ios::AccountTrackerServiceFactory::GetForBrowserState(
+          chrome_browser_state),
+      ios::GaiaCookieManagerServiceFactory::GetForBrowserState(
+          chrome_browser_state));
+  service->Initialize(GetApplicationContext()->GetLocalState());
+  FOR_EACH_OBSERVER(Observer, observer_list_, SigninManagerCreated(service));
+  return make_scoped_ptr(service);
+}
+
+void SigninManagerFactory::BrowserStateShutdown(web::BrowserState* context) {
+  SigninManagerBase* manager = static_cast<SigninManagerBase*>(

[sdefresne, 2015/07/24 12:38:46]

Why mix SigninManagerBase and SigninManager in the same file? You only create
SigninManger, so cast to the most specific one.

+      GetServiceForBrowserState(context, false));
+  if (manager)
+    FOR_EACH_OBSERVER(Observer, observer_list_, SigninManagerShutdown(manager));
+  BrowserStateKeyedServiceFactory::BrowserStateShutdown(context);
+}
+
+}  // namespace ios