[Sync] Move change-related methods out of SyncManager::Observer

chrome/browser/sync/internal_api/sync_manager.h

 // Copyright (c) 2011 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_INTERNAL_API_SYNC_MANAGER_H_
 #define CHROME_BROWSER_SYNC_INTERNAL_API_SYNC_MANAGER_H_
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/callback_old.h"
+#include "base/threading/thread_checker.h"
 #include "chrome/browser/sync/internal_api/change_record.h"
 #include "chrome/browser/sync/internal_api/configure_reason.h"
 #include "chrome/browser/sync/protocol/sync_protocol_error.h"
 #include "chrome/browser/sync/syncable/model_type.h"
 #include "chrome/browser/sync/util/weak_handle.h"
 #include "chrome/common/net/gaia/google_service_auth_error.h"
 
 class FilePath;
 
 namespace base {
@@ skipping 35 matching lines @@
                                        // decryption with the cached passphrase
                                        // was unsuccessful.
 };
 
 // Contains everything needed to talk to and identify a user account.
 struct SyncCredentials {
   std::string email;
   std::string sync_token;
 };
 
-// SyncManager encapsulates syncable::DirectoryManager and serves as the parent
-// of all other objects in the sync API.  SyncManager is thread-safe.  If
-// multiple threads interact with the same local sync repository (i.e. the
-// same sqlite database), they should share a single SyncManager instance.  The
-// caller should typically create one SyncManager for the lifetime of a user
-// session.
+// SyncManager encapsulates syncable::DirectoryManager and serves as
+// the parent of all other objects in the sync API.  If multiple
+// threads interact with the same local sync repository (i.e. the same
+// sqlite database), they should share a single SyncManager instance.
+// The caller should typically create one SyncManager for the lifetime
+// of a user session.
+//
+// Unless stated otherwise, all methods of SyncManager should be
+// called on the same thread.
 class SyncManager {
  public:
   // SyncInternal contains the implementation of SyncManager, while abstracting
   // internal types from clients of the interface.
   class SyncInternal;
 
   // Status encapsulates detailed state about the internals of the SyncManager.
   struct Status {
     // Summary is a distilled set of important information that the end-user may
     // wish to be informed about (through UI, for example). Note that if a
@@ skipping 72 matching lines @@
     // Count of useless and useful syncs we perform.
     int useless_sync_cycles;
     int useful_sync_cycles;
 
     // Encryption related.
     syncable::ModelTypeSet encrypted_types;
     bool cryptographer_ready;
     bool crypto_has_pending_keys;
   };
 
-  // An interface the embedding application implements to receive notifications
-  // from the SyncManager.  Register an observer via SyncManager::AddObserver.
-  // This observer is an event driven model as the events may be raised from
-  // different internal threads, and simply providing an "OnStatusChanged" type
-  // notification complicates things such as trying to determine "what changed",
-  // if different members of the Status object are modified from different
-  // threads.  This way, the event is explicit, and it is safe for the Observer
-  // to dispatch to a native thread or synchronize accordingly.
-  class Observer {
+  // An interface the embedding application implements to be notified
+  // on change events.  Note that these methods may be called on *any*
+  // thread.
+  class ChangeDelegate {
    public:
-    // Notify the observer that changes have been applied to the sync model.
+    // Notify the delegate that changes have been applied to the sync model.
     //
     // This will be invoked on the same thread as on which ApplyChanges was
     // called. |changes| is an array of size |change_count|, and contains the
     // ID of each individual item that was changed. |changes| exists only for
     // the duration of the call. If items of multiple data types change at
     // the same time, this method is invoked once per data type and |changes|
     // is restricted to items of the ModelType indicated by |model_type|.
     // Because the observer is passed a |trans|, the observer can assume a
     // read lock on the sync model that will be released after the function
     // returns.
@@ skipping 22 matching lines @@
     //
     // The purpose of this function is to support processors that require
     // split-transactions changes. For example, if a model processor wants to
     // perform blocking I/O due to a change, it should calculate the changes
     // while holding the transaction lock (from within OnChangesApplied), buffer
     // those changes, let the transaction fall out of scope, and then commit
     // those changes from within OnChangesComplete (postponing the blocking
     // I/O to when it no longer holds any lock).
     virtual void OnChangesComplete(syncable::ModelType model_type) = 0;
 
+   protected:
+    virtual ~ChangeDelegate();
+  };
+
+  // Like ChangeDelegate, except called only on the sync thread and
+  // not while a transaction is held.  For objects that want to know
+  // when changes happen, but don't need to process them.
+  class ChangeObserver {
+   public:
+    // Ids referred to in |changes| may or may not be in |mutations|.
+    // If they're not, that means that the node didn't actually
+    // change, but we marked them as changed for some other reason
+    // (e.g., siblings of re-ordered nodes).
+    //
+    // TODO(sync, long-term): Ideally, ChangeDelegate would also be
+    // passed |mutations| instead of a transaction that would have to
+    // be used to look up the changed nodes.  That is,
+    // ChangeDelegate::OnChangesApplied() would still be called under
+    // the transaction, but all the needed data will be passed down.
+    //
+    // Even more ideally, we would have sync semantics such that we'd
+    // be able to apply changes without being under a transaction.
+    // But that's a ways off...
+    virtual void OnChangesApplied(
+        syncable::ModelType model_type,
+        const syncable::ImmutableEntryKernelMutationMap& mutations,

[tim (not reviewing), 2011/09/20 16:04:59]

We shouldn't add new deps on syncable:: stuff here.  Adding deps rules was a
first step in formalizing this, some things ( like model_type ) still leak
through and they're called out in the deps files (for ex, see glue/DEPS)

[akalin, 2011/09/20 21:14:42]

Introduced the notion of a write transaction ID, and just use that.  Done.

+        const ImmutableChangeRecordList& changes) = 0;
+
+    virtual void OnChangesComplete(syncable::ModelType model_type) = 0;
+
+   protected:
+    virtual ~ChangeObserver();
+  };
+
+  // An interface the embedding application implements to receive
+  // notifications from the SyncManager.  Register an observer via
+  // SyncManager::AddObserver.  All methods are called only on the
+  // sync thread.
+  class Observer {
+   public:
     // A round-trip sync-cycle took place and the syncer has resolved any
     // conflicts that may have arisen.
     virtual void OnSyncCycleCompleted(
         const browser_sync::sessions::SyncSessionSnapshot* snapshot) = 0;
 
     // Called when user interaction may be required due to an auth problem.
     virtual void OnAuthError(const GoogleServiceAuthError& auth_error) = 0;
 
     // Called when a new auth token is provided by the sync server.
     virtual void OnUpdatedToken(const std::string& token) = 0;
@@ skipping 104 matching lines @@
     // After a request to clear server data, these callbacks are invoked to
     // indicate success or failure.
     virtual void OnClearServerDataSucceeded() = 0;
     virtual void OnClearServerDataFailed() = 0;
 
     // Called after we finish encrypting all appropriate datatypes.
     virtual void OnEncryptionComplete(
         const syncable::ModelTypeSet& encrypted_types) = 0;
 
     virtual void OnActionableError(
-        const browser_sync::SyncProtocolError& sync_protocol_error)
-            = 0;
+        const browser_sync::SyncProtocolError& sync_protocol_error) = 0;
 
    protected:
     virtual ~Observer();
   };
 
   typedef Callback0::Type ModeChangeCallback;
 
   // Create an uninitialized SyncManager.  Callers must Init() before using.
   explicit SyncManager(const std::string& name);
   virtual ~SyncManager();
@@ skipping 14 matching lines @@
   // HTTP header. Used internally when collecting stats to classify clients.
   // |sync_notifier| is owned and used to listen for notifications.
   bool Init(const FilePath& database_location,
             const browser_sync::WeakHandle<browser_sync::JsEventHandler>&
                 event_handler,
             const std::string& sync_server_and_path,
             int sync_server_port,
             bool use_ssl,
             HttpPostProviderFactory* post_factory,
             browser_sync::ModelSafeWorkerRegistrar* registrar,
+            ChangeDelegate* change_delegate,
             const std::string& user_agent,
             const SyncCredentials& credentials,
             sync_notifier::SyncNotifier* sync_notifier,
             const std::string& restored_key_for_bootstrapping,
             bool setup_for_test_mode);
 
   // Returns the username last used for a successful authentication.
-  // Returns empty if there is no such username.
+  // Returns empty if there is no such username.  May be called on any
+  // thread.
   const std::string& GetAuthenticatedUsername();
 
   // Check if the database has been populated with a full "initial" download of
   // sync items for each data type currently present in the routing info.
   // Prerequisite for calling this is that OnInitializationComplete has been
-  // called.
+  // called.  May be called from any thread.
   bool InitialSyncEndedForAllEnabledTypes();
 
   // Update tokens that we're using in Sync. Email must stay the same.
   void UpdateCredentials(const SyncCredentials& credentials);
 
   // Called when the user disables or enables a sync type.
   void UpdateEnabledTypes();
 
   // Conditionally sets the flag in the Nigori node which instructs other
   // clients to start syncing tabs.
@@ skipping 24 matching lines @@
   // Switches the mode of operation to CONFIGURATION_MODE and
   // schedules a config task to fetch updates for |types|.
   void RequestConfig(const syncable::ModelTypeBitSet& types,
      sync_api::ConfigureReason reason);
 
   void RequestCleanupDisabledTypes();
 
   // Request a clearing of all data on the server
   void RequestClearServerData();
 
+  // Add/remove change observers.
+  void AddChangeObserver(ChangeObserver* observer);
+  void RemoveChangeObserver(ChangeObserver* observer);
+
   // Adds a listener to be notified of sync events.
   // NOTE: It is OK (in fact, it's probably a good idea) to call this before
   // having received OnInitializationCompleted.
   void AddObserver(Observer* observer);
 
   // Remove the given observer.  Make sure to call this if the
   // Observer is being destroyed so the SyncManager doesn't
   // potentially dereference garbage.
   void RemoveObserver(Observer* observer);
 
   // Status-related getters. Typically GetStatusSummary will suffice, but
   // GetDetailedSyncStatus can be useful for gathering debug-level details of
-  // the internals of the sync engine.
+  // the internals of the sync engine.  May be called on any thread.
   Status::Summary GetStatusSummary() const;
   Status GetDetailedStatus() const;
 
   // Whether or not the Nigori node is encrypted using an explicit passphrase.
+  // May be called on any thread.
   bool IsUsingExplicitPassphrase();
 
-  // Get the internal implementation for use by BaseTransaction, etc.
-  SyncInternal* GetImpl() const;
-
   // Call periodically from a database-safe thread to persist recent changes
   // to the syncapi model.
   void SaveChanges();
 
+  // Requests the syncer to stop as soon as possible.  May be called
+  // from any thread.
   void RequestEarlyExit();
 
   // Issue a final SaveChanges, close sqlite handles, and stop running threads.
-  // Must be called from the same thread that called Init().
   void Shutdown();
 
+  // May be called from any thread.
   UserShare* GetUserShare() const;
 
   // Inform the cryptographer of the most recent passphrase and set of encrypted
   // types (from nigori node), then ensure all data that needs encryption is
   // encrypted with the appropriate passphrase.
   // Note: opens a transaction and can trigger ON_PASSPHRASE_REQUIRED, so must
   // only be called after syncapi has been initialized.
   void RefreshEncryption();
 
   // Enable encryption of all sync data. Once enabled, it can never be disabled
   // without clearing the server data.
   void EnableEncryptEverything();
 
-  // Returns true if we are currently encrypting all sync data.
+  // Returns true if we are currently encrypting all sync data.  May
+  // be called on any thread.
   bool EncryptEverythingEnabled() const;
 
   // Gets the set of encrypted types from the cryptographer
-  // Note: opens a transaction.
+  // Note: opens a transaction.  May be called from any thread.
   syncable::ModelTypeSet GetEncryptedDataTypes() const;
 
   // Reads the nigori node to determine if any experimental types should be
   // enabled.
-  // Note: opens a transaction.
+  // Note: opens a transaction.  May be called on any thread.
   bool ReceivedExperimentalTypes(syncable::ModelTypeSet* to_add) const;
 
   // Uses a read-only transaction to determine if the directory being synced has
-  // any remaining unsynced items.
+  // any remaining unsynced items.  May be called on any thread.
   bool HasUnsyncedItems() const;
 
-  // Logs the list of unsynced meta handles.
-  void LogUnsyncedItems(int level) const;
-
   // Functions used for testing.
 
   void TriggerOnNotificationStateChangeForTest(
       bool notifications_enabled);
 
   void TriggerOnIncomingNotificationForTest(
       const syncable::ModelTypeBitSet& model_types);
 
  private:
+  base::ThreadChecker thread_checker_;
+
   // An opaque pointer to the nested private class.
   SyncInternal* data_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncManager);
 };
 
 bool InitialSyncEndedForTypes(syncable::ModelTypeSet types, UserShare* share);
 
 // Returns the string representation of a PassphraseRequiredReason value.
 std::string PassphraseRequiredReasonToString(PassphraseRequiredReason reason);
 
 } // namespace sync_api
 
 #endif  // CHROME_BROWSER_SYNC_INTERNAL_API_SYNC_MANAGER_H_