sync/internal_api/sync_manager.cc - Issue 10701046: sync: Remove SyncManager::TestingMode in favour of InternalComponentsFactory.

Unified Diff: sync/internal_api/sync_manager.cc

Issue 10701046: sync: Remove SyncManager::TestingMode in favour of InternalComponentsFactory. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: test Created 8 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« sync/internal_api/public/test/test_internal_components_factory.h ('K') | « sync/internal_api/public/test/test_internal_components_factory.h ('k') | sync/internal_api/syncapi_unittest.cc » ('j') | sync/internal_api/test/test_internal_components_factory.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: sync/internal_api/sync_manager.cc

diff --git a/sync/internal_api/sync_manager.cc b/sync/internal_api/sync_manager.cc

index e8ba9dd45dcadeb6335191a7f8cb8a61723361e8..173c6fafc6c462ed30debb9c15be1102eae3e633 100644

--- a/sync/internal_api/sync_manager.cc

+++ b/sync/internal_api/sync_manager.cc

@@ -32,6 +32,7 @@

#include "sync/internal_api/public/base_node.h"

#include "sync/internal_api/public/configure_reason.h"

#include "sync/internal_api/public/engine/polling_constants.h"

+#include "sync/internal_api/public/internal_components_factory_impl.h"

Nicolas Zea 2012/07/18 21:10:12 this doesn't need to be the impl does it?

tim (not reviewing) 2012/07/18 21:57:47 Done.

#include "sync/internal_api/public/read_node.h"

#include "sync/internal_api/public/read_transaction.h"

#include "sync/internal_api/public/user_share.h"

@@ -84,7 +85,6 @@ GetUpdatesCallerInfo::GetUpdatesSource GetSourceFromReason(

default:

NOTREACHED();

}

return GetUpdatesCallerInfo::UNKNOWN;

}

@@ -123,14 +123,12 @@ class SyncManager::SyncInternal

weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),

change_delegate_(NULL),

initialized_(false),

- testing_mode_(NON_TEST),

observing_ip_address_changes_(false),

throttled_data_type_tracker_(&allstatus_),

traffic_recorder_(kMaxMessagesToRecord, kMaxMessageSizeToRecord),

encryptor_(NULL),

unrecoverable_error_handler_(NULL),

report_unrecoverable_error_function_(NULL),

- created_on_loop_(MessageLoop::current()),

nigori_overwrite_count_(0) {

// Pre-fill |notification_info_map_|.

for (int i = syncer::FIRST_REAL_MODEL_TYPE;

@@ -185,7 +183,7 @@ class SyncManager::SyncInternal

const SyncCredentials& credentials,

syncer::SyncNotifier* sync_notifier,

const std::string& restored_key_for_bootstrapping,

- TestingMode testing_mode,

+ scoped_ptr<InternalComponentsFactory> internal_components_factory,

Nicolas Zea 2012/07/18 21:10:12 see comment in SBH about whether this should take

Encryptor* encryptor,

UnrecoverableErrorHandler* unrecoverable_error_handler,

ReportUnrecoverableErrorFunction

@@ -587,12 +585,6 @@ class SyncManager::SyncInternal

// Set to true once Init has been called.

bool initialized_;

- // Controls the disabling of certain SyncManager features.

- // Can be used to disable communication with the server and the use of an

- // on-disk file for maintaining syncer state.

- // TODO(117836): Clean up implementation of SyncManager unit tests.

- TestingMode testing_mode_;

bool observing_ip_address_changes_;

// Map used to store the notification info to be displayed in

@@ -616,8 +608,6 @@ class SyncManager::SyncInternal

UnrecoverableErrorHandler* unrecoverable_error_handler_;

ReportUnrecoverableErrorFunction report_unrecoverable_error_function_;

- MessageLoop* const created_on_loop_;

// The number of times we've automatically (i.e. not via SetPassphrase or

// conflict resolver) updated the nigori's encryption keys in this chrome

// instantiation.

@@ -722,7 +712,7 @@ bool SyncManager::Init(

const SyncCredentials& credentials,

syncer::SyncNotifier* sync_notifier,

const std::string& restored_key_for_bootstrapping,

- TestingMode testing_mode,

+ scoped_ptr<InternalComponentsFactory> internal_components_factory,

Encryptor* encryptor,

UnrecoverableErrorHandler* unrecoverable_error_handler,

ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {

@@ -744,7 +734,7 @@ bool SyncManager::Init(

credentials,

sync_notifier,

restored_key_for_bootstrapping,

- testing_mode,

+ internal_components_factory.Pass(),

encryptor,

unrecoverable_error_handler,

report_unrecoverable_error_function);

@@ -874,7 +864,7 @@ bool SyncManager::SyncInternal::Init(

const SyncCredentials& credentials,

syncer::SyncNotifier* sync_notifier,

const std::string& restored_key_for_bootstrapping,

- TestingMode testing_mode,

+ scoped_ptr<InternalComponentsFactory> internal_components_factory,

Encryptor* encryptor,

UnrecoverableErrorHandler* unrecoverable_error_handler,

ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {

@@ -889,7 +879,6 @@ bool SyncManager::SyncInternal::Init(

blocking_task_runner_ = blocking_task_runner;

change_delegate_ = change_delegate;

- testing_mode_ = testing_mode;

sync_notifier_.reset(sync_notifier);

@@ -904,18 +893,11 @@ bool SyncManager::SyncInternal::Init(

unrecoverable_error_handler_ = unrecoverable_error_handler;

report_unrecoverable_error_function_ = report_unrecoverable_error_function;

- syncable::DirectoryBackingStore* backing_store = NULL;

- if (testing_mode_ == TEST_IN_MEMORY) {

- // TODO(tim): 117836. Use a factory or delegate to create this and don't

- // depend on TEST_IN_MEMORY here.

- backing_store =

- new syncable::InMemoryDirectoryBackingStore(credentials.email);

- } else {

- FilePath absolute_db_path(database_path_);

- file_util::AbsolutePath(&absolute_db_path);

- backing_store = new syncable::OnDiskDirectoryBackingStore(

- credentials.email, absolute_db_path);

- }

+ FilePath absolute_db_path(database_path_);

+ file_util::AbsolutePath(&absolute_db_path);

+ syncable::DirectoryBackingStore* backing_store =

+ internal_components_factory->BuildDirectoryBackingStore(

+ credentials.email, absolute_db_path);

DCHECK(backing_store);

share_.directory.reset(

@@ -932,33 +914,29 @@ bool SyncManager::SyncInternal::Init(

connection_manager()->AddListener(this);

- // Test mode does not use a syncer context or syncer thread.

- if (testing_mode_ == NON_TEST) {

- // Build a SyncSessionContext and store the worker in it.

- DVLOG(1) << "Sync is bringing up SyncSessionContext.";

- std::vector<SyncEngineEventListener*> listeners;

- listeners.push_back(&allstatus_);

- listeners.push_back(this);

- session_context_.reset(new SyncSessionContext(

- connection_manager_.get(),

- directory(),

- model_safe_routing_info,

- workers,

- extensions_activity_monitor,

- &throttled_data_type_tracker_,

- listeners,

- &debug_info_event_listener_,

- &traffic_recorder_));

- session_context()->set_account_name(credentials.email);

- scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer()));

- }

+ // Build a SyncSessionContext and store the worker in it.

+ DVLOG(1) << "Sync is bringing up SyncSessionContext.";

+ std::vector<SyncEngineEventListener*> listeners;

+ listeners.push_back(&allstatus_);

+ listeners.push_back(this);

+ session_context_.reset(internal_components_factory->BuildContext(

+ connection_manager_.get(),

+ directory(),

+ model_safe_routing_info,

+ workers,

+ extensions_activity_monitor,

+ &throttled_data_type_tracker_,

+ listeners,

+ &debug_info_event_listener_,

+ &traffic_recorder_));

+ session_context()->set_account_name(credentials.email);

+ scheduler_.reset(internal_components_factory->BuildScheduler(

+ name_, session_context()));

bool success = SignIn(credentials);

if (success) {

- if (scheduler()) {

- scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE);

- }

+ scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE);

initialized_ = true;

@@ -994,8 +972,7 @@ bool SyncManager::SyncInternal::Init(

OnInitializationComplete(

MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()),

success));

- if (!success && testing_mode_ == NON_TEST)

+ if (!success)

return false;

sync_notifier_->AddObserver(this);

@@ -1077,7 +1054,6 @@ void SyncManager::SyncInternal::UpdateCryptographerAndNigoriCallback(

pending_keys));

}

// Add or update device information.

bool contains_this_device = false;

for (int i = 0; i < nigori.device_information_size(); ++i) {

@@ -1145,14 +1121,12 @@ void SyncManager::SyncInternal::NotifyCryptographerState(

void SyncManager::SyncInternal::StartSyncingNormally(

const syncer::ModelSafeRoutingInfo& routing_info) {

// Start the sync scheduler.

- if (scheduler()) { // NULL during certain unittests.

- // TODO(sync): We always want the newest set of routes when we switch back

- // to normal mode. Figure out how to enforce set_routing_info is always

- // appropriately set and that it's only modified when switching to normal

- // mode.

- session_context()->set_routing_info(routing_info);

- scheduler()->Start(SyncScheduler::NORMAL_MODE);

- }

+ // TODO(sync): We always want the newest set of routes when we switch back

+ // to normal mode. Figure out how to enforce set_routing_info is always

+ // appropriately set and that it's only modified when switching to normal

+ // mode.

+ session_context()->set_routing_info(routing_info);

+ scheduler()->Start(SyncScheduler::NORMAL_MODE);

}

bool SyncManager::SyncInternal::OpenDirectory() {

@@ -1694,11 +1668,7 @@ void SyncManager::StopSyncingForShutdown(const base::Closure& callback) {

void SyncManager::SyncInternal::StopSyncingForShutdown(

const base::Closure& callback) {

DVLOG(2) << "StopSyncingForShutdown";

- if (scheduler()) // May be null in tests.

- scheduler()->RequestStop(callback);

- else

- created_on_loop_->PostTask(FROM_HERE, callback);

+ scheduler()->RequestStop(callback);

if (connection_manager_.get())

connection_manager_->TerminateAllIO();

}

@@ -1771,8 +1741,7 @@ void SyncManager::SyncInternal::OnIPAddressChanged() {

void SyncManager::SyncInternal::OnIPAddressChangedImpl() {

DCHECK(thread_checker_.CalledOnValidThread());

- if (scheduler())

- scheduler()->OnConnectionStatusChange();

+ scheduler()->OnConnectionStatusChange();

}

void SyncManager::SyncInternal::OnServerConnectionEvent(

@@ -1979,11 +1948,9 @@ SyncStatus SyncManager::SyncInternal::GetStatus() {

void SyncManager::SyncInternal::RequestNudge(

const tracked_objects::Location& location) {

- if (scheduler()) {

- scheduler()->ScheduleNudgeAsync(

- TimeDelta::FromMilliseconds(0), syncer::NUDGE_SOURCE_LOCAL,

- ModelTypeSet(), location);

- }

+ scheduler()->ScheduleNudgeAsync(

+ TimeDelta::FromMilliseconds(0), syncer::NUDGE_SOURCE_LOCAL,

+ ModelTypeSet(), location);

}

TimeDelta SyncManager::SyncInternal::GetNudgeDelayTimeDelta(

@@ -2311,9 +2278,8 @@ void SyncManager::SyncInternal::UpdateNotificationInfo(

void SyncManager::SyncInternal::OnNotificationsEnabled() {

DVLOG(1) << "Notifications enabled";

allstatus_.SetNotificationsEnabled(true);

- if (scheduler()) {

- scheduler()->set_notifications_enabled(true);

- }

+ scheduler()->SetNotificationsEnabled(true);

// TODO(akalin): Separate onNotificationStateChange into

// enabled/disabled events.

if (js_event_handler_.IsInitialized()) {

@@ -2331,9 +2297,7 @@ void SyncManager::SyncInternal::OnNotificationsDisabled(

DVLOG(1) << "Notifications disabled with reason "

<< syncer::NotificationsDisabledReasonToString(reason);

allstatus_.SetNotificationsEnabled(false);

- if (scheduler()) {

- scheduler()->set_notifications_enabled(false);

- }

+ scheduler()->SetNotificationsEnabled(false);

if (js_event_handler_.IsInitialized()) {

DictionaryValue details;

details.Set("enabled", Value::CreateBooleanValue(false));

@@ -2351,19 +2315,15 @@ void SyncManager::SyncInternal::OnIncomingNotification(

syncer::IncomingNotificationSource source) {

DCHECK(thread_checker_.CalledOnValidThread());

if (source == syncer::LOCAL_NOTIFICATION) {

- if (scheduler()) {

- scheduler()->ScheduleNudgeWithPayloadsAsync(

- TimeDelta::FromMilliseconds(kSyncRefreshDelayMsec),

- syncer::NUDGE_SOURCE_LOCAL_REFRESH,

- type_payloads, FROM_HERE);

- }

+ scheduler()->ScheduleNudgeWithPayloadsAsync(

+ TimeDelta::FromMilliseconds(kSyncRefreshDelayMsec),

+ syncer::NUDGE_SOURCE_LOCAL_REFRESH,

+ type_payloads, FROM_HERE);

} else if (!type_payloads.empty()) {

- if (scheduler()) {

- scheduler()->ScheduleNudgeWithPayloadsAsync(

- TimeDelta::FromMilliseconds(kSyncSchedulerDelayMsec),

- syncer::NUDGE_SOURCE_NOTIFICATION,

- type_payloads, FROM_HERE);

- }

+ scheduler()->ScheduleNudgeWithPayloadsAsync(

+ TimeDelta::FromMilliseconds(kSyncSchedulerDelayMsec),

+ syncer::NUDGE_SOURCE_NOTIFICATION,

+ type_payloads, FROM_HERE);

allstatus_.IncrementNotificationsReceived();

UpdateNotificationInfo(type_payloads);

debug_info_event_listener_.OnIncomingNotification(type_payloads);