Revert "[Sync] Move DataTypeStatusTable ownership into DataTypeManager."

This reverts commit fe2ae5fb.

BUG=368834,409965
TBR=zea,scottmg

Review URL: https://codereview.chromium.org/534733002

Cr-Commit-Position: refs/heads/master@{#292981}

chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc

-// Copyright 2014 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.
-
-#include "chrome/browser/sync/glue/history_delete_directives_data_type_controller.h"
-
-#include "chrome/browser/sync/glue/chrome_report_unrecoverable_error.h"
-#include "chrome/browser/sync/profile_sync_service.h"
-#include "content/public/browser/browser_thread.h"
-
-using content::BrowserThread;
-
-namespace browser_sync {
-
-HistoryDeleteDirectivesDataTypeController::
-    HistoryDeleteDirectivesDataTypeController(
-        sync_driver::SyncApiComponentFactory* factory,
-        ProfileSyncService* sync_service)
-    : sync_driver::UIDataTypeController(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
-          base::Bind(&ChromeReportUnrecoverableError),
-          syncer::HISTORY_DELETE_DIRECTIVES,
-          factory),
-      sync_service_(sync_service) {
-  sync_service_->AddObserver(this);
-}
-
-HistoryDeleteDirectivesDataTypeController::
-    ~HistoryDeleteDirectivesDataTypeController() {
-  sync_service_->RemoveObserver(this);
-}
-
-bool HistoryDeleteDirectivesDataTypeController::ReadyForStart() const {
-  return !sync_service_->EncryptEverythingEnabled();
-}
-
-void HistoryDeleteDirectivesDataTypeController::OnStateChanged() {
-  if ((state() != NOT_RUNNING || state() != STOPPING) &&
-      sync_service_->ShouldPushChanges() && !ReadyForStart()) {
-    syncer::SyncError error(
-        FROM_HERE,
-        syncer::SyncError::DATATYPE_POLICY_ERROR,
-        "Delete directives not supported with encryption.",
-        type());
-    OnSingleDataTypeUnrecoverableError(error);
-  }
-}
-
-}  // namespace browser_sync

chrome/browser/sync/glue/history_delete_directives_data_type_controller.h

-// Copyright 2014 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_HISTORY_DELETE_DIRECTIVES_DATA_TYPE_CONTROLLER_H_
-#define CHROME_BROWSER_SYNC_GLUE_HISTORY_DELETE_DIRECTIVES_DATA_TYPE_CONTROLLER_H_
-
-#include "chrome/browser/sync/glue/local_device_info_provider.h"
-#include "chrome/browser/sync/profile_sync_service_observer.h"
-#include "components/sync_driver/ui_data_type_controller.h"
-
-class Profile;
-class ProfileSyncService;
-
-namespace browser_sync {
-
-// A controller for delete directives, which cannot sync when full encryption
-// is enabled.
-class HistoryDeleteDirectivesDataTypeController
-    : public sync_driver::UIDataTypeController,
-      public ProfileSyncServiceObserver {
- public:
-  HistoryDeleteDirectivesDataTypeController(
-      sync_driver::SyncApiComponentFactory* factory,
-      ProfileSyncService* sync_service);
-
-  // DataTypeController override.
-  virtual bool ReadyForStart() const OVERRIDE;
-
-  // ProfileSyncServiceBaseObserver implementation.
-  virtual void OnStateChanged() OVERRIDE;
-
- private:
-  // Refcounted.
-  virtual ~HistoryDeleteDirectivesDataTypeController();
-
-  ProfileSyncService* sync_service_;
-
-  DISALLOW_COPY_AND_ASSIGN(HistoryDeleteDirectivesDataTypeController);
-};
-
-}  // namespace browser_sync
-
-#endif  // CHROME_BROWSER_SYNC_GLUE_HISTORY_DELETE_DIRECTIVES_DATA_TYPE_CONTROLLER_H_

chrome/browser/sync/profile_sync_components_factory.h

@@ -87,7 +87,8 @@ class ProfileSyncComponentsFactory
       const sync_driver::DataTypeController::TypeMap* controllers,
       const sync_driver::DataTypeEncryptionHandler* encryption_handler,
       browser_sync::SyncBackendHost* backend,
-      sync_driver::DataTypeManagerObserver* observer) = 0;
+      sync_driver::DataTypeManagerObserver* observer,
+      sync_driver::DataTypeStatusTable* data_type_status_table) = 0;
 
   // Creating this in the factory helps us mock it out in testing.
   virtual browser_sync::SyncBackendHost* CreateSyncBackendHost(

chrome/browser/sync/profile_sync_components_factory_impl.cc

@@ -27,7 +27,6 @@
 #include "chrome/browser/sync/glue/extension_backed_data_type_controller.h"
 #include "chrome/browser/sync/glue/extension_data_type_controller.h"
 #include "chrome/browser/sync/glue/extension_setting_data_type_controller.h"
-#include "chrome/browser/sync/glue/history_delete_directives_data_type_controller.h"
 #include "chrome/browser/sync/glue/local_device_info_provider_impl.h"
 #include "chrome/browser/sync/glue/password_data_type_controller.h"
 #include "chrome/browser/sync/glue/search_engine_data_type_controller.h"
@@ -107,7 +106,6 @@ using browser_sync::ChromeReportUnrecoverableError;
 using browser_sync::ExtensionBackedDataTypeController;
 using browser_sync::ExtensionDataTypeController;
 using browser_sync::ExtensionSettingDataTypeController;
-using browser_sync::HistoryDeleteDirectivesDataTypeController;
 using browser_sync::PasswordDataTypeController;
 using browser_sync::SearchEngineDataTypeController;
 using browser_sync::SessionDataTypeController;
@@ -217,10 +215,13 @@ void ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes(
 
   // Delete directive sync is enabled by default.  Register unless full history
   // sync is disabled.
-  if (!disabled_types.Has(syncer::HISTORY_DELETE_DIRECTIVES) &&
-      !history_disabled) {
+  if (!disabled_types.Has(syncer::HISTORY_DELETE_DIRECTIVES)) {
     pss->RegisterDataTypeController(
-        new HistoryDeleteDirectivesDataTypeController(this, pss));
+        new UIDataTypeController(
+            BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+            base::Bind(&ChromeReportUnrecoverableError),
+            syncer::HISTORY_DELETE_DIRECTIVES,
+            this));
   }
 
   // Session sync is enabled by default.  Register unless explicitly disabled.
@@ -239,8 +240,7 @@ void ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes(
 
   // Favicon sync is enabled by default. Register unless explicitly disabled.
   if (!disabled_types.Has(syncer::FAVICON_IMAGES) &&
-      !disabled_types.Has(syncer::FAVICON_TRACKING) &&
-      !history_disabled) {
+      !disabled_types.Has(syncer::FAVICON_TRACKING)) {
     // crbug/384552. We disable error uploading for this data types for now.
     pss->RegisterDataTypeController(
         new UIDataTypeController(
@@ -411,13 +411,15 @@ DataTypeManager* ProfileSyncComponentsFactoryImpl::CreateDataTypeManager(
     const DataTypeController::TypeMap* controllers,
     const sync_driver::DataTypeEncryptionHandler* encryption_handler,
     SyncBackendHost* backend,
-    DataTypeManagerObserver* observer) {
+    DataTypeManagerObserver* observer,
+    sync_driver::DataTypeStatusTable* data_type_status_table) {
   return new DataTypeManagerImpl(base::Bind(ChromeReportUnrecoverableError),
                                  debug_info_listener,
                                  controllers,
                                  encryption_handler,
                                  backend,
-                                 observer);
+                                 observer,
+                                 data_type_status_table);
 }
 
 browser_sync::SyncBackendHost*

chrome/browser/sync/profile_sync_components_factory_impl.h

@@ -50,7 +50,8 @@ class ProfileSyncComponentsFactoryImpl : public ProfileSyncComponentsFactory {
       const sync_driver::DataTypeController::TypeMap* controllers,
       const sync_driver::DataTypeEncryptionHandler* encryption_handler,
       browser_sync::SyncBackendHost* backend,
-      sync_driver::DataTypeManagerObserver* observer)
+      sync_driver::DataTypeManagerObserver* observer,
+      sync_driver::DataTypeStatusTable* data_type_status_table)
           OVERRIDE;
 
   virtual browser_sync::SyncBackendHost* CreateSyncBackendHost(

chrome/browser/sync/profile_sync_components_factory_mock.h

@@ -28,13 +28,15 @@ class ProfileSyncComponentsFactoryMock : public ProfileSyncComponentsFactory {
   virtual ~ProfileSyncComponentsFactoryMock();
 
   MOCK_METHOD1(RegisterDataTypes, void(ProfileSyncService*));
-  MOCK_METHOD5(CreateDataTypeManager,
+  MOCK_METHOD6(CreateDataTypeManager,
                sync_driver::DataTypeManager*(
                    const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>&,
                    const sync_driver::DataTypeController::TypeMap*,
                    const sync_driver::DataTypeEncryptionHandler*,
                    browser_sync::SyncBackendHost*,
-                   sync_driver::DataTypeManagerObserver* observer));
+                   sync_driver::DataTypeManagerObserver* observer,
+                   sync_driver::DataTypeStatusTable*
+                       data_type_status_table));
   MOCK_METHOD5(CreateSyncBackendHost,
                browser_sync::SyncBackendHost*(
                    const std::string& name,

chrome/browser/sync/profile_sync_service.cc

@@ -962,9 +962,7 @@ void ProfileSyncService::ClearStaleErrors() {
   ClearUnrecoverableError();
   last_actionable_error_ = SyncProtocolError();
   // Clear the data type errors as well.
-  if (directory_data_type_manager_.get())
-    directory_data_type_manager_->ResetDataTypeErrors();
-
+  data_type_status_table_.Reset();
 }
 
 void ProfileSyncService::ClearUnrecoverableError() {
@@ -1412,7 +1410,20 @@ void ProfileSyncService::OnEncryptedTypesChanged(
            << (encrypt_everything_ ? "true" : "false") << ")";
   DCHECK(encrypted_types_.Has(syncer::PASSWORDS));
 
-  NotifyObservers();
+  // If sessions are encrypted, full history sync is not possible, and
+  // delete directives are unnecessary.
+  if (GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES) &&
+      encrypted_types_.Has(syncer::SESSIONS)) {
+    syncer::SyncError error(
+        FROM_HERE,
+        syncer::SyncError::DATATYPE_POLICY_ERROR,
+        "Delete directives not supported with encryption.",
+        syncer::HISTORY_DELETE_DIRECTIVES);
+    DataTypeStatusTable::TypeErrorMap error_map;
+    error_map[error.model_type()] = error;
+    data_type_status_table_.UpdateFailedDataTypes(error_map);
+    ReconfigureDatatypeManager();
+  }
 }
 
 void ProfileSyncService::OnEncryptionComplete() {
@@ -1500,7 +1511,6 @@ void ProfileSyncService::OnActionableError(const SyncProtocolError& error) {
 void ProfileSyncService::OnConfigureDone(
     const DataTypeManager::ConfigureResult& result) {
   configure_status_ = result.status;
-  data_type_status_table_ = result.data_type_status_table;
 
   if (backend_mode_ != SYNC) {
     if (configure_status_ == DataTypeManager::OK) {
@@ -1830,8 +1840,18 @@ void ProfileSyncService::OnUserChoseDatatypes(
   UpdateSelectedTypesHistogram(sync_everything, chosen_types);
   sync_prefs_.SetKeepEverythingSynced(sync_everything);
 
-  if (directory_data_type_manager_.get())
-    directory_data_type_manager_->ResetDataTypeErrors();
+  data_type_status_table_.Reset();
+  if (GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES) &&
+      encrypted_types_.Has(syncer::SESSIONS)) {
+    syncer::SyncError error(
+        FROM_HERE,
+        syncer::SyncError::DATATYPE_POLICY_ERROR,
+        "Delete directives not supported with encryption.",
+        syncer::HISTORY_DELETE_DIRECTIVES);
+    DataTypeStatusTable::TypeErrorMap error_map;
+    error_map[error.model_type()] = error;
+    data_type_status_table_.UpdateFailedDataTypes(error_map);
+  }
   ChangePreferredDataTypes(chosen_types);
   AcknowledgeSyncedTypes();
   NotifyObservers();
@@ -1966,7 +1986,8 @@ void ProfileSyncService::ConfigureDataTypeManager() {
                                         &directory_data_type_controllers_,
                                         this,
                                         backend_.get(),
-                                        this));
+                                        this,
+                                        &data_type_status_table_));
 
     // We create the migrator at the same time.
     migrator_.reset(

chrome/browser/sync/profile_sync_service_autofill_unittest.cc

@@ -362,7 +362,8 @@ ACTION_P(ReturnNewDataTypeManagerWithDebugListener, debug_listener) {
       arg1,
       arg2,
       arg3,
-      arg4);
+      arg4,
+      arg5);
 }
 
 ACTION_P(MakeAutofillProfileSyncComponents, wds) {
@@ -556,7 +557,7 @@ class ProfileSyncServiceAutofillTest
                             web_data_service_.get(),
                             data_type_controller);
 
-    EXPECT_CALL(*components, CreateDataTypeManager(_, _, _, _, _)).
+    EXPECT_CALL(*components, CreateDataTypeManager(_, _, _, _, _, _)).
         WillOnce(ReturnNewDataTypeManagerWithDebugListener(
                      syncer::MakeWeakHandle(debug_ptr_factory_.GetWeakPtr())));
 

chrome/browser/sync/profile_sync_service_startup_unittest.cc

@@ -59,7 +59,7 @@ ACTION_P3(InvokeOnConfigureDone, pss, error_callback, result) {
   DataTypeManager::ConfigureResult configure_result =
       static_cast<DataTypeManager::ConfigureResult>(result);
   if (result.status == sync_driver::DataTypeManager::ABORTED)
-    error_callback.Run(&configure_result);
+    error_callback.Run();
   service->OnConfigureDone(configure_result);
 }
 
@@ -87,7 +87,8 @@ class ProfileSyncServiceStartupTest : public testing::Test {
                        content::TestBrowserThreadBundle::REAL_FILE_THREAD |
                        content::TestBrowserThreadBundle::REAL_IO_THREAD),
         profile_manager_(TestingBrowserProcess::GetGlobal()),
-        sync_(NULL) {}
+        sync_(NULL),
+        data_type_status_table_(NULL) {}
 
   virtual ~ProfileSyncServiceStartupTest() {
   }
@@ -147,14 +148,14 @@ class ProfileSyncServiceStartupTest : public testing::Test {
     return static_cast<FakeSigninManagerForTesting*>(sync_->signin());
   }
 
-  void SetError(DataTypeManager::ConfigureResult* result) {
+  void SetError() {
     sync_driver::DataTypeStatusTable::TypeErrorMap errors;
     errors[syncer::BOOKMARKS] =
         syncer::SyncError(FROM_HERE,
                           syncer::SyncError::UNRECOVERABLE_ERROR,
                           "Error",
                           syncer::BOOKMARKS);
-    result->data_type_status_table.UpdateFailedDataTypes(errors);
+    data_type_status_table_->UpdateFailedDataTypes(errors);
   }
 
  protected:
@@ -174,8 +175,9 @@ class ProfileSyncServiceStartupTest : public testing::Test {
   DataTypeManagerMock* SetUpDataTypeManager() {
     DataTypeManagerMock* data_type_manager = new DataTypeManagerMock();
     EXPECT_CALL(*components_factory_mock(),
-                CreateDataTypeManager(_, _, _, _, _)).
-        WillOnce(Return(data_type_manager));
+                CreateDataTypeManager(_, _, _, _, _, _)).
+        WillOnce(DoAll(SaveArg<5>(&data_type_status_table_),
+                       Return(data_type_manager)));
     return data_type_manager;
   }
 
@@ -193,7 +195,7 @@ class ProfileSyncServiceStartupTest : public testing::Test {
   TestingProfile* profile_;
   ProfileSyncService* sync_;
   ProfileSyncServiceObserverMock observer_;
-  sync_driver::DataTypeStatusTable data_type_status_table_;
+  sync_driver::DataTypeStatusTable* data_type_status_table_;
 };
 
 class ProfileSyncServiceStartupCrosTest : public ProfileSyncServiceStartupTest {
@@ -277,7 +279,7 @@ TEST_F(ProfileSyncServiceStartupTest, DISABLED_StartNoCredentials) {
   // Should not actually start, rather just clean things up and wait
   // to be enabled.
   EXPECT_CALL(*components_factory_mock(),
-              CreateDataTypeManager(_, _, _, _, _)).Times(0);
+              CreateDataTypeManager(_, _, _, _, _, _)).Times(0);
   EXPECT_CALL(observer_, OnStateChanged()).Times(AnyNumber());
   sync_->Initialize();
 
@@ -351,7 +353,7 @@ TEST_F(ProfileSyncServiceStartupTest, DISABLED_StartInvalidCredentials) {
 #endif
 TEST_F(ProfileSyncServiceStartupCrosTest, MAYBE_StartCrosNoCredentials) {
   EXPECT_CALL(*components_factory_mock(),
-              CreateDataTypeManager(_, _, _, _, _)).Times(0);
+              CreateDataTypeManager(_, _, _, _, _, _)).Times(0);
   EXPECT_CALL(*components_factory_mock(),
               CreateSyncBackendHost(_, _, _, _, _)).Times(0);
   profile_->GetPrefs()->ClearPref(sync_driver::prefs::kSyncHasSetupCompleted);
@@ -494,7 +496,7 @@ TEST_F(ProfileSyncServiceStartupTest, MAYBE_ManagedStartup) {
   // Disable sync through policy.
   profile_->GetPrefs()->SetBoolean(sync_driver::prefs::kSyncManaged, true);
   EXPECT_CALL(*components_factory_mock(),
-              CreateDataTypeManager(_, _, _, _, _)).Times(0);
+              CreateDataTypeManager(_, _, _, _, _, _)).Times(0);
   EXPECT_CALL(observer_, OnStateChanged()).Times(AnyNumber());
 
   sync_->Initialize();
@@ -526,7 +528,7 @@ TEST_F(ProfileSyncServiceStartupTest, SwitchManaged) {
   // should not start up automatically (kSyncSetupCompleted will be false).
   Mock::VerifyAndClearExpectations(data_type_manager);
   EXPECT_CALL(*components_factory_mock(),
-              CreateDataTypeManager(_, _, _, _, _)).Times(0);
+              CreateDataTypeManager(_, _, _, _, _, _)).Times(0);
   EXPECT_CALL(observer_, OnStateChanged()).Times(AnyNumber());
   profile_->GetPrefs()->ClearPref(sync_driver::prefs::kSyncManaged);
 }

chrome/browser/sync/profile_sync_service_typed_url_unittest.cc

@@ -273,7 +273,7 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest {
                                               data_type_controller,
                                               &error_handler_,
                                               &model_associator));
-      EXPECT_CALL(*components, CreateDataTypeManager(_, _, _, _, _)).
+      EXPECT_CALL(*components, CreateDataTypeManager(_, _, _, _, _, _)).
           WillOnce(ReturnNewDataTypeManager());
 
       ProfileOAuth2TokenService* oauth2_token_service =

chrome/browser/sync/profile_sync_service_unittest.cc

@@ -58,7 +58,6 @@ class FakeDataTypeManager : public sync_driver::DataTypeManager {
   }
 
   virtual void ReenableType(syncer::ModelType type) OVERRIDE {}
-  virtual void ResetDataTypeErrors() OVERRIDE {}
   virtual void PurgeForMigration(syncer::ModelTypeSet undesired_types,
                                  syncer::ConfigureReason reason) OVERRIDE {}
   virtual void Stop() OVERRIDE {};
@@ -252,7 +251,7 @@ class ProfileSyncServiceTest : public ::testing::Test {
   }
 
   void ExpectDataTypeManagerCreation(int times) {
-    EXPECT_CALL(*components_factory_, CreateDataTypeManager(_, _, _, _, _))
+    EXPECT_CALL(*components_factory_, CreateDataTypeManager(_, _, _, _, _, _))
         .Times(times)
         .WillRepeatedly(ReturnNewDataTypeManager());
   }

chrome/browser/sync/test/integration/sync_errors_test.cc

@@ -37,36 +37,12 @@ class SyncDisabledChecker : public SingleClientStatusChangeChecker {
   }
 };
 
-class TypeDisabledChecker : public SingleClientStatusChangeChecker {
- public:
-  explicit TypeDisabledChecker(ProfileSyncService* service,
-                               syncer::ModelType type)
-      : SingleClientStatusChangeChecker(service), type_(type) {}
-
-  virtual bool IsExitConditionSatisfied() OVERRIDE {
-    return !service()->GetActiveDataTypes().Has(type_);
-  }
-
-  virtual std::string GetDebugMessage() const OVERRIDE {
-    return "Type disabled";
-  }
- private:
-   syncer::ModelType type_;
-};
-
 bool AwaitSyncDisabled(ProfileSyncService* service) {
   SyncDisabledChecker checker(service);
   checker.Wait();
   return !checker.TimedOut();
 }
 
-bool AwaitTypeDisabled(ProfileSyncService* service,
-                       syncer::ModelType type) {
-  TypeDisabledChecker checker(service, type);
-  checker.Wait();
-  return !checker.TimedOut();
-}
-
 class SyncErrorTest : public SyncTest {
  public:
   SyncErrorTest() : SyncTest(SINGLE_CLIENT) {}
@@ -227,9 +203,12 @@ IN_PROC_BROWSER_TEST_F(LegacySyncErrorTest, DisableDatatypeWhileRunning) {
   GetProfile(0)->GetPrefs()->SetBoolean(
       prefs::kSavingBrowserHistoryDisabled, true);
 
-  // Wait for reconfigurations.
-  ASSERT_TRUE(AwaitTypeDisabled(GetSyncService(0), syncer::TYPED_URLS));
-  ASSERT_TRUE(AwaitTypeDisabled(GetSyncService(0), syncer::SESSIONS));
+  // Flush any tasks posted by observers of the pref change.
+  base::RunLoop().RunUntilIdle();
+
+  synced_datatypes = GetSyncService((0))->GetActiveDataTypes();
+  ASSERT_FALSE(synced_datatypes.Has(syncer::TYPED_URLS));
+  ASSERT_FALSE(synced_datatypes.Has(syncer::SESSIONS));
 
   const BookmarkNode* node1 = AddFolder(0, 0, "title1");
   SetTitle(0, node1, "new_title1");

chrome/browser/sync/test_profile_sync_service.h

@@ -29,7 +29,8 @@ ACTION(ReturnNewDataTypeManager) {
                                               arg1,
                                               arg2,
                                               arg3,
-                                              arg4);
+                                              arg4,
+                                              arg5);
 }
 
 namespace browser_sync {

chrome/chrome_browser.gypi

@@ -1265,8 +1265,6 @@
       'browser/sync/glue/favicon_cache.h',
       'browser/sync/glue/frontend_data_type_controller.cc',
       'browser/sync/glue/frontend_data_type_controller.h',
-      'browser/sync/glue/history_delete_directives_data_type_controller.cc',
-      'browser/sync/glue/history_delete_directives_data_type_controller.h',
       'browser/sync/glue/history_model_worker.cc',
       'browser/sync/glue/history_model_worker.h',
       'browser/sync/glue/invalidation_adapter.cc',

components/sync_driver/data_type_manager.h

@@ -10,7 +10,6 @@
 #include <string>
 
 #include "components/sync_driver/data_type_controller.h"
-#include "components/sync_driver/data_type_status_table.h"
 #include "sync/api/sync_error.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/configure_reason.h"
@@ -53,7 +52,6 @@ class DataTypeManager {
 
     ConfigureStatus status;
     syncer::ModelTypeSet requested_types;
-    DataTypeStatusTable data_type_status_table;
   };
 
   virtual ~DataTypeManager() {}
@@ -80,9 +78,6 @@ class DataTypeManager {
   // necessary.
   virtual void ReenableType(syncer::ModelType type) = 0;
 
-  // Resets all data type error state.
-  virtual void ResetDataTypeErrors() = 0;
-
   virtual void PurgeForMigration(syncer::ModelTypeSet undesired_types,
                                  syncer::ConfigureReason reason) = 0;
 

components/sync_driver/data_type_manager_impl.cc

@@ -51,7 +51,8 @@ DataTypeManagerImpl::DataTypeManagerImpl(
     const DataTypeController::TypeMap* controllers,
     const DataTypeEncryptionHandler* encryption_handler,
     BackendDataTypeConfigurer* configurer,
-    DataTypeManagerObserver* observer)
+    DataTypeManagerObserver* observer,
+    DataTypeStatusTable* data_type_status_table)
     : configurer_(configurer),
       controllers_(controllers),
       state_(DataTypeManager::STOPPED),
@@ -60,9 +61,11 @@ DataTypeManagerImpl::DataTypeManagerImpl(
       debug_info_listener_(debug_info_listener),
       model_association_manager_(controllers, this),
       observer_(observer),
+      data_type_status_table_(data_type_status_table),
       encryption_handler_(encryption_handler),
       unrecoverable_error_method_(unrecoverable_error_method),
       weak_ptr_factory_(this) {
+  DCHECK(data_type_status_table_);
   DCHECK(configurer_);
   DCHECK(observer_);
 }
@@ -86,7 +89,7 @@ void DataTypeManagerImpl::Configure(syncer::ModelTypeSet desired_types,
         iter != controllers_->end()) {
       if (iter != controllers_->end()) {
         if (!iter->second->ReadyForStart() &&
-            !data_type_status_table_.GetUnreadyErrorTypes().Has(
+            !data_type_status_table_->GetUnreadyErrorTypes().Has(
                 type.Get())) {
           // Add the type to the unready types set to prevent purging it. It's
           // up to the datatype controller to, if necessary, explicitly
@@ -97,9 +100,9 @@ void DataTypeManagerImpl::Configure(syncer::ModelTypeSet desired_types,
                                   type.Get());
           std::map<syncer::ModelType, syncer::SyncError> errors;
           errors[type.Get()] = error;
-          data_type_status_table_.UpdateFailedDataTypes(errors);
+          data_type_status_table_->UpdateFailedDataTypes(errors);
         } else if (iter->second->ReadyForStart()) {
-          data_type_status_table_.ResetUnreadyErrorFor(type.Get());
+          data_type_status_table_->ResetUnreadyErrorFor(type.Get());
         }
       }
       filtered_desired_types.Put(type.Get());
@@ -112,8 +115,8 @@ void DataTypeManagerImpl::ReenableType(syncer::ModelType type) {
   // TODO(zea): move the "should we reconfigure" logic into the datatype handler
   // itself.
   // Only reconfigure if the type actually had a data type or unready error.
-  if (!data_type_status_table_.ResetDataTypeErrorFor(type) &&
-      !data_type_status_table_.ResetUnreadyErrorFor(type)) {
+  if (!data_type_status_table_->ResetDataTypeErrorFor(type) &&
+      !data_type_status_table_->ResetUnreadyErrorFor(type)) {
     return;
   }
 
@@ -127,11 +130,6 @@ void DataTypeManagerImpl::ReenableType(syncer::ModelType type) {
   ProcessReconfigure();
 }
 
-void DataTypeManagerImpl::ResetDataTypeErrors() {
-  DCHECK_EQ(state_, CONFIGURED);
-  data_type_status_table_.Reset();
-}
-
 void DataTypeManagerImpl::PurgeForMigration(
     syncer::ModelTypeSet undesired_types,
     syncer::ConfigureReason reason) {
@@ -179,18 +177,18 @@ DataTypeManagerImpl::BuildDataTypeConfigStateMap(
   // 4. Set non-enabled user types as DISABLED.
   // 5. Set the fatal, crypto, and unready types to their respective states.
   syncer::ModelTypeSet error_types =
-      data_type_status_table_.GetFailedTypes();
+      data_type_status_table_->GetFailedTypes();
   syncer::ModelTypeSet fatal_types =
-      data_type_status_table_.GetFatalErrorTypes();
+      data_type_status_table_->GetFatalErrorTypes();
   syncer::ModelTypeSet crypto_types =
-      data_type_status_table_.GetCryptoErrorTypes();
+      data_type_status_table_->GetCryptoErrorTypes();
   syncer::ModelTypeSet unready_types=
-      data_type_status_table_.GetUnreadyErrorTypes();
+      data_type_status_table_->GetUnreadyErrorTypes();
 
   // Types with persistence errors are only purged/resynced when they're
   // actively being configured.
   syncer::ModelTypeSet persistence_types =
-      data_type_status_table_.GetPersistenceErrorTypes();
+      data_type_status_table_->GetPersistenceErrorTypes();
   persistence_types.RetainAll(types_being_configured);
 
   // Types with unready errors do not count as unready if they've been disabled.
@@ -242,16 +240,16 @@ void DataTypeManagerImpl::Restart(syncer::ConfigureReason reason) {
         encryption_handler_->GetEncryptedDataTypes();
     encrypted_types.RetainAll(last_requested_types_);
     encrypted_types.RemoveAll(
-        data_type_status_table_.GetCryptoErrorTypes());
+        data_type_status_table_->GetCryptoErrorTypes());
     DataTypeStatusTable::TypeErrorMap crypto_errors =
         GenerateCryptoErrorsForTypes(encrypted_types);
-    data_type_status_table_.UpdateFailedDataTypes(crypto_errors);
+    data_type_status_table_->UpdateFailedDataTypes(crypto_errors);
   } else {
-    data_type_status_table_.ResetCryptoErrors();
+    data_type_status_table_->ResetCryptoErrors();
   }
 
   syncer::ModelTypeSet failed_types =
-      data_type_status_table_.GetFailedTypes();
+      data_type_status_table_->GetFailedTypes();
   syncer::ModelTypeSet enabled_types =
       syncer::Difference(last_requested_types_, failed_types);
 
@@ -355,7 +353,7 @@ void DataTypeManagerImpl::DownloadReady(
 
   // Persistence errors are reset after each backend configuration attempt
   // during which they would have been purged.
-  data_type_status_table_.ResetPersistenceErrorsFrom(types_to_download);
+  data_type_status_table_->ResetPersistenceErrorsFrom(types_to_download);
 
   // Ignore |failed_configuration_types| if we need to reconfigure
   // anyway.
@@ -378,7 +376,7 @@ void DataTypeManagerImpl::DownloadReady(
           iter.Get());
       errors[iter.Get()] = error;
     }
-    data_type_status_table_.UpdateFailedDataTypes(errors);
+    data_type_status_table_->UpdateFailedDataTypes(errors);
     Abort(UNRECOVERABLE_ERROR);
     return;
   }
@@ -433,7 +431,7 @@ void DataTypeManagerImpl::OnSingleDataTypeWillStop(
   if (error.IsSet()) {
     DataTypeStatusTable::TypeErrorMap failed_types;
     failed_types[type] = error;
-    data_type_status_table_.UpdateFailedDataTypes(
+    data_type_status_table_->UpdateFailedDataTypes(
             failed_types);
 
     // Unrecoverable errors will shut down the entire backend, so no need to
@@ -559,12 +557,9 @@ void DataTypeManagerImpl::NotifyStart() {
   observer_->OnConfigureStart();
 }
 
-void DataTypeManagerImpl::NotifyDone(const ConfigureResult& raw_result) {
+void DataTypeManagerImpl::NotifyDone(const ConfigureResult& result) {
   AddToConfigureTime();
 
-  ConfigureResult result = raw_result;
-  result.data_type_status_table = data_type_status_table_;
-
   DVLOG(1) << "Total time spent configuring: "
            << configure_time_delta_.InSecondsF() << "s";
   switch (result.status) {

components/sync_driver/data_type_manager_impl.h

@@ -30,6 +30,7 @@ namespace sync_driver {
 class DataTypeController;
 class DataTypeEncryptionHandler;
 class DataTypeManagerObserver;
+class DataTypeStatusTable;
 
 // List of data types grouped by priority and ordered from high priority to
 // low priority.
@@ -45,14 +46,14 @@ class DataTypeManagerImpl : public DataTypeManager,
       const DataTypeController::TypeMap* controllers,
       const DataTypeEncryptionHandler* encryption_handler,
       BackendDataTypeConfigurer* configurer,
-      DataTypeManagerObserver* observer);
+      DataTypeManagerObserver* observer,
+      DataTypeStatusTable* data_type_status_table);
   virtual ~DataTypeManagerImpl();
 
   // DataTypeManager interface.
   virtual void Configure(syncer::ModelTypeSet desired_types,
                          syncer::ConfigureReason reason) OVERRIDE;
   virtual void ReenableType(syncer::ModelType type) OVERRIDE;
-  virtual void ResetDataTypeErrors() OVERRIDE;
 
   // Needed only for backend migration.
   virtual void PurgeForMigration(
@@ -162,7 +163,7 @@ class DataTypeManagerImpl : public DataTypeManager,
 
   // For querying failed data types (having unrecoverable error) when
   // configuring backend.
-  DataTypeStatusTable data_type_status_table_;
+  DataTypeStatusTable* data_type_status_table_;
 
   // Types waiting to be downloaded.
   TypeSetPriorityList download_types_queue_;

components/sync_driver/data_type_manager_mock.h

@@ -18,7 +18,6 @@ class DataTypeManagerMock : public DataTypeManager {
 
   MOCK_METHOD2(Configure, void(syncer::ModelTypeSet, syncer::ConfigureReason));
   MOCK_METHOD1(ReenableType, void(syncer::ModelType));
-  MOCK_METHOD0(ResetDataTypeErrors, void());
   MOCK_METHOD2(PurgeForMigration, void(syncer::ModelTypeSet,
                                        syncer::ConfigureReason));
   MOCK_METHOD0(Stop, void());

components/sync_driver/data_type_status_table.h

@@ -16,7 +16,7 @@ class DataTypeStatusTable {
  public:
   typedef std::map<syncer::ModelType, syncer::SyncError> TypeErrorMap;
 
-  DataTypeStatusTable();
+  explicit DataTypeStatusTable();
   ~DataTypeStatusTable();
 
   // Copy and assign welcome.

components/sync_driver/fake_data_type_controller.cc

@@ -80,15 +80,13 @@ void FakeDataTypeController::FinishStart(ConfigureResult result) {
                           syncer::SyncError::UNRECOVERABLE_ERROR,
                           "Unrecoverable error",
                           type()));
-  } else if (result == NEEDS_CRYPTO) {
+  } else {
     state_ = NOT_RUNNING;
     local_merge_result.set_error(
         syncer::SyncError(FROM_HERE,
-                          syncer::SyncError::CRYPTO_ERROR,
-                          "Crypto error",
+                          syncer::SyncError::DATATYPE_ERROR,
+                          "Fake error",
                           type()));
-  } else {
-    NOTREACHED();
   }
   last_start_callback_.Run(result, local_merge_result, syncer_merge_result);
 }