diff options
Diffstat (limited to 'chrome')
32 files changed, 387 insertions, 613 deletions
diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc index 72e1f6d..5beaa80 100644 --- a/chrome/browser/sync/engine/all_status.cc +++ b/chrome/browser/sync/engine/all_status.cc @@ -9,18 +9,12 @@ #include "base/logging.h" #include "base/port.h" #include "chrome/browser/sync/engine/net/server_connection_manager.h" -#include "chrome/browser/sync/engine/syncer.h" -#include "chrome/browser/sync/engine/syncer_thread.h" #include "chrome/browser/sync/protocol/service_constants.h" #include "chrome/browser/sync/sessions/session_state.h" #include "chrome/browser/sync/syncable/directory_manager.h" -#include "chrome/common/deprecated/event_sys-inl.h" -#include "jingle/notifier/listener/talk_mediator.h" namespace browser_sync { -static const time_t kMinSyncObserveInterval = 10; // seconds - const char* AllStatus::GetSyncStatusString(SyncStatus icon) { const char* strings[] = {"OFFLINE", "OFFLINE_UNSYNCED", "SYNCING", "READY", "CONFLICT", "OFFLINE_UNUSABLE"}; @@ -33,23 +27,12 @@ const char* AllStatus::GetSyncStatusString(SyncStatus icon) { static const AllStatus::Status init_status = { AllStatus::OFFLINE }; -static const AllStatusEvent shutdown_event = - { AllStatusEvent::SHUTDOWN, init_status }; - -AllStatus::AllStatus() : status_(init_status), - channel_(new Channel(shutdown_event)) { +AllStatus::AllStatus() : status_(init_status) { status_.initial_sync_ended = true; status_.notifications_enabled = false; } AllStatus::~AllStatus() { - syncer_thread_hookup_.reset(); - delete channel_; -} - -void AllStatus::WatchSyncerThread(SyncerThread* syncer_thread) { - syncer_thread_hookup_.reset(syncer_thread == NULL ? NULL : - syncer_thread->relay_channel()->AddObserver(this)); } AllStatus::Status AllStatus::CreateBlankStatus() const { @@ -66,7 +49,7 @@ AllStatus::Status AllStatus::CreateBlankStatus() const { return status; } -AllStatus::Status AllStatus::CalcSyncing(const SyncerEvent &event) const { +AllStatus::Status AllStatus::CalcSyncing(const SyncEngineEvent &event) const { Status status = CreateBlankStatus(); const sessions::SyncSessionSnapshot* snapshot = event.snapshot; status.unsynced_count += static_cast<int>(snapshot->unsynced_count); @@ -93,29 +76,7 @@ AllStatus::Status AllStatus::CalcSyncing(const SyncerEvent &event) const { return status; } -int AllStatus::CalcStatusChanges(Status* old_status) { - int what_changed = 0; - - // Calculate what changed and what the new icon should be. - if (status_.syncing != old_status->syncing) - what_changed |= AllStatusEvent::SYNCING; - if (status_.unsynced_count != old_status->unsynced_count) - what_changed |= AllStatusEvent::UNSYNCED_COUNT; - if (status_.server_up != old_status->server_up) - what_changed |= AllStatusEvent::SERVER_UP; - if (status_.server_reachable != old_status->server_reachable) - what_changed |= AllStatusEvent::SERVER_REACHABLE; - if (status_.notifications_enabled != old_status->notifications_enabled) - what_changed |= AllStatusEvent::NOTIFICATIONS_ENABLED; - if (status_.notifications_received != old_status->notifications_received) - what_changed |= AllStatusEvent::NOTIFICATIONS_RECEIVED; - if (status_.notifications_sent != old_status->notifications_sent) - what_changed |= AllStatusEvent::NOTIFICATIONS_SENT; - if (status_.initial_sync_ended != old_status->initial_sync_ended) - what_changed |= AllStatusEvent::INITIAL_SYNC_ENDED; - if (status_.authenticated != old_status->authenticated) - what_changed |= AllStatusEvent::AUTHENTICATED; - +void AllStatus::CalcStatusChanges() { const bool unsynced_changes = status_.unsynced_count > 0; const bool online = status_.authenticated && status_.server_reachable && status_.server_up && !status_.server_broken; @@ -133,47 +94,24 @@ int AllStatus::CalcStatusChanges(Status* old_status) { } else { status_.icon = OFFLINE; } - - if (status_.icon != old_status->icon) - what_changed |= AllStatusEvent::ICON; - - if (0 == what_changed) - return 0; - *old_status = status_; - return what_changed; } -void AllStatus::HandleChannelEvent(const SyncerEvent& event) { - ScopedStatusLockWithNotify lock(this); +void AllStatus::OnSyncEngineEvent(const SyncEngineEvent& event) { + ScopedStatusLock lock(this); switch (event.what_happened) { - case SyncerEvent::COMMITS_SUCCEEDED: - break; - case SyncerEvent::SYNC_CYCLE_ENDED: - case SyncerEvent::STATUS_CHANGED: + case SyncEngineEvent::SYNC_CYCLE_ENDED: + case SyncEngineEvent::STATUS_CHANGED: status_ = CalcSyncing(event); break; - case SyncerEvent::SHUTDOWN_USE_WITH_CARE: - // We're safe to use this value here because we don't call into the syncer - // or block on any processes. - lock.set_notify_plan(DONT_NOTIFY); - syncer_thread_hookup_.reset(); - break; - case SyncerEvent::OVER_QUOTA: - LOG(WARNING) << "User has gone over quota."; - lock.NotifyOverQuota(); - break; - case SyncerEvent::REQUEST_SYNC_NUDGE: - case SyncerEvent::PAUSED: - case SyncerEvent::RESUMED: - case SyncerEvent::WAITING_FOR_CONNECTION: - case SyncerEvent::CONNECTED: - case SyncerEvent::STOP_SYNCING_PERMANENTLY: - case SyncerEvent::SYNCER_THREAD_EXITING: - lock.set_notify_plan(DONT_NOTIFY); + case SyncEngineEvent::SYNCER_THREAD_PAUSED: + case SyncEngineEvent::SYNCER_THREAD_RESUMED: + case SyncEngineEvent::SYNCER_THREAD_WAITING_FOR_CONNECTION: + case SyncEngineEvent::SYNCER_THREAD_CONNECTED: + case SyncEngineEvent::STOP_SYNCING_PERMANENTLY: + case SyncEngineEvent::SYNCER_THREAD_EXITING: break; default: LOG(ERROR) << "Unrecognized Syncer Event: " << event.what_happened; - lock.set_notify_plan(DONT_NOTIFY); break; } } @@ -181,7 +119,7 @@ void AllStatus::HandleChannelEvent(const SyncerEvent& event) { void AllStatus::HandleServerConnectionEvent( const ServerConnectionEvent& event) { if (ServerConnectionEvent::STATUS_CHANGED == event.what_happened) { - ScopedStatusLockWithNotify lock(this); + ScopedStatusLock lock(this); status_.server_up = IsGoodReplyFromServer(event.connection_code); status_.server_reachable = event.server_reachable; @@ -202,40 +140,28 @@ AllStatus::Status AllStatus::status() const { } void AllStatus::SetNotificationsEnabled(bool notifications_enabled) { - ScopedStatusLockWithNotify lock(this); + ScopedStatusLock lock(this); status_.notifications_enabled = notifications_enabled; } void AllStatus::IncrementNotificationsSent() { - ScopedStatusLockWithNotify lock(this); + ScopedStatusLock lock(this); ++status_.notifications_sent; } void AllStatus::IncrementNotificationsReceived() { - ScopedStatusLockWithNotify lock(this); + ScopedStatusLock lock(this); ++status_.notifications_received; } -ScopedStatusLockWithNotify::ScopedStatusLockWithNotify(AllStatus* allstatus) - : allstatus_(allstatus), plan_(NOTIFY_IF_STATUS_CHANGED) { - event_.what_changed = 0; +ScopedStatusLock::ScopedStatusLock(AllStatus* allstatus) + : allstatus_(allstatus) { allstatus->mutex_.Acquire(); - event_.status = allstatus->status_; } -ScopedStatusLockWithNotify::~ScopedStatusLockWithNotify() { - if (DONT_NOTIFY == plan_) { - allstatus_->mutex_.Release(); - return; - } - event_.what_changed |= allstatus_->CalcStatusChanges(&event_.status); +ScopedStatusLock::~ScopedStatusLock() { + allstatus_->CalcStatusChanges(); allstatus_->mutex_.Release(); - if (event_.what_changed) - allstatus_->channel()->NotifyListeners(event_); -} - -void ScopedStatusLockWithNotify::NotifyOverQuota() { - event_.what_changed |= AllStatusEvent::OVER_QUOTA; } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/all_status.h b/chrome/browser/sync/engine/all_status.h index b843a1a..d9947d0 100644 --- a/chrome/browser/sync/engine/all_status.h +++ b/chrome/browser/sync/engine/all_status.h @@ -13,25 +13,20 @@ #include "base/lock.h" #include "base/scoped_ptr.h" -#include "chrome/browser/sync/util/channel.h" -#include "chrome/common/deprecated/event_sys.h" +#include "chrome/browser/sync/engine/syncer_types.h" namespace browser_sync { -class ScopedStatusLockWithNotify; +class ScopedStatusLock; class ServerConnectionManager; class Syncer; class SyncerThread; -struct AllStatusEvent; struct AuthWatcherEvent; struct ServerConnectionEvent; -struct SyncerEvent; -class AllStatus : public ChannelEventHandler<SyncerEvent> { - friend class ScopedStatusLockWithNotify; +class AllStatus : public SyncEngineEventListener { + friend class ScopedStatusLock; public: - typedef EventChannel<AllStatusEvent, Lock> Channel; - // Status of the entire sync process distilled into a single enum. enum SyncStatus { // Can't connect to server, but there are no pending changes in @@ -93,15 +88,12 @@ class AllStatus : public ChannelEventHandler<SyncerEvent> { void HandleAuthWatcherEvent(const AuthWatcherEvent& event); - void WatchSyncerThread(SyncerThread* syncer_thread); - void HandleChannelEvent(const SyncerEvent& event); + virtual void OnSyncEngineEvent(const SyncEngineEvent& event); // Returns a string description of the SyncStatus (currently just the ascii // version of the enum). Will LOG(FATAL) if the status us out of range. static const char* GetSyncStatusString(SyncStatus status); - Channel* channel() const { return channel_; } - Status status() const; void SetNotificationsEnabled(bool notifications_enabled); @@ -111,72 +103,26 @@ class AllStatus : public ChannelEventHandler<SyncerEvent> { void IncrementNotificationsReceived(); protected: - typedef std::map<Syncer*, EventListenerHookup*> Syncers; - // Examines syncer to calculate syncing and the unsynced count, // and returns a Status with new values. - Status CalcSyncing(const SyncerEvent& event) const; + Status CalcSyncing(const SyncEngineEvent& event) const; Status CreateBlankStatus() const; // Examines status to see what has changed, updates old_status in place. - int CalcStatusChanges(Status* old_status); + void CalcStatusChanges(); Status status_; - Channel* const channel_; - scoped_ptr<ChannelHookup<SyncerEvent> > syncer_thread_hookup_; - scoped_ptr<EventListenerHookup> diskfull_hookup_; - scoped_ptr<EventListenerHookup> talk_mediator_hookup_; mutable Lock mutex_; // Protects all data members. DISALLOW_COPY_AND_ASSIGN(AllStatus); }; -struct AllStatusEvent { - enum { // A bit mask of which members have changed. - SHUTDOWN = 0x0000, - ICON = 0x0001, - UNSYNCED_COUNT = 0x0002, - AUTHENTICATED = 0x0004, - SYNCING = 0x0008, - SERVER_UP = 0x0010, - NOTIFICATIONS_ENABLED = 0x0020, - INITIAL_SYNC_ENDED = 0x0080, - SERVER_REACHABLE = 0x0100, - DISK_FULL = 0x0200, - OVER_QUOTA = 0x0400, - NOTIFICATIONS_RECEIVED = 0x0800, - NOTIFICATIONS_SENT = 0x1000, - TRASH_WARNING = 0x40000, - }; - int what_changed; - AllStatus::Status status; - - typedef AllStatusEvent EventType; - static inline bool IsChannelShutdownEvent(const AllStatusEvent& e) { - return SHUTDOWN == e.what_changed; - } -}; - -enum StatusNotifyPlan { - NOTIFY_IF_STATUS_CHANGED, - // A small optimization, don't do the big compare when we know - // nothing has changed. - DONT_NOTIFY, -}; - -class ScopedStatusLockWithNotify { +class ScopedStatusLock { public: - explicit ScopedStatusLockWithNotify(AllStatus* allstatus); - ~ScopedStatusLockWithNotify(); - // Defaults to true, but can be explicitly reset so we don't have to - // do the big compare in the destructor. Small optimization. - - inline void set_notify_plan(StatusNotifyPlan plan) { plan_ = plan; } - void NotifyOverQuota(); + explicit ScopedStatusLock(AllStatus* allstatus); + ~ScopedStatusLock(); protected: - AllStatusEvent event_; - AllStatus* const allstatus_; - StatusNotifyPlan plan_; + AllStatus* allstatus_; }; } // namespace browser_sync diff --git a/chrome/browser/sync/engine/cleanup_disabled_types_command_unittest.cc b/chrome/browser/sync/engine/cleanup_disabled_types_command_unittest.cc index 4cb4edb..9a3e6ab 100644 --- a/chrome/browser/sync/engine/cleanup_disabled_types_command_unittest.cc +++ b/chrome/browser/sync/engine/cleanup_disabled_types_command_unittest.cc @@ -16,8 +16,6 @@ using testing::_; namespace browser_sync { -using sessions::ScopedSessionContextSyncerEventChannel; - class CleanupDisabledTypesCommandTest : public MockDirectorySyncerCommandTest { public: CleanupDisabledTypesCommandTest() { @@ -87,9 +85,6 @@ TEST_F(CleanupDisabledTypesCommandTest, TypeDisabled) { TEST_F(CleanupDisabledTypesCommandTest, SyncerEndCommandSetsPreviousRoutingInfo) { SyncerEndCommand command; - // Need channel for SyncerEndCommand. - scoped_ptr<SyncerEventChannel> c(new SyncerEventChannel()); - ScopedSessionContextSyncerEventChannel s(session()->context(), c.get()); ModelSafeRoutingInfo info; EXPECT_TRUE(info == session()->context()->previous_session_routing_info()); diff --git a/chrome/browser/sync/engine/clear_data_command.cc b/chrome/browser/sync/engine/clear_data_command.cc index a7b7700..e2ceb20 100644 --- a/chrome/browser/sync/engine/clear_data_command.cc +++ b/chrome/browser/sync/engine/clear_data_command.cc @@ -62,16 +62,16 @@ void ClearDataCommand::ExecuteImpl(SyncSession* session) { // On failure, subsequent requests to the server will cause it to attempt // to resume the clear. The client will handle disabling of sync in // response to a store birthday error from the server. - SyncerEvent event(SyncerEvent::CLEAR_SERVER_DATA_FAILED); - session->context()->syncer_event_channel()->Notify(event); + SyncEngineEvent event(SyncEngineEvent::CLEAR_SERVER_DATA_FAILED); + session->context()->NotifyListeners(event); LOG(ERROR) << "Error posting ClearData."; return; } - SyncerEvent event(SyncerEvent::CLEAR_SERVER_DATA_SUCCEEDED); - session->context()->syncer_event_channel()->Notify(event); + SyncEngineEvent event(SyncEngineEvent::CLEAR_SERVER_DATA_SUCCEEDED); + session->context()->NotifyListeners(event); session->delegate()->OnShouldStopSyncingPermanently(); diff --git a/chrome/browser/sync/engine/clear_data_command_unittest.cc b/chrome/browser/sync/engine/clear_data_command_unittest.cc index a11325a..4ae4c8b 100644 --- a/chrome/browser/sync/engine/clear_data_command_unittest.cc +++ b/chrome/browser/sync/engine/clear_data_command_unittest.cc @@ -10,10 +10,11 @@ #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/test/sync/engine/proto_extension_validator.h" #include "chrome/test/sync/engine/syncer_command_test.h" +#include "chrome/test/sync/sessions/test_scoped_session_event_listener.h" namespace browser_sync { -using sessions::ScopedSessionContextSyncerEventChannel; +using sessions::TestScopedSessionEventListener; using syncable::FIRST_REAL_MODEL_TYPE; using syncable::MODEL_TYPE_COUNT; @@ -34,7 +35,7 @@ class ClearDataCommandTest : public SyncerCommandTest { DISALLOW_COPY_AND_ASSIGN(ClearDataCommandTest); }; -class ClearEventHandler : public ChannelEventHandler<SyncerEvent> { +class ClearEventHandler : public SyncEngineEventListener { public: ClearEventHandler() { ResetReceivedEvents(); @@ -46,31 +47,27 @@ class ClearEventHandler : public ChannelEventHandler<SyncerEvent> { received_clear_failed_event_ = false; } - void HandleChannelEvent(const SyncerEvent& event); + virtual void OnSyncEngineEvent(const SyncEngineEvent& event) { + if (event.what_happened == SyncEngineEvent::CLEAR_SERVER_DATA_FAILED) { + received_clear_failed_event_ = true; + } else if (event.what_happened == + SyncEngineEvent::CLEAR_SERVER_DATA_SUCCEEDED) { + received_clear_success_event_ = true; + } + } private: bool received_clear_success_event_; bool received_clear_failed_event_; }; -void ClearEventHandler::HandleChannelEvent(const SyncerEvent& event) { - if (event.what_happened == SyncerEvent::CLEAR_SERVER_DATA_FAILED) { - received_clear_failed_event_ = true; - } else if (event.what_happened == SyncerEvent::CLEAR_SERVER_DATA_SUCCEEDED) { - received_clear_success_event_ = true; - } -} - TEST_F(ClearDataCommandTest, ClearDataCommandExpectFailed) { syncable::ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); ASSERT_TRUE(dir.good()); ConfigureMockServerConnection(); - scoped_ptr<SyncerEventChannel> channel(new SyncerEventChannel()); scoped_ptr<ClearEventHandler> handler(new ClearEventHandler()); - scoped_ptr<browser_sync::ChannelHookup<SyncerEvent> > hookup( - channel.get()->AddObserver(handler.get())); - ScopedSessionContextSyncerEventChannel s_channel(context(), channel.get()); + TestScopedSessionEventListener reg(context(), handler.get()); dir->set_store_birthday(mock_server()->store_birthday()); mock_server()->SetServerNotReachable(); @@ -105,11 +102,8 @@ TEST_F(ClearDataCommandTest, ClearDataCommandExpectSuccess) { ASSERT_TRUE(dir.good()); ConfigureMockServerConnection(); - scoped_ptr<SyncerEventChannel> channel(new SyncerEventChannel()); scoped_ptr<ClearEventHandler> handler(new ClearEventHandler()); - scoped_ptr<browser_sync::ChannelHookup<SyncerEvent> > hookup( - channel.get()->AddObserver(handler.get())); - ScopedSessionContextSyncerEventChannel s_channel(context(), channel.get()); + TestScopedSessionEventListener reg(context(), handler.get()); dir->set_store_birthday(mock_server()->store_birthday()); mock_server()->SetClearUserDataResponseStatus( @@ -123,7 +117,7 @@ TEST_F(ClearDataCommandTest, ClearDataCommandExpectSuccess) { const sync_pb::ClientToServerMessage& r = mock_server()->last_request(); EXPECT_TRUE(r.has_clear_user_data()); - EXPECT_TRUE(handler.get()->ReceievedClearSuccessEvent()); + EXPECT_TRUE(handler->ReceievedClearSuccessEvent()); EXPECT_TRUE(on_should_stop_syncing_permanently_called_); EXPECT_FALSE(handler->ReceievedClearFailedEvent()); diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc index da88447..4677f87 100644 --- a/chrome/browser/sync/engine/download_updates_command.cc +++ b/chrome/browser/sync/engine/download_updates_command.cc @@ -50,9 +50,9 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { } syncable::MultiTypeTimeStamp target = dir->GetTypesWithOldestLastDownloadTimestamp(enabled_types); - LOG(INFO) << "Getting updates from ts " << target.timestamp - << " for types " << target.data_types.to_string() - << " (of possible " << enabled_types.to_string() << ")"; + VLOG(1) << "Getting updates from ts " << target.timestamp + << " for types " << target.data_types.to_string() + << " (of possible " << enabled_types.to_string() << ")"; DCHECK(target.data_types.any()); target.data_types.set(syncable::TOP_LEVEL_FOLDER); // Always fetched. @@ -78,7 +78,7 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { &update_response, session); - DLOG(INFO) << SyncerProtoUtil::ClientToServerResponseDebugString( + VLOG(1) << SyncerProtoUtil::ClientToServerResponseDebugString( update_response); StatusController* status = session->status_controller(); @@ -92,11 +92,11 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { status->mutable_updates_response()->CopyFrom(update_response); - LOG(INFO) << "GetUpdates from ts " << get_updates->from_timestamp() - << " returned " << update_response.get_updates().entries_size() - << " updates and indicated " - << update_response.get_updates().changes_remaining() - << " updates left on server."; + VLOG(1) << "GetUpdates from ts " << get_updates->from_timestamp() + << " returned " << update_response.get_updates().entries_size() + << " updates and indicated " + << update_response.get_updates().changes_remaining() + << " updates left on server."; } void DownloadUpdatesCommand::SetRequestedTypes( diff --git a/chrome/browser/sync/engine/get_commit_ids_command.cc b/chrome/browser/sync/engine/get_commit_ids_command.cc index f4e6334..7348146 100644 --- a/chrome/browser/sync/engine/get_commit_ids_command.cc +++ b/chrome/browser/sync/engine/get_commit_ids_command.cc @@ -41,7 +41,7 @@ void GetCommitIdsCommand::ExecuteImpl(SyncSession* session) { ordered_commit_set_->GetAllCommitIds(); for (size_t i = 0; i < verified_commit_ids.size(); i++) - LOG(INFO) << "Debug commit batch result:" << verified_commit_ids[i]; + VLOG(1) << "Debug commit batch result:" << verified_commit_ids[i]; status->set_commit_set(*ordered_commit_set_.get()); } diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc index 7845c04..a65660e9 100644 --- a/chrome/browser/sync/engine/process_commit_response_command.cc +++ b/chrome/browser/sync/engine/process_commit_response_command.cc @@ -131,7 +131,6 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( int conflicting_commits = 0; int error_commits = 0; int successes = 0; - bool over_quota = false; set<syncable::Id> conflicting_new_folder_ids; set<syncable::Id> deleted_folders; ConflictProgress* conflict_progress = status->mutable_conflict_progress(); @@ -163,7 +162,6 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( status->increment_num_successful_commits(); break; case CommitResponse::OVER_QUOTA: - over_quota = true; // We handle over quota like a retry, which is same as transient. case CommitResponse::RETRY: case CommitResponse::TRANSIENT_ERROR: @@ -195,7 +193,6 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( ResetErrorCounters(status); } SyncerUtil::MarkDeletedChildrenSynced(dir, &deleted_folders); - status->set_over_quota(over_quota); return; } @@ -253,7 +250,7 @@ ProcessCommitResponseCommand::ProcessSingleCommitResponse( return response; } if (CommitResponse::OVER_QUOTA == response) { - LOG(INFO) << "Hit Quota Committing: " << local_entry; + LOG(WARNING) << "Hit deprecated OVER_QUOTA Committing: " << local_entry; return response; } if (!server_entry.has_id_string()) { diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc index cdb14ac..fffab96 100644 --- a/chrome/browser/sync/engine/syncapi.cc +++ b/chrome/browser/sync/engine/syncapi.cc @@ -61,15 +61,15 @@ #include "net/base/network_change_notifier.h" using browser_sync::AllStatus; -using browser_sync::AllStatusEvent; using browser_sync::Cryptographer; using browser_sync::KeyParams; using browser_sync::ModelSafeRoutingInfo; using browser_sync::ModelSafeWorker; using browser_sync::ModelSafeWorkerRegistrar; using browser_sync::ServerConnectionEvent; +using browser_sync::SyncEngineEvent; +using browser_sync::SyncEngineEventListener; using browser_sync::Syncer; -using browser_sync::SyncerEvent; using browser_sync::SyncerThread; using browser_sync::kNigoriTag; using browser_sync::sessions::SyncSessionContext; @@ -913,7 +913,7 @@ class SyncManager::SyncInternal public TalkMediator::Delegate, public sync_notifier::StateWriter, public browser_sync::ChannelEventHandler<syncable::DirectoryChangeEvent>, - public browser_sync::ChannelEventHandler<SyncerEvent>{ + public SyncEngineEventListener { static const int kDefaultNudgeDelayMilliseconds; static const int kPreferencesNudgeDelayMilliseconds; public: @@ -928,7 +928,7 @@ class SyncManager::SyncInternal DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } - ~SyncInternal() { + virtual ~SyncInternal() { DCHECK(!core_message_loop_); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } @@ -976,9 +976,6 @@ class SyncManager::SyncInternal void HandleCalculateChangesChangeEventFromSyncer( const syncable::DirectoryChangeEvent& event); - // This listener is called by the syncer channel for all syncer events. - virtual void HandleChannelEvent(const SyncerEvent& event); - // Listens for notifications from the ServerConnectionManager void HandleServerConnectionEvent(const ServerConnectionEvent& event); @@ -1066,6 +1063,8 @@ class SyncManager::SyncInternal return true; } + // SyncEngineEventListener implementation. + virtual void OnSyncEngineEvent(const SyncEngineEvent& event); private: // Helper to call OnAuthError when no authentication credentials are // available. @@ -1187,9 +1186,6 @@ class SyncManager::SyncInternal // Event listener hookup for the ServerConnectionManager. scoped_ptr<EventListenerHookup> connection_manager_hookup_; - // The event listener hookup registered for HandleSyncerEvent. - scoped_ptr<browser_sync::ChannelHookup<SyncerEvent> > syncer_event_; - // The sync dir_manager to which we belong. SyncManager* const sync_manager_; @@ -1366,15 +1362,17 @@ bool SyncManager::SyncInternal::Init( if (!setup_for_test_mode) { // Build a SyncSessionContext and store the worker in it. LOG(INFO) << "Sync is bringing up SyncSessionContext."; + std::vector<SyncEngineEventListener*> listeners; + listeners.push_back(&allstatus_); + listeners.push_back(this); SyncSessionContext* context = new SyncSessionContext( - connection_manager_.get(), dir_manager(), model_safe_worker_registrar); + connection_manager_.get(), + dir_manager(), + model_safe_worker_registrar, + listeners); // The SyncerThread takes ownership of |context|. syncer_thread_ = new SyncerThread(context); - allstatus_.WatchSyncerThread(syncer_thread()); - - // Subscribe to the syncer thread's channel. - syncer_event_.reset(syncer_thread()->relay_channel()->AddObserver(this)); } return SignIn(credentials); @@ -1572,7 +1570,6 @@ void SyncManager::SyncInternal::Shutdown() { if (!syncer_thread()->Stop(kThreadExitTimeoutMsec)) { LOG(FATAL) << "Unable to stop the syncer, it won't be happy..."; } - syncer_event_.reset(); syncer_thread_ = NULL; } @@ -1586,7 +1583,7 @@ void SyncManager::SyncInternal::Shutdown() { } // Pump any messages the auth watcher, syncer thread, or talk - // mediator posted before they shut down. (See HandleSyncerEvent(), + // mediator posted before they shut down. (See OnSyncEngineEvent(), // and HandleTalkMediatorEvent() for the // events that may be posted.) { @@ -1855,7 +1852,8 @@ SyncManager::Status SyncManager::SyncInternal::ComputeAggregatedStatus() { return return_status; } -void SyncManager::SyncInternal::HandleChannelEvent(const SyncerEvent& event) { +void SyncManager::SyncInternal::OnSyncEngineEvent( + const SyncEngineEvent& event) { if (!observer_) return; @@ -1866,7 +1864,7 @@ void SyncManager::SyncInternal::HandleChannelEvent(const SyncerEvent& event) { // // Notifications are sent at the end of every sync cycle, regardless of // whether we should sync again. - if (event.what_happened == SyncerEvent::SYNC_CYCLE_ENDED) { + if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) { ModelSafeRoutingInfo enabled_types; registrar_->GetModelSafeRoutingInfo(&enabled_types); if (enabled_types.count(syncable::PASSWORDS) > 0) { @@ -1915,32 +1913,32 @@ void SyncManager::SyncInternal::HandleChannelEvent(const SyncerEvent& event) { } } - if (event.what_happened == SyncerEvent::PAUSED) { + if (event.what_happened == SyncEngineEvent::SYNCER_THREAD_PAUSED) { observer_->OnPaused(); return; } - if (event.what_happened == SyncerEvent::RESUMED) { + if (event.what_happened == SyncEngineEvent::SYNCER_THREAD_RESUMED) { observer_->OnResumed(); return; } - if (event.what_happened == SyncerEvent::STOP_SYNCING_PERMANENTLY) { + if (event.what_happened == SyncEngineEvent::STOP_SYNCING_PERMANENTLY) { observer_->OnStopSyncingPermanently(); return; } - if (event.what_happened == SyncerEvent::CLEAR_SERVER_DATA_SUCCEEDED) { + if (event.what_happened == SyncEngineEvent::CLEAR_SERVER_DATA_SUCCEEDED) { observer_->OnClearServerDataSucceeded(); return; } - if (event.what_happened == SyncerEvent::CLEAR_SERVER_DATA_FAILED) { + if (event.what_happened == SyncEngineEvent::CLEAR_SERVER_DATA_FAILED) { observer_->OnClearServerDataFailed(); return; } - if (event.what_happened == SyncerEvent::UPDATED_TOKEN) { + if (event.what_happened == SyncEngineEvent::UPDATED_TOKEN) { observer_->OnUpdatedToken(event.updated_token); return; } diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc index 112f474..1a719b6 100644 --- a/chrome/browser/sync/engine/syncer.cc +++ b/chrome/browser/sync/engine/syncer.cc @@ -56,24 +56,17 @@ using sessions::ConflictProgress; Syncer::Syncer(sessions::SyncSessionContext* context) : early_exit_requested_(false), max_commit_batch_size_(kDefaultMaxCommitBatchSize), - syncer_event_channel_(new SyncerEventChannel()), resolver_scoper_(context, &resolver_), - event_channel_scoper_(context, syncer_event_channel_.get()), context_(context), updates_source_(sync_pb::GetUpdatesCallerInfo::UNKNOWN), pre_conflict_resolution_closure_(NULL) { - shutdown_channel_.reset(new ShutdownChannel()); ScopedDirLookup dir(context->directory_manager(), context->account_name()); // The directory must be good here. CHECK(dir.good()); } -Syncer::~Syncer() { - syncer_event_channel_->Notify( - SyncerEvent(SyncerEvent::SHUTDOWN_USE_WITH_CARE)); - shutdown_channel_->Notify(SyncerShutdownEvent(this)); -} +Syncer::~Syncer() {} bool Syncer::ExitRequested() { AutoLock lock(early_exit_requested_lock_); @@ -85,12 +78,6 @@ void Syncer::RequestEarlyExit() { early_exit_requested_ = true; } -void Syncer::RequestNudge(int milliseconds) { - SyncerEvent event(SyncerEvent::REQUEST_SYNC_NUDGE); - event.nudge_delay_milliseconds = milliseconds; - syncer_event_channel_->Notify(event); -} - bool Syncer::SyncShare(sessions::SyncSession::Delegate* delegate) { sessions::SyncSession session(context_, delegate); return SyncShare(&session); @@ -136,45 +123,45 @@ void Syncer::SyncShare(sessions::SyncSession* session, while (!ExitRequested()) { switch (current_step) { case SYNCER_BEGIN: - LOG(INFO) << "Syncer Begin"; + VLOG(1) << "Syncer Begin"; next_step = CLEANUP_DISABLED_TYPES; break; case CLEANUP_DISABLED_TYPES: { - LOG(INFO) << "Cleaning up disabled types"; + VLOG(1) << "Cleaning up disabled types"; CleanupDisabledTypesCommand cleanup; cleanup.Execute(session); next_step = DOWNLOAD_UPDATES; break; } case DOWNLOAD_UPDATES: { - LOG(INFO) << "Downloading Updates"; + VLOG(1) << "Downloading Updates"; DownloadUpdatesCommand download_updates; download_updates.Execute(session); next_step = PROCESS_CLIENT_COMMAND; break; } case PROCESS_CLIENT_COMMAND: { - LOG(INFO) << "Processing Client Command"; + VLOG(1) << "Processing Client Command"; ProcessClientCommand(session); next_step = VERIFY_UPDATES; break; } case VERIFY_UPDATES: { - LOG(INFO) << "Verifying Updates"; + VLOG(1) << "Verifying Updates"; VerifyUpdatesCommand verify_updates; verify_updates.Execute(session); next_step = PROCESS_UPDATES; break; } case PROCESS_UPDATES: { - LOG(INFO) << "Processing Updates"; + VLOG(1) << "Processing Updates"; ProcessUpdatesCommand process_updates; process_updates.Execute(session); next_step = STORE_TIMESTAMPS; break; } case STORE_TIMESTAMPS: { - LOG(INFO) << "Storing timestamps"; + VLOG(1) << "Storing timestamps"; StoreTimestampsCommand store_timestamps; store_timestamps.Execute(session); // We should download all of the updates before attempting to process @@ -188,7 +175,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case APPLY_UPDATES: { - LOG(INFO) << "Applying Updates"; + VLOG(1) << "Applying Updates"; ApplyUpdatesCommand apply_updates; apply_updates.Execute(session); next_step = BUILD_COMMIT_REQUEST; @@ -199,7 +186,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, case BUILD_COMMIT_REQUEST: { session->status_controller()->set_syncing(true); - LOG(INFO) << "Processing Commit Request"; + VLOG(1) << "Processing Commit Request"; ScopedDirLookup dir(context_->directory_manager(), context_->account_name()); if (!dir.good()) { @@ -209,12 +196,12 @@ void Syncer::SyncShare(sessions::SyncSession* session, WriteTransaction trans(dir, SYNCER, __FILE__, __LINE__); sessions::ScopedSetSessionWriteTransaction set_trans(session, &trans); - LOG(INFO) << "Getting the Commit IDs"; + VLOG(1) << "Getting the Commit IDs"; GetCommitIdsCommand get_commit_ids_command(max_commit_batch_size_); get_commit_ids_command.Execute(session); if (!session->status_controller()->commit_ids().empty()) { - LOG(INFO) << "Building a commit message"; + VLOG(1) << "Building a commit message"; BuildCommitCommand build_commit_command; build_commit_command.Execute(session); @@ -226,14 +213,14 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case POST_COMMIT_MESSAGE: { - LOG(INFO) << "Posting a commit request"; + VLOG(1) << "Posting a commit request"; PostCommitMessageCommand post_commit_command; post_commit_command.Execute(session); next_step = PROCESS_COMMIT_RESPONSE; break; } case PROCESS_COMMIT_RESPONSE: { - LOG(INFO) << "Processing the commit response"; + VLOG(1) << "Processing the commit response"; session->status_controller()->reset_num_conflicting_commits(); ProcessCommitResponseCommand process_response_command; process_response_command.Execute(session); @@ -241,7 +228,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case BUILD_AND_PROCESS_CONFLICT_SETS: { - LOG(INFO) << "Building and Processing Conflict Sets"; + VLOG(1) << "Building and Processing Conflict Sets"; BuildAndProcessConflictSetsCommand build_process_conflict_sets; build_process_conflict_sets.Execute(session); if (session->status_controller()->conflict_sets_built()) @@ -251,7 +238,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case RESOLVE_CONFLICTS: { - LOG(INFO) << "Resolving Conflicts"; + VLOG(1) << "Resolving Conflicts"; // Trigger the pre_conflict_resolution_closure_, which is a testing // hook for the unit tests, if it is non-NULL. @@ -271,7 +258,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, } case APPLY_UPDATES_TO_RESOLVE_CONFLICTS: { StatusController* status = session->status_controller(); - LOG(INFO) << "Applying updates to resolve conflicts"; + VLOG(1) << "Applying updates to resolve conflicts"; ApplyUpdatesCommand apply_updates; int before_conflicting_updates = status->TotalNumConflictingItems(); apply_updates.Execute(session); @@ -285,13 +272,13 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case CLEAR_PRIVATE_DATA: { - LOG(INFO) << "Clear Private Data"; + VLOG(1) << "Clear Private Data"; ClearDataCommand clear_data_command; clear_data_command.Execute(session); next_step = SYNCER_END; } case SYNCER_END: { - LOG(INFO) << "Syncer End"; + VLOG(1) << "Syncer End"; SyncerEndCommand syncer_end_command; // This will set "syncing" to false, and send out a notification. syncer_end_command.Execute(session); diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h index 9a07400..e2d1c65 100644 --- a/chrome/browser/sync/engine/syncer.h +++ b/chrome/browser/sync/engine/syncer.h @@ -103,14 +103,6 @@ class Syncer { // Limit the batch size of commit operations to a specified number of items. void set_max_commit_batch_size(int x) { max_commit_batch_size_ = x; } - ShutdownChannel* shutdown_channel() const { return shutdown_channel_.get(); } - - // Syncer will take ownership of this channel and it will be destroyed along - // with the Syncer instance. - void set_shutdown_channel(ShutdownChannel* channel) { - shutdown_channel_.reset(channel); - } - // Volatile reader for the source member of the syncer session object. The // value is set to the SYNC_CYCLE_CONTINUATION value to signal that it has // been read. @@ -147,13 +139,9 @@ class Syncer { int32 max_commit_batch_size_; ConflictResolver resolver_; - scoped_ptr<SyncerEventChannel> syncer_event_channel_; sessions::ScopedSessionContextConflictResolver resolver_scoper_; - sessions::ScopedSessionContextSyncerEventChannel event_channel_scoper_; sessions::SyncSessionContext* context_; - scoped_ptr<ShutdownChannel> shutdown_channel_; - // The source of the last nudge. sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source_; diff --git a/chrome/browser/sync/engine/syncer_command.cc b/chrome/browser/sync/engine/syncer_command.cc index 4e13918..fbcc9fe 100644 --- a/chrome/browser/sync/engine/syncer_command.cc +++ b/chrome/browser/sync/engine/syncer_command.cc @@ -29,16 +29,10 @@ void SyncerCommand::SendNotifications(SyncSession* session) { } if (session->status_controller()->TestAndClearIsDirty()) { - SyncerEvent event(SyncerEvent::STATUS_CHANGED); + SyncEngineEvent event(SyncEngineEvent::STATUS_CHANGED); const sessions::SyncSessionSnapshot& snapshot(session->TakeSnapshot()); event.snapshot = &snapshot; - DCHECK(session->context()->syncer_event_channel()); - session->context()->syncer_event_channel()->Notify(event); - if (session->status_controller()->syncer_status().over_quota) { - SyncerEvent quota_event(SyncerEvent::OVER_QUOTA); - quota_event.snapshot = &snapshot; - session->context()->syncer_event_channel()->Notify(quota_event); - } + session->context()->NotifyListeners(event); } } diff --git a/chrome/browser/sync/engine/syncer_end_command.cc b/chrome/browser/sync/engine/syncer_end_command.cc index 0b8792d..1311b57 100644 --- a/chrome/browser/sync/engine/syncer_end_command.cc +++ b/chrome/browser/sync/engine/syncer_end_command.cc @@ -41,10 +41,10 @@ void SyncerEndCommand::ExecuteImpl(sessions::SyncSession* session) { } } - SyncerEvent event(SyncerEvent::SYNC_CYCLE_ENDED); + SyncEngineEvent event(SyncEngineEvent::SYNC_CYCLE_ENDED); sessions::SyncSessionSnapshot snapshot(session->TakeSnapshot()); event.snapshot = &snapshot; - session->context()->syncer_event_channel()->Notify(event); + session->context()->NotifyListeners(event); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/syncer_proto_util.cc b/chrome/browser/sync/engine/syncer_proto_util.cc index 2cc2300..38531d9 100644 --- a/chrome/browser/sync/engine/syncer_proto_util.cc +++ b/chrome/browser/sync/engine/syncer_proto_util.cc @@ -144,9 +144,9 @@ bool SyncerProtoUtil::PostAndProcessHeaders(ServerConnectionManager* scm, std::string new_token = http_response.update_client_auth_header; if (!new_token.empty()) { - SyncerEvent event(SyncerEvent::UPDATED_TOKEN); + SyncEngineEvent event(SyncEngineEvent::UPDATED_TOKEN); event.updated_token = new_token; - session->context()->syncer_event_channel()->Notify(event); + session->context()->NotifyListeners(event); } if (response->ParseFromString(rx)) { diff --git a/chrome/browser/sync/engine/syncer_thread.cc b/chrome/browser/sync/engine/syncer_thread.cc index 92322b4..10d28be 100644 --- a/chrome/browser/sync/engine/syncer_thread.cc +++ b/chrome/browser/sync/engine/syncer_thread.cc @@ -66,18 +66,14 @@ SyncerThread::SyncerThread(sessions::SyncSessionContext* context) : thread_main_started_(false, false), thread_("SyncEngine_SyncerThread"), vault_field_changed_(&lock_), - p2p_authenticated_(false), - p2p_subscribed_(false), conn_mgr_hookup_(NULL), syncer_short_poll_interval_seconds_(kDefaultShortPollIntervalSeconds), syncer_long_poll_interval_seconds_(kDefaultLongPollIntervalSeconds), syncer_polling_interval_(kDefaultShortPollIntervalSeconds), syncer_max_interval_(kDefaultMaxPollIntervalMs), - syncer_events_(NULL), session_context_(context), disable_idle_detection_(false) { DCHECK(context); - syncer_event_relay_channel_.reset(new SyncerEventChannel()); if (context->connection_manager()) WatchConnectionManager(context->connection_manager()); @@ -86,10 +82,6 @@ SyncerThread::SyncerThread(sessions::SyncSessionContext* context) SyncerThread::~SyncerThread() { conn_mgr_hookup_.reset(); - syncer_event_relay_channel_->Notify(SyncerEvent( - SyncerEvent::SHUTDOWN_USE_WITH_CARE)); - syncer_event_relay_channel_.reset(); - syncer_events_.reset(); delete vault_.syncer_; CHECK(!thread_.IsRunning()); } @@ -182,6 +174,10 @@ bool SyncerThread::RequestPause() { return true; } +void SyncerThread::Notify(SyncEngineEvent::EventCause cause) { + session_context_->NotifyListeners(SyncEngineEvent(cause)); +} + bool SyncerThread::RequestResume() { AutoLock lock(lock_); // Only valid to request a resume when we are already paused or we @@ -193,8 +189,7 @@ bool SyncerThread::RequestResume() { if (vault_.pause_requested_) { // If pause was requested we have not yet paused. In this case, // the resume cancels the pause request. - SyncerEvent event(SyncerEvent::RESUMED); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::SYNCER_THREAD_RESUMED); LOG(INFO) << "Pending pause canceled by resume."; } else { // Unpause and notify. @@ -235,9 +230,7 @@ bool SyncerThread::IsSyncingCurrentlySilenced() { void SyncerThread::OnShouldStopSyncingPermanently() { RequestSyncerExitAndSetThreadStopConditions(); - - SyncerEvent event(SyncerEvent::STOP_SYNCING_PERMANENTLY); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::STOP_SYNCING_PERMANENTLY); } void SyncerThread::ThreadMainLoop() { @@ -342,8 +335,7 @@ void SyncerThread::ThreadMainLoop() { void SyncerThread::WaitUntilConnectedOrQuit() { LOG(INFO) << "Syncer thread waiting for connection."; - SyncerEvent event(SyncerEvent::WAITING_FOR_CONNECTION); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::SYNCER_THREAD_WAITING_FOR_CONNECTION); bool is_paused = vault_.paused_; @@ -366,8 +358,7 @@ void SyncerThread::WaitUntilConnectedOrQuit() { } if (!vault_.stop_syncer_thread_) { - SyncerEvent event(SyncerEvent::CONNECTED); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::SYNCER_THREAD_CONNECTED); LOG(INFO) << "Syncer thread found connection."; } } @@ -396,16 +387,14 @@ void SyncerThread::EnterPausedState() { vault_.pause_requested_ = false; vault_.paused_ = true; vault_field_changed_.Broadcast(); - SyncerEvent event(SyncerEvent::PAUSED); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::SYNCER_THREAD_PAUSED); } void SyncerThread::ExitPausedState() { lock_.AssertAcquired(); vault_.paused_ = false; vault_field_changed_.Broadcast(); - SyncerEvent event(SyncerEvent::RESUMED); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::SYNCER_THREAD_RESUMED); } // We check how long the user's been idle and sync less often if the machine is @@ -497,8 +486,7 @@ void SyncerThread::ThreadMain() { thread_main_started_.Signal(); ThreadMainLoop(); LOG(INFO) << "Syncer thread ThreadMain is done."; - SyncerEvent event(SyncerEvent::SYNCER_THREAD_EXITING); - relay_channel()->Notify(event); + Notify(SyncEngineEvent::SYNCER_THREAD_EXITING); } void SyncerThread::SyncMain(Syncer* syncer) { @@ -575,15 +563,6 @@ void SyncerThread::SetUpdatesSource(bool nudged, NudgeSource nudge_source, vault_.syncer_->set_updates_source(updates_source); } -void SyncerThread::HandleChannelEvent(const SyncerEvent& event) { - AutoLock lock(lock_); - relay_channel()->Notify(event); - if (SyncerEvent::REQUEST_SYNC_NUDGE != event.what_happened) { - return; - } - NudgeSyncImpl(event.nudge_delay_milliseconds, kUnknown); -} - void SyncerThread::CreateSyncer(const std::string& dirname) { AutoLock lock(lock_); LOG(INFO) << "Creating syncer up for: " << dirname; @@ -592,9 +571,6 @@ void SyncerThread::CreateSyncer(const std::string& dirname) { CHECK(vault_.syncer_ == NULL); session_context_->set_account_name(dirname); vault_.syncer_ = new Syncer(session_context_.get()); - - syncer_events_.reset( - session_context_->syncer_event_channel()->AddObserver(this)); vault_field_changed_.Broadcast(); } @@ -643,10 +619,6 @@ void SyncerThread::HandleServerConnectionEvent( } } -SyncerEventChannel* SyncerThread::relay_channel() { - return syncer_event_relay_channel_.get(); -} - int SyncerThread::GetRecommendedDelaySeconds(int base_delay_seconds) { if (base_delay_seconds >= kMaxBackoffSeconds) return kMaxBackoffSeconds; diff --git a/chrome/browser/sync/engine/syncer_thread.h b/chrome/browser/sync/engine/syncer_thread.h index 3bf96c3..60d986e 100644 --- a/chrome/browser/sync/engine/syncer_thread.h +++ b/chrome/browser/sync/engine/syncer_thread.h @@ -23,6 +23,7 @@ #if defined(OS_LINUX) #include "chrome/browser/sync/engine/idle_query_linux.h" #endif +#include "chrome/browser/sync/engine/syncer_types.h" #include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/common/deprecated/event_sys-inl.h" @@ -35,12 +36,9 @@ class ServerConnectionManager; class Syncer; class URLFactory; struct ServerConnectionEvent; -struct SyncerEvent; -struct SyncerShutdownEvent; class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, - public sessions::SyncSession::Delegate, - public ChannelEventHandler<SyncerEvent> { + public sessions::SyncSession::Delegate { FRIEND_TEST_ALL_PREFIXES(SyncerThreadTest, CalculateSyncWaitTime); FRIEND_TEST_ALL_PREFIXES(SyncerThreadTest, CalculatePollingWaitTime); FRIEND_TEST_ALL_PREFIXES(SyncerThreadWithSyncerTest, Polling); @@ -114,14 +112,14 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, // Request that the thread pauses. Returns false if the request can // not be completed (e.g. the thread is not running). When the - // thread actually pauses, a SyncerEvent::PAUSED event notification + // thread actually pauses, a SyncEngineEvent::PAUSED event notification // will be sent to the relay channel. virtual bool RequestPause(); // Request that the thread resumes from pause. Returns false if the // request can not be completed (e.g. the thread is not running or // is not currently paused). When the thread actually resumes, a - // SyncerEvent::RESUMED event notification will be sent to the relay + // SyncEngineEvent::RESUMED event notification will be sent to the relay // channel. virtual bool RequestResume(); @@ -131,8 +129,6 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, void SetNotificationsEnabled(bool notifications_enabled); - virtual SyncerEventChannel* relay_channel(); - // Call this when a directory is opened void CreateSyncer(const std::string& dirname); @@ -219,10 +215,6 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, // Threshold multipler for how long before user should be considered idle. static const int kPollBackoffThresholdMultiplier = 10; - friend void* RunSyncerThread(void* syncer_thread); - void* Run(); - void HandleChannelEvent(const SyncerEvent& event); - // SyncSession::Delegate implementation. virtual void OnSilencedUntil(const base::TimeTicks& silenced_until); virtual bool IsSyncingCurrentlySilenced(); @@ -287,9 +279,7 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, // time after to fully stop and clean up. void RequestSyncerExitAndSetThreadStopConditions(); - // State of the notification framework is tracked by these values. - bool p2p_authenticated_; - bool p2p_subscribed_; + void Notify(SyncEngineEvent::EventCause cause); scoped_ptr<EventListenerHookup> conn_mgr_hookup_; @@ -312,8 +302,6 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, // this is called. void NudgeSyncImpl(int milliseconds_from_now, NudgeSource source); - scoped_ptr<ChannelHookup<SyncerEvent> > syncer_events_; - #if defined(OS_LINUX) // On Linux, we need this information in order to query idle time. scoped_ptr<IdleQueryLinux> idle_query_; @@ -321,13 +309,6 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, scoped_ptr<sessions::SyncSessionContext> session_context_; - // Events from the Syncer's syncer_event_channel are first processed by the - // SyncerThread and then get relayed onto this channel for consumers. - // TODO(timsteele): Wow did this confused me. I had removed the channel from - // here thinking there was only one, and then realized this relay was - // happening. Is this strict event handling order needed?! - scoped_ptr<SyncerEventChannel> syncer_event_relay_channel_; - // Set whenever the server instructs us to stop sending it requests until // a specified time, and reset for each call to SyncShare. (Note that the // WaitInterval::THROTTLED contract is such that we don't call SyncShare at diff --git a/chrome/browser/sync/engine/syncer_thread_unittest.cc b/chrome/browser/sync/engine/syncer_thread_unittest.cc index ded07f0..f4af377 100644 --- a/chrome/browser/sync/engine/syncer_thread_unittest.cc +++ b/chrome/browser/sync/engine/syncer_thread_unittest.cc @@ -4,7 +4,6 @@ #include <list> #include <map> -#include <set> #include "base/lock.h" #include "base/scoped_ptr.h" @@ -17,6 +16,7 @@ #include "chrome/browser/sync/util/channel.h" #include "chrome/test/sync/engine/mock_connection_manager.h" #include "chrome/test/sync/engine/test_directory_setter_upper.h" +#include "chrome/test/sync/sessions/test_scoped_session_event_listener.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -29,6 +29,7 @@ using testing::Field; namespace browser_sync { using sessions::ErrorCounters; +using sessions::TestScopedSessionEventListener; using sessions::SyncSessionContext; using sessions::SyncSessionSnapshot; using sessions::SyncerStatus; @@ -48,14 +49,14 @@ SyncSessionSnapshot SessionSnapshotForTest( syncable::ModelTypeBitSet(), false, false, unsynced_count, 0, false); } -class ListenerMock : public ChannelEventHandler<SyncerEvent> { +class ListenerMock : public SyncEngineEventListener { public: - MOCK_METHOD1(HandleChannelEvent, void(const SyncerEvent&)); + MOCK_METHOD1(OnSyncEngineEvent, void(const SyncEngineEvent&)); }; class SyncerThreadWithSyncerTest : public testing::Test, public ModelSafeWorkerRegistrar, - public ChannelEventHandler<SyncerEvent> { + public SyncEngineEventListener { public: SyncerThreadWithSyncerTest() : max_wait_time_(TimeDelta::FromSeconds(10)), @@ -65,18 +66,18 @@ class SyncerThreadWithSyncerTest : public testing::Test, connection_.reset(new MockConnectionManager(metadb_.manager(), metadb_.name())); worker_ = new ModelSafeWorker(); - SyncSessionContext* context = new SyncSessionContext(connection_.get(), - metadb_.manager(), this); - syncer_thread_ = new SyncerThread(context); - syncer_event_hookup_.reset( - syncer_thread_->relay_channel()->AddObserver(this)); + std::vector<SyncEngineEventListener*> listeners; + listeners.push_back(this); + context_ = new SyncSessionContext(connection_.get(), metadb_.manager(), + this, listeners); + syncer_thread_ = new SyncerThread(context_); syncer_thread_->SetConnected(true); syncable::ModelTypeBitSet expected_types; expected_types[syncable::BOOKMARKS] = true; connection_->ExpectGetUpdatesRequestTypes(expected_types); } virtual void TearDown() { - syncer_event_hookup_.reset(); + context_ = NULL; syncer_thread_ = NULL; connection_.reset(); metadb_.TearDown(); @@ -127,8 +128,9 @@ class SyncerThreadWithSyncerTest : public testing::Test, WaitableEvent event(false, false); { AutoLock lock(syncer_thread()->lock_); - EXPECT_CALL(*listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::PAUSED))). + EXPECT_CALL(*listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_PAUSED))). WillOnce(SignalEvent(&event)); } if (!syncer_thread()->RequestPause()) @@ -140,8 +142,9 @@ class SyncerThreadWithSyncerTest : public testing::Test, WaitableEvent event(false, false); { AutoLock lock(syncer_thread()->lock_); - EXPECT_CALL(*listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::RESUMED))). + EXPECT_CALL(*listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_RESUMED))). WillOnce(SignalEvent(&event)); } if (!syncer_thread()->RequestResume()) @@ -156,20 +159,20 @@ class SyncerThreadWithSyncerTest : public testing::Test, private: - void HandleChannelEvent(const SyncerEvent& event) { - if (event.what_happened == SyncerEvent::SYNC_CYCLE_ENDED) + virtual void OnSyncEngineEvent(const SyncEngineEvent& event) { + if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) sync_cycle_ended_event_.Signal(); } protected: TimeDelta max_wait_time_; + SyncSessionContext* context_; private: ManuallyOpenedTestDirectorySetterUpper metadb_; scoped_ptr<MockConnectionManager> connection_; scoped_refptr<SyncerThread> syncer_thread_; scoped_refptr<ModelSafeWorker> worker_; - scoped_ptr<ChannelHookup<SyncerEvent> > syncer_event_hookup_; base::WaitableEvent sync_cycle_ended_event_; DISALLOW_COPY_AND_ASSIGN(SyncerThreadWithSyncerTest); }; @@ -217,12 +220,14 @@ class SyncShareIntercept }; TEST_F(SyncerThreadTest, Construction) { - SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL, + std::vector<SyncEngineEventListener*>()); scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context)); } TEST_F(SyncerThreadTest, StartStop) { - SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL, + std::vector<SyncEngineEventListener*>()); scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context)); EXPECT_TRUE(syncer_thread->Start()); EXPECT_TRUE(syncer_thread->Stop(2000)); @@ -247,7 +252,8 @@ TEST(SyncerThread, GetRecommendedDelay) { } TEST_F(SyncerThreadTest, CalculateSyncWaitTime) { - SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL, + std::vector<SyncEngineEventListener*>()); scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context)); syncer_thread->DisableIdleDetection(); @@ -307,7 +313,8 @@ TEST_F(SyncerThreadTest, CalculateSyncWaitTime) { TEST_F(SyncerThreadTest, CalculatePollingWaitTime) { // Set up the environment. int user_idle_milliseconds_param = 0; - SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL, + std::vector<SyncEngineEventListener*>()); scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context)); syncer_thread->DisableIdleDetection(); // Hold the lock to appease asserts in code. @@ -731,23 +738,25 @@ TEST_F(SyncerThreadWithSyncerTest, StopSyncPermanently) { ListenerMock listener; WaitableEvent sync_cycle_ended_event(false, false); WaitableEvent syncer_thread_exiting_event(false, false); - scoped_ptr<ChannelHookup<SyncerEvent> > hookup; - hookup.reset(syncer_thread()->relay_channel()->AddObserver(&listener)); + TestScopedSessionEventListener reg(context_, &listener); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::STATUS_CHANGED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::STATUS_CHANGED))). Times(AnyNumber()); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). Times(AnyNumber()). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, - SyncerEvent::STOP_SYNCING_PERMANENTLY))); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNCER_THREAD_EXITING))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::STOP_SYNCING_PERMANENTLY))); + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_EXITING))). WillOnce(SignalEvent(&syncer_thread_exiting_event)); EXPECT_TRUE(syncer_thread()->Start()); @@ -815,19 +824,21 @@ TEST_F(SyncerThreadWithSyncerTest, FLAKY_Pause) { PreventThreadFromPolling(); ListenerMock listener; - scoped_ptr<ChannelHookup<SyncerEvent> > hookup; - hookup.reset(syncer_thread()->relay_channel()->AddObserver(&listener)); - - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::STATUS_CHANGED))). + TestScopedSessionEventListener reg(context_, &listener); + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::STATUS_CHANGED))). Times(AnyNumber()); // Wait for the initial sync to complete. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNCER_THREAD_EXITING))); + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_EXITING))); + ASSERT_TRUE(syncer_thread()->Start()); metadb()->Open(); syncer_thread()->CreateSyncer(metadb()->name()); @@ -849,8 +860,9 @@ TEST_F(SyncerThreadWithSyncerTest, FLAKY_Pause) { syncer_thread()->NudgeSyncer(0, SyncerThread::kUnknown); // Resuming will cause the nudge to be processed and a sync cycle to run. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); ASSERT_TRUE(Resume(&listener)); ASSERT_TRUE(sync_cycle_ended_event.TimedWait(max_wait_time_)); @@ -862,15 +874,16 @@ TEST_F(SyncerThreadWithSyncerTest, StartWhenNotConnected) { WaitableEvent sync_cycle_ended_event(false, false); WaitableEvent event(false, false); ListenerMock listener; - scoped_ptr<ChannelHookup<SyncerEvent> > hookup; - hookup.reset(syncer_thread()->relay_channel()->AddObserver(&listener)); + TestScopedSessionEventListener reg(context_, &listener); PreventThreadFromPolling(); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::STATUS_CHANGED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::STATUS_CHANGED))). Times(AnyNumber()); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNCER_THREAD_EXITING))); + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_EXITING))); connection()->SetServerNotReachable(); metadb()->Open(); @@ -878,26 +891,30 @@ TEST_F(SyncerThreadWithSyncerTest, StartWhenNotConnected) { // Syncer thread will always go through once cycle at the start, // then it will wait for a connection. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::WAITING_FOR_CONNECTION))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_WAITING_FOR_CONNECTION))). WillOnce(SignalEvent(&event)); ASSERT_TRUE(syncer_thread()->Start()); ASSERT_TRUE(sync_cycle_ended_event.TimedWait(max_wait_time_)); ASSERT_TRUE(event.TimedWait(max_wait_time_)); // Connect, will put the syncer thread into its usually poll wait. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::CONNECTED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_CONNECTED))). WillOnce(SignalEvent(&event)); connection()->SetServerReachable(); ASSERT_TRUE(event.TimedWait(max_wait_time_)); // Nudge the syncer to complete a cycle. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); syncer_thread()->NudgeSyncer(0, SyncerThread::kUnknown); ASSERT_TRUE(sync_cycle_ended_event.TimedWait(max_wait_time_)); @@ -912,21 +929,23 @@ TEST_F(SyncerThreadWithSyncerTest, DISABLED_PauseWhenNotConnected) { WaitableEvent sync_cycle_ended_event(false, false); WaitableEvent event(false, false); ListenerMock listener; - scoped_ptr<ChannelHookup<SyncerEvent> > hookup; - hookup.reset(syncer_thread()->relay_channel()->AddObserver(&listener)); + TestScopedSessionEventListener reg(context_, &listener); PreventThreadFromPolling(); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::STATUS_CHANGED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::STATUS_CHANGED))). Times(AnyNumber()); // Put the thread into a "waiting for connection" state. connection()->SetServerNotReachable(); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::WAITING_FOR_CONNECTION))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_WAITING_FOR_CONNECTION))). WillOnce(SignalEvent(&event)); metadb()->Open(); syncer_thread()->CreateSyncer(metadb()->name()); @@ -940,11 +959,13 @@ TEST_F(SyncerThreadWithSyncerTest, DISABLED_PauseWhenNotConnected) { ASSERT_TRUE(Resume(&listener)); // Make a connection and let the syncer cycle. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::CONNECTED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_CONNECTED))). WillOnce(SignalEvent(&event)); connection()->SetServerReachable(); ASSERT_TRUE(event.TimedWait(max_wait_time_)); @@ -952,8 +973,9 @@ TEST_F(SyncerThreadWithSyncerTest, DISABLED_PauseWhenNotConnected) { ASSERT_TRUE(sync_cycle_ended_event.TimedWait(max_wait_time_)); // Disconnect and get into the waiting for a connection state. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::WAITING_FOR_CONNECTION))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_WAITING_FOR_CONNECTION))). WillOnce(SignalEvent(&event)); connection()->SetServerNotReachable(); ASSERT_TRUE(event.TimedWait(max_wait_time_)); @@ -962,8 +984,9 @@ TEST_F(SyncerThreadWithSyncerTest, DISABLED_PauseWhenNotConnected) { ASSERT_TRUE(Pause(&listener)); // Get a connection then resume. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::CONNECTED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_CONNECTED))). WillOnce(SignalEvent(&event)); connection()->SetServerReachable(); ASSERT_TRUE(event.TimedWait(max_wait_time_)); @@ -971,11 +994,13 @@ TEST_F(SyncerThreadWithSyncerTest, DISABLED_PauseWhenNotConnected) { ASSERT_TRUE(Resume(&listener)); // Cycle the syncer to show we are not longer paused. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNCER_THREAD_EXITING))); + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_EXITING))); syncer_thread()->NudgeSyncer(0, SyncerThread::kUnknown); ASSERT_TRUE(sync_cycle_ended_event.TimedWait(max_wait_time_)); @@ -987,12 +1012,12 @@ TEST_F(SyncerThreadWithSyncerTest, PauseResumeWhenNotRunning) { WaitableEvent sync_cycle_ended_event(false, false); WaitableEvent event(false, false); ListenerMock listener; - scoped_ptr<ChannelHookup<SyncerEvent> > hookup; - hookup.reset(syncer_thread()->relay_channel()->AddObserver(&listener)); + TestScopedSessionEventListener reg(context_, &listener); PreventThreadFromPolling(); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::STATUS_CHANGED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::STATUS_CHANGED))). Times(AnyNumber()); // Pause and resume the syncer while not running @@ -1006,11 +1031,13 @@ TEST_F(SyncerThreadWithSyncerTest, PauseResumeWhenNotRunning) { ASSERT_TRUE(syncer_thread()->Start()); // Resume and let the syncer cycle. - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNC_CYCLE_ENDED))). + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNC_CYCLE_ENDED))). WillOnce(SignalEvent(&sync_cycle_ended_event)); - EXPECT_CALL(listener, HandleChannelEvent( - Field(&SyncerEvent::what_happened, SyncerEvent::SYNCER_THREAD_EXITING))); + EXPECT_CALL(listener, OnSyncEngineEvent( + Field(&SyncEngineEvent::what_happened, + SyncEngineEvent::SYNCER_THREAD_EXITING))); ASSERT_TRUE(Resume(&listener)); ASSERT_TRUE(sync_cycle_ended_event.TimedWait(max_wait_time_)); diff --git a/chrome/browser/sync/engine/syncer_types.h b/chrome/browser/sync/engine/syncer_types.h index 7be16b0..0ed1f19 100644 --- a/chrome/browser/sync/engine/syncer_types.h +++ b/chrome/browser/sync/engine/syncer_types.h @@ -9,6 +9,7 @@ #include <map> #include <vector> +#include "base/observer_list.h" #include "chrome/browser/sync/util/channel.h" namespace syncable { @@ -70,46 +71,44 @@ enum VerifyCommitResult { VERIFY_OK, }; -struct SyncerEvent { - typedef SyncerEvent EventType; - +struct SyncEngineEvent { enum EventCause { - COMMITS_SUCCEEDED, // Count is stored in successful_commit_count. - + //////////////////////////////////////////////////////////////// + // SyncerCommand generated events. STATUS_CHANGED, - UPDATED_TOKEN, // new token in updated_token - - // Take care not to wait in shutdown handlers for the syncer to stop as it - // causes a race in the event system. Use SyncerShutdownEvent instead. - SHUTDOWN_USE_WITH_CARE, - - // We're over our quota. - OVER_QUOTA, - - // This event is how the syncer requests that it be synced. - REQUEST_SYNC_NUDGE, - // We have reached the SYNCER_END state in the main sync loop. // Check the SyncerSession for information like whether we need to continue // syncing (SyncerSession::HasMoreToSync). SYNC_CYCLE_ENDED, + //////////////////////////////////////////////////////////////// + // SyncerThread generated events. + // This event is sent when the thread is paused in response to a // pause request. - PAUSED, + SYNCER_THREAD_PAUSED, // This event is sent when the thread is resumed in response to a // resume request. - RESUMED, + SYNCER_THREAD_RESUMED, // This event is sent when the thread is waiting for a connection // to be established. - WAITING_FOR_CONNECTION, + SYNCER_THREAD_WAITING_FOR_CONNECTION, // This event is sent when a connection has been established and // the thread continues. - CONNECTED, + SYNCER_THREAD_CONNECTED, + + // Sent when the main syncer loop finishes. + SYNCER_THREAD_EXITING, + + //////////////////////////////////////////////////////////////// + // Generated in response to specific protocol actions or events. + + // New token in updated_token. + UPDATED_TOKEN, // This is sent after the Syncer (and SyncerThread) have initiated self // halt due to no longer being permitted to communicate with the server. @@ -121,52 +120,30 @@ struct SyncerEvent { // server data have failed or succeeded. CLEAR_SERVER_DATA_SUCCEEDED, CLEAR_SERVER_DATA_FAILED, - - // Sent when the main syncer loop finishes. - SYNCER_THREAD_EXITING, }; - explicit SyncerEvent(EventCause cause) : what_happened(cause), - snapshot(NULL), - successful_commit_count(0), - nudge_delay_milliseconds(0) {} - - static bool IsChannelShutdownEvent(const SyncerEvent& e) { - return SHUTDOWN_USE_WITH_CARE == e.what_happened; - } - - // This is used to put SyncerEvents into sorted STL structures. - bool operator < (const SyncerEvent& r) const { - return this->what_happened < r.what_happened; - } + explicit SyncEngineEvent(EventCause cause) : what_happened(cause), + snapshot(NULL) { +} EventCause what_happened; // The last session used for syncing. const sessions::SyncSessionSnapshot* snapshot; - int successful_commit_count; - - // How many milliseconds later should the syncer kick in? For - // REQUEST_SYNC_NUDGE only. - int nudge_delay_milliseconds; - // Update-Client-Auth returns a new token for sync use. std::string updated_token; }; -struct SyncerShutdownEvent { - explicit SyncerShutdownEvent(Syncer *syncer_ptr) : syncer(syncer_ptr) {} - Syncer* syncer; - static bool IsChannelShutdownEvent(Syncer* syncer) { - return true; - } +class SyncEngineEventListener { + public: + // TODO(tim): Consider splitting this up to multiple callbacks, rather than + // have to do Event e(type); OnSyncEngineEvent(e); at all callsites, + virtual void OnSyncEngineEvent(const SyncEngineEvent& event) = 0; + protected: + virtual ~SyncEngineEventListener() {} }; -typedef Channel<SyncerEvent> SyncerEventChannel; - -typedef Channel<SyncerShutdownEvent> ShutdownChannel; - // This struct is passed between parts of the syncer during the processing of // one sync loop. It lives on the stack. We don't expose the number of // conflicts during SyncShare as the conflicts may be solved automatically diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc index 74e5122..1598696 100644 --- a/chrome/browser/sync/engine/syncer_unittest.cc +++ b/chrome/browser/sync/engine/syncer_unittest.cc @@ -94,9 +94,9 @@ using sessions::SyncSession; class SyncerTest : public testing::Test, public SyncSession::Delegate, public ModelSafeWorkerRegistrar, - public ChannelEventHandler<SyncerEvent> { + public SyncEngineEventListener { protected: - SyncerTest() : syncer_(NULL) {} + SyncerTest() : syncer_(NULL), saw_syncer_event_(false) {} // SyncSession::Delegate implementation. virtual void OnSilencedUntil(const base::TimeTicks& silenced_until) { @@ -131,25 +131,18 @@ class SyncerTest : public testing::Test, } } - void HandleChannelEvent(const SyncerEvent& event) { - LOG(INFO) << "HandleSyncerEvent in unittest " << event.what_happened; + virtual void OnSyncEngineEvent(const SyncEngineEvent& event) { + VLOG(1) << "HandleSyncEngineEvent in unittest " << event.what_happened; // we only test for entry-specific events, not status changed ones. switch (event.what_happened) { - case SyncerEvent::STATUS_CHANGED: + case SyncEngineEvent::STATUS_CHANGED: // fall through - case SyncerEvent::SYNC_CYCLE_ENDED: - // fall through - case SyncerEvent::COMMITS_SUCCEEDED: + case SyncEngineEvent::SYNC_CYCLE_ENDED: return; - case SyncerEvent::SHUTDOWN_USE_WITH_CARE: - case SyncerEvent::OVER_QUOTA: - case SyncerEvent::REQUEST_SYNC_NUDGE: - LOG(INFO) << "Handling event type " << event.what_happened; - break; default: CHECK(false) << "Handling unknown error type in unit tests!!"; } - syncer_events_.insert(event); + saw_syncer_event_ = true; } void LoopSyncShare(Syncer* syncer) { @@ -168,17 +161,15 @@ class SyncerTest : public testing::Test, new MockConnectionManager(syncdb_.manager(), syncdb_.name())); EnableDatatype(syncable::BOOKMARKS); worker_ = new ModelSafeWorker(); + std::vector<SyncEngineEventListener*> listeners; + listeners.push_back(this); context_.reset(new SyncSessionContext(mock_server_.get(), - syncdb_.manager(), this)); + syncdb_.manager(), this, listeners)); context_->set_account_name(syncdb_.name()); - ASSERT_FALSE(context_->syncer_event_channel()); ASSERT_FALSE(context_->resolver()); syncer_ = new Syncer(context_.get()); // The Syncer installs some components on the context. - ASSERT_TRUE(context_->syncer_event_channel()); ASSERT_TRUE(context_->resolver()); - - hookup_.reset(context_->syncer_event_channel()->AddObserver(this)); session_.reset(new SyncSession(context_.get(), this)); ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); @@ -187,7 +178,7 @@ class SyncerTest : public testing::Test, syncable::Directory::ChildHandles children; dir->GetChildHandles(&trans, trans.root_id(), &children); ASSERT_TRUE(0 == children.size()); - syncer_events_.clear(); + saw_syncer_event_ = false; root_id_ = TestIdFactory::root(); parent_id_ = ids_.MakeServer("parent id"); child_id_ = ids_.MakeServer("child id"); @@ -195,7 +186,6 @@ class SyncerTest : public testing::Test, virtual void TearDown() { mock_server_.reset(); - hookup_.reset(); delete syncer_; syncer_ = NULL; syncdb_.TearDown(); @@ -455,13 +445,12 @@ class SyncerTest : public testing::Test, TestDirectorySetterUpper syncdb_; scoped_ptr<MockConnectionManager> mock_server_; - scoped_ptr<ChannelHookup<SyncerEvent> > hookup_; Syncer* syncer_; scoped_ptr<SyncSession> session_; scoped_ptr<SyncSessionContext> context_; - std::set<SyncerEvent> syncer_events_; + bool saw_syncer_event_; base::TimeDelta last_short_poll_interval_received_; base::TimeDelta last_long_poll_interval_received_; scoped_refptr<ModelSafeWorker> worker_; @@ -1179,7 +1168,7 @@ TEST_F(SyncerTest, DontGetStuckWithTwoSameNames) { mock_server_->AddUpdateDirectory(2, 0, "foo:", 1, 20); SyncRepeatedlyToTriggerStuckSignal(session_.get()); EXPECT_FALSE(session_->status_controller()->syncer_status().syncer_stuck); - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, TestBasicUpdate) { @@ -1338,7 +1327,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { EXPECT_TRUE(10 == circular_parent_target.Get(BASE_VERSION)); } - EXPECT_TRUE(0 == syncer_events_.size()); + EXPECT_FALSE(saw_syncer_event_); EXPECT_EQ(4, status->TotalNumConflictingItems()); { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); @@ -1670,7 +1659,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesUnsanitizedNames) { B.Put(SERVER_VERSION, 20); } LoopSyncShare(syncer_); - syncer_events_.clear(); + saw_syncer_event_ = false; mock_server_->set_conflict_all_commits(false); { @@ -1712,7 +1701,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesNormalNames) { B.Put(SERVER_VERSION, 20); } LoopSyncShare(syncer_); - syncer_events_.clear(); + saw_syncer_event_ = false; mock_server_->set_conflict_all_commits(false); { @@ -1890,7 +1879,7 @@ TEST_F(SyncerTest, DoublyChangedWithResolver) { // Only one entry, since we just overwrite one. EXPECT_TRUE(1 == children.size()); - syncer_events_.clear(); + saw_syncer_event_ = false; } // We got this repro case when someone was editing bookmarks while sync was @@ -1981,7 +1970,7 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) { syncable::Directory::UnsyncedMetaHandles unsynced; dir->GetUnsyncedMetaHandles(&trans, &unsynced); EXPECT_TRUE(0 == unsynced.size()); - syncer_events_.clear(); + saw_syncer_event_ = false; } } @@ -2027,7 +2016,7 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) { syncer_->SyncShare(session_.get()); syncer_->SyncShare(session_.get()); EXPECT_TRUE(0 == session_->status_controller()->TotalNumConflictingItems()); - syncer_events_.clear(); + saw_syncer_event_ = false; } // Original problem synopsis: @@ -2124,7 +2113,7 @@ TEST_F(SyncerTest, FolderSwapUpdate) { EXPECT_TRUE("bob" == id2.Get(NON_UNIQUE_NAME)); EXPECT_TRUE(root_id_ == id2.Get(PARENT_ID)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) { @@ -2168,7 +2157,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) { EXPECT_TRUE("bob" == id3.Get(NON_UNIQUE_NAME)); EXPECT_TRUE(root_id_ == id3.Get(PARENT_ID)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, CommitManyItemsInOneGo) { @@ -2260,7 +2249,7 @@ TEST_F(SyncerTest, DontCrashOnCaseChange) { mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateDirectory(1, 0, "BOB", 2, 20); syncer_->SyncShare(this); // USED TO CAUSE AN ASSERT - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, UnsyncedItemAndUpdate) { @@ -2271,7 +2260,7 @@ TEST_F(SyncerTest, UnsyncedItemAndUpdate) { mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateDirectory(2, 0, "bob", 2, 20); syncer_->SyncShare(this); // USED TO CAUSE AN ASSERT - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) { @@ -2296,7 +2285,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) { mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20); mock_server_->set_conflict_all_commits(true); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; { // Update #20 should have been dropped in favor of the local version. WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -2315,13 +2304,13 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) { // Allow local changes to commit. mock_server_->set_conflict_all_commits(false); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; // Now add a server change to make the two names equal. There should // be no conflict with that, since names are not unique. mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 30, 30); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry server(&wtrans, GET_BY_ID, ids_.FromNumber(1)); @@ -2364,7 +2353,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) { mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20); mock_server_->set_conflict_all_commits(true); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; { // Update #20 should have been dropped in favor of the local version. WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -2383,13 +2372,13 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) { // Allow local changes to commit. mock_server_->set_conflict_all_commits(false); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; // Now add a server change to make the two names equal. There should // be no conflict with that, since names are not unique. mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 30, 30); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry server(&wtrans, GET_BY_ID, ids_.FromNumber(1)); @@ -2428,7 +2417,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) { mock_server_->AddUpdateDirectory(2, 1, "A", 20, 20); mock_server_->set_conflict_all_commits(true); syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1)); @@ -2462,7 +2451,7 @@ TEST_F(SyncerTest, ConflictSetClassificationError) { B.Put(SERVER_NON_UNIQUE_NAME, "A"); } syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, SwapEntryNames) { @@ -2486,7 +2475,7 @@ TEST_F(SyncerTest, SwapEntryNames) { ASSERT_TRUE(A.Put(NON_UNIQUE_NAME, "B")); } syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) { @@ -2513,7 +2502,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) { EXPECT_FALSE(B.Get(IS_UNSYNCED)); EXPECT_FALSE(B.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, FixDirectoryLoopConflict) { @@ -2544,7 +2533,7 @@ TEST_F(SyncerTest, FixDirectoryLoopConflict) { EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) { @@ -2576,7 +2565,7 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) { EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_DEL)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, ServerDeletingFolderWeHaveMovedSomethingInto) { @@ -2626,7 +2615,7 @@ TEST_F(SyncerTest, ServerDeletingFolderWeHaveMovedSomethingInto) { EXPECT_TRUE(fred.Get(IS_UNSYNCED)); EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } // TODO(ncarter): This test is bogus, but it actually seems to hit an @@ -2659,7 +2648,7 @@ TEST_F(SyncerTest, DISABLED_ServerDeletingFolderWeHaveAnOpenEntryIn) { mock_server_->AddUpdateDirectory(2, 0, "fred", 2, 20); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_events_.clear(); + saw_syncer_event_ = false; // These SyncShares would cause a CHECK because we'd think we were stuck. syncer_->SyncShare(this); syncer_->SyncShare(this); @@ -2669,7 +2658,7 @@ TEST_F(SyncerTest, DISABLED_ServerDeletingFolderWeHaveAnOpenEntryIn) { syncer_->SyncShare(this); syncer_->SyncShare(this); syncer_->SyncShare(this); - EXPECT_TRUE(0 == syncer_events_.size()); + EXPECT_FALSE(saw_syncer_event_); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2683,7 +2672,7 @@ TEST_F(SyncerTest, DISABLED_ServerDeletingFolderWeHaveAnOpenEntryIn) { EXPECT_TRUE(bob.Get(PARENT_ID) == fred.Get(ID)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, WeMovedSomethingIntoAFolderServerHasDeleted) { @@ -2737,7 +2726,7 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderServerHasDeleted) { EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } class FolderMoveDeleteRenameTest : public SyncerTest { @@ -2834,7 +2823,7 @@ TEST_F(FolderMoveDeleteRenameTest, EXPECT_FALSE(alice.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } @@ -2893,7 +2882,7 @@ TEST_F(SyncerTest, ASSERT_TRUE(new_item.good()); EXPECT_EQ(new_item.Get(PARENT_ID), fred.Get(ID)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, ServerMovedSomethingIntoAFolderWeHaveDeleted) { @@ -2926,7 +2915,7 @@ TEST_F(SyncerTest, ServerMovedSomethingIntoAFolderWeHaveDeleted) { EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, ServerMovedAFolderIntoAFolderWeHaveDeletedAndMovedIntoIt) { @@ -2962,7 +2951,7 @@ TEST_F(SyncerTest, ServerMovedAFolderIntoAFolderWeHaveDeletedAndMovedIntoIt) { EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, NewServerItemInAFolderWeHaveDeleted) { @@ -2994,7 +2983,7 @@ TEST_F(SyncerTest, NewServerItemInAFolderWeHaveDeleted) { EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, NewServerItemInAFolderHierarchyWeHaveDeleted) { @@ -3036,7 +3025,7 @@ TEST_F(SyncerTest, NewServerItemInAFolderHierarchyWeHaveDeleted) { EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(joe.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, NewServerItemInAFolderHierarchyWeHaveDeleted2) { @@ -3086,7 +3075,7 @@ TEST_F(SyncerTest, NewServerItemInAFolderHierarchyWeHaveDeleted2) { EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(joe.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } @@ -3201,7 +3190,7 @@ TEST_F(SusanDeletingTest, EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE)); EXPECT_FALSE(joe.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted) { @@ -3269,7 +3258,7 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted) { EXPECT_FALSE(fred.Get(IS_UNAPPLIED_UPDATE)); EXPECT_TRUE(fred.Get(NON_UNIQUE_NAME) == "fred"); } - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted2) { @@ -3349,7 +3338,7 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted2) { EXPECT_TRUE(susan.Get(PARENT_ID) == root_id_); EXPECT_FALSE(susan.Get(IS_UNAPPLIED_UPDATE)); } - syncer_events_.clear(); + saw_syncer_event_ = false; } // This test is to reproduce a check failure. Sometimes we would get a bad ID @@ -3379,7 +3368,7 @@ TEST_F(SyncerTest, DuplicateIDReturn) { EXPECT_TRUE(1 == dir->unsynced_entity_count()); syncer_->SyncShare(this); // another bad id in here. EXPECT_TRUE(0 == dir->unsynced_entity_count()); - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) { @@ -3828,7 +3817,7 @@ TEST_F(SyncerTest, ConflictSetSizeReducedToOne) { mock_server_->set_conflict_all_commits(true); // This SyncShare call used to result in a CHECK failure. syncer_->SyncShare(this); - syncer_events_.clear(); + saw_syncer_event_ = false; } TEST_F(SyncerTest, TestClientCommand) { diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc index c2639d6..c279e26 100644 --- a/chrome/browser/sync/engine/syncer_util.cc +++ b/chrome/browser/sync/engine/syncer_util.cc @@ -519,7 +519,7 @@ void SyncerUtil::UpdateLocalDataFromServerData( DCHECK(!entry->Get(IS_UNSYNCED)); DCHECK(entry->Get(IS_UNAPPLIED_UPDATE)); - LOG(INFO) << "Updating entry : " << *entry; + VLOG(1) << "Updating entry : " << *entry; // Start by setting the properties that determine the model_type. entry->Put(SPECIFICS, entry->Get(SERVER_SPECIFICS)); entry->Put(IS_DIR, entry->Get(SERVER_IS_DIR)); diff --git a/chrome/browser/sync/engine/update_applicator.cc b/chrome/browser/sync/engine/update_applicator.cc index f184a93..aee4aae 100644 --- a/chrome/browser/sync/engine/update_applicator.cc +++ b/chrome/browser/sync/engine/update_applicator.cc @@ -31,7 +31,7 @@ UpdateApplicator::UpdateApplicator(ConflictResolver* resolver, progress_(false), routing_info_(routes) { size_t item_count = end - begin; - LOG(INFO) << "UpdateApplicator created for " << item_count << " items."; + VLOG(1) << "UpdateApplicator created for " << item_count << " items."; successful_ids_.reserve(item_count); } @@ -79,8 +79,8 @@ bool UpdateApplicator::AttemptOneApplication( NOTREACHED(); break; } - LOG(INFO) << "Apply Status for " << entry.Get(syncable::META_HANDLE) - << " is " << updateResponse; + VLOG(1) << "Apply Status for " << entry.Get(syncable::META_HANDLE) + << " is " << updateResponse; return true; } diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h index c345fb4..aca18dc 100644 --- a/chrome/browser/sync/sessions/session_state.h +++ b/chrome/browser/sync/sessions/session_state.h @@ -35,11 +35,10 @@ class UpdateProgress; // Data pertaining to the status of an active Syncer object. struct SyncerStatus { SyncerStatus() - : over_quota(false), invalid_store(false), syncer_stuck(false), + : invalid_store(false), syncer_stuck(false), syncing(false), num_successful_commits(0), num_successful_bookmark_commits(0) {} - bool over_quota; // True when we get such an INVALID_STORE error from the server. bool invalid_store; // True iff we're stuck. diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc index f7e159c..7c1602d 100644 --- a/chrome/browser/sync/sessions/status_controller.cc +++ b/chrome/browser/sync/sessions/status_controller.cc @@ -101,11 +101,6 @@ void StatusController::set_num_server_changes_remaining( *(shared_.num_server_changes_remaining.mutate()) = changes_remaining; } -void StatusController::set_over_quota(bool over_quota) { - if (shared_.syncer_status.value().over_quota != over_quota) - shared_.syncer_status.mutate()->over_quota = over_quota; -} - void StatusController::set_invalid_store(bool invalid_store) { if (shared_.syncer_status.value().invalid_store != invalid_store) shared_.syncer_status.mutate()->invalid_store = invalid_store; diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h index d038ed5..7663e4e 100644 --- a/chrome/browser/sync/sessions/status_controller.h +++ b/chrome/browser/sync/sessions/status_controller.h @@ -209,7 +209,6 @@ class StatusController { void set_current_download_timestamp(syncable::ModelType model, int64 current_timestamp); void set_num_server_changes_remaining(int64 changes_remaining); - void set_over_quota(bool over_quota); void set_invalid_store(bool invalid_store); void set_syncer_stuck(bool syncer_stuck); void set_syncing(bool syncing); diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc index 2f07039..88605bf 100644 --- a/chrome/browser/sync/sessions/status_controller_unittest.cc +++ b/chrome/browser/sync/sessions/status_controller_unittest.cc @@ -56,11 +56,6 @@ TEST_F(StatusControllerTest, GetsDirty) { status.set_num_server_changes_remaining(30); EXPECT_TRUE(status.TestAndClearIsDirty()); - status.set_over_quota(true); - EXPECT_TRUE(status.TestAndClearIsDirty()); - status.set_over_quota(false); - EXPECT_TRUE(status.TestAndClearIsDirty()); - status.set_invalid_store(true); EXPECT_TRUE(status.TestAndClearIsDirty()); status.set_invalid_store(false); @@ -144,10 +139,6 @@ TEST_F(StatusControllerTest, ReadYourWrites) { status.set_num_server_changes_remaining(13); EXPECT_EQ(13, status.num_server_changes_remaining()); - EXPECT_FALSE(status.syncer_status().over_quota); - status.set_over_quota(true); - EXPECT_TRUE(status.syncer_status().over_quota); - EXPECT_FALSE(status.syncer_status().invalid_store); status.set_invalid_store(true); EXPECT_TRUE(status.syncer_status().invalid_store); diff --git a/chrome/browser/sync/sessions/sync_session_context.cc b/chrome/browser/sync/sessions/sync_session_context.cc index 527cb2d..04d79c1 100644 --- a/chrome/browser/sync/sessions/sync_session_context.cc +++ b/chrome/browser/sync/sessions/sync_session_context.cc @@ -14,14 +14,17 @@ namespace sessions { SyncSessionContext::SyncSessionContext( ServerConnectionManager* connection_manager, syncable::DirectoryManager* directory_manager, - ModelSafeWorkerRegistrar* model_safe_worker_registrar) + ModelSafeWorkerRegistrar* model_safe_worker_registrar, + const std::vector<SyncEngineEventListener*>& listeners) : resolver_(NULL), - syncer_event_channel_(NULL), connection_manager_(connection_manager), directory_manager_(directory_manager), registrar_(model_safe_worker_registrar), extensions_activity_monitor_(new ExtensionsActivityMonitor()), notifications_enabled_(false) { + std::vector<SyncEngineEventListener*>::const_iterator it; + for (it = listeners.begin(); it != listeners.end(); ++it) + listeners_.AddObserver(*it); } SyncSessionContext::~SyncSessionContext() { diff --git a/chrome/browser/sync/sessions/sync_session_context.h b/chrome/browser/sync/sessions/sync_session_context.h index b782876..c6d87a1 100644 --- a/chrome/browser/sync/sessions/sync_session_context.h +++ b/chrome/browser/sync/sessions/sync_session_context.h @@ -38,14 +38,15 @@ class ServerConnectionManager; namespace sessions { class ScopedSessionContextConflictResolver; -class ScopedSessionContextSyncerEventChannel; struct SyncSessionSnapshot; +class TestScopedSessionEventListener; class SyncSessionContext { public: SyncSessionContext(ServerConnectionManager* connection_manager, syncable::DirectoryManager* directory_manager, - ModelSafeWorkerRegistrar* model_safe_worker_registrar); + ModelSafeWorkerRegistrar* model_safe_worker_registrar, + const std::vector<SyncEngineEventListener*>& listeners); ~SyncSessionContext(); ConflictResolver* resolver() { return resolver_; } @@ -55,9 +56,6 @@ class SyncSessionContext { syncable::DirectoryManager* directory_manager() { return directory_manager_; } - SyncerEventChannel* syncer_event_channel() { - return syncer_event_channel_; - } ModelSafeWorkerRegistrar* registrar() { return registrar_; } @@ -92,16 +90,22 @@ class SyncSessionContext { void set_last_snapshot(const SyncSessionSnapshot& snapshot); + void NotifyListeners(const SyncEngineEvent& event) { + FOR_EACH_OBSERVER(SyncEngineEventListener, listeners_, + OnSyncEngineEvent(event)); + } + private: // Rather than force clients to set and null-out various context members, we // extend our encapsulation boundary to scoped helpers that take care of this // once they are allocated. See definitions of these below. friend class ScopedSessionContextConflictResolver; - friend class ScopedSessionContextSyncerEventChannel; + friend class TestScopedSessionEventListener; - // These are installed by Syncer objects when needed and may be NULL. + // This is installed by Syncer objects when needed and may be NULL. ConflictResolver* resolver_; - SyncerEventChannel* syncer_event_channel_; + + ObserverList<SyncEngineEventListener> listeners_; ServerConnectionManager* const connection_manager_; syncable::DirectoryManager* const directory_manager_; @@ -153,27 +157,6 @@ class ScopedSessionContextConflictResolver { DISALLOW_COPY_AND_ASSIGN(ScopedSessionContextConflictResolver); }; -// Installs a SyncerEventChannel to a given session context for the lifetime of -// the ScopedSessionContextSyncerEventChannel. There should never be more than -// one SyncerEventChannel in the context, so it is an error to use this if the -// context already has a channel. -class ScopedSessionContextSyncerEventChannel { - public: - ScopedSessionContextSyncerEventChannel(SyncSessionContext* context, - SyncerEventChannel* channel) - : context_(context), channel_(channel) { - DCHECK(NULL == context->syncer_event_channel_); - context->syncer_event_channel_ = channel_; - } - ~ScopedSessionContextSyncerEventChannel() { - context_->syncer_event_channel_ = NULL; - } - private: - SyncSessionContext* context_; - SyncerEventChannel* channel_; - DISALLOW_COPY_AND_ASSIGN(ScopedSessionContextSyncerEventChannel); -}; - } // namespace sessions } // namespace browser_sync diff --git a/chrome/browser/sync/sessions/sync_session_unittest.cc b/chrome/browser/sync/sessions/sync_session_unittest.cc index 011f667..1751434 100644 --- a/chrome/browser/sync/sessions/sync_session_unittest.cc +++ b/chrome/browser/sync/sessions/sync_session_unittest.cc @@ -27,7 +27,8 @@ class SyncSessionTest : public testing::Test, GetModelSafeRoutingInfo(&routes_); } virtual void SetUp() { - context_.reset(new SyncSessionContext(NULL, NULL, this)); + context_.reset(new SyncSessionContext(NULL, NULL, this, + std::vector<SyncEngineEventListener*>())); session_.reset(new SyncSession(context_.get(), this)); } virtual void TearDown() { @@ -91,26 +92,20 @@ class SyncSessionTest : public testing::Test, TEST_F(SyncSessionTest, ScopedContextHelpers) { ConflictResolver resolver; - SyncerEventChannel* channel = new SyncerEventChannel(); EXPECT_FALSE(context_->resolver()); - EXPECT_FALSE(context_->syncer_event_channel()); { ScopedSessionContextConflictResolver s_resolver(context_.get(), &resolver); - ScopedSessionContextSyncerEventChannel s_channel(context_.get(), channel); EXPECT_EQ(&resolver, context_->resolver()); - EXPECT_EQ(channel, context_->syncer_event_channel()); } EXPECT_FALSE(context_->resolver()); - EXPECT_FALSE(context_->syncer_event_channel()); - channel->Notify(SyncerEvent(SyncerEvent::SHUTDOWN_USE_WITH_CARE)); - delete channel; } TEST_F(SyncSessionTest, SetWriteTransaction) { TestDirectorySetterUpper db; db.SetUp(); session_.reset(NULL); - context_.reset(new SyncSessionContext(NULL, db.manager(), this)); + context_.reset(new SyncSessionContext(NULL, db.manager(), this, + std::vector<SyncEngineEventListener*>())); session_.reset(new SyncSession(context_.get(), this)); context_->set_account_name(db.name()); syncable::ScopedDirLookup dir(context_->directory_manager(), diff --git a/chrome/browser/sync/syncable/directory_backing_store.cc b/chrome/browser/sync/syncable/directory_backing_store.cc index 9d738fd..abba0a6 100644 --- a/chrome/browser/sync/syncable/directory_backing_store.cc +++ b/chrome/browser/sync/syncable/directory_backing_store.cc @@ -471,7 +471,7 @@ DirOpenResult DirectoryBackingStore::InitializeTables() { return FAILED_NEWER_VERSION; } // Fallback (re-sync everything) migration path. - LOG(INFO) << "Old/null sync database, version " << version_on_disk; + VLOG(1) << "Old/null sync database, version " << version_on_disk; // Delete the existing database (if any), and create a fresh one. if (SQLITE_OK == last_result) { DropAllTables(); @@ -492,7 +492,7 @@ DirOpenResult DirectoryBackingStore::InitializeTables() { string db_create_version = statement.column_text(0); int db_create_time = statement.column_int(1); statement.reset(); - LOG(INFO) << "DB created at " << db_create_time << " by version " << + VLOG(1) << "DB created at " << db_create_time << " by version " << db_create_version; } // COMMIT TRANSACTION rolls back on failure. @@ -927,7 +927,7 @@ bool DirectoryBackingStore::MigrateVersion72To73() { } int DirectoryBackingStore::CreateTables() { - LOG(INFO) << "First run, creating tables"; + VLOG(1) << "First run, creating tables"; // Create two little tables share_version and share_info int result = ExecQuery(load_dbhandle_, "CREATE TABLE share_version (" diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 51598a3..4469b27 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -2514,6 +2514,7 @@ 'test/sync/engine/test_id_factory.h', 'test/sync/engine/test_syncable_utils.cc', 'test/sync/engine/test_syncable_utils.h', + 'test/sync/sessions/test_scoped_session_event_listener.h', ], 'include_dirs': [ '..', diff --git a/chrome/test/sync/engine/syncer_command_test.h b/chrome/test/sync/engine/syncer_command_test.h index 02ea57d..faeb4b5 100644 --- a/chrome/test/sync/engine/syncer_command_test.h +++ b/chrome/test/sync/engine/syncer_command_test.h @@ -94,7 +94,8 @@ class SyncerCommandTestWithParam : public testing::TestWithParam<T>, void ResetContext() { context_.reset(new sessions::SyncSessionContext( - mock_server_.get(), syncdb_->manager(), registrar())); + mock_server_.get(), syncdb_->manager(), registrar(), + std::vector<SyncEngineEventListener*>())); context_->set_account_name(syncdb_->name()); ClearSession(); } diff --git a/chrome/test/sync/sessions/test_scoped_session_event_listener.h b/chrome/test/sync/sessions/test_scoped_session_event_listener.h new file mode 100644 index 0000000..9ede09b --- /dev/null +++ b/chrome/test/sync/sessions/test_scoped_session_event_listener.h @@ -0,0 +1,36 @@ +// Copyright (c) 2010 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_TEST_SYNC_SESSIONS_TEST_SCOPED_SESSION_EVENT_LISTENER_H_ +#define CHROME_TEST_SYNC_SESSIONS_TEST_SCOPED_SESSION_EVENT_LISTENER_H_ +#pragma once + +#include "chrome/browser/sync/sessions/sync_session_context.h" + +namespace browser_sync { +namespace sessions { + +// Installs a SyncEventListener to a given session context for the lifetime of +// the TestScopedSessionEventListener. +class TestScopedSessionEventListener { + public: + TestScopedSessionEventListener( + SyncSessionContext* context, + SyncEngineEventListener* listener) + : context_(context), listener_(listener) { + context->listeners_.AddObserver(listener); + } + ~TestScopedSessionEventListener() { + context_->listeners_.RemoveObserver(listener_); + } + private: + SyncSessionContext* context_; + SyncEngineEventListener* listener_; + DISALLOW_COPY_AND_ASSIGN(TestScopedSessionEventListener); +}; + +} // namespace sessions +} // namespace browser_sync + +#endif // CHROME_TEST_SYNC_SESSIONS_TEST_SCOPED_SESSION_EVENT_LISTENER_H_ |