From 3d4fde10da1c443a9128023977fc819d26798071 Mon Sep 17 00:00:00 2001 From: "rlarocque@chromium.org" Date: Wed, 23 Oct 2013 23:28:46 +0000 Subject: sync: Init backend without UI thread task Remove the part of sync backend initialize flow that posts a task to the UI thread. The only part of this task that could not easily be moved on to the sync thread or defered for later was the setting of the initialization_state_ flag. However, as of r224014 that flag is no longer useful, so the refactoring can proceed. Replace the initialization_state_ variable with a bool. The code no longer requires a set of enum values to represent the backend's initialization state. This change broke the TestProfileSyncService pretty badly. That test class was fixed by moving around some code in order to allow us to inject testable hooks into SyncManager. BUG=295806 Review URL: https://codereview.chromium.org/23698015 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230550 0039d316-1c4b-4281-b951-d872f2087c98 --- .../sync_manager_factory_for_profile_sync_test.h | 30 ++++++++++++++ sync/internal_api/sync_manager_impl.cc | 19 ++++++--- sync/internal_api/sync_manager_impl.h | 5 +++ .../sync_manager_factory_for_profile_sync_test.cc | 29 +++++++++++++ .../test/sync_manager_for_profile_sync_test.cc | 47 ++++++++++++++++++++++ .../test/sync_manager_for_profile_sync_test.h | 34 ++++++++++++++++ sync/sync_tests.gypi | 4 ++ 7 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 sync/internal_api/public/test/sync_manager_factory_for_profile_sync_test.h create mode 100644 sync/internal_api/test/sync_manager_factory_for_profile_sync_test.cc create mode 100644 sync/internal_api/test/sync_manager_for_profile_sync_test.cc create mode 100644 sync/internal_api/test/sync_manager_for_profile_sync_test.h (limited to 'sync') diff --git a/sync/internal_api/public/test/sync_manager_factory_for_profile_sync_test.h b/sync/internal_api/public/test/sync_manager_factory_for_profile_sync_test.h new file mode 100644 index 0000000..d8615c0 --- /dev/null +++ b/sync/internal_api/public/test/sync_manager_factory_for_profile_sync_test.h @@ -0,0 +1,30 @@ +// Copyright 2013 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 SYNC_INTERNAL_API_PUBLIC_TEST_SYNC_MANAGER_FACTORY_FOR_PROFILE_SYNC_TEST_H_ +#define SYNC_INTERNAL_API_PUBLIC_TEST_SYNC_MANAGER_FACTORY_FOR_PROFILE_SYNC_TEST_H_ + +#include + +#include "base/callback.h" +#include "base/memory/scoped_ptr.h" +#include "sync/internal_api/public/sync_manager_factory.h" + +namespace syncer { + +class SyncManagerFactoryForProfileSyncTest : public syncer::SyncManagerFactory { + public: + SyncManagerFactoryForProfileSyncTest(base::Closure init_callback, + bool set_initial_sync_ended); + virtual ~SyncManagerFactoryForProfileSyncTest(); + virtual scoped_ptr CreateSyncManager( + std::string name) OVERRIDE; + private: + base::Closure init_callback_; + bool set_initial_sync_ended_; +}; + +} // namespace syncer + +#endif // SYNC_INTERNAL_API_PUBLIC_TEST_SYNC_MANAGER_FACTORY_FOR_PROFILE_SYNC_TEST_H_ diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index 6cad681..6ad9580 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -409,12 +409,7 @@ void SyncManagerImpl::Init( DVLOG(1) << "Username: " << username; if (!OpenDirectory(username)) { - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnInitializationComplete( - MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()), - MakeWeakHandle( - debug_info_event_listener_.GetWeakPtr()), - false, ModelTypeSet())); + NotifyInitializationFailure(); LOG(ERROR) << "Sync manager initialization failed!"; return; } @@ -461,6 +456,10 @@ void SyncManagerImpl::Init( UpdateCredentials(credentials); + NotifyInitializationSuccess(); +} + +void SyncManagerImpl::NotifyInitializationSuccess() { FOR_EACH_OBSERVER(SyncManager::Observer, observers_, OnInitializationComplete( MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()), @@ -468,6 +467,14 @@ void SyncManagerImpl::Init( true, InitialSyncEndedTypes())); } +void SyncManagerImpl::NotifyInitializationFailure() { + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnInitializationComplete( + MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()), + MakeWeakHandle(debug_info_event_listener_.GetWeakPtr()), + false, ModelTypeSet())); +} + void SyncManagerImpl::OnPassphraseRequired( PassphraseRequiredReason reason, const sync_pb::EncryptedData& pending_keys) { diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h index 2387690..5d5cfaa 100644 --- a/sync/internal_api/sync_manager_impl.h +++ b/sync/internal_api/sync_manager_impl.h @@ -180,6 +180,11 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl : bool GetHasInvalidAuthTokenForTest() const; + protected: + // Helper functions. Virtual for testing. + virtual void NotifyInitializationSuccess(); + virtual void NotifyInitializationFailure(); + private: friend class SyncManagerTest; FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, NudgeDelayTest); diff --git a/sync/internal_api/test/sync_manager_factory_for_profile_sync_test.cc b/sync/internal_api/test/sync_manager_factory_for_profile_sync_test.cc new file mode 100644 index 0000000..df4073b --- /dev/null +++ b/sync/internal_api/test/sync_manager_factory_for_profile_sync_test.cc @@ -0,0 +1,29 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sync/internal_api/public/test/sync_manager_factory_for_profile_sync_test.h" + +#include "sync/internal_api/test/sync_manager_for_profile_sync_test.h" + +namespace syncer { + +SyncManagerFactoryForProfileSyncTest::SyncManagerFactoryForProfileSyncTest( + base::Closure init_callback, + bool set_initial_sync_ended) + : init_callback_(init_callback), + set_initial_sync_ended_(set_initial_sync_ended) { +} + +SyncManagerFactoryForProfileSyncTest::~SyncManagerFactoryForProfileSyncTest() {} + +scoped_ptr +SyncManagerFactoryForProfileSyncTest::CreateSyncManager(std::string name) { + return scoped_ptr( + new SyncManagerForProfileSyncTest( + name, + init_callback_, + set_initial_sync_ended_)); +} + +} // namespace syncer diff --git a/sync/internal_api/test/sync_manager_for_profile_sync_test.cc b/sync/internal_api/test/sync_manager_for_profile_sync_test.cc new file mode 100644 index 0000000..aaa6c8d --- /dev/null +++ b/sync/internal_api/test/sync_manager_for_profile_sync_test.cc @@ -0,0 +1,47 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sync/internal_api/test/sync_manager_for_profile_sync_test.h" + +#include "sync/internal_api/public/test/test_user_share.h" +#include "sync/internal_api/public/user_share.h" +#include "sync/syncable/directory.h" + +namespace syncer { + +SyncManagerForProfileSyncTest::SyncManagerForProfileSyncTest( + std::string name, + base::Closure init_callback, + bool set_initial_sync_ended) + : SyncManagerImpl(name), + init_callback_(init_callback), + set_initial_sync_ended_(set_initial_sync_ended) {} + +SyncManagerForProfileSyncTest::~SyncManagerForProfileSyncTest() {} + +void SyncManagerForProfileSyncTest::NotifyInitializationSuccess() { + UserShare* user_share = GetUserShare(); + syncable::Directory* directory = user_share->directory.get(); + + if (!init_callback_.is_null()) + init_callback_.Run(); + + if (set_initial_sync_ended_) { + ModelTypeSet early_download_types; + early_download_types.PutAll(ControlTypes()); + early_download_types.PutAll(PriorityUserTypes()); + for (ModelTypeSet::Iterator it = early_download_types.First(); + it.Good(); it.Inc()) { + if (!directory->InitialSyncEndedForType(it.Get())) { + syncer::TestUserShare::CreateRoot(it.Get(), user_share); + } + } + } else { + VLOG(2) << "Skipping directory init"; + } + + SyncManagerImpl::NotifyInitializationSuccess(); +} + +}; diff --git a/sync/internal_api/test/sync_manager_for_profile_sync_test.h b/sync/internal_api/test/sync_manager_for_profile_sync_test.h new file mode 100644 index 0000000..c9af22d --- /dev/null +++ b/sync/internal_api/test/sync_manager_for_profile_sync_test.h @@ -0,0 +1,34 @@ +// Copyright 2013 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 SYNC_INTERNAL_API_PUBLIC_TEST_SYNC_MANAGER_FOR_PROFILE_SYNC_TEST_H_ +#define SYNC_INTERNAL_API_PUBLIC_TEST_SYNC_MANAGER_FOR_PROFILE_SYNC_TEST_H_ + +#include + +#include "base/callback.h" +#include "base/compiler_specific.h" +#include "sync/internal_api/sync_manager_impl.h" + +namespace syncer { + +// This class is used to help implement the TestProfileSyncService. +// Those tests try to test sync without instantiating a real backend. +class SyncManagerForProfileSyncTest + : public syncer::SyncManagerImpl { + public: + SyncManagerForProfileSyncTest(std::string name, + base::Closure init_callback, + bool set_initial_sync_ended); + virtual ~SyncManagerForProfileSyncTest(); + virtual void NotifyInitializationSuccess() OVERRIDE; + + private: + base::Closure init_callback_; + bool set_initial_sync_ended_; +}; + +} // namespace syncer + +#endif // SYNC_INTERNAL_API_PUBLIC_TEST_SYNC_MANAGER_FOR_PROFILE_SYNC_TEST_H_ diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 56ddeed..2081066 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -150,10 +150,14 @@ 'internal_api/public/base/object_id_invalidation_map_test_util.cc', 'internal_api/public/base/object_id_invalidation_map_test_util.h', 'internal_api/public/test/fake_sync_manager.h', + 'internal_api/public/test/sync_manager_factory_for_profile_sync_test.h', 'internal_api/public/test/test_entry_factory.h', 'internal_api/public/test/test_internal_components_factory.h', 'internal_api/public/test/test_user_share.h', 'internal_api/test/fake_sync_manager.cc', + 'internal_api/test/sync_manager_factory_for_profile_sync_test.cc', + 'internal_api/test/sync_manager_for_profile_sync_test.cc', + 'internal_api/test/sync_manager_for_profile_sync_test.h', 'internal_api/test/test_entry_factory.cc', 'internal_api/test/test_internal_components_factory.cc', 'internal_api/test/test_user_share.cc', -- cgit v1.1