Extract abstract base to API directory for ProfileSyncService.

chrome/browser/sync/profile_sync_service.h

Index: chrome/browser/sync/profile_sync_service.h
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index a09766bf8e220e73f7a8a1f7de199081690630a2..5fa42a6f0b3a5df17b5088e3272128221b8f9ca6 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -18,6 +18,8 @@
 #include "base/string16.h"
 #include "base/time.h"
 #include "base/timer.h"
+#include "chrome/browser/api/sync/profile_sync_service_base.h"
+#include "chrome/browser/api/sync/profile_sync_service_observer.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/browser/sync/backend_unrecoverable_error_handler.h"
 #include "chrome/browser/sync/failed_datatypes_handler.h"
@@ -27,7 +29,6 @@
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/invalidation_frontend.h"
 #include "chrome/browser/sync/invalidations/invalidator_storage.h"
-#include "chrome/browser/sync/profile_sync_service_observer.h"
 #include "chrome/browser/sync/sync_prefs.h"
 #include "chrome/common/net/gaia/google_service_auth_error.h"
 #include "content/public/browser/notification_observer.h"
@@ -149,7 +150,8 @@ class EncryptedData;
 //   tell the sync engine that setup is completed and it can begin downloading
 //   data from the sync server.
 //
-class ProfileSyncService : public browser_sync::SyncFrontend,
+class ProfileSyncService : public ProfileSyncServiceBase,
+                           public browser_sync::SyncFrontend,
                            public browser_sync::SyncPrefObserver,
                            public browser_sync::DataTypeManagerObserver,
                            public syncer::UnrecoverableErrorHandler,
@@ -157,7 +159,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
                            public ProfileKeyedService,
                            public InvalidationFrontend {
  public:
-  typedef ProfileSyncServiceObserver Observer;
   typedef browser_sync::SyncBackendHost::Status Status;
 
   enum SyncEventCodes  {
@@ -221,6 +222,14 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // class is constructed.
   void Initialize();
 
+  // ProfileSyncServiceBase implementation.
+  virtual bool HasSyncSetupCompleted() const OVERRIDE;
+  virtual bool ShouldPushChanges() OVERRIDE;
+  virtual syncer::ModelTypeSet GetPreferredDataTypes() const OVERRIDE;
+  virtual void AddObserver(Observer* observer) OVERRIDE;
+  virtual void RemoveObserver(Observer* observer) OVERRIDE;
+  virtual bool HasObserver(Observer* observer) const OVERRIDE;

[tim (not reviewing), 2012/09/06 17:00:06]

If we describe ProfileSyncServiceBase as "what normal feature code" should use,
there are more pieces we need to include in that API.  On one hand I feel it's
better to only add as necessary, but on the other hand I can't reason about what
PSSBase actually _is_ without seeing the full picture of what it claims to be in
the methods it exposes.  But it seems like it's a work in progress, so for now
it's likely better to not add extra pieces.

[Jói, 2012/09/06 20:08:35]

OK.  I agree it's probably best to evolve it as we start using it for other
(non-UI) components.

+
   void RegisterAuthNotifications();
 
   // Returns true if sync is enabled/not suppressed and the user is logged in.
@@ -254,8 +263,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // Disables sync for user. Use ShowLoginDialog to enable.
   virtual void DisableForUser();
 
-  // Whether sync is enabled by user or not.
-  virtual bool HasSyncSetupCompleted() const;
   virtual void SetSyncSetupCompleted();

[tim (not reviewing), 2012/09/06 17:00:06]

It's weird that this is so asymmetric now. Just double checking, but "browser
UI" doesn't qualify as "common feature code" as per your earlier comment on this
review, right?  If the goal was to have browser UI depend on browser/api
(instead of things like browser/sync/*) we'd really need all of
ProfileSyncService.

[Jói, 2012/09/06 20:08:35]

Correct, the APIs are intended for "business logic" type components, whereas UI
in general is allowed to depend concretely on everything.  This may of course
evolve over time but for now we should consider the APIs as what's needed for
the business logic of feature code.  Instantiation, set-up and configuration,
that require a larger interface surface, would either happen under
chrome/browser/configuration (which is a directory for that kind of glue code
that we are establishing) or if UI-specific, under chrome/browser/ui somewhere,
and both of those could use the full concrete implementation directly.

I moved SetSyncSetupCompleted to be right before HasSyncSetupCompleted, i.e.
right after the ProfileSyncServiceBase methods, so it's easier to see that the
concrete class has both the setter and getter.

 
   // syncer::InvalidationHandler implementation (via SyncFrontend).
@@ -371,14 +378,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // The profile we are syncing for.
   Profile* profile() const { return profile_; }
 
-  // Adds/removes an observer. ProfileSyncService does not take ownership of
-  // the observer.
-  virtual void AddObserver(Observer* observer);
-  virtual void RemoveObserver(Observer* observer);
-
-  // Returns true if |observer| has already been added as an observer.
-  bool HasObserver(Observer* observer) const;
-
   // Returns a weak pointer to the service's JsController.
   // Overrideable for testing purposes.
   virtual base::WeakPtr<syncer::JsController> GetJsController();
@@ -469,11 +468,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   virtual void ChangePreferredDataTypes(
       syncer::ModelTypeSet preferred_types);
 
-  // Get the set of currently enabled data types (as chosen or configured by
-  // the user).  See class comment for more on what it means for a datatype
-  // to be Preferred.
-  virtual syncer::ModelTypeSet GetPreferredDataTypes() const;
-
   // Gets the set of all data types that could be allowed (the set that
   // should be advertised to the user).  These will typically only change
   // via a command-line option.  See class comment for more on what it means
@@ -528,10 +522,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // Returns true if the syncer is waiting for new datatypes to be encrypted.
   virtual bool encryption_pending() const;
 
-  // Returns whether processing changes is allowed.  Check this before doing
-  // any model-modifying operations.
-  bool ShouldPushChanges();
-
   const GURL& sync_service_url() const { return sync_service_url_; }
   bool auto_start_enabled() const { return auto_start_enabled_; }
   SigninManager* signin() const { return signin_; }