Extract abstract base to API directory for ProfileSyncService.

chrome/browser/sync/profile_sync_service.h

 // 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.
 
 #ifndef CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_H_
 #define CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_H_
 
 #include <list>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #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"
 #include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/browser/sync/glue/data_type_manager.h"
 #include "chrome/browser/sync/glue/data_type_manager_observer.h"
 #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"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_types.h"
 #include "googleurl/src/gurl.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/sync_manager_factory.h"
 #include "sync/internal_api/public/util/experiments.h"
@@ skipping 101 matching lines @@
 //   called while the user is actively configuring their account, and then
 //   SetSetupInProgress(false) should be called when configuration is complete.
 //   When SetSyncSetupCompleted() == false, but SetSetupInProgress(true) has
 //   been called, then the sync engine knows not to download any user data.
 //
 //   When initial sync is complete, the UI code should call
 //   SetSyncSetupCompleted() followed by SetSetupInProgress(false) - this will
 //   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,
                            public content::NotificationObserver,
                            public ProfileKeyedService,
                            public InvalidationFrontend {
  public:
-  typedef ProfileSyncServiceObserver Observer;
   typedef browser_sync::SyncBackendHost::Status Status;
 
   enum SyncEventCodes  {
     MIN_SYNC_EVENT_CODE = 0,
 
     // Events starting the sync service.
     START_FROM_NTP = 1,      // Sync was started from the ad in NTP
     START_FROM_WRENCH = 2,   // Sync was started from the Wrench menu.
     START_FROM_OPTIONS = 3,  // Sync was started from Wrench->Options.
     START_FROM_BOOKMARK_MANAGER = 4,  // Sync was started from Bookmark manager.
@@ skipping 43 matching lines @@
   ProfileSyncService(ProfileSyncComponentsFactory* factory,
                      Profile* profile,
                      SigninManager* signin,
                      StartBehavior start_behavior);
   virtual ~ProfileSyncService();
 
   // Initializes the object. This should be called every time an object of this
   // 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.
   // (being logged in does not mean that tokens are available - tokens may
   // be missing because they have not loaded yet, or because they were deleted
   // due to http://crbug.com/121755).
   // Virtual to enable mocking in tests.
   virtual bool IsSyncEnabledAndLoggedIn();
 
   // Return whether all sync tokens are loaded and available for the backend to
@@ skipping 13 matching lines @@
   browser_sync::SessionModelAssociator* GetSessionModelAssociator();
 
   // Fills state_map with a map of current data types that are possible to
   // sync, as well as their states.
   void GetDataTypeControllerStates(
     browser_sync::DataTypeController::StateMap* state_map) const;
 
   // 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).
   virtual void OnNotificationsEnabled() OVERRIDE;
   virtual void OnNotificationsDisabled(
       syncer::NotificationsDisabledReason reason) OVERRIDE;
   virtual void OnIncomingNotification(
       const syncer::ObjectIdStateMap& id_state_map,
       syncer::IncomingNotificationSource source) OVERRIDE;
 
   // SyncFrontend implementation.
@@ skipping 94 matching lines @@
   }
 
   // Returns a user-friendly string form of last synced time (in minutes).
   virtual string16 GetLastSyncedTimeString() const;
 
   ProfileSyncComponentsFactory* factory() { return factory_.get(); }
 
   // 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();
 
   // Record stats on various events.
   static void SyncEvent(SyncEventCodes code);
 
   // Returns whether sync is enabled.  Sync can be enabled/disabled both
   // at compile time (e.g., on a per-OS basis) or at run time (e.g.,
   // command-line switches).
@@ skipping 70 matching lines @@
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
   // Changes which data types we're going to be syncing to |preferred_types|.
   // If it is running, the DataTypeManager will be instructed to reconfigure
   // the sync backend so that exactly these datatypes are actively synced.  See
   // class comment for more on what it means for a datatype to be Preferred.
   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
   // for a datatype to be Registered.
   virtual syncer::ModelTypeSet GetRegisteredDataTypes() const;
 
   // Checks whether the Cryptographer is ready to encrypt and decrypt updates
   // for sensitive data types. Caller must be holding a
   // syncapi::BaseTransaction to ensure thread safety.
   virtual bool IsCryptographerReady(
@@ skipping 34 matching lines @@
   // encryption_pending() must be checked.
   virtual bool EncryptEverythingEnabled() const;
 
   // Fills |encrypted_types| with the set of currently encrypted types. Does
   // not account for types pending encryption.
   virtual syncer::ModelTypeSet GetEncryptedDataTypes() const;
 
   // 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_; }
   bool setup_in_progress() const { return setup_in_progress_; }
 
   // Stops the sync backend and sets the flag for suppressing sync startup.
   void StopAndSuppress();
 
   // Resets the flag for suppressing sync startup and starts the sync backend.
   void UnsuppressAndStart();
@@ skipping 349 matching lines @@
   syncer::InvalidatorRegistrar invalidator_registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileSyncService);
 };
 
 bool ShouldShowActionOnUI(
     const syncer::SyncProtocolError& error);
 
 
 #endif  // CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_H_