Commit bfa75811 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Rename DataTypeController::RegisterWithBackend to ActivateDataType

This better describes what it does, and is symmetric with
DeactivateDataType.
Similarly, DataTypeManagerImpl::RegisterTypesWithBackend is renamed to
ActivateDataTypes.

Bug: 647505
Change-Id: I780d4a3e31b93ca067f647c4465ec0e33e23230e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462271
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarVictor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#816224}
parent 3acd71ab
...@@ -40,8 +40,8 @@ class DataTypeController : public base::SupportsWeakPtr<DataTypeController> { ...@@ -40,8 +40,8 @@ class DataTypeController : public base::SupportsWeakPtr<DataTypeController> {
FAILED // The controller was started but encountered an error. FAILED // The controller was started but encountered an error.
}; };
// Returned from RegisterWithBackend. // Returned from ActivateDataType.
enum RegisterWithBackendResult { enum ActivateDataTypeResult {
// Indicates that the initial download for this type is already complete, or // Indicates that the initial download for this type is already complete, or
// wasn't needed in the first place (e.g. for proxy types). // wasn't needed in the first place (e.g. for proxy types).
TYPE_ALREADY_DOWNLOADED, TYPE_ALREADY_DOWNLOADED,
...@@ -75,11 +75,10 @@ class DataTypeController : public base::SupportsWeakPtr<DataTypeController> { ...@@ -75,11 +75,10 @@ class DataTypeController : public base::SupportsWeakPtr<DataTypeController> {
virtual void LoadModels(const ConfigureContext& configure_context, virtual void LoadModels(const ConfigureContext& configure_context,
const ModelLoadCallback& model_load_callback) = 0; const ModelLoadCallback& model_load_callback) = 0;
// Registers with sync backend if needed. This function is called by // Called by DataTypeManager once the local model has loaded, but before
// DataTypeManager before downloading initial data. Returns whether the // downloading initial data (if necessary). Returns whether the initial
// initial download for this type is already complete. // download for this type is already complete.
// TODO(crbug.com/647505): Rename this function to ActivateDataType(). virtual ActivateDataTypeResult ActivateDataType(
virtual RegisterWithBackendResult RegisterWithBackend(
ModelTypeConfigurer* configurer) = 0; ModelTypeConfigurer* configurer) = 0;
// Called by DataTypeManager to deactivate the controlled data type. // Called by DataTypeManager to deactivate the controlled data type.
......
...@@ -194,18 +194,18 @@ void DataTypeManagerImpl::ConfigureImpl(ModelTypeSet desired_types, ...@@ -194,18 +194,18 @@ void DataTypeManagerImpl::ConfigureImpl(ModelTypeSet desired_types,
Restart(); Restart();
} }
void DataTypeManagerImpl::RegisterTypesWithBackend() { void DataTypeManagerImpl::ActivateDataTypes() {
for (ModelType type : last_enabled_types_) { for (ModelType type : last_enabled_types_) {
const auto& dtc_iter = controllers_->find(type); const auto& dtc_iter = controllers_->find(type);
if (dtc_iter == controllers_->end()) if (dtc_iter == controllers_->end())
continue; continue;
DataTypeController* dtc = dtc_iter->second.get(); DataTypeController* dtc = dtc_iter->second.get();
if (dtc->state() == DataTypeController::MODEL_LOADED) { if (dtc->state() == DataTypeController::MODEL_LOADED) {
// Only call RegisterWithBackend for types that completed LoadModels // Only call ActivateDataType for types that completed LoadModels
// successfully. Such types shouldn't be in an error state at the same // successfully. Such types shouldn't be in an error state at the same
// time. // time.
DCHECK(!data_type_status_table_.GetFailedTypes().Has(dtc->type())); DCHECK(!data_type_status_table_.GetFailedTypes().Has(dtc->type()));
switch (dtc->RegisterWithBackend(configurer_)) { switch (dtc->ActivateDataType(configurer_)) {
case DataTypeController::TYPE_ALREADY_DOWNLOADED: case DataTypeController::TYPE_ALREADY_DOWNLOADED:
// Proxy types (as opposed to protocol types) don't actually have any // Proxy types (as opposed to protocol types) don't actually have any
// data, so keep proxy types out of |downloaded_types_|. // data, so keep proxy types out of |downloaded_types_|.
...@@ -348,7 +348,7 @@ void DataTypeManagerImpl::OnAllDataTypesReadyForConfigure() { ...@@ -348,7 +348,7 @@ void DataTypeManagerImpl::OnAllDataTypesReadyForConfigure() {
// TODO(pavely): By now some of datatypes in |download_types_queue_| could // TODO(pavely): By now some of datatypes in |download_types_queue_| could
// have failed loading and should be excluded from configuration. I need to // have failed loading and should be excluded from configuration. I need to
// adjust |download_types_queue_| for such types. // adjust |download_types_queue_| for such types.
RegisterTypesWithBackend(); ActivateDataTypes();
StartNextDownload(ModelTypeSet()); StartNextDownload(ModelTypeSet());
} }
......
...@@ -160,8 +160,8 @@ class DataTypeManagerImpl : public DataTypeManager, ...@@ -160,8 +160,8 @@ class DataTypeManagerImpl : public DataTypeManager,
void ConfigureImpl(ModelTypeSet desired_types, void ConfigureImpl(ModelTypeSet desired_types,
const ConfigureContext& context); const ConfigureContext& context);
// Calls data type controllers of requested types to register with backend. // Calls data type controllers of requested types to activate.
void RegisterTypesWithBackend(); void ActivateDataTypes();
DataTypeConfigStateMap BuildDataTypeConfigStateMap( DataTypeConfigStateMap BuildDataTypeConfigStateMap(
const ModelTypeSet& types_being_configured) const; const ModelTypeSet& types_being_configured) const;
......
...@@ -1313,12 +1313,12 @@ TEST_F(SyncDataTypeManagerImplTest, DelayConfigureForUSSTypes) { ...@@ -1313,12 +1313,12 @@ TEST_F(SyncDataTypeManagerImplTest, DelayConfigureForUSSTypes) {
// Bookmarks model isn't loaded yet and it is required to complete before // Bookmarks model isn't loaded yet and it is required to complete before
// call to configure. Ensure that configure wasn't called. // call to configure. Ensure that configure wasn't called.
EXPECT_EQ(0, configurer_.configure_call_count()); EXPECT_EQ(0, configurer_.configure_call_count());
EXPECT_EQ(0, GetController(BOOKMARKS)->register_with_backend_call_count()); EXPECT_EQ(0, GetController(BOOKMARKS)->activate_call_count());
// Finishing model load should trigger configure. // Finishing model load should trigger configure.
GetController(BOOKMARKS)->model()->SimulateModelStartFinished(); GetController(BOOKMARKS)->model()->SimulateModelStartFinished();
EXPECT_EQ(1, configurer_.configure_call_count()); EXPECT_EQ(1, configurer_.configure_call_count());
EXPECT_EQ(1, GetController(BOOKMARKS)->register_with_backend_call_count()); EXPECT_EQ(1, GetController(BOOKMARKS)->activate_call_count());
FinishDownload(ModelTypeSet(), ModelTypeSet()); // control types FinishDownload(ModelTypeSet(), ModelTypeSet()); // control types
FinishDownload(ModelTypeSet(BOOKMARKS), ModelTypeSet()); FinishDownload(ModelTypeSet(BOOKMARKS), ModelTypeSet());
...@@ -1327,8 +1327,8 @@ TEST_F(SyncDataTypeManagerImplTest, DelayConfigureForUSSTypes) { ...@@ -1327,8 +1327,8 @@ TEST_F(SyncDataTypeManagerImplTest, DelayConfigureForUSSTypes) {
} }
// Test that when encryption fails for a given type, the corresponding // Test that when encryption fails for a given type, the corresponding
// controller is not told to register with its backend. // data type is not activated.
TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendOnEncryptionError) { TEST_F(SyncDataTypeManagerImplTest, ActivateDataTypeOnEncryptionError) {
AddController(BOOKMARKS); AddController(BOOKMARKS);
AddController(PASSWORDS); AddController(PASSWORDS);
GetController(BOOKMARKS)->model()->EnableManualModelStart(); GetController(BOOKMARKS)->model()->EnableManualModelStart();
...@@ -1341,17 +1341,17 @@ TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendOnEncryptionError) { ...@@ -1341,17 +1341,17 @@ TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendOnEncryptionError) {
EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state()); EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state());
EXPECT_EQ(DataTypeController::MODEL_STARTING, EXPECT_EQ(DataTypeController::MODEL_STARTING,
GetController(PASSWORDS)->state()); GetController(PASSWORDS)->state());
EXPECT_EQ(0, GetController(BOOKMARKS)->register_with_backend_call_count()); EXPECT_EQ(0, GetController(BOOKMARKS)->activate_call_count());
EXPECT_EQ(0, GetController(PASSWORDS)->register_with_backend_call_count()); EXPECT_EQ(0, GetController(PASSWORDS)->activate_call_count());
GetController(PASSWORDS)->model()->SimulateModelStartFinished(); GetController(PASSWORDS)->model()->SimulateModelStartFinished();
EXPECT_EQ(0, GetController(BOOKMARKS)->register_with_backend_call_count()); EXPECT_EQ(0, GetController(BOOKMARKS)->activate_call_count());
EXPECT_EQ(1, GetController(PASSWORDS)->register_with_backend_call_count()); EXPECT_EQ(1, GetController(PASSWORDS)->activate_call_count());
} }
// Test that RegisterWithBackend is not called for datatypes that failed // Test that ActivateDataType is not called for datatypes that failed
// LoadModels(). // LoadModels().
TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendAfterLoadModelsError) { TEST_F(SyncDataTypeManagerImplTest, ActivateDataTypeAfterLoadModelsError) {
// Initiate configuration for two datatypes but block them at LoadModels. // Initiate configuration for two datatypes but block them at LoadModels.
AddController(BOOKMARKS); AddController(BOOKMARKS);
AddController(PASSWORDS); AddController(PASSWORDS);
...@@ -1369,9 +1369,9 @@ TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendAfterLoadModelsError) { ...@@ -1369,9 +1369,9 @@ TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendAfterLoadModelsError) {
ModelError(FROM_HERE, "test error")); ModelError(FROM_HERE, "test error"));
GetController(PASSWORDS)->model()->SimulateModelStartFinished(); GetController(PASSWORDS)->model()->SimulateModelStartFinished();
// RegisterWithBackend should be called for passwords, but not bookmarks. // ActivateDataType should be called for passwords, but not bookmarks.
EXPECT_EQ(0, GetController(BOOKMARKS)->register_with_backend_call_count()); EXPECT_EQ(0, GetController(BOOKMARKS)->activate_call_count());
EXPECT_EQ(1, GetController(PASSWORDS)->register_with_backend_call_count()); EXPECT_EQ(1, GetController(PASSWORDS)->activate_call_count());
} }
// Test that Stop with DISABLE_SYNC calls DTC Stop with CLEAR_METADATA for // Test that Stop with DISABLE_SYNC calls DTC Stop with CLEAR_METADATA for
......
...@@ -39,10 +39,10 @@ FakeDataTypeController::GetPreconditionState() const { ...@@ -39,10 +39,10 @@ FakeDataTypeController::GetPreconditionState() const {
return precondition_state_; return precondition_state_;
} }
DataTypeController::RegisterWithBackendResult DataTypeController::ActivateDataTypeResult
FakeDataTypeController::RegisterWithBackend(ModelTypeConfigurer* configurer) { FakeDataTypeController::ActivateDataType(ModelTypeConfigurer* configurer) {
++register_with_backend_call_count_; ++activate_call_count_;
return ModelTypeController::RegisterWithBackend(configurer); return ModelTypeController::ActivateDataType(configurer);
} }
} // namespace syncer } // namespace syncer
...@@ -25,18 +25,16 @@ class FakeDataTypeController : public ModelTypeController { ...@@ -25,18 +25,16 @@ class FakeDataTypeController : public ModelTypeController {
// |enable_transport_only_model| is set upon construction. // |enable_transport_only_model| is set upon construction.
FakeModelTypeControllerDelegate* model(SyncMode sync_mode = SyncMode::kFull); FakeModelTypeControllerDelegate* model(SyncMode sync_mode = SyncMode::kFull);
int register_with_backend_call_count() const { int activate_call_count() const { return activate_call_count_; }
return register_with_backend_call_count_;
}
// ModelTypeController overrides. // ModelTypeController overrides.
PreconditionState GetPreconditionState() const override; PreconditionState GetPreconditionState() const override;
RegisterWithBackendResult RegisterWithBackend( ActivateDataTypeResult ActivateDataType(
ModelTypeConfigurer* configurer) override; ModelTypeConfigurer* configurer) override;
private: private:
PreconditionState precondition_state_ = PreconditionState::kPreconditionsMet; PreconditionState precondition_state_ = PreconditionState::kPreconditionsMet;
int register_with_backend_call_count_ = 0; int activate_call_count_ = 0;
}; };
} // namespace syncer } // namespace syncer
......
...@@ -120,8 +120,8 @@ void ModelTypeController::LoadModels( ...@@ -120,8 +120,8 @@ void ModelTypeController::LoadModels(
base::AsWeakPtr(this))); base::AsWeakPtr(this)));
} }
DataTypeController::RegisterWithBackendResult DataTypeController::ActivateDataTypeResult
ModelTypeController::RegisterWithBackend(ModelTypeConfigurer* configurer) { ModelTypeController::ActivateDataType(ModelTypeConfigurer* configurer) {
DCHECK(CalledOnValidThread()); DCHECK(CalledOnValidThread());
DCHECK(configurer); DCHECK(configurer);
DCHECK(activation_response_); DCHECK(activation_response_);
......
...@@ -41,14 +41,14 @@ class ModelTypeController : public DataTypeController { ...@@ -41,14 +41,14 @@ class ModelTypeController : public DataTypeController {
// Steals the activation response, only used for Nigori. // Steals the activation response, only used for Nigori.
// TODO(crbug.com/967677): Once all datatypes are in USS, we should redesign // TODO(crbug.com/967677): Once all datatypes are in USS, we should redesign
// or remove RegisterWithBackend, and expose the activation response via // or remove ActivateDataType, and expose the activation response via
// LoadModels(), which is more natural in USS. // LoadModels(), which is more natural in USS.
std::unique_ptr<DataTypeActivationResponse> ActivateManuallyForNigori(); std::unique_ptr<DataTypeActivationResponse> ActivateManuallyForNigori();
// DataTypeController implementation. // DataTypeController implementation.
void LoadModels(const ConfigureContext& configure_context, void LoadModels(const ConfigureContext& configure_context,
const ModelLoadCallback& model_load_callback) override; const ModelLoadCallback& model_load_callback) override;
RegisterWithBackendResult RegisterWithBackend( ActivateDataTypeResult ActivateDataType(
ModelTypeConfigurer* configurer) override; ModelTypeConfigurer* configurer) override;
void DeactivateDataType(ModelTypeConfigurer* configurer) override; void DeactivateDataType(ModelTypeConfigurer* configurer) override;
void Stop(ShutdownReason shutdown_reason, StopCallback callback) override; void Stop(ShutdownReason shutdown_reason, StopCallback callback) override;
......
...@@ -175,11 +175,11 @@ class ModelTypeControllerTest : public testing::Test { ...@@ -175,11 +175,11 @@ class ModelTypeControllerTest : public testing::Test {
return true; return true;
} }
void RegisterWithBackend(bool expect_downloaded) { void ActivateDataType(bool expect_downloaded) {
auto result = expect_downloaded auto result = expect_downloaded
? DataTypeController::TYPE_ALREADY_DOWNLOADED ? DataTypeController::TYPE_ALREADY_DOWNLOADED
: DataTypeController::TYPE_NOT_YET_DOWNLOADED; : DataTypeController::TYPE_NOT_YET_DOWNLOADED;
EXPECT_EQ(result, controller_.RegisterWithBackend(&configurer_)); EXPECT_EQ(result, controller_.ActivateDataType(&configurer_));
// ModelTypeProcessorProxy does posting of tasks. // ModelTypeProcessorProxy does posting of tasks.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
...@@ -238,7 +238,7 @@ TEST_F(ModelTypeControllerTest, Activate) { ...@@ -238,7 +238,7 @@ TEST_F(ModelTypeControllerTest, Activate) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ASSERT_TRUE(LoadModels()); ASSERT_TRUE(LoadModels());
EXPECT_EQ(DataTypeController::MODEL_LOADED, controller()->state()); EXPECT_EQ(DataTypeController::MODEL_LOADED, controller()->state());
RegisterWithBackend(/*expect_downloaded=*/false); ActivateDataType(/*expect_downloaded=*/false);
EXPECT_TRUE(processor()->is_connected()); EXPECT_TRUE(processor()->is_connected());
EXPECT_EQ(DataTypeController::RUNNING, controller()->state()); EXPECT_EQ(DataTypeController::RUNNING, controller()->state());
histogram_tester.ExpectTotalCount(kStartFailuresHistogram, 0); histogram_tester.ExpectTotalCount(kStartFailuresHistogram, 0);
...@@ -248,7 +248,7 @@ TEST_F(ModelTypeControllerTest, ActivateWithInitialSyncDone) { ...@@ -248,7 +248,7 @@ TEST_F(ModelTypeControllerTest, ActivateWithInitialSyncDone) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ASSERT_TRUE(LoadModels(/*initial_sync_done=*/true)); ASSERT_TRUE(LoadModels(/*initial_sync_done=*/true));
EXPECT_EQ(DataTypeController::MODEL_LOADED, controller()->state()); EXPECT_EQ(DataTypeController::MODEL_LOADED, controller()->state());
RegisterWithBackend(/*expect_downloaded=*/true); ActivateDataType(/*expect_downloaded=*/true);
EXPECT_TRUE(processor()->is_connected()); EXPECT_TRUE(processor()->is_connected());
histogram_tester.ExpectTotalCount(kStartFailuresHistogram, 0); histogram_tester.ExpectTotalCount(kStartFailuresHistogram, 0);
} }
...@@ -282,7 +282,7 @@ TEST_F(ModelTypeControllerTest, ActivateWithError) { ...@@ -282,7 +282,7 @@ TEST_F(ModelTypeControllerTest, ActivateWithError) {
TEST_F(ModelTypeControllerTest, Stop) { TEST_F(ModelTypeControllerTest, Stop) {
ASSERT_TRUE(LoadModels()); ASSERT_TRUE(LoadModels());
RegisterWithBackend(/*expect_downloaded=*/false); ActivateDataType(/*expect_downloaded=*/false);
EXPECT_TRUE(processor()->is_connected()); EXPECT_TRUE(processor()->is_connected());
DeactivateDataTypeAndStop(STOP_SYNC); DeactivateDataTypeAndStop(STOP_SYNC);
EXPECT_EQ(DataTypeController::NOT_RUNNING, controller()->state()); EXPECT_EQ(DataTypeController::NOT_RUNNING, controller()->state());
...@@ -651,7 +651,7 @@ TEST_F(ModelTypeControllerTest, ReportErrorAfterRegisteredWithBackend) { ...@@ -651,7 +651,7 @@ TEST_F(ModelTypeControllerTest, ReportErrorAfterRegisteredWithBackend) {
std::move(start_callback).Run(std::move(activation_response)); std::move(start_callback).Run(std::move(activation_response));
ASSERT_EQ(DataTypeController::MODEL_LOADED, controller()->state()); ASSERT_EQ(DataTypeController::MODEL_LOADED, controller()->state());
RegisterWithBackend(/*expect_downloaded=*/false); ActivateDataType(/*expect_downloaded=*/false);
ASSERT_EQ(DataTypeController::RUNNING, controller()->state()); ASSERT_EQ(DataTypeController::RUNNING, controller()->state());
// Now trigger the run-time error. // Now trigger the run-time error.
......
...@@ -30,8 +30,8 @@ void ProxyTabsDataTypeController::LoadModels( ...@@ -30,8 +30,8 @@ void ProxyTabsDataTypeController::LoadModels(
model_load_callback.Run(type(), syncer::SyncError()); model_load_callback.Run(type(), syncer::SyncError());
} }
syncer::DataTypeController::RegisterWithBackendResult syncer::DataTypeController::ActivateDataTypeResult
ProxyTabsDataTypeController::RegisterWithBackend( ProxyTabsDataTypeController::ActivateDataType(
syncer::ModelTypeConfigurer* configurer) { syncer::ModelTypeConfigurer* configurer) {
DCHECK(configurer); DCHECK(configurer);
DCHECK_EQ(MODEL_LOADED, state_); DCHECK_EQ(MODEL_LOADED, state_);
......
...@@ -26,7 +26,7 @@ class ProxyTabsDataTypeController : public syncer::DataTypeController { ...@@ -26,7 +26,7 @@ class ProxyTabsDataTypeController : public syncer::DataTypeController {
// DataTypeController interface. // DataTypeController interface.
void LoadModels(const syncer::ConfigureContext& configure_context, void LoadModels(const syncer::ConfigureContext& configure_context,
const ModelLoadCallback& model_load_callback) override; const ModelLoadCallback& model_load_callback) override;
RegisterWithBackendResult RegisterWithBackend( ActivateDataTypeResult ActivateDataType(
syncer::ModelTypeConfigurer* configurer) override; syncer::ModelTypeConfigurer* configurer) override;
void Stop(syncer::ShutdownReason shutdown_reason, void Stop(syncer::ShutdownReason shutdown_reason,
StopCallback callback) override; StopCallback callback) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment