diff options
| author | zea <zea@chromium.org> | 2014-09-02 11:30:33 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2014-09-02 18:36:44 +0000 |
| commit | fe2ae5fbf23243039bdc94f8f8672bc371f5339f (patch) | |
| tree | e6fab663dfc01928e8a7ebf26a0626645e40936e | |
| parent | bf2cd357fc230d28ccb9708097cc8a16aebe3e71 (diff) | |
| download | chromium_src-fe2ae5fbf23243039bdc94f8f8672bc371f5339f.zip chromium_src-fe2ae5fbf23243039bdc94f8f8672bc371f5339f.tar.gz chromium_src-fe2ae5fbf23243039bdc94f8f8672bc371f5339f.tar.bz2 | |
[Sync] Move DataTypeStatusTable ownership into DataTypeManager.
The DataTypeManager now maintains its own status table, which it posts a copy
of on each configuration completion. This makes testing configuration results
easier as we can now just check the type status table, and makes the DTM
more self contained.
To make this work a HistoryDeleteDirectives datatype controller was added to
encapsulate the encryption dependency the type has. Additionally, the PSS
is now able to tell the DTM to reset type errors (e.g. when the user is attempting
to re-configure with or without certain types).
BUG=368834
Review URL: https://codereview.chromium.org/523043005
Cr-Commit-Position: refs/heads/master@{#292962}
21 files changed, 408 insertions, 209 deletions
diff --git a/chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc b/chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc new file mode 100644 index 0000000..3a90985 --- /dev/null +++ b/chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc @@ -0,0 +1,49 @@ +// 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 diff --git a/chrome/browser/sync/glue/history_delete_directives_data_type_controller.h b/chrome/browser/sync/glue/history_delete_directives_data_type_controller.h new file mode 100644 index 0000000..7f956ad --- /dev/null +++ b/chrome/browser/sync/glue/history_delete_directives_data_type_controller.h @@ -0,0 +1,44 @@ +// 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_ diff --git a/chrome/browser/sync/profile_sync_components_factory.h b/chrome/browser/sync/profile_sync_components_factory.h index c55c301f..8c77e41 100644 --- a/chrome/browser/sync/profile_sync_components_factory.h +++ b/chrome/browser/sync/profile_sync_components_factory.h @@ -87,8 +87,7 @@ class ProfileSyncComponentsFactory const sync_driver::DataTypeController::TypeMap* controllers, const sync_driver::DataTypeEncryptionHandler* encryption_handler, browser_sync::SyncBackendHost* backend, - sync_driver::DataTypeManagerObserver* observer, - sync_driver::DataTypeStatusTable* data_type_status_table) = 0; + sync_driver::DataTypeManagerObserver* observer) = 0; // Creating this in the factory helps us mock it out in testing. virtual browser_sync::SyncBackendHost* CreateSyncBackendHost( diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.cc b/chrome/browser/sync/profile_sync_components_factory_impl.cc index b0881df..52390b0 100644 --- a/chrome/browser/sync/profile_sync_components_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_components_factory_impl.cc @@ -27,6 +27,7 @@ #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" @@ -106,6 +107,7 @@ 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; @@ -215,13 +217,10 @@ 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)) { + if (!disabled_types.Has(syncer::HISTORY_DELETE_DIRECTIVES) && + !history_disabled) { pss->RegisterDataTypeController( - new UIDataTypeController( - BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI), - base::Bind(&ChromeReportUnrecoverableError), - syncer::HISTORY_DELETE_DIRECTIVES, - this)); + new HistoryDeleteDirectivesDataTypeController(this, pss)); } // Session sync is enabled by default. Register unless explicitly disabled. @@ -240,7 +239,8 @@ 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)) { + !disabled_types.Has(syncer::FAVICON_TRACKING) && + !history_disabled) { // crbug/384552. We disable error uploading for this data types for now. pss->RegisterDataTypeController( new UIDataTypeController( @@ -411,15 +411,13 @@ DataTypeManager* ProfileSyncComponentsFactoryImpl::CreateDataTypeManager( const DataTypeController::TypeMap* controllers, const sync_driver::DataTypeEncryptionHandler* encryption_handler, SyncBackendHost* backend, - DataTypeManagerObserver* observer, - sync_driver::DataTypeStatusTable* data_type_status_table) { + DataTypeManagerObserver* observer) { return new DataTypeManagerImpl(base::Bind(ChromeReportUnrecoverableError), debug_info_listener, controllers, encryption_handler, backend, - observer, - data_type_status_table); + observer); } browser_sync::SyncBackendHost* diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.h b/chrome/browser/sync/profile_sync_components_factory_impl.h index 8604805..5ccd6c26 100644 --- a/chrome/browser/sync/profile_sync_components_factory_impl.h +++ b/chrome/browser/sync/profile_sync_components_factory_impl.h @@ -50,8 +50,7 @@ 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::DataTypeStatusTable* data_type_status_table) + sync_driver::DataTypeManagerObserver* observer) OVERRIDE; virtual browser_sync::SyncBackendHost* CreateSyncBackendHost( diff --git a/chrome/browser/sync/profile_sync_components_factory_mock.h b/chrome/browser/sync/profile_sync_components_factory_mock.h index 358e4c9..6dc7161 100644 --- a/chrome/browser/sync/profile_sync_components_factory_mock.h +++ b/chrome/browser/sync/profile_sync_components_factory_mock.h @@ -28,15 +28,13 @@ class ProfileSyncComponentsFactoryMock : public ProfileSyncComponentsFactory { virtual ~ProfileSyncComponentsFactoryMock(); MOCK_METHOD1(RegisterDataTypes, void(ProfileSyncService*)); - MOCK_METHOD6(CreateDataTypeManager, + MOCK_METHOD5(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::DataTypeStatusTable* - data_type_status_table)); + sync_driver::DataTypeManagerObserver* observer)); MOCK_METHOD5(CreateSyncBackendHost, browser_sync::SyncBackendHost*( const std::string& name, diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 1adc368..6375c34 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -962,7 +962,9 @@ void ProfileSyncService::ClearStaleErrors() { ClearUnrecoverableError(); last_actionable_error_ = SyncProtocolError(); // Clear the data type errors as well. - data_type_status_table_.Reset(); + if (directory_data_type_manager_.get()) + directory_data_type_manager_->ResetDataTypeErrors(); + } void ProfileSyncService::ClearUnrecoverableError() { @@ -1410,20 +1412,7 @@ void ProfileSyncService::OnEncryptedTypesChanged( << (encrypt_everything_ ? "true" : "false") << ")"; DCHECK(encrypted_types_.Has(syncer::PASSWORDS)); - // 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(); - } + NotifyObservers(); } void ProfileSyncService::OnEncryptionComplete() { @@ -1511,6 +1500,7 @@ 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) { @@ -1840,18 +1830,8 @@ void ProfileSyncService::OnUserChoseDatatypes( UpdateSelectedTypesHistogram(sync_everything, chosen_types); sync_prefs_.SetKeepEverythingSynced(sync_everything); - 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); - } + if (directory_data_type_manager_.get()) + directory_data_type_manager_->ResetDataTypeErrors(); ChangePreferredDataTypes(chosen_types); AcknowledgeSyncedTypes(); NotifyObservers(); @@ -1986,8 +1966,7 @@ void ProfileSyncService::ConfigureDataTypeManager() { &directory_data_type_controllers_, this, backend_.get(), - this, - &data_type_status_table_)); + this)); // We create the migrator at the same time. migrator_.reset( diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc index 8c7cddf..10fb761 100644 --- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc @@ -362,8 +362,7 @@ ACTION_P(ReturnNewDataTypeManagerWithDebugListener, debug_listener) { arg1, arg2, arg3, - arg4, - arg5); + arg4); } ACTION_P(MakeAutofillProfileSyncComponents, wds) { @@ -557,7 +556,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()))); diff --git a/chrome/browser/sync/profile_sync_service_startup_unittest.cc b/chrome/browser/sync/profile_sync_service_startup_unittest.cc index d18f051..79354aa 100644 --- a/chrome/browser/sync/profile_sync_service_startup_unittest.cc +++ b/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(); + error_callback.Run(&configure_result); service->OnConfigureDone(configure_result); } @@ -87,8 +87,7 @@ class ProfileSyncServiceStartupTest : public testing::Test { content::TestBrowserThreadBundle::REAL_FILE_THREAD | content::TestBrowserThreadBundle::REAL_IO_THREAD), profile_manager_(TestingBrowserProcess::GetGlobal()), - sync_(NULL), - data_type_status_table_(NULL) {} + sync_(NULL) {} virtual ~ProfileSyncServiceStartupTest() { } @@ -148,14 +147,14 @@ class ProfileSyncServiceStartupTest : public testing::Test { return static_cast<FakeSigninManagerForTesting*>(sync_->signin()); } - void SetError() { + void SetError(DataTypeManager::ConfigureResult* result) { sync_driver::DataTypeStatusTable::TypeErrorMap errors; errors[syncer::BOOKMARKS] = syncer::SyncError(FROM_HERE, syncer::SyncError::UNRECOVERABLE_ERROR, "Error", syncer::BOOKMARKS); - data_type_status_table_->UpdateFailedDataTypes(errors); + result->data_type_status_table.UpdateFailedDataTypes(errors); } protected: @@ -175,9 +174,8 @@ class ProfileSyncServiceStartupTest : public testing::Test { DataTypeManagerMock* SetUpDataTypeManager() { DataTypeManagerMock* data_type_manager = new DataTypeManagerMock(); EXPECT_CALL(*components_factory_mock(), - CreateDataTypeManager(_, _, _, _, _, _)). - WillOnce(DoAll(SaveArg<5>(&data_type_status_table_), - Return(data_type_manager))); + CreateDataTypeManager(_, _, _, _, _)). + WillOnce(Return(data_type_manager)); return data_type_manager; } @@ -195,7 +193,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 { @@ -279,7 +277,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(); @@ -353,7 +351,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); @@ -496,7 +494,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(); @@ -528,7 +526,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); } diff --git a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc index adc2aa9..1a2e241 100644 --- a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc +++ b/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 = diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc index 8093358..accbcb4 100644 --- a/chrome/browser/sync/profile_sync_service_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_unittest.cc @@ -58,6 +58,7 @@ 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 {}; @@ -251,7 +252,7 @@ class ProfileSyncServiceTest : public ::testing::Test { } void ExpectDataTypeManagerCreation(int times) { - EXPECT_CALL(*components_factory_, CreateDataTypeManager(_, _, _, _, _, _)) + EXPECT_CALL(*components_factory_, CreateDataTypeManager(_, _, _, _, _)) .Times(times) .WillRepeatedly(ReturnNewDataTypeManager()); } diff --git a/chrome/browser/sync/test/integration/sync_errors_test.cc b/chrome/browser/sync/test/integration/sync_errors_test.cc index 48fbf46..6fe399f 100644 --- a/chrome/browser/sync/test/integration/sync_errors_test.cc +++ b/chrome/browser/sync/test/integration/sync_errors_test.cc @@ -37,12 +37,36 @@ 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) {} @@ -203,12 +227,9 @@ IN_PROC_BROWSER_TEST_F(LegacySyncErrorTest, DisableDatatypeWhileRunning) { GetProfile(0)->GetPrefs()->SetBoolean( prefs::kSavingBrowserHistoryDisabled, true); - // 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)); + // Wait for reconfigurations. + ASSERT_TRUE(AwaitTypeDisabled(GetSyncService(0), syncer::TYPED_URLS)); + ASSERT_TRUE(AwaitTypeDisabled(GetSyncService(0), syncer::SESSIONS)); const BookmarkNode* node1 = AddFolder(0, 0, "title1"); SetTitle(0, node1, "new_title1"); diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h index 529bbfc..b683f56 100644 --- a/chrome/browser/sync/test_profile_sync_service.h +++ b/chrome/browser/sync/test_profile_sync_service.h @@ -29,8 +29,7 @@ ACTION(ReturnNewDataTypeManager) { arg1, arg2, arg3, - arg4, - arg5); + arg4); } namespace browser_sync { diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index c4cff6f..cc38eab 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1265,6 +1265,8 @@ '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', diff --git a/components/sync_driver/data_type_manager.h b/components/sync_driver/data_type_manager.h index e4657b6..9c65af5 100644 --- a/components/sync_driver/data_type_manager.h +++ b/components/sync_driver/data_type_manager.h @@ -10,6 +10,7 @@ #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" @@ -52,6 +53,7 @@ class DataTypeManager { ConfigureStatus status; syncer::ModelTypeSet requested_types; + DataTypeStatusTable data_type_status_table; }; virtual ~DataTypeManager() {} @@ -78,6 +80,9 @@ 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; diff --git a/components/sync_driver/data_type_manager_impl.cc b/components/sync_driver/data_type_manager_impl.cc index 856722e..092f362 100644 --- a/components/sync_driver/data_type_manager_impl.cc +++ b/components/sync_driver/data_type_manager_impl.cc @@ -51,8 +51,7 @@ DataTypeManagerImpl::DataTypeManagerImpl( const DataTypeController::TypeMap* controllers, const DataTypeEncryptionHandler* encryption_handler, BackendDataTypeConfigurer* configurer, - DataTypeManagerObserver* observer, - DataTypeStatusTable* data_type_status_table) + DataTypeManagerObserver* observer) : configurer_(configurer), controllers_(controllers), state_(DataTypeManager::STOPPED), @@ -61,11 +60,9 @@ 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_); } @@ -89,7 +86,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 @@ -100,9 +97,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()); @@ -115,8 +112,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; } @@ -130,6 +127,11 @@ 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) { @@ -177,18 +179,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. @@ -240,16 +242,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); @@ -353,7 +355,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. @@ -376,7 +378,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; } @@ -431,7 +433,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 @@ -557,9 +559,12 @@ void DataTypeManagerImpl::NotifyStart() { observer_->OnConfigureStart(); } -void DataTypeManagerImpl::NotifyDone(const ConfigureResult& result) { +void DataTypeManagerImpl::NotifyDone(const ConfigureResult& raw_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) { diff --git a/components/sync_driver/data_type_manager_impl.h b/components/sync_driver/data_type_manager_impl.h index abd7fbc..7d0b101 100644 --- a/components/sync_driver/data_type_manager_impl.h +++ b/components/sync_driver/data_type_manager_impl.h @@ -30,7 +30,6 @@ 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. @@ -46,14 +45,14 @@ class DataTypeManagerImpl : public DataTypeManager, const DataTypeController::TypeMap* controllers, const DataTypeEncryptionHandler* encryption_handler, BackendDataTypeConfigurer* configurer, - DataTypeManagerObserver* observer, - DataTypeStatusTable* data_type_status_table); + DataTypeManagerObserver* observer); 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( @@ -163,7 +162,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_; diff --git a/components/sync_driver/data_type_manager_impl_unittest.cc b/components/sync_driver/data_type_manager_impl_unittest.cc index 054e267..c4d2ec2 100644 --- a/components/sync_driver/data_type_manager_impl_unittest.cc +++ b/components/sync_driver/data_type_manager_impl_unittest.cc @@ -14,11 +14,11 @@ #include "components/sync_driver/fake_data_type_controller.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/configure_reason.h" -#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace sync_driver { +using syncer::SyncError; using syncer::ModelType; using syncer::ModelTypeSet; using syncer::ModelTypeToString; @@ -27,25 +27,48 @@ using syncer::APPS; using syncer::PASSWORDS; using syncer::PREFERENCES; using syncer::NIGORI; -using testing::_; -using testing::Mock; -using testing::ResultOf; namespace { -// Used by SetConfigureDoneExpectation. -DataTypeManager::ConfigureStatus GetStatus( - const DataTypeManager::ConfigureResult& result) { - return result.status; -} - // Helper for unioning with priority types. -syncer::ModelTypeSet AddHighPriorityTypesTo(syncer::ModelTypeSet types) { - syncer::ModelTypeSet result = syncer::ControlTypes(); +ModelTypeSet AddHighPriorityTypesTo(ModelTypeSet types) { + ModelTypeSet result = syncer::ControlTypes(); result.PutAll(types); return result; } +DataTypeStatusTable BuildStatusTable(ModelTypeSet crypto_errors, + ModelTypeSet association_errors, + ModelTypeSet unready_errors, + ModelTypeSet unrecoverable_errors) { + DataTypeStatusTable::TypeErrorMap error_map; + for (ModelTypeSet::Iterator iter = crypto_errors.First(); iter.Good(); + iter.Inc()) { + error_map[iter.Get()] = SyncError( + FROM_HERE, SyncError::CRYPTO_ERROR, "crypto error", iter.Get()); + } + for (ModelTypeSet::Iterator iter = association_errors.First(); iter.Good(); + iter.Inc()) { + error_map[iter.Get()] = SyncError( + FROM_HERE, SyncError::DATATYPE_ERROR, "association error", iter.Get()); + } + for (ModelTypeSet::Iterator iter = unready_errors.First(); iter.Good(); + iter.Inc()) { + error_map[iter.Get()] = SyncError( + FROM_HERE, SyncError::UNREADY_ERROR, "unready errors", iter.Get()); + } + for (ModelTypeSet::Iterator iter = unrecoverable_errors.First(); iter.Good(); + iter.Inc()) { + error_map[iter.Get()] = SyncError(FROM_HERE, + SyncError::UNRECOVERABLE_ERROR, + "unrecoverable errors", + iter.Get()); + } + DataTypeStatusTable status_table; + status_table.UpdateFailedDataTypes(error_map); + return status_table; +} + // Fake BackendDataTypeConfigurer implementation that simply stores away the // callback passed into ConfigureDataTypes. class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer { @@ -65,9 +88,9 @@ class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer { EXPECT_TRUE( expected_configure_types_.Equals( GetDataTypesInState(CONFIGURE_ACTIVE, config_state_map))) - << syncer::ModelTypeSetToString(expected_configure_types_) + << ModelTypeSetToString(expected_configure_types_) << " v.s. " - << syncer::ModelTypeSetToString( + << ModelTypeSetToString( GetDataTypesInState(CONFIGURE_ACTIVE, config_state_map)); } } @@ -85,28 +108,72 @@ class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer { return last_ready_task_; } - void set_expected_configure_types(syncer::ModelTypeSet types) { + void set_expected_configure_types(ModelTypeSet types) { expected_configure_types_ = types; } - const syncer::ModelTypeSet activated_types() { return activated_types_; } + const ModelTypeSet activated_types() { return activated_types_; } private: base::Callback<void(ModelTypeSet, ModelTypeSet)> last_ready_task_; - syncer::ModelTypeSet expected_configure_types_; - syncer::ModelTypeSet activated_types_; + ModelTypeSet expected_configure_types_; + ModelTypeSet activated_types_; }; -// Mock DataTypeManagerObserver implementation. -class DataTypeManagerObserverMock : public DataTypeManagerObserver { +// DataTypeManagerObserver implementation. +class FakeDataTypeManagerObserver : public DataTypeManagerObserver { public: - DataTypeManagerObserverMock() {} - virtual ~DataTypeManagerObserverMock() {} + FakeDataTypeManagerObserver() { ResetExpectations(); } + virtual ~FakeDataTypeManagerObserver() { + EXPECT_FALSE(start_expected_); + DataTypeManager::ConfigureResult default_result; + EXPECT_EQ(done_expectation_.status, default_result.status); + EXPECT_TRUE( + done_expectation_.data_type_status_table.GetFailedTypes().Empty()); + } + + void ExpectStart() { + start_expected_ = true; + } + void ExpectDone(const DataTypeManager::ConfigureResult& result) { + done_expectation_ = result; + } + void ResetExpectations() { + start_expected_ = false; + done_expectation_ = DataTypeManager::ConfigureResult(); + } + + virtual void OnConfigureDone( + const DataTypeManager::ConfigureResult& result) OVERRIDE { + EXPECT_EQ(done_expectation_.status, result.status); + DataTypeStatusTable::TypeErrorMap errors = + result.data_type_status_table.GetAllErrors(); + DataTypeStatusTable::TypeErrorMap expected_errors = + done_expectation_.data_type_status_table.GetAllErrors(); + ASSERT_EQ(expected_errors.size(), errors.size()); + for (DataTypeStatusTable::TypeErrorMap::const_iterator iter = + expected_errors.begin(); + iter != expected_errors.end(); + ++iter) { + ASSERT_TRUE(errors.find(iter->first) != errors.end()); + ASSERT_EQ(iter->second.error_type(), + errors.find(iter->first)->second.error_type()); + } + done_expectation_ = DataTypeManager::ConfigureResult(); + } + + virtual void OnConfigureRetry() OVERRIDE{ + // Not verified. + } + + virtual void OnConfigureStart() OVERRIDE { + EXPECT_TRUE(start_expected_); + start_expected_ = false; + } - MOCK_METHOD1(OnConfigureDone, - void(const DataTypeManager::ConfigureResult&)); - MOCK_METHOD0(OnConfigureRetry, void()); - MOCK_METHOD0(OnConfigureStart, void()); + private: + bool start_expected_ = true; + DataTypeManager::ConfigureResult done_expectation_; }; class FakeDataTypeEncryptionHandler : public DataTypeEncryptionHandler { @@ -115,17 +182,17 @@ class FakeDataTypeEncryptionHandler : public DataTypeEncryptionHandler { virtual ~FakeDataTypeEncryptionHandler(); virtual bool IsPassphraseRequired() const OVERRIDE; - virtual syncer::ModelTypeSet GetEncryptedDataTypes() const OVERRIDE; + virtual ModelTypeSet GetEncryptedDataTypes() const OVERRIDE; void set_passphrase_required(bool passphrase_required) { passphrase_required_ = passphrase_required; } - void set_encrypted_types(syncer::ModelTypeSet encrypted_types) { + void set_encrypted_types(ModelTypeSet encrypted_types) { encrypted_types_ = encrypted_types; } private: bool passphrase_required_; - syncer::ModelTypeSet encrypted_types_; + ModelTypeSet encrypted_types_; }; FakeDataTypeEncryptionHandler::FakeDataTypeEncryptionHandler() @@ -136,7 +203,7 @@ bool FakeDataTypeEncryptionHandler::IsPassphraseRequired() const { return passphrase_required_; } -syncer::ModelTypeSet +ModelTypeSet FakeDataTypeEncryptionHandler::GetEncryptedDataTypes() const { return encrypted_types_; } @@ -151,18 +218,16 @@ class TestDataTypeManager : public DataTypeManagerImpl { BackendDataTypeConfigurer* configurer, const DataTypeController::TypeMap* controllers, const DataTypeEncryptionHandler* encryption_handler, - DataTypeManagerObserver* observer, - DataTypeStatusTable* data_type_status_table) + DataTypeManagerObserver* observer) : DataTypeManagerImpl(base::Closure(), debug_info_listener, controllers, encryption_handler, configurer, - observer, - data_type_status_table), + observer), custom_priority_types_(syncer::ControlTypes()) {} - void set_priority_types(const syncer::ModelTypeSet& priority_types) { + void set_priority_types(const ModelTypeSet& priority_types) { custom_priority_types_ = priority_types; } @@ -177,11 +242,11 @@ class TestDataTypeManager : public DataTypeManagerImpl { } private: - virtual syncer::ModelTypeSet GetPriorityTypes() const OVERRIDE { + virtual ModelTypeSet GetPriorityTypes() const OVERRIDE { return custom_priority_types_; } - syncer::ModelTypeSet custom_priority_types_; + ModelTypeSet custom_priority_types_; DataTypeManager::ConfigureResult configure_result_; }; @@ -202,21 +267,24 @@ class SyncDataTypeManagerImplTest : public testing::Test { &configurer_, &controllers_, &encryption_handler_, - &observer_, - &data_type_status_table_)); + &observer_)); } void SetConfigureStartExpectation() { - EXPECT_CALL(observer_, OnConfigureStart()); + observer_.ExpectStart(); } - void SetConfigureDoneExpectation(DataTypeManager::ConfigureStatus status) { - EXPECT_CALL(observer_, OnConfigureDone(ResultOf(&GetStatus, status))); + void SetConfigureDoneExpectation(DataTypeManager::ConfigureStatus status, + const DataTypeStatusTable& status_table) { + DataTypeManager::ConfigureResult result; + result.status = status; + result.data_type_status_table = status_table; + observer_.ExpectDone(result); } // Configure the given DTM with the given desired types. void Configure(DataTypeManagerImpl* dtm, - const syncer::ModelTypeSet& desired_types) { + const ModelTypeSet& desired_types) { dtm->Configure(desired_types, syncer::CONFIGURE_REASON_RECONFIGURATION); } @@ -252,7 +320,7 @@ class SyncDataTypeManagerImplTest : public testing::Test { static_cast<FakeDataTypeController*>(it->second.get())); } - void FailEncryptionFor(syncer::ModelTypeSet encrypted_types) { + void FailEncryptionFor(ModelTypeSet encrypted_types) { encryption_handler_.set_passphrase_required(true); encryption_handler_.set_encrypted_types(encrypted_types); } @@ -260,9 +328,8 @@ class SyncDataTypeManagerImplTest : public testing::Test { base::MessageLoopForUI ui_loop_; DataTypeController::TypeMap controllers_; FakeBackendDataTypeConfigurer configurer_; - DataTypeManagerObserverMock observer_; + FakeDataTypeManagerObserver observer_; scoped_ptr<TestDataTypeManager> dtm_; - DataTypeStatusTable data_type_status_table_; FakeDataTypeEncryptionHandler encryption_handler_; }; @@ -270,7 +337,7 @@ class SyncDataTypeManagerImplTest : public testing::Test { // and then stop it. TEST_F(SyncDataTypeManagerImplTest, NoControllers) { SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); Configure(dtm_.get(), ModelTypeSet()); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -288,7 +355,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOne) { AddController(BOOKMARKS); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -314,7 +381,8 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) { { SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::ABORTED); + SetConfigureDoneExpectation(DataTypeManager::ABORTED, + DataTypeStatusTable()); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -336,7 +404,8 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) { { SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::ABORTED); + SetConfigureDoneExpectation(DataTypeManager::ABORTED, + DataTypeStatusTable()); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -364,7 +433,8 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) { { SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::ABORTED); + SetConfigureDoneExpectation(DataTypeManager::ABORTED, + DataTypeStatusTable()); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -394,7 +464,11 @@ TEST_F(SyncDataTypeManagerImplTest, OneWaitingForCrypto) { AddController(PASSWORDS); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, + BuildStatusTable(ModelTypeSet(PASSWORDS), + ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet())); const ModelTypeSet types(PASSWORDS); dtm_->set_priority_types(AddHighPriorityTypesTo(types)); @@ -435,7 +509,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); @@ -450,9 +524,9 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) { GetController(BOOKMARKS)->FinishStart(DataTypeController::OK); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 4. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); @@ -488,7 +562,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); @@ -503,9 +577,9 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) { GetController(BOOKMARKS)->FinishStart(DataTypeController::OK); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 4. Configure(dtm_.get(), ModelTypeSet(PREFERENCES)); @@ -541,7 +615,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); @@ -583,7 +657,11 @@ TEST_F(SyncDataTypeManagerImplTest, OneFailingController) { AddController(BOOKMARKS); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR); + SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(BOOKMARKS))); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -612,7 +690,11 @@ TEST_F(SyncDataTypeManagerImplTest, SecondControllerFails) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR); + SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(PREFERENCES))); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); @@ -651,7 +733,11 @@ TEST_F(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(PREFERENCES), + ModelTypeSet(), + ModelTypeSet())); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); @@ -696,7 +782,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); @@ -742,7 +828,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) { AddController(PREFERENCES); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Step 1. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); @@ -780,7 +866,7 @@ TEST_F(SyncDataTypeManagerImplTest, MigrateAll) { dtm_->set_priority_types(AddHighPriorityTypesTo(ModelTypeSet(BOOKMARKS))); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Initial setup. Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); @@ -789,7 +875,7 @@ TEST_F(SyncDataTypeManagerImplTest, MigrateAll) { // We've now configured bookmarks and (implicitly) the control types. EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); // Pretend we were told to migrate all types. ModelTypeSet to_migrate; @@ -797,7 +883,7 @@ TEST_F(SyncDataTypeManagerImplTest, MigrateAll) { to_migrate.PutAll(syncer::ControlTypes()); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); dtm_->PurgeForMigration(to_migrate, syncer::CONFIGURE_REASON_MIGRATION); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -805,11 +891,11 @@ TEST_F(SyncDataTypeManagerImplTest, MigrateAll) { // The DTM will call ConfigureDataTypes(), even though it is unnecessary. FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); // Re-enable the migrated types. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); Configure(dtm_.get(), to_migrate); FinishDownload(*dtm_, to_migrate, ModelTypeSet()); GetController(BOOKMARKS)->FinishStart(DataTypeController::OK); @@ -823,20 +909,20 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureDuringPurge) { // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); FinishDownload(*dtm_, ModelTypeSet(BOOKMARKS), ModelTypeSet()); GetController(BOOKMARKS)->FinishStart(DataTypeController::OK); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); // Purge the Nigori type. SetConfigureStartExpectation(); dtm_->PurgeForMigration(ModelTypeSet(NIGORI), syncer::CONFIGURE_REASON_MIGRATION); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); // Before the backend configuration completes, ask for a different // set of types. This request asks for @@ -849,9 +935,9 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureDuringPurge) { // Invoke the callback we've been waiting for since we asked to purge NIGORI. FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); // Now invoke the callback for the second configure request. @@ -871,11 +957,11 @@ TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfiguration) { AddController(PREFERENCES); dtm_->set_priority_types( - AddHighPriorityTypesTo(syncer::ModelTypeSet(PREFERENCES))); + AddHighPriorityTypesTo(ModelTypeSet(PREFERENCES))); // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Initially only PREFERENCES is configured. configurer_.set_expected_configure_types( @@ -902,11 +988,11 @@ TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationReconfigure) { AddController(APPS); dtm_->set_priority_types( - AddHighPriorityTypesTo(syncer::ModelTypeSet(PREFERENCES))); + AddHighPriorityTypesTo(ModelTypeSet(PREFERENCES))); // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); // Reconfigure while associating PREFERENCES and downloading BOOKMARKS. configurer_.set_expected_configure_types( @@ -949,11 +1035,11 @@ TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationStop) { AddController(PREFERENCES); dtm_->set_priority_types( - AddHighPriorityTypesTo(syncer::ModelTypeSet(PREFERENCES))); + AddHighPriorityTypesTo(ModelTypeSet(PREFERENCES))); // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::ABORTED); + SetConfigureDoneExpectation(DataTypeManager::ABORTED, DataTypeStatusTable()); // Initially only PREFERENCES is configured. configurer_.set_expected_configure_types( @@ -984,11 +1070,15 @@ TEST_F(SyncDataTypeManagerImplTest, PrioritizedConfigurationDownloadError) { AddController(PREFERENCES); dtm_->set_priority_types( - AddHighPriorityTypesTo(syncer::ModelTypeSet(PREFERENCES))); + AddHighPriorityTypesTo(ModelTypeSet(PREFERENCES))); // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR); + SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(BOOKMARKS))); // Initially only PREFERENCES is configured. configurer_.set_expected_configure_types( @@ -1020,11 +1110,15 @@ TEST_F(SyncDataTypeManagerImplTest, HighPriorityAssociationFailure) { AddController(BOOKMARKS); // Will succeed. dtm_->set_priority_types( - AddHighPriorityTypesTo(syncer::ModelTypeSet(PREFERENCES))); + AddHighPriorityTypesTo(ModelTypeSet(PREFERENCES))); // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(PREFERENCES), + ModelTypeSet(), + ModelTypeSet())); // Initially only PREFERENCES is configured. configurer_.set_expected_configure_types( @@ -1072,11 +1166,15 @@ TEST_F(SyncDataTypeManagerImplTest, LowPriorityAssociationFailure) { AddController(BOOKMARKS); // Will fail. dtm_->set_priority_types( - AddHighPriorityTypesTo(syncer::ModelTypeSet(PREFERENCES))); + AddHighPriorityTypesTo(ModelTypeSet(PREFERENCES))); // Initial configure. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(BOOKMARKS), + ModelTypeSet(), + ModelTypeSet())); // Initially only PREFERENCES is configured. configurer_.set_expected_configure_types( @@ -1125,9 +1223,9 @@ TEST_F(SyncDataTypeManagerImplTest, FilterDesiredTypes) { dtm_->set_priority_types(AddHighPriorityTypesTo(types)); SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); - syncer::ModelTypeSet expected_types = syncer::ControlTypes(); + ModelTypeSet expected_types = syncer::ControlTypes(); expected_types.Put(BOOKMARKS); // APPS is filtered out because there's no controller for it. configurer_.set_expected_configure_types(expected_types); @@ -1144,7 +1242,6 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureForBackupRollback) { SetConfigureStartExpectation(); - ModelTypeSet expected_types = syncer::ControlTypes(); expected_types.Put(BOOKMARKS); configurer_.set_expected_configure_types(expected_types); @@ -1155,32 +1252,32 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureForBackupRollback) { } TEST_F(SyncDataTypeManagerImplTest, ReenableAfterDataTypeError) { - syncer::SyncError error(FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Datatype disabled", - syncer::BOOKMARKS); - std::map<syncer::ModelType, syncer::SyncError> errors; - errors[syncer::BOOKMARKS] = error; - data_type_status_table_.UpdateFailedDataTypes(errors); - AddController(PREFERENCES); // Will succeed. AddController(BOOKMARKS); // Will be disabled due to datatype error. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(BOOKMARKS), + ModelTypeSet(), + ModelTypeSet())); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES)); FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); - FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet()); + FinishDownload(*dtm_, ModelTypeSet(PREFERENCES, BOOKMARKS), ModelTypeSet()); GetController(PREFERENCES)->FinishStart(DataTypeController::OK); + GetController(BOOKMARKS) + ->FinishStart(DataTypeController::ASSOCIATION_FAILED); + FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); // Reconfig for error. + FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); // Reconfig for error. EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); EXPECT_EQ(DataTypeController::RUNNING, GetController(PREFERENCES)->state()); EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); // Re-enable bookmarks. - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); dtm_->ReenableType(syncer::BOOKMARKS); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -1202,17 +1299,21 @@ TEST_F(SyncDataTypeManagerImplTest, UnreadyType) { // Bookmarks is never started due to being unready. SetConfigureStartExpectation(); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, + BuildStatusTable(ModelTypeSet(), + ModelTypeSet(), + ModelTypeSet(BOOKMARKS), + ModelTypeSet())); Configure(dtm_.get(), ModelTypeSet(BOOKMARKS)); FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet()); EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state()); EXPECT_EQ(0U, configurer_.activated_types().Size()); - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); // Bookmarks should start normally now. GetController(BOOKMARKS)->SetReadyForStart(true); - SetConfigureDoneExpectation(DataTypeManager::OK); + SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable()); dtm_->ReenableType(BOOKMARKS); EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state()); @@ -1225,7 +1326,7 @@ TEST_F(SyncDataTypeManagerImplTest, UnreadyType) { EXPECT_EQ(1U, configurer_.activated_types().Size()); // Should do nothing. - Mock::VerifyAndClearExpectations(&observer_); + observer_.ResetExpectations(); dtm_->ReenableType(BOOKMARKS); dtm_->Stop(); diff --git a/components/sync_driver/data_type_manager_mock.h b/components/sync_driver/data_type_manager_mock.h index fac6522..237e285 100644 --- a/components/sync_driver/data_type_manager_mock.h +++ b/components/sync_driver/data_type_manager_mock.h @@ -18,6 +18,7 @@ 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()); diff --git a/components/sync_driver/data_type_status_table.h b/components/sync_driver/data_type_status_table.h index f3bb19a..bf2e0a8 100644 --- a/components/sync_driver/data_type_status_table.h +++ b/components/sync_driver/data_type_status_table.h @@ -16,7 +16,7 @@ class DataTypeStatusTable { public: typedef std::map<syncer::ModelType, syncer::SyncError> TypeErrorMap; - explicit DataTypeStatusTable(); + DataTypeStatusTable(); ~DataTypeStatusTable(); // Copy and assign welcome. diff --git a/components/sync_driver/fake_data_type_controller.cc b/components/sync_driver/fake_data_type_controller.cc index 9f9c769..3fb646c 100644 --- a/components/sync_driver/fake_data_type_controller.cc +++ b/components/sync_driver/fake_data_type_controller.cc @@ -80,13 +80,15 @@ void FakeDataTypeController::FinishStart(ConfigureResult result) { syncer::SyncError::UNRECOVERABLE_ERROR, "Unrecoverable error", type())); - } else { + } else if (result == NEEDS_CRYPTO) { state_ = NOT_RUNNING; local_merge_result.set_error( syncer::SyncError(FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Fake error", + syncer::SyncError::CRYPTO_ERROR, + "Crypto error", type())); + } else { + NOTREACHED(); } last_start_callback_.Run(result, local_merge_result, syncer_merge_result); } |