RefCounted types should not have public destructors, chrome/browser/ part 2

chrome/browser/sync/glue/non_frontend_data_type_controller.cc

Index: chrome/browser/sync/glue/non_frontend_data_type_controller.cc
diff --git a/chrome/browser/sync/glue/non_frontend_data_type_controller.cc b/chrome/browser/sync/glue/non_frontend_data_type_controller.cc
index 2aa51491eda0ee384851873fd6df64c6fd93863b..7a0b89a7fc6a4855d871e0665195ed8a916f284f 100644
--- a/chrome/browser/sync/glue/non_frontend_data_type_controller.cc
+++ b/chrome/browser/sync/glue/non_frontend_data_type_controller.cc
@@ -22,15 +22,6 @@ using content::BrowserThread;
 
 namespace browser_sync {
 
-NonFrontendDataTypeController::NonFrontendDataTypeController()
-    : profile_sync_factory_(NULL),
-      profile_(NULL),
-      profile_sync_service_(NULL),
-      state_(NOT_RUNNING),
-      abort_association_(false),
-      abort_association_complete_(false, false),
-      datatype_stopped_(false, false) {}
-
 NonFrontendDataTypeController::NonFrontendDataTypeController(
     ProfileSyncComponentsFactory* profile_sync_factory,
     Profile* profile,
@@ -48,10 +39,6 @@ NonFrontendDataTypeController::NonFrontendDataTypeController(
   DCHECK(profile_sync_service_);
 }
 
-NonFrontendDataTypeController::~NonFrontendDataTypeController() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-}
-
 void NonFrontendDataTypeController::Start(const StartCallback& start_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!start_callback.is_null());
@@ -80,110 +67,6 @@ void NonFrontendDataTypeController::Start(const StartCallback& start_callback) {
   }
 }
 
-bool NonFrontendDataTypeController::StartModels() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(state_, MODEL_STARTING);
-  // By default, no additional services need to be started before we can proceed
-  // with model association, so do nothing.
-  return true;
-}
-
-void NonFrontendDataTypeController::StartAssociation() {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(state_, ASSOCIATING);
-  {
-    base::AutoLock lock(abort_association_lock_);
-    if (abort_association_) {
-      abort_association_complete_.Signal();
-      return;
-    }
-    CreateSyncComponents();
-  }
-
-  if (!model_associator_->CryptoReadyIfNecessary()) {
-    StartFailed(NEEDS_CRYPTO, SyncError());
-    return;
-  }
-
-  bool sync_has_nodes = false;
-  if (!model_associator_->SyncModelHasUserCreatedNodes(&sync_has_nodes)) {
-    SyncError error(FROM_HERE, "Failed to load sync nodes", type());
-    StartFailed(UNRECOVERABLE_ERROR, error);
-    return;
-  }
-
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  SyncError error;
-  error = model_associator_->AssociateModels();
-  // TODO(lipalani): crbug.com/122690 - handle abort.
-  RecordAssociationTime(base::TimeTicks::Now() - start_time);
-  if (error.IsSet()) {
-    StartFailed(ASSOCIATION_FAILED, error);
-    return;
-  }
-
-  profile_sync_service_->ActivateDataType(type(), model_safe_group(),
-                                          change_processor());
-  StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, RUNNING, SyncError());
-}
-
-void NonFrontendDataTypeController::StartFailed(StartResult result,
-                                                const SyncError& error) {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  if (IsUnrecoverableResult(result))
-    RecordUnrecoverableError(FROM_HERE, "StartFailed");
-  StopAssociation();
-  StartDone(result,
-            result == ASSOCIATION_FAILED ? DISABLED : NOT_RUNNING,
-            error);
-}
-
-void NonFrontendDataTypeController::StartDone(
-    DataTypeController::StartResult result,
-    DataTypeController::State new_state,
-    const SyncError& error) {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
-  abort_association_complete_.Signal();
-  base::AutoLock lock(abort_association_lock_);
-  if (!abort_association_) {
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-        base::Bind(&NonFrontendDataTypeController::StartDoneImpl,
-                   this,
-                   result,
-                   new_state,
-                   error));
-  }
-}
-
-void NonFrontendDataTypeController::StartDoneImpl(
-    DataTypeController::StartResult result,
-    DataTypeController::State new_state,
-    const SyncError& error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // It's possible to have StartDoneImpl called first from the UI thread
-  // (due to Stop being called) and then posted from the non-UI thread. In
-  // this case, we drop the second call because we've already been stopped.
-  if (state_ == NOT_RUNNING) {
-    DCHECK(start_callback_.is_null());
-    return;
-  }
-
-  state_ = new_state;
-  if (state_ != RUNNING) {
-    // Start failed.
-    StopModels();
-    RecordStartFailure(result);
-  }
-
-  // We have to release the callback before we call it, since it's possible
-  // invoking the callback will trigger a call to STOP(), which will get
-  // confused by the non-NULL start_callback_.
-  StartCallback callback = start_callback_;
-  start_callback_.Reset();
-  callback.Run(result, error);
-}
-
 // TODO(sync): Blocking the UI thread at shutdown is bad. The new API avoids
 // this. Once all non-frontend datatypes use the new API, we can get rid of this
 // locking (see implementation in AutofillProfileDataTypeController).
@@ -239,24 +122,6 @@ void NonFrontendDataTypeController::Stop() {
   state_ = NOT_RUNNING;
 }
 
-void NonFrontendDataTypeController::StopModels() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(state_ == STOPPING || state_ == NOT_RUNNING || state_ == DISABLED);
-  DVLOG(1) << "NonFrontendDataTypeController::StopModels(): State = " << state_;
-  // Do nothing by default.
-}
-
-void NonFrontendDataTypeController::StopAssociation() {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (model_associator_.get()) {
-    SyncError error;  // Not used.
-    error = model_associator_->DisassociateModels();
-  }
-  model_associator_.reset();
-  change_processor_.reset();
-  datatype_stopped_.Signal();
-}
-
 std::string NonFrontendDataTypeController::name() const {
   // For logging only.
   return syncable::ModelTypeToString(type());
@@ -290,28 +155,97 @@ void NonFrontendDataTypeController::OnSingleDatatypeUnrecoverableError(
                  message));
 }
 
-void NonFrontendDataTypeController::OnUnrecoverableErrorImpl(
-    const tracked_objects::Location& from_here,
-    const std::string& message) {
+NonFrontendDataTypeController::NonFrontendDataTypeController()
+    : profile_sync_factory_(NULL),
+      profile_(NULL),
+      profile_sync_service_(NULL),
+      state_(NOT_RUNNING),
+      abort_association_(false),
+      abort_association_complete_(false, false),
+      datatype_stopped_(false, false) {
+}
+
+NonFrontendDataTypeController::~NonFrontendDataTypeController() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  profile_sync_service_->OnUnrecoverableError(from_here, message);
 }
 
-bool NonFrontendDataTypeController::StartAssociationAsync() {
+bool NonFrontendDataTypeController::StartModels() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(state(), ASSOCIATING);
-  return PostTaskOnBackendThread(
-      FROM_HERE,
-      base::Bind(&NonFrontendDataTypeController::StartAssociation, this));
+  DCHECK_EQ(state_, MODEL_STARTING);
+  // By default, no additional services need to be started before we can proceed
+  // with model association, so do nothing.
+  return true;
 }
 
-bool NonFrontendDataTypeController::StopAssociationAsync() {
+void NonFrontendDataTypeController::StartFailed(StartResult result,
+                                                const SyncError& error) {
+  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (IsUnrecoverableResult(result))
+    RecordUnrecoverableError(FROM_HERE, "StartFailed");
+  StopAssociation();
+  StartDone(result,
+            result == ASSOCIATION_FAILED ? DISABLED : NOT_RUNNING,
+            error);
+}
+
+void NonFrontendDataTypeController::StartDone(
+    DataTypeController::StartResult result,
+    DataTypeController::State new_state,
+    const SyncError& error) {
+  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
+  abort_association_complete_.Signal();
+  base::AutoLock lock(abort_association_lock_);
+  if (!abort_association_) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(&NonFrontendDataTypeController::StartDoneImpl,
+                   this,
+                   result,
+                   new_state,
+                   error));
+  }
+}
+
+void NonFrontendDataTypeController::StartDoneImpl(
+    DataTypeController::StartResult result,
+    DataTypeController::State new_state,
+    const SyncError& error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(state(), STOPPING);
-  return PostTaskOnBackendThread(
-      FROM_HERE,
-      base::Bind(
-          &NonFrontendDataTypeController::StopAssociation, this));
+  // It's possible to have StartDoneImpl called first from the UI thread
+  // (due to Stop being called) and then posted from the non-UI thread. In
+  // this case, we drop the second call because we've already been stopped.
+  if (state_ == NOT_RUNNING) {
+    DCHECK(start_callback_.is_null());
+    return;
+  }
+
+  state_ = new_state;
+  if (state_ != RUNNING) {
+    // Start failed.
+    StopModels();
+    RecordStartFailure(result);
+  }
+
+  // We have to release the callback before we call it, since it's possible
+  // invoking the callback will trigger a call to STOP(), which will get
+  // confused by the non-NULL start_callback_.
+  StartCallback callback = start_callback_;
+  start_callback_.Reset();
+  callback.Run(result, error);
+}
+
+void NonFrontendDataTypeController::StopModels() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(state_ == STOPPING || state_ == NOT_RUNNING || state_ == DISABLED);
+  DVLOG(1) << "NonFrontendDataTypeController::StopModels(): State = " << state_;
+  // Do nothing by default.
+}
+
+void NonFrontendDataTypeController::OnUnrecoverableErrorImpl(
+    const tracked_objects::Location& from_here,
+    const std::string& message) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  profile_sync_service_->OnUnrecoverableError(from_here, message);
 }
 
 void NonFrontendDataTypeController::DisableImpl(
@@ -341,6 +275,14 @@ void NonFrontendDataTypeController::RecordStartFailure(StartResult result) {
 #undef PER_DATA_TYPE_MACRO
 }
 
+bool NonFrontendDataTypeController::StartAssociationAsync() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(state(), ASSOCIATING);
+  return PostTaskOnBackendThread(
+      FROM_HERE,
+      base::Bind(&NonFrontendDataTypeController::StartAssociation, this));
+}
+
 ProfileSyncComponentsFactory*
     NonFrontendDataTypeController::profile_sync_factory() const {
   return profile_sync_factory_;
@@ -359,6 +301,7 @@ void NonFrontendDataTypeController::set_start_callback(
     const StartCallback& callback) {
   start_callback_ = callback;
 }
+
 void NonFrontendDataTypeController::set_state(State state) {
   state_ = state;
 }
@@ -381,4 +324,63 @@ void NonFrontendDataTypeController::set_change_processor(
   change_processor_.reset(change_processor);
 }
 
+void NonFrontendDataTypeController::StartAssociation() {
+  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(state_, ASSOCIATING);
+  {
+    base::AutoLock lock(abort_association_lock_);
+    if (abort_association_) {
+      abort_association_complete_.Signal();
+      return;
+    }
+    CreateSyncComponents();
+  }
+
+  if (!model_associator_->CryptoReadyIfNecessary()) {
+    StartFailed(NEEDS_CRYPTO, SyncError());
+    return;
+  }
+
+  bool sync_has_nodes = false;
+  if (!model_associator_->SyncModelHasUserCreatedNodes(&sync_has_nodes)) {
+    SyncError error(FROM_HERE, "Failed to load sync nodes", type());
+    StartFailed(UNRECOVERABLE_ERROR, error);
+    return;
+  }
+
+  base::TimeTicks start_time = base::TimeTicks::Now();
+  SyncError error;
+  error = model_associator_->AssociateModels();
+  // TODO(lipalani): crbug.com/122690 - handle abort.
+  RecordAssociationTime(base::TimeTicks::Now() - start_time);
+  if (error.IsSet()) {
+    StartFailed(ASSOCIATION_FAILED, error);
+    return;
+  }
+
+  profile_sync_service_->ActivateDataType(type(), model_safe_group(),
+                                          change_processor());
+  StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, RUNNING, SyncError());
+}
+
+bool NonFrontendDataTypeController::StopAssociationAsync() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(state(), STOPPING);
+  return PostTaskOnBackendThread(
+      FROM_HERE,
+      base::Bind(
+          &NonFrontendDataTypeController::StopAssociation, this));
+}
+
+void NonFrontendDataTypeController::StopAssociation() {
+  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (model_associator_.get()) {
+    SyncError error;  // Not used.
+    error = model_associator_->DisassociateModels();
+  }
+  model_associator_.reset();
+  change_processor_.reset();
+  datatype_stopped_.Signal();
+}
+
 }  // namespace browser_sync