[Sync] Make generic non-frontend thread datatype controller.

chrome/browser/sync/glue/non_frontend_data_type_controller.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_GLUE_NON_FRONTEND_DATA_TYPE_CONTROLLER_H__
+#define CHROME_BROWSER_SYNC_GLUE_NON_FRONTEND_DATA_TYPE_CONTROLLER_H__
+#pragma once
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/waitable_event.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
+
+class Profile;
+class ProfileSyncService;
+class ProfileSyncFactory;
+
+namespace base { class TimeDelta; }
+namespace browser_sync {
+
+class AssociatorInterface;
+class ChangeProcessor;
+
+// Implementation for datatypes that do not reside on the frontend thread
+// (UI thread). This is the same thread we perform initialization
+// on, so we don't have to worry about thread safety. The main start/stop
+// funtionality is implemented by default. Derived classes must implement:

[tim (not reviewing), 2011/04/07 22:04:25]

I like the comment.  Just a suggestion -- abbreviate the signatures e.g.
"... must implement: 

type()
CreateSyncComponents()
RecordUnrecoverableError(...)
RecordAssociationTime(...)
RecordStartFailure(...)
"

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+//    syncable::ModelType type() const
+//    browser_sync::ModelSafeGroup model_safe_group() const = 0;
+//    void CreateSyncComponents();
+//    void RecordUnrecoverableError(
+//        const tracked_objects::Location& from_here,
+//        const std::string& message);
+//    void RecordAssociationTime(base::TimeDelta time);
+//    void RecordStartFailure(StartResult result);
+class NonFrontendDataTypeController : public DataTypeController {
+ public:
+  NonFrontendDataTypeController(
+      ProfileSyncFactory* profile_sync_factory,
+      Profile* profile,
+      ProfileSyncService* sync_service);

[tim (not reviewing), 2011/04/07 22:04:25]

do we really need both the profile and service if one can be derived?

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+  virtual ~NonFrontendDataTypeController();
+
+  // DataTypeController interface.
+  virtual void Start(StartCallback* start_callback);
+  virtual void Stop();
+  virtual syncable::ModelType type() const = 0;
+  virtual browser_sync::ModelSafeGroup model_safe_group() const = 0;
+  virtual std::string name() const;
+  virtual State state() const;
+
+  // UnrecoverableErrorHandler interface.
+  // Note: this is performed on the datatype's thread.
+  virtual void OnUnrecoverableError(const tracked_objects::Location& from_here,
+                                    const std::string& message);
+ protected:
+  // For testing only.
+  NonFrontendDataTypeController();
+
+  // Kick of any dependent services that need to be running before we can

[tim (not reviewing), 2011/04/07 22:04:25]

a comment like "kick off" on a method called Start when there is also a method
called "KickOff..." is confusing :)

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+  // associate models. The default implementation is a no-op.
+  // Return value:
+  //   True - if models are ready and association can proceed.
+  //   False - if models are not ready. KickOffAssociation should be called
+  //           when the models are ready. Refer to Start(_) implementation.
+  // Note: this is performed on the frontend (UI) thread.
+  virtual bool StartModels();
+
+  // Kick off the the association on the thread the datatype lives on.

[tim (not reviewing), 2011/04/07 22:04:25]

"the the" > "the"

I think we should be as consistent as possible with naming.  I'd prefer
StartAssociateAsync() Associate() AssociateFailed AssociateDone 

vs Start/Kickoff/Association/StartFailed/StartDone?

Lastly, I'm fine with Kickoff but I think convention in chrome would be to use
the verb 'Post'

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+  // Note: this is performed on the frontend (UI) thread.
+  // Return value: True if task posted successfully, False otherwise.
+  virtual bool KickOffAssociation() = 0;
+
+  // Build sync components and associate models.
+  // Note: this is performed on the datatype's thread.
+  virtual void Associate();
+
+  // Datatype specific creation of sync components.
+  // Note: this is performed on the datatype's thread.
+  virtual void CreateSyncComponents() = 0;
+
+  // Start failed, make sure we record it, clean up state, and invoke the
+  // callback on the frontend thread.
+  // Note: this is performed on the datatype's thread.
+  virtual void StartFailed(StartResult result,
+                           const tracked_objects::Location& location);
+
+  // Start up complete, update the state and invoke the callback.
+  // Note: this is performed on the datatype's thread.
+  virtual void StartDone(DataTypeController::StartResult result,
+                         DataTypeController::State new_state,
+                         const tracked_objects::Location& location);
+
+  // UI thread implementation of StartDone.
+  virtual void StartDoneImpl(DataTypeController::StartResult result,
+                             DataTypeController::State new_state,
+                             const tracked_objects::Location& location);
+
+  // Perform any DataType controller specific state cleanup before stopping
+  // the datatype controller. The default implementation is a no-op.
+  // Note: this is performed on the frontend (UI) thread.
+  virtual void CleanUpState();

[tim (not reviewing), 2011/04/07 22:04:25]

I think just Cleanup or Teardown is a more familiar name for this sort of thing.
 Consider suffixing "OnFrontendLoop" or something.

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+
+  // Kick off disassociation/destruction of the sync components on the
+  // datatype's thread.
+  // Note: this is performed on the frontend (UI) thread.
+  // Return value: True if task posted successfully, False otherwise.
+  virtual bool KickOffDestroy() = 0;

[tim (not reviewing), 2011/04/07 22:04:25]

same as above

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+
+  // Disassociate the models and destroy the sync components.
+  // Note: this is performed on the datatype's thread.
+  virtual void Destroy();
+
+  // Implementation of OnUnrecoverableError that lives on UI thread.
+  virtual void OnUnrecoverableErrorImpl(
+      const tracked_objects::Location& from_here,
+      const std::string& message);
+
+  // DataType specific histogram methods. Because histograms use static's, the
+  // specific datatype controllers must implement this themselves.
+  // Important: calling them on other threads can lead to memory corruption!
+  // Record unrecoverable errors. Called on Datatype's thread.
+  virtual void RecordUnrecoverableError(
+      const tracked_objects::Location& from_here,
+      const std::string& message) = 0;
+  // Record association time. Called on Datatype's thread.
+  virtual void RecordAssociationTime(base::TimeDelta time) = 0;
+  // Record causes of start failure. Called on UI thread.
+  virtual void RecordStartFailure(StartResult result) = 0;
+
+  ProfileSyncFactory* const profile_sync_factory_;

[tim (not reviewing), 2011/04/07 22:04:25]

strictly speaking, style-guide-says members should be private with accessors... 

[Nicolas Zea, 2011/04/08 16:53:42]

Done.

+  Profile* const profile_;
+  ProfileSyncService* const sync_service_;
+
+  State state_;
+
+  scoped_ptr<StartCallback> start_callback_;
+  scoped_ptr<AssociatorInterface> model_associator_;
+  scoped_ptr<ChangeProcessor> change_processor_;
+
+  // Locks/Barriers for aborting association early.
+  base::Lock abort_association_lock_;
+  bool abort_association_;
+  base::WaitableEvent abort_association_complete_;
+
+  // Barrier to ensure that the datatype has been stopped on the DB thread
+  // from the UI thread.
+  base::WaitableEvent datatype_stopped_;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NonFrontendDataTypeController);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_NON_FRONTEND_DATA_TYPE_CONTROLLER_H__