signin: pull basic SigninManager functionality into new SigninManagerBase class.

chrome/browser/signin/signin_manager_factory.h

Index: chrome/browser/signin/signin_manager_factory.h
diff --git a/chrome/browser/signin/signin_manager_factory.h b/chrome/browser/signin/signin_manager_factory.h
index 7cb2b81f3f4d34ee9e3b4632b2e2d6a1b4a71594..878d28f56bbf5fb26c025ba18f82d645d4995e4e 100644
--- a/chrome/browser/signin/signin_manager_factory.h
+++ b/chrome/browser/signin/signin_manager_factory.h
@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile_keyed_service_factory.h"
 
 class SigninManager;
+class SigninManagerBase;
 class PrefRegistrySimple;
 class PrefRegistrySyncable;
 class Profile;
@@ -21,11 +22,20 @@ class SigninManagerFactory : public ProfileKeyedServiceFactory {
   // Returns the instance of SigninManager associated with this profile
   // (creating one if none exists). Returns NULL if this profile cannot have a
   // SigninManager (for example, if |profile| is incognito).
-  static SigninManager* GetForProfile(Profile* profile);
+  static SigninManagerBase* GetBaseForProfile(Profile* profile);

[Roger Tawa OOO till Jul 10th, 2013/03/27 17:36:33]

Instead of two methods GetBaseForProfile() and GetForProfile(), why not
something like:

#ifdef chromeos
  static SigninManagerBase* GetForProfile();
#else
  static SigninManager* GetForProfile();
#endif

Same for XXXIfExists().  I guess you're thinking that on desktop some callers
only need the "basic" functionality and could call GetBaseForProfile().  Not
sure if the extra complexity is worth it.

[tim (not reviewing), 2013/03/29 18:03:16]

Right I noticed the vast majority of callsites just want Base functionality /
dependencies, so it seemed worth it to me. But I guess it'd be easier to do it
your way... I"ll change it.

 
   // Returns the instance of SigninManager associated with this profile. Returns
   // null if no SigninManager instance currently exists (will not create a new
   // instance).
+  static SigninManagerBase* GetBaseForProfileIfExists(Profile* profile);
+
+  // On non-ChromeOS platforms, the SigninManager the factory creates will be
+  // an instance of the extended SigninManager class.
+  //
+  // TODO(tim): Make the above statement true in BuildServiceInstanceFor, and
+  // rename these (to e.g GetExtendedForProfile) and ifdef out on chrome os.

[Roger Tawa OOO till Jul 10th, 2013/03/27 17:36:33]

Why change BuildServiceInstanceFor() in a second step?

[tim (not reviewing), 2013/03/29 18:03:16]

I wanted to split it up to avoid a massive CL mixed with real changes and
renames. But with the ifdef suggestion I implemented I'll need to audit all the
callsites. It also looks like I'll have to refactor ProfileSyncServiceHarness,
since it's using StartSignin on all platforms.

I'll let you know when I have a working patch again.

+  // Most callers should use SigninManagerBase.
+  static SigninManager* GetForProfile(Profile* profile);
   static SigninManager* GetForProfileIfExists(Profile* profile);
 
   // Returns an instance of the SigninManagerFactory singleton.