0

Sync ModelTypeController: Rename delegate_on_disk/delegate_in_memory

These have evolved to mean something different, so those names weren't
appropriate anymore. They're now called delegate_for_full_sync_mode/
delegate_for_transport_mode.

Similarly, the StorageOption enum is renamed to SyncMode and its
entries from STORAGE_ON_DISK/STORAGE_IN_MEMORY to kFull/kTransportOnly.

TBRing trivial changes (renaming some parameters) in
autofill_wallet_model_type_controller.h/cc
TBR=jsaul@google.com

Bug: 999980
Change-Id: I21ecfb6620ac42d323b91fc284e0593d19ff4379
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1795605
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697552}
This commit is contained in:
Marc Treib
2019-09-18 10:49:07 +00:00
committed by Commit Bot
parent 9c6d6340f0
commit 659e6050df
29 changed files with 162 additions and 148 deletions

@ -292,14 +292,14 @@ ChromeSyncClient::CreateDataTypeControllers(syncer::SyncService* sync_service) {
SecurityEventRecorderFactory::GetForProfile(profile_)
->GetControllerDelegate()
.get();
// Forward both on-disk and in-memory storage modes to the same delegate,
// Forward both full-sync and transport-only modes to the same delegate,
// since behavior for SECURITY_EVENTS does not differ.
controllers.push_back(std::make_unique<syncer::ModelTypeController>(
syncer::SECURITY_EVENTS,
/*delegate_on_disk=*/
/*delegate_for_full_sync_mode=*/
std::make_unique<syncer::ForwardingModelTypeControllerDelegate>(
delegate),
/*delegate_in_memory=*/
/*delegate_for_transport_mode=*/
std::make_unique<syncer::ForwardingModelTypeControllerDelegate>(
delegate)));
}

@ -18,10 +18,11 @@ namespace browser_sync {
AutofillWalletModelTypeController::AutofillWalletModelTypeController(
syncer::ModelType type,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
PrefService* pref_service,
syncer::SyncService* sync_service)
: ModelTypeController(type, std::move(delegate_on_disk)),
: ModelTypeController(type, std::move(delegate_for_full_sync_mode)),
pref_service_(pref_service),
sync_service_(sync_service) {
DCHECK(type == syncer::AUTOFILL_WALLET_DATA ||
@ -34,13 +35,15 @@ AutofillWalletModelTypeController::AutofillWalletModelTypeController(
AutofillWalletModelTypeController::AutofillWalletModelTypeController(
syncer::ModelType type,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_in_memory,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_transport_mode,
PrefService* pref_service,
syncer::SyncService* sync_service)
: ModelTypeController(type,
std::move(delegate_on_disk),
std::move(delegate_in_memory)),
std::move(delegate_for_full_sync_mode),
std::move(delegate_for_transport_mode)),
pref_service_(pref_service),
sync_service_(sync_service) {
DCHECK(type == syncer::AUTOFILL_WALLET_DATA ||

@ -29,13 +29,16 @@ class AutofillWalletModelTypeController : public syncer::ModelTypeController,
// |sync_client| must outlive this object.
AutofillWalletModelTypeController(
syncer::ModelType type,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
PrefService* pref_service,
syncer::SyncService* sync_service);
AutofillWalletModelTypeController(
syncer::ModelType type,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_in_memory,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_transport_mode,
PrefService* pref_service,
syncer::SyncService* sync_service);
~AutofillWalletModelTypeController() override;

@ -14,11 +14,12 @@
namespace browser_sync {
AutofillProfileModelTypeController::AutofillProfileModelTypeController(
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
PrefService* pref_service,
syncer::SyncService* sync_service)
: ModelTypeController(syncer::AUTOFILL_PROFILE,
std::move(delegate_on_disk)),
std::move(delegate_for_full_sync_mode)),
pref_service_(pref_service),
sync_service_(sync_service) {
pref_registrar_.Init(pref_service_);

@ -24,7 +24,8 @@ namespace browser_sync {
class AutofillProfileModelTypeController : public syncer::ModelTypeController {
public:
AutofillProfileModelTypeController(
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
PrefService* pref_service,
syncer::SyncService* sync_service);
~AutofillProfileModelTypeController() override;

@ -361,14 +361,14 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
.get())));
}
// Forward both on-disk and in-memory storage modes to the same delegate,
// Forward both full-sync and transport-only modes to the same delegate,
// since behavior for USER_CONSENTS does not differ (they are always
// persisted).
controllers.push_back(std::make_unique<ModelTypeController>(
syncer::USER_CONSENTS,
/*delegate_on_disk=*/
/*delegate_for_full_sync_mode=*/
CreateForwardingControllerDelegate(syncer::USER_CONSENTS),
/*delegate_in_memory=*/
/*delegate_for_transport_mode=*/
CreateForwardingControllerDelegate(syncer::USER_CONSENTS)));
return controllers;
@ -459,12 +459,12 @@ std::unique_ptr<ModelTypeController> ProfileSyncComponentsFactoryImpl::
autofill::AutofillWebDataService*)>& delegate_from_web_data,
syncer::SyncService* sync_service) {
return std::make_unique<AutofillWalletModelTypeController>(
type, /*delegate_on_disk=*/
type, /*delegate_for_full_sync_mode=*/
std::make_unique<syncer::ProxyModelTypeControllerDelegate>(
db_thread_,
base::BindRepeating(delegate_from_web_data,
base::RetainedRef(web_data_service_on_disk_))),
/*delegate_in_memory=*/
/*delegate_for_transport_mode=*/
std::make_unique<syncer::ProxyModelTypeControllerDelegate>(
db_thread_,
base::BindRepeating(delegate_from_web_data,

@ -14,13 +14,15 @@
namespace password_manager {
PasswordModelTypeController::PasswordModelTypeController(
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_in_memory,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_transport_mode,
syncer::SyncService* sync_service,
const base::RepeatingClosure& state_changed_callback)
: ModelTypeController(syncer::PASSWORDS,
std::move(delegate_on_disk),
std::move(delegate_in_memory)),
std::move(delegate_for_full_sync_mode),
std::move(delegate_for_transport_mode)),
sync_service_(sync_service),
state_changed_callback_(state_changed_callback) {}
@ -31,7 +33,7 @@ void PasswordModelTypeController::LoadModels(
const ModelLoadCallback& model_load_callback) {
DCHECK(CalledOnValidThread());
sync_service_->AddObserver(this);
storage_option_ = configure_context.storage_option;
sync_mode_ = configure_context.sync_mode;
ModelTypeController::LoadModels(configure_context, model_load_callback);
state_changed_callback_.Run();
}
@ -40,12 +42,12 @@ void PasswordModelTypeController::Stop(syncer::ShutdownReason shutdown_reason,
StopCallback callback) {
DCHECK(CalledOnValidThread());
sync_service_->RemoveObserver(this);
// The account storage should be cleared if Sync is stopped for any reason
// (other than just browser shutdown). One common case it switching from
// Sync-the-transport to Sync-the-feature; in that case we don't want to end
// up with two copied of the passwords (one in the profile DB, one in the
// account DB).
if (storage_option_ == syncer::STORAGE_IN_MEMORY) {
// In transport-only mode, our storage is scoped to the Gaia account. That
// means it should be cleared if Sync is stopped for any reason (other than
// just browser shutdown). E.g. when switching to full-Sync mode, we don't
// want to end up with two copies of the passwords (one in the profile DB, one
// in the account DB).
if (sync_mode_ == syncer::SyncMode::kTransportOnly) {
switch (shutdown_reason) {
case syncer::STOP_SYNC:
shutdown_reason = syncer::DISABLE_SYNC;

@ -24,8 +24,10 @@ class PasswordModelTypeController : public syncer::ModelTypeController,
public syncer::SyncServiceObserver {
public:
PasswordModelTypeController(
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<syncer::ModelTypeControllerDelegate> delegate_in_memory,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_full_sync_mode,
std::unique_ptr<syncer::ModelTypeControllerDelegate>
delegate_for_transport_mode,
syncer::SyncService* sync_service,
const base::RepeatingClosure& state_changed_callback);
~PasswordModelTypeController() override;
@ -44,7 +46,7 @@ class PasswordModelTypeController : public syncer::ModelTypeController,
const base::RepeatingClosure state_changed_callback_;
// Passed in to LoadModels(), and cached here for later use in Stop().
syncer::StorageOption storage_option_ = syncer::STORAGE_ON_DISK;
syncer::SyncMode sync_mode_ = syncer::SyncMode::kFull;
DISALLOW_COPY_AND_ASSIGN(PasswordModelTypeController);
};

@ -43,9 +43,9 @@ jumbo_static_library("base") {
"report_unrecoverable_error.cc",
"report_unrecoverable_error.h",
"stop_source.h",
"storage_option.h",
"sync_base_switches.cc",
"sync_base_switches.h",
"sync_mode.h",
"sync_prefs.cc",
"sync_prefs.h",
"sync_stop_metadata_fate.cc",

@ -1,15 +0,0 @@
// Copyright 2018 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 COMPONENTS_SYNC_BASE_STORAGE_OPTION_H_
#define COMPONENTS_SYNC_BASE_STORAGE_OPTION_H_
namespace syncer {
// Passed as an argument when configuring sync / individual data types.
enum StorageOption { STORAGE_ON_DISK, STORAGE_IN_MEMORY };
} // namespace syncer
#endif // COMPONENTS_SYNC_BASE_STORAGE_OPTION_H_

@ -0,0 +1,18 @@
// Copyright 2018 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 COMPONENTS_SYNC_BASE_SYNC_MODE_H_
#define COMPONENTS_SYNC_BASE_SYNC_MODE_H_
namespace syncer {
// Specifies whether the sync machinery is running in full-Sync mode (aka
// Sync-the-feature) or transport-only mode. In transport-only mode, only a
// subset of data types is allowed, and any local data is removed on sign-out.
// Passed as an argument when configuring sync / individual data types.
enum class SyncMode { kTransportOnly, kFull };
} // namespace syncer
#endif // COMPONENTS_SYNC_BASE_SYNC_MODE_H_

@ -50,7 +50,7 @@ void AsyncDirectoryTypeController::LoadModels(
const ConfigureContext& configure_context,
const ModelLoadCallback& model_load_callback) {
DCHECK(CalledOnValidThread());
DCHECK_EQ(configure_context.storage_option, STORAGE_ON_DISK)
DCHECK_EQ(configure_context.sync_mode, SyncMode::kFull)
<< " for type " << ModelTypeToString(type());
model_load_callback_ = model_load_callback;

@ -7,7 +7,7 @@
#include <string>
#include "components/sync/base/storage_option.h"
#include "components/sync/base/sync_mode.h"
#include "components/sync/engine/configure_reason.h"
#include "google_apis/gaia/core_account_id.h"
@ -22,7 +22,7 @@ namespace syncer {
struct ConfigureContext {
CoreAccountId authenticated_account_id;
std::string cache_guid;
StorageOption storage_option = STORAGE_ON_DISK;
SyncMode sync_mode = SyncMode::kFull;
ConfigureReason reason = CONFIGURE_REASON_UNKNOWN;
base::Time configuration_start_time;
// TODO(mastiz): Consider adding |requested_types| here, but currently there

@ -623,11 +623,11 @@ ModelTypeSet DataTypeManagerImpl::PrepareConfigureParams(
// response will be sent.
ModelTypeSet types_to_purge;
// If we're using in-memory storage, don't clear any old data. The reason is
// If we're using transport-only mode, don't clear any old data. The reason is
// that if a user temporarily disables Sync, we don't want to wipe (and later
// redownload) all their data, just because Sync restarted in transport-only
// mode.
if (last_requested_context_.storage_option == STORAGE_ON_DISK) {
if (last_requested_context_.sync_mode == SyncMode::kFull) {
types_to_purge = Difference(ModelTypeSet::All(), downloaded_types_);
// Include clean_types in types_to_purge, they are part of
// |downloaded_types_|, but still need to be cleared.
@ -663,7 +663,7 @@ ModelTypeSet DataTypeManagerImpl::PrepareConfigureParams(
base::Bind(&DataTypeManagerImpl::DownloadReady,
weak_ptr_factory_.GetWeakPtr(), download_types_queue_.front());
params->is_sync_feature_enabled =
last_requested_context_.storage_option == STORAGE_ON_DISK;
last_requested_context_.sync_mode == SyncMode::kFull;
DCHECK(Intersection(active_types, types_to_purge).Empty());
DCHECK(Intersection(active_types, fatal_types).Empty());

@ -11,7 +11,7 @@
#include "base/test/metrics/histogram_tester.h"
#include "base/test/task_environment.h"
#include "components/sync/base/model_type.h"
#include "components/sync/base/storage_option.h"
#include "components/sync/base/sync_mode.h"
#include "components/sync/driver/configure_context.h"
#include "components/sync/driver/data_type_encryption_handler.h"
#include "components/sync/driver/data_type_manager_observer.h"
@ -34,12 +34,11 @@ ModelTypeSet AddControlTypesTo(ModelTypeSet types) {
return Union(ControlTypes(), types);
}
ConfigureContext BuildConfigureContext(
ConfigureReason reason,
StorageOption storage_option = STORAGE_ON_DISK) {
ConfigureContext BuildConfigureContext(ConfigureReason reason,
SyncMode sync_mode = SyncMode::kFull) {
ConfigureContext context;
context.reason = reason;
context.storage_option = storage_option;
context.sync_mode = sync_mode;
return context;
}
@ -276,10 +275,9 @@ class SyncDataTypeManagerImplTest : public testing::Test {
}
void Configure(ModelTypeSet desired_types,
StorageOption storage_option,
SyncMode sync_mode,
ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION) {
dtm_->Configure(desired_types,
BuildConfigureContext(reason, storage_option));
dtm_->Configure(desired_types, BuildConfigureContext(reason, sync_mode));
}
// Finish downloading for the given DTM. Should be done only after
@ -1889,7 +1887,7 @@ TEST_F(SyncDataTypeManagerImplTest, PurgeDataOnStartingPersistent) {
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), STORAGE_ON_DISK);
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), SyncMode::kFull);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -1910,7 +1908,7 @@ TEST_F(SyncDataTypeManagerImplTest, PurgeDataOnStartingPersistent) {
// Now we restart with a reduced set of data types.
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), STORAGE_ON_DISK);
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), SyncMode::kFull);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -1937,7 +1935,7 @@ TEST_F(SyncDataTypeManagerImplTest, DontPurgeDataOnStartingEphemeral) {
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), STORAGE_ON_DISK);
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), SyncMode::kFull);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -1958,7 +1956,7 @@ TEST_F(SyncDataTypeManagerImplTest, DontPurgeDataOnStartingEphemeral) {
// Now we restart in ephemeral mode, with a reduced set of data types.
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), STORAGE_IN_MEMORY);
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), SyncMode::kTransportOnly);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -1982,7 +1980,7 @@ TEST_F(SyncDataTypeManagerImplTest, PurgeDataOnReconfiguringPersistent) {
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), STORAGE_ON_DISK);
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), SyncMode::kFull);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -1997,7 +1995,7 @@ TEST_F(SyncDataTypeManagerImplTest, PurgeDataOnReconfiguringPersistent) {
// Now we reconfigure with a reduced set of data types.
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), STORAGE_ON_DISK);
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), SyncMode::kFull);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -2020,7 +2018,7 @@ TEST_F(SyncDataTypeManagerImplTest, DontPurgeDataOnReconfiguringEphemeral) {
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), STORAGE_ON_DISK);
Configure(ModelTypeSet(BOOKMARKS, AUTOFILL_WALLET_DATA), SyncMode::kFull);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -2035,7 +2033,7 @@ TEST_F(SyncDataTypeManagerImplTest, DontPurgeDataOnReconfiguringEphemeral) {
// Now we reconfigure into ephemeral mode, with a reduced set of data types.
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), STORAGE_IN_MEMORY);
Configure(ModelTypeSet(AUTOFILL_WALLET_DATA), SyncMode::kTransportOnly);
ASSERT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -2060,7 +2058,7 @@ TEST_F(SyncDataTypeManagerImplTest, ShouldRecordInitialConfigureTimeHistogram) {
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(BOOKMARKS), STORAGE_ON_DISK,
Configure(ModelTypeSet(BOOKMARKS), SyncMode::kFull,
CONFIGURE_REASON_NEW_CLIENT);
FinishDownload(ModelTypeSet(), ModelTypeSet());
@ -2078,7 +2076,7 @@ TEST_F(SyncDataTypeManagerImplTest,
SetConfigureStartExpectation();
SetConfigureDoneExpectation(DataTypeManager::OK, DataTypeStatusTable());
Configure(ModelTypeSet(BOOKMARKS), STORAGE_ON_DISK,
Configure(ModelTypeSet(BOOKMARKS), SyncMode::kFull,
CONFIGURE_REASON_RECONFIGURATION);
FinishDownload(ModelTypeSet(), ModelTypeSet());

@ -31,7 +31,7 @@ void FrontendDataTypeController::LoadModels(
const ConfigureContext& configure_context,
const ModelLoadCallback& model_load_callback) {
DCHECK(CalledOnValidThread());
DCHECK_EQ(configure_context.storage_option, STORAGE_ON_DISK);
DCHECK_EQ(configure_context.sync_mode, SyncMode::kFull);
model_load_callback_ = model_load_callback;

@ -696,8 +696,8 @@ void SyncEngineBackend::LoadAndConnectNigoriController() {
configure_context.authenticated_account_id = authenticated_account_id_;
configure_context.cache_guid = sync_manager_->cache_guid();
// TODO(crbug.com/922900): investigate whether we want to use
// STORAGE_IN_MEMORY in Butter mode.
configure_context.storage_option = STORAGE_ON_DISK;
// kTransportOnly in Butter mode.
configure_context.sync_mode = SyncMode::kFull;
configure_context.configuration_start_time = base::Time::Now();
nigori_controller_->LoadModels(configure_context, base::DoNothing());
DCHECK_EQ(nigori_controller_->state(), DataTypeController::MODEL_LOADED);

@ -114,8 +114,7 @@ void ModelAssociationManager::Initialize(ModelTypeSet desired_types,
// |desired_types| must be a subset of |preferred_types|.
DCHECK(preferred_types.HasAll(desired_types));
bool storage_option_changed =
configure_context_.storage_option != context.storage_option;
bool sync_mode_changed = configure_context_.sync_mode != context.sync_mode;
configure_context_ = context;
@ -145,8 +144,7 @@ void ModelAssociationManager::Initialize(ModelTypeSet desired_types,
// We generally stop all data types which are not desired. When the storage
// option changes, we need to restart all data types so that they can
// re-wire to the correct storage.
bool should_stop =
!desired_types_.Has(dtc->type()) || storage_option_changed;
bool should_stop = !desired_types_.Has(dtc->type()) || sync_mode_changed;
// If the datatype is already STOPPING, we also wait for it to stop, to make
// sure it's ready to start again (if appropriate).
if ((should_stop && dtc->state() != DataTypeController::NOT_RUNNING) ||
@ -155,12 +153,12 @@ void ModelAssociationManager::Initialize(ModelTypeSet desired_types,
// means we'll clear it.
ShutdownReason reason =
preferred_types.Has(dtc->type()) ? STOP_SYNC : DISABLE_SYNC;
// If we're switchingt o in-memory storage, don't clear any old data. The
// reason is that if a user temporarily disables Sync, we don't want to
// wipe (and later redownload) all their data, just because Sync restarted
// in transport-only mode.
if (storage_option_changed &&
configure_context_.storage_option == STORAGE_IN_MEMORY) {
// If we're switching to transport-only mode, don't clear any old data.
// The reason is that if a user temporarily disables Sync, we don't want
// to wipe (and later redownload) all their data, just because Sync
// restarted in transport-only mode.
if (sync_mode_changed &&
configure_context_.sync_mode == SyncMode::kTransportOnly) {
reason = STOP_SYNC;
}
types_to_stop[dtc] = reason;

@ -733,7 +733,7 @@ TEST_F(SyncModelAssociationManagerTest,
ModelTypeSet desired_types = preferred_types;
ConfigureContext configure_context;
configure_context.storage_option = STORAGE_ON_DISK;
configure_context.sync_mode = SyncMode::kFull;
EXPECT_CALL(delegate_, OnSingleDataTypeWillStart(BOOKMARKS));
EXPECT_CALL(delegate_, OnSingleDataTypeWillStart(APPS));
@ -757,7 +757,7 @@ TEST_F(SyncModelAssociationManagerTest,
testing::Mock::VerifyAndClearExpectations(&delegate_);
// Switch to in-memory storage.
configure_context.storage_option = STORAGE_IN_MEMORY;
configure_context.sync_mode = SyncMode::kTransportOnly;
desired_types.Remove(APPS);
preferred_types.Remove(APPS);
@ -786,7 +786,7 @@ TEST_F(SyncModelAssociationManagerTest,
}
TEST_F(SyncModelAssociationManagerTest,
SwitchFromInMemoryToOnDiskRestartsTypes) {
SwitchFromTransportOnlyToFullSyncRestartsTypes) {
// Associate model with two data types.
controllers_[BOOKMARKS] = std::make_unique<FakeDataTypeController>(BOOKMARKS);
controllers_[APPS] = std::make_unique<FakeDataTypeController>(APPS);
@ -795,7 +795,7 @@ TEST_F(SyncModelAssociationManagerTest,
ModelTypeSet desired_types = preferred_types;
ConfigureContext configure_context;
configure_context.storage_option = STORAGE_IN_MEMORY;
configure_context.sync_mode = SyncMode::kTransportOnly;
EXPECT_CALL(delegate_, OnSingleDataTypeWillStart(BOOKMARKS));
EXPECT_CALL(delegate_, OnSingleDataTypeWillStart(APPS));
@ -818,8 +818,8 @@ TEST_F(SyncModelAssociationManagerTest,
DataTypeController::RUNNING);
testing::Mock::VerifyAndClearExpectations(&delegate_);
// Switch to on-disk storage.
configure_context.storage_option = STORAGE_ON_DISK;
// Switch to full-sync mode.
configure_context.sync_mode = SyncMode::kFull;
desired_types.Remove(APPS);
preferred_types.Remove(APPS);

@ -45,17 +45,19 @@ SyncStopMetadataFate TakeStrictestMetadataFate(SyncStopMetadataFate fate1,
ModelTypeController::ModelTypeController(
ModelType type,
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk)
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode)
: DataTypeController(type) {
delegate_map_.emplace(STORAGE_ON_DISK, std::move(delegate_on_disk));
delegate_map_.emplace(SyncMode::kFull,
std::move(delegate_for_full_sync_mode));
}
ModelTypeController::ModelTypeController(
ModelType type,
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<ModelTypeControllerDelegate> delegate_in_memory)
: ModelTypeController(type, std::move(delegate_on_disk)) {
delegate_map_.emplace(STORAGE_IN_MEMORY, std::move(delegate_in_memory));
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode,
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_transport_mode)
: ModelTypeController(type, std::move(delegate_for_full_sync_mode)) {
delegate_map_.emplace(SyncMode::kTransportOnly,
std::move(delegate_for_transport_mode));
}
ModelTypeController::~ModelTypeController() {}
@ -84,7 +86,7 @@ void ModelTypeController::LoadModels(
DCHECK(model_load_callback);
DCHECK_EQ(NOT_RUNNING, state_);
auto it = delegate_map_.find(configure_context.storage_option);
auto it = delegate_map_.find(configure_context.sync_mode);
DCHECK(it != delegate_map_.end());
delegate_ = it->second.get();
DCHECK(delegate_);
@ -100,7 +102,7 @@ void ModelTypeController::LoadModels(
base::AsWeakPtr(this), SyncError::DATATYPE_ERROR));
request.authenticated_account_id = configure_context.authenticated_account_id;
request.cache_guid = configure_context.cache_guid;
request.storage_option = configure_context.storage_option;
request.sync_mode = configure_context.sync_mode;
request.configuration_start_time = configure_context.configuration_start_time;
// Note that |request.authenticated_account_id| may be empty for local sync.

@ -14,7 +14,7 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "components/sync/base/model_type.h"
#include "components/sync/base/storage_option.h"
#include "components/sync/base/sync_mode.h"
#include "components/sync/driver/configure_context.h"
#include "components/sync/driver/data_type_controller.h"
#include "components/sync/model/model_error.h"
@ -30,12 +30,12 @@ class ModelTypeController : public DataTypeController {
public:
ModelTypeController(
ModelType type,
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk);
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode);
// For datatypes that have support for STORAGE_IN_MEMORY.
ModelTypeController(
ModelType type,
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk,
std::unique_ptr<ModelTypeControllerDelegate> delegate_in_memory);
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode,
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_transport_mode);
~ModelTypeController() override;
// Steals the activation response, only used for Nigori.
@ -71,7 +71,7 @@ class ModelTypeController : public DataTypeController {
std::unique_ptr<DataTypeActivationResponse> activation_response);
void TriggerCompletionCallbacks(const SyncError& error);
base::flat_map<StorageOption, std::unique_ptr<ModelTypeControllerDelegate>>
base::flat_map<SyncMode, std::unique_ptr<ModelTypeControllerDelegate>>
delegate_map_;
// State of this datatype controller.

@ -132,8 +132,9 @@ class TestModelTypeConfigurer : public ModelTypeConfigurer {
class TestModelTypeController : public ModelTypeController {
public:
explicit TestModelTypeController(
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk)
: ModelTypeController(kTestModelType, std::move(delegate_on_disk)) {}
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode)
: ModelTypeController(kTestModelType,
std::move(delegate_for_full_sync_mode)) {}
~TestModelTypeController() override {}
using ModelTypeController::ReportModelError;
@ -555,19 +556,19 @@ TEST_F(ModelTypeControllerTest, StopAndReportErrorWhileStarting) {
EXPECT_EQ(DataTypeController::FAILED, controller()->state());
}
// Tests that StorageOption is honored when the controller has been constructed
// Tests that SyncMode is honored when the controller has been constructed
// with two delegates.
TEST(ModelTypeControllerWithMultiDelegateTest, ToggleStorageOption) {
TEST(ModelTypeControllerWithMultiDelegateTest, ToggleSyncMode) {
base::test::SingleThreadTaskEnvironment task_environment;
NiceMock<MockDelegate> delegate_on_disk;
NiceMock<MockDelegate> delegate_in_memory;
NiceMock<MockDelegate> delegate_for_full_sync_mode;
NiceMock<MockDelegate> delegate_for_transport_mode;
ModelTypeController controller(
kTestModelType,
std::make_unique<ForwardingModelTypeControllerDelegate>(
&delegate_on_disk),
&delegate_for_full_sync_mode),
std::make_unique<ForwardingModelTypeControllerDelegate>(
&delegate_in_memory));
&delegate_for_transport_mode));
ConfigureContext context;
context.authenticated_account_id = kAccountId;
@ -575,14 +576,14 @@ TEST(ModelTypeControllerWithMultiDelegateTest, ToggleStorageOption) {
ModelTypeControllerDelegate::StartCallback start_callback;
// Start sync with STORAGE_IN_MEMORY.
EXPECT_CALL(delegate_on_disk, OnSyncStarting(_, _)).Times(0);
EXPECT_CALL(delegate_in_memory, OnSyncStarting(_, _))
// Start sync with SyncMode::kTransportOnly.
EXPECT_CALL(delegate_for_full_sync_mode, OnSyncStarting(_, _)).Times(0);
EXPECT_CALL(delegate_for_transport_mode, OnSyncStarting(_, _))
.WillOnce([&](const DataTypeActivationRequest& request,
ModelTypeControllerDelegate::StartCallback callback) {
start_callback = std::move(callback);
});
context.storage_option = STORAGE_IN_MEMORY;
context.sync_mode = SyncMode::kTransportOnly;
controller.LoadModels(context, base::DoNothing());
ASSERT_EQ(DataTypeController::MODEL_STARTING, controller.state());
@ -593,19 +594,19 @@ TEST(ModelTypeControllerWithMultiDelegateTest, ToggleStorageOption) {
ASSERT_EQ(DataTypeController::MODEL_LOADED, controller.state());
// Stop sync.
EXPECT_CALL(delegate_on_disk, OnSyncStopping(_)).Times(0);
EXPECT_CALL(delegate_in_memory, OnSyncStopping(_));
EXPECT_CALL(delegate_for_full_sync_mode, OnSyncStopping(_)).Times(0);
EXPECT_CALL(delegate_for_transport_mode, OnSyncStopping(_));
controller.Stop(DISABLE_SYNC, base::DoNothing());
ASSERT_EQ(DataTypeController::NOT_RUNNING, controller.state());
// Start sync with STORAGE_ON_DISK.
EXPECT_CALL(delegate_in_memory, OnSyncStarting(_, _)).Times(0);
EXPECT_CALL(delegate_on_disk, OnSyncStarting(_, _))
// Start sync with SyncMode::kFull.
EXPECT_CALL(delegate_for_transport_mode, OnSyncStarting(_, _)).Times(0);
EXPECT_CALL(delegate_for_full_sync_mode, OnSyncStarting(_, _))
.WillOnce([&](const DataTypeActivationRequest& request,
ModelTypeControllerDelegate::StartCallback callback) {
start_callback = std::move(callback);
});
context.storage_option = STORAGE_ON_DISK;
context.sync_mode = SyncMode::kFull;
controller.LoadModels(context, base::DoNothing());
ASSERT_EQ(DataTypeController::MODEL_STARTING, controller.state());
@ -616,8 +617,8 @@ TEST(ModelTypeControllerWithMultiDelegateTest, ToggleStorageOption) {
ASSERT_EQ(DataTypeController::MODEL_LOADED, controller.state());
// Stop sync.
EXPECT_CALL(delegate_in_memory, OnSyncStopping(_)).Times(0);
EXPECT_CALL(delegate_on_disk, OnSyncStopping(_));
EXPECT_CALL(delegate_for_transport_mode, OnSyncStopping(_)).Times(0);
EXPECT_CALL(delegate_for_full_sync_mode, OnSyncStopping(_));
controller.Stop(DISABLE_SYNC, base::DoNothing());
ASSERT_EQ(DataTypeController::NOT_RUNNING, controller.state());
}

@ -1230,7 +1230,7 @@ void ProfileSyncService::ConfigureDataTypeManager(ConfigureReason reason) {
configure_context.authenticated_account_id =
GetAuthenticatedAccountInfo().account_id;
configure_context.cache_guid = sync_prefs_.GetCacheGuid();
configure_context.storage_option = STORAGE_ON_DISK;
configure_context.sync_mode = SyncMode::kFull;
configure_context.reason = reason;
configure_context.configuration_start_time = base::Time::Now();
@ -1283,7 +1283,7 @@ void ProfileSyncService::ConfigureDataTypeManager(ConfigureReason reason) {
}
types = Intersection(types, allowed_types);
configure_context.storage_option = STORAGE_IN_MEMORY;
configure_context.sync_mode = SyncMode::kTransportOnly;
}
data_type_manager_->Configure(types, configure_context);

@ -8,7 +8,7 @@
#include <string>
#include "base/time/time.h"
#include "components/sync/base/storage_option.h"
#include "components/sync/base/sync_mode.h"
#include "components/sync/model/model_error.h"
#include "google_apis/gaia/core_account_id.h"
@ -29,7 +29,7 @@ struct DataTypeActivationRequest {
ModelErrorHandler error_handler;
CoreAccountId authenticated_account_id;
std::string cache_guid;
StorageOption storage_option = STORAGE_ON_DISK;
SyncMode sync_mode = SyncMode::kFull;
// The start time of the confuguration this activation is part of.
base::Time configuration_start_time;

@ -738,7 +738,7 @@ void ClientTagBasedModelTypeProcessor::OnUpdateReceived(
base::UmaHistogramCustomTimes(
base::StringPrintf(
"Sync.ModelTypeConfigurationTime.%s.%s",
(activation_request_.storage_option == STORAGE_IN_MEMORY)
(activation_request_.sync_mode == SyncMode::kTransportOnly)
? "Ephemeral"
: "Persistent",
ModelTypeToHistogramSuffix(type_)),

@ -17,7 +17,7 @@
#include "base/test/task_environment.h"
#include "base/threading/platform_thread.h"
#include "components/sync/base/model_type.h"
#include "components/sync/base/storage_option.h"
#include "components/sync/base/sync_mode.h"
#include "components/sync/base/time.h"
#include "components/sync/engine/data_type_activation_response.h"
#include "components/sync/model/data_type_activation_request.h"
@ -234,14 +234,14 @@ class ClientTagBasedModelTypeProcessorTest : public ::testing::Test {
void OnSyncStarting(
const std::string& authenticated_account_id = "SomeAccountId",
const std::string& cache_guid = "TestCacheGuid",
StorageOption storage_option = STORAGE_ON_DISK) {
SyncMode sync_mode = SyncMode::kFull) {
DataTypeActivationRequest request;
request.error_handler = base::BindRepeating(
&ClientTagBasedModelTypeProcessorTest::ErrorReceived,
base::Unretained(this));
request.cache_guid = cache_guid;
request.authenticated_account_id = authenticated_account_id;
request.storage_option = storage_option;
request.sync_mode = sync_mode;
request.configuration_start_time = base::Time::Now();
type_processor()->OnSyncStarting(
request,
@ -1793,12 +1793,12 @@ TEST_F(ClientTagBasedModelTypeProcessorTest,
EXPECT_EQ(0U, db()->data_count());
}
// Tests that initial updates for ephemeral storage result in reporting setup
// duration.
// Tests that initial updates for transport-only mode (called "ephemeral
// storage" for historical reasons) result in reporting setup duration.
TEST_F(ClientTagBasedModelTypeProcessorTest,
ShouldReportEphemeralConfigurationTime) {
InitializeToMetadataLoaded(/*initial_sync_done=*/false);
OnSyncStarting("SomeAccountId", "TestCacheGuid", STORAGE_IN_MEMORY);
OnSyncStarting("SomeAccountId", "TestCacheGuid", SyncMode::kTransportOnly);
base::HistogramTester histogram_tester;
@ -1817,12 +1817,12 @@ TEST_F(ClientTagBasedModelTypeProcessorTest,
/*count=*/0);
}
// Tests that initial updates for persistent storage do not result in reporting
// setup duration.
// Tests that initial updates for full-sync mode (called "persistent storage"
// for historical reasons) do not result in reporting setup duration.
TEST_F(ClientTagBasedModelTypeProcessorTest,
ShouldReportPersistentConfigurationTime) {
InitializeToMetadataLoaded(/*initial_sync_done=*/false);
OnSyncStarting("SomeAccountId", "TestCacheGuid", STORAGE_ON_DISK);
OnSyncStarting("SomeAccountId", "TestCacheGuid", SyncMode::kFull);
base::HistogramTester histogram_tester;
@ -1886,13 +1886,12 @@ TEST_F(FullUpdateClientTagBasedModelTypeProcessorTest,
EXPECT_EQ(0U, db()->metadata_count());
EXPECT_EQ(0U, worker()->GetNumPendingCommits());
}
// Tests that full updates for ephemeral storage result in reporting setup
// duration.
// Tests that full updates for transport-only mode (called "ephemeral storage"
// for historical reasons) result in reporting setup duration.
TEST_F(FullUpdateClientTagBasedModelTypeProcessorTest,
ShouldReportEphemeralConfigurationTimeOnlyForFirstFullUpdate) {
InitializeToMetadataLoaded(/*initial_sync_done=*/false);
OnSyncStarting("SomeAccountId", "TestCacheGuid", STORAGE_IN_MEMORY);
OnSyncStarting("SomeAccountId", "TestCacheGuid", SyncMode::kTransportOnly);
UpdateResponseDataList updates1;
updates1.push_back(worker()->GenerateUpdateData(

@ -40,7 +40,7 @@ void ProxyTabsDataTypeController::LoadModels(
const syncer::ConfigureContext& configure_context,
const ModelLoadCallback& model_load_callback) {
DCHECK(CalledOnValidThread());
DCHECK_EQ(configure_context.storage_option, syncer::STORAGE_ON_DISK);
DCHECK_EQ(configure_context.sync_mode, syncer::SyncMode::kFull);
state_ = MODEL_LOADED;
state_changed_cb_.Run(state_);
model_load_callback.Run(type(), syncer::SyncError());

@ -16,8 +16,9 @@ namespace syncer {
UserEventModelTypeController::UserEventModelTypeController(
SyncService* sync_service,
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk)
: ModelTypeController(syncer::USER_EVENTS, std::move(delegate_on_disk)),
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode)
: ModelTypeController(syncer::USER_EVENTS,
std::move(delegate_for_full_sync_mode)),
sync_service_(sync_service) {
DCHECK(sync_service_);
sync_service_->AddObserver(this);

@ -21,7 +21,7 @@ class UserEventModelTypeController : public syncer::ModelTypeController,
// |sync_service| must not be null and must outlive this object.
UserEventModelTypeController(
SyncService* sync_service,
std::unique_ptr<ModelTypeControllerDelegate> delegate_on_disk);
std::unique_ptr<ModelTypeControllerDelegate> delegate_for_full_sync_mode);
~UserEventModelTypeController() override;
// syncer::DataTypeController implementation.