diff --git a/chrome/browser/sync/chrome_sync_client.cc b/chrome/browser/sync/chrome_sync_client.cc index bb55f5baa3a327..106e7e18a0b93b 100644 --- a/chrome/browser/sync/chrome_sync_client.cc +++ b/chrome/browser/sync/chrome_sync_client.cc @@ -7,6 +7,7 @@ #include "chrome/browser/autofill/personal_data_manager_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/dom_distiller/dom_distiller_service_factory.h" +#include "chrome/browser/favicon/favicon_service_factory.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/prefs/pref_service_syncable_util.h" @@ -17,6 +18,7 @@ #include "chrome/browser/themes/theme_service.h" #include "chrome/browser/themes/theme_service_factory.h" #include "chrome/browser/themes/theme_syncable_service.h" +#include "chrome/browser/undo/bookmark_undo_service_factory.h" #include "chrome/browser/web_data_service_factory.h" #include "components/autofill/core/browser/webdata/autocomplete_syncable_service.h" #include "components/autofill/core/browser/webdata/autofill_profile_syncable_service.h" @@ -100,6 +102,12 @@ bookmarks::BookmarkModel* ChromeSyncClient::GetBookmarkModel() { return BookmarkModelFactory::GetForProfile(profile_); } +favicon::FaviconService* ChromeSyncClient::GetFaviconService() { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + return FaviconServiceFactory::GetForProfile( + profile_, ServiceAccessType::EXPLICIT_ACCESS); +} + history::HistoryService* ChromeSyncClient::GetHistoryService() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); return HistoryServiceFactory::GetForProfile( @@ -125,6 +133,10 @@ ChromeSyncClient::GetWebDataService() { profile_, ServiceAccessType::EXPLICIT_ACCESS); } +BookmarkUndoService* ChromeSyncClient::GetBookmarkUndoServiceIfExists() { + return BookmarkUndoServiceFactory::GetForProfileIfExists(profile_); +} + base::WeakPtr ChromeSyncClient::GetSyncableServiceForType(syncer::ModelType type) { if (!profile_) { // For tests. diff --git a/chrome/browser/sync/chrome_sync_client.h b/chrome/browser/sync/chrome_sync_client.h index d16d0e7cd0399d..6839c78d4d9ee6 100644 --- a/chrome/browser/sync/chrome_sync_client.h +++ b/chrome/browser/sync/chrome_sync_client.h @@ -30,10 +30,12 @@ class ChromeSyncClient : public sync_driver::SyncClient { sync_driver::SyncService* GetSyncService() override; PrefService* GetPrefService() override; bookmarks::BookmarkModel* GetBookmarkModel() override; + favicon::FaviconService* GetFaviconService() override; history::HistoryService* GetHistoryService() override; scoped_refptr GetPasswordStore() override; autofill::PersonalDataManager* GetPersonalDataManager() override; scoped_refptr GetWebDataService() override; + BookmarkUndoService* GetBookmarkUndoServiceIfExists() override; base::WeakPtr GetSyncableServiceForType( syncer::ModelType type) override; diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc index d61224037cf69e..c336401df578de 100644 --- a/chrome/browser/sync/glue/bookmark_change_processor.cc +++ b/chrome/browser/sync/glue/bookmark_change_processor.cc @@ -13,19 +13,14 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" -#include "chrome/browser/bookmarks/bookmark_model_factory.h" -#include "chrome/browser/favicon/favicon_service_factory.h" -#include "chrome/browser/history/history_service_factory.h" -#include "chrome/browser/sync/profile_sync_service.h" -#include "chrome/browser/undo/bookmark_undo_service_factory.h" #include "components/bookmarks/browser/bookmark_client.h" #include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_utils.h" #include "components/favicon/core/favicon_service.h" #include "components/history/core/browser/history_service.h" +#include "components/sync_driver/sync_client.h" #include "components/undo/bookmark_undo_service.h" #include "components/undo/bookmark_undo_utils.h" -#include "content/public/browser/browser_thread.h" #include "sync/internal_api/public/change_record.h" #include "sync/internal_api/public/read_node.h" #include "sync/internal_api/public/write_node.h" @@ -37,7 +32,6 @@ using bookmarks::BookmarkModel; using bookmarks::BookmarkNode; -using content::BrowserThread; using syncer::ChangeRecord; using syncer::ChangeRecordList; @@ -46,16 +40,15 @@ namespace browser_sync { static const char kMobileBookmarksTag[] = "synced_bookmarks"; BookmarkChangeProcessor::BookmarkChangeProcessor( - Profile* profile, + sync_driver::SyncClient* sync_client, BookmarkModelAssociator* model_associator, sync_driver::DataTypeErrorHandler* error_handler) : sync_driver::ChangeProcessor(error_handler), bookmark_model_(NULL), - profile_(profile), + sync_client_(sync_client), model_associator_(model_associator) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(model_associator); - DCHECK(profile); + DCHECK(sync_client); DCHECK(error_handler); } @@ -65,9 +58,9 @@ BookmarkChangeProcessor::~BookmarkChangeProcessor() { } void BookmarkChangeProcessor::StartImpl() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(!bookmark_model_); - bookmark_model_ = BookmarkModelFactory::GetForProfile(profile_); + bookmark_model_ = sync_client_->GetBookmarkModel(); DCHECK(bookmark_model_->loaded()); bookmark_model_->AddObserver(this); } @@ -554,7 +547,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( const syncer::BaseTransaction* trans, int64 model_version, const syncer::ImmutableChangeRecordList& changes) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(thread_checker_.CalledOnValidThread()); // A note about ordering. Sync backend is responsible for ordering the change // records in the following order: // @@ -580,7 +573,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( // Changes made to the bookmark model due to sync should not be undoable. ScopedSuspendBookmarkUndo suspend_undo( - BookmarkUndoServiceFactory::GetForProfileIfExists(profile_)); + sync_client_->GetBookmarkUndoServiceIfExists()); // Notify UI intensive observers of BookmarkModel that we are about to make // potentially significant changes to it, so the updates may be batched. For @@ -698,7 +691,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( if (dst) { DCHECK(it->action == ChangeRecord::ACTION_UPDATE) << "ACTION_UPDATE should be seen if and only if the node is known."; - UpdateBookmarkWithSyncData(src, model, dst, profile_); + UpdateBookmarkWithSyncData(src, model, dst, sync_client_); // Move all modified entries to the right. We'll fix it later. model->Move(dst, parent, parent->child_count()); @@ -706,10 +699,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( DCHECK(it->action == ChangeRecord::ACTION_ADD) << "ACTION_ADD should be seen if and only if the node is unknown."; - dst = CreateBookmarkNode(&src, - parent, - model, - profile_, + dst = CreateBookmarkNode(&src, parent, model, sync_client_, parent->child_count()); if (!dst) { // We ignore bookmarks we can't add. Chances are this is caused by @@ -766,7 +756,7 @@ void BookmarkChangeProcessor::UpdateBookmarkWithSyncData( const syncer::BaseNode& sync_node, BookmarkModel* model, const BookmarkNode* node, - Profile* profile) { + sync_driver::SyncClient* sync_client) { DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder()); const sync_pb::BookmarkSpecifics& specifics = sync_node.GetBookmarkSpecifics(); @@ -778,7 +768,7 @@ void BookmarkChangeProcessor::UpdateBookmarkWithSyncData( node, base::Time::FromInternalValue(specifics.creation_time_us())); } - SetBookmarkFavicon(&sync_node, node, model, profile); + SetBookmarkFavicon(&sync_node, node, model, sync_client); model->SetNodeMetaInfoMap(node, *GetBookmarkMetaInfo(&sync_node)); } @@ -802,11 +792,11 @@ const BookmarkNode* BookmarkChangeProcessor::CreateBookmarkNode( const syncer::BaseNode* sync_node, const BookmarkNode* parent, BookmarkModel* model, - Profile* profile, + sync_driver::SyncClient* sync_client, int index) { return CreateBookmarkNode(base::UTF8ToUTF16(sync_node->GetTitle()), GURL(sync_node->GetBookmarkSpecifics().url()), - sync_node, parent, model, profile, index); + sync_node, parent, model, sync_client, index); } // static @@ -818,7 +808,7 @@ const BookmarkNode* BookmarkChangeProcessor::CreateBookmarkNode( const syncer::BaseNode* sync_node, const BookmarkNode* parent, BookmarkModel* model, - Profile* profile, + sync_driver::SyncClient* sync_client, int index) { DCHECK(parent); @@ -837,7 +827,7 @@ const BookmarkNode* BookmarkChangeProcessor::CreateBookmarkNode( parent, index, title, url, create_time, GetBookmarkMetaInfo(sync_node).get()); if (node) - SetBookmarkFavicon(sync_node, node, model, profile); + SetBookmarkFavicon(sync_node, node, model, sync_client); } return node; @@ -849,7 +839,7 @@ bool BookmarkChangeProcessor::SetBookmarkFavicon( const syncer::BaseNode* sync_node, const BookmarkNode* bookmark_node, BookmarkModel* bookmark_model, - Profile* profile) { + sync_driver::SyncClient* sync_client) { const sync_pb::BookmarkSpecifics& specifics = sync_node->GetBookmarkSpecifics(); const std::string& icon_bytes_str = specifics.favicon(); @@ -867,7 +857,7 @@ bool BookmarkChangeProcessor::SetBookmarkFavicon( if (icon_url.is_empty()) icon_url = bookmark_node->url(); - ApplyBookmarkFavicon(bookmark_node, profile, icon_url, icon_bytes); + ApplyBookmarkFavicon(bookmark_node, sync_client, icon_url, icon_bytes); return true; } @@ -938,14 +928,11 @@ void BookmarkChangeProcessor::SetSyncNodeMetaInfo( // static void BookmarkChangeProcessor::ApplyBookmarkFavicon( const BookmarkNode* bookmark_node, - Profile* profile, + sync_driver::SyncClient* sync_client, const GURL& icon_url, const scoped_refptr& bitmap_data) { - history::HistoryService* history = HistoryServiceFactory::GetForProfile( - profile, ServiceAccessType::EXPLICIT_ACCESS); - favicon::FaviconService* favicon_service = - FaviconServiceFactory::GetForProfile(profile, - ServiceAccessType::EXPLICIT_ACCESS); + history::HistoryService* history = sync_client->GetHistoryService(); + favicon::FaviconService* favicon_service = sync_client->GetFaviconService(); history->AddPageNoVisitForBookmark(bookmark_node->url(), bookmark_node->GetTitle()); diff --git a/chrome/browser/sync/glue/bookmark_change_processor.h b/chrome/browser/sync/glue/bookmark_change_processor.h index 9c69cc5d957809..031021861ea0b0 100644 --- a/chrome/browser/sync/glue/bookmark_change_processor.h +++ b/chrome/browser/sync/glue/bookmark_change_processor.h @@ -8,6 +8,7 @@ #include #include "base/compiler_specific.h" +#include "base/threading/thread_checker.h" #include "chrome/browser/sync/glue/bookmark_model_associator.h" #include "components/bookmarks/browser/bookmark_model_observer.h" #include "components/bookmarks/browser/bookmark_node.h" @@ -25,6 +26,10 @@ class WriteNode; class WriteTransaction; } // namespace syncer +namespace sync_driver { +class SyncClient; +} + namespace browser_sync { // This class is responsible for taking changes from the BookmarkModel @@ -34,7 +39,7 @@ namespace browser_sync { class BookmarkChangeProcessor : public bookmarks::BookmarkModelObserver, public sync_driver::ChangeProcessor { public: - BookmarkChangeProcessor(Profile* profile, + BookmarkChangeProcessor(sync_driver::SyncClient* sync_client, BookmarkModelAssociator* model_associator, sync_driver::DataTypeErrorHandler* error_handler); ~BookmarkChangeProcessor() override; @@ -89,7 +94,7 @@ class BookmarkChangeProcessor : public bookmarks::BookmarkModelObserver, static void UpdateBookmarkWithSyncData(const syncer::BaseNode& sync_node, bookmarks::BookmarkModel* model, const bookmarks::BookmarkNode* node, - Profile* profile); + sync_driver::SyncClient* sync_client); // Creates a bookmark node under the given parent node from the given sync // node. Returns the newly created node. The created node is placed at the @@ -98,7 +103,7 @@ class BookmarkChangeProcessor : public bookmarks::BookmarkModelObserver, const syncer::BaseNode* sync_node, const bookmarks::BookmarkNode* parent, bookmarks::BookmarkModel* model, - Profile* profile, + sync_driver::SyncClient* sync_client, int index); // Overload of CreateBookmarkNode function above that helps to avoid @@ -109,7 +114,7 @@ class BookmarkChangeProcessor : public bookmarks::BookmarkModelObserver, const syncer::BaseNode* sync_node, const bookmarks::BookmarkNode* parent, bookmarks::BookmarkModel* model, - Profile* profile, + sync_driver::SyncClient* sync_client, int index); // Sets the favicon of the given bookmark node from the given sync node. @@ -119,14 +124,14 @@ class BookmarkChangeProcessor : public bookmarks::BookmarkModelObserver, static bool SetBookmarkFavicon(const syncer::BaseNode* sync_node, const bookmarks::BookmarkNode* bookmark_node, bookmarks::BookmarkModel* model, - Profile* profile); + sync_driver::SyncClient* sync_client); // Applies the 1x favicon |bitmap_data| and |icon_url| to |bookmark_node|. // |profile| is the profile that contains the HistoryService and BookmarkModel // for the bookmark in question. static void ApplyBookmarkFavicon( const bookmarks::BookmarkNode* bookmark_node, - Profile* profile, + sync_driver::SyncClient* sync_client, const GURL& icon_url, const scoped_refptr& bitmap_data); @@ -233,11 +238,13 @@ class BookmarkChangeProcessor : public bookmarks::BookmarkModelObserver, // Returns false if |node| should not be synced. bool CanSyncNode(const bookmarks::BookmarkNode* node); + base::ThreadChecker thread_checker_; + // The bookmark model we are processing changes from. Non-NULL when // |running_| is true. bookmarks::BookmarkModel* bookmark_model_; - Profile* profile_; + sync_driver::SyncClient* sync_client_; // The two models should be associated according to this ModelAssociator. BookmarkModelAssociator* model_associator_; diff --git a/chrome/browser/sync/glue/bookmark_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc index 5bb4f9afe97d77..44dcaee4c5c051 100644 --- a/chrome/browser/sync/glue/bookmark_model_associator.cc +++ b/chrome/browser/sync/glue/bookmark_model_associator.cc @@ -18,12 +18,11 @@ #include "base/strings/utf_string_conversions.h" #include "base/thread_task_runner_handle.h" #include "chrome/browser/sync/glue/bookmark_change_processor.h" -#include "chrome/browser/undo/bookmark_undo_service_factory.h" #include "components/bookmarks/browser/bookmark_client.h" #include "components/bookmarks/browser/bookmark_model.h" +#include "components/sync_driver/sync_client.h" #include "components/undo/bookmark_undo_service.h" #include "components/undo/bookmark_undo_utils.h" -#include "content/public/browser/browser_thread.h" #include "sync/api/sync_error.h" #include "sync/api/sync_merge_result.h" #include "sync/internal_api/public/delete_journal.h" @@ -39,7 +38,6 @@ using bookmarks::BookmarkModel; using bookmarks::BookmarkNode; -using content::BrowserThread; namespace browser_sync { @@ -353,29 +351,28 @@ void BookmarkModelAssociator::Context::MarkForVersionUpdate( BookmarkModelAssociator::BookmarkModelAssociator( BookmarkModel* bookmark_model, - Profile* profile, + sync_driver::SyncClient* sync_client, syncer::UserShare* user_share, sync_driver::DataTypeErrorHandler* unrecoverable_error_handler, bool expect_mobile_bookmarks_folder) : bookmark_model_(bookmark_model), - profile_(profile), + sync_client_(sync_client), user_share_(user_share), unrecoverable_error_handler_(unrecoverable_error_handler), expect_mobile_bookmarks_folder_(expect_mobile_bookmarks_folder), optimistic_association_enabled_(IsOptimisticAssociationEnabled()), weak_factory_(this) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(bookmark_model_); DCHECK(user_share_); DCHECK(unrecoverable_error_handler_); } BookmarkModelAssociator::~BookmarkModelAssociator() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(thread_checker_.CalledOnValidThread()); } void BookmarkModelAssociator::UpdatePermanentNodeVisibility() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(bookmark_model_->loaded()); BookmarkNode::Type bookmark_node_types[] = { @@ -427,7 +424,7 @@ bool BookmarkModelAssociator::InitSyncNodeFromChromeId( void BookmarkModelAssociator::AddAssociation(const BookmarkNode* node, int64 sync_id) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(thread_checker_.CalledOnValidThread()); int64 node_id = node->id(); DCHECK_NE(sync_id, syncer::kInvalidId); DCHECK(id_map_.find(node_id) == id_map_.end()); @@ -454,7 +451,7 @@ void BookmarkModelAssociator::Associate(const BookmarkNode* node, } void BookmarkModelAssociator::Disassociate(int64 sync_id) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(thread_checker_.CalledOnValidThread()); SyncIdToBookmarkNodeMap::iterator iter = id_map_inverse_.find(sync_id); if (iter == id_map_inverse_.end()) return; @@ -519,7 +516,7 @@ syncer::SyncError BookmarkModelAssociator::AssociateModels( // Since any changes to the bookmark model made here are not user initiated, // these change should not be undoable and so suspend the undo tracking. ScopedSuspendBookmarkUndo suspend_undo( - BookmarkUndoServiceFactory::GetForProfileIfExists(profile_)); + sync_client_->GetBookmarkUndoServiceIfExists()); Context context(local_merge_result, syncer_merge_result); @@ -756,7 +753,7 @@ syncer::SyncError BookmarkModelAssociator::BuildAssociations( // nothing has changed. // TODO(sync): Only modify the bookmark model if necessary. BookmarkChangeProcessor::UpdateBookmarkWithSyncData( - sync_child_node, bookmark_model_, child_node, profile_); + sync_child_node, bookmark_model_, child_node, sync_client_); bookmark_model_->Move(child_node, parent_node, index); context->IncrementLocalItemsModified(); } else { @@ -951,7 +948,7 @@ const BookmarkNode* BookmarkModelAssociator::CreateBookmarkNode( base::string16 bookmark_title = base::UTF8ToUTF16(sync_title); const BookmarkNode* child_node = BookmarkChangeProcessor::CreateBookmarkNode( bookmark_title, url, sync_child_node, parent_node, bookmark_model_, - profile_, bookmark_index); + sync_client_, bookmark_index); if (!child_node) { *error = unrecoverable_error_handler_->CreateAndUploadError( FROM_HERE, "Failed to create bookmark node with title " + sync_title + diff --git a/chrome/browser/sync/glue/bookmark_model_associator.h b/chrome/browser/sync/glue/bookmark_model_associator.h index 33d08c93001b68..3cf802f6051c25 100644 --- a/chrome/browser/sync/glue/bookmark_model_associator.h +++ b/chrome/browser/sync/glue/bookmark_model_associator.h @@ -15,11 +15,11 @@ #include "base/compiler_specific.h" #include "base/hash.h" #include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" #include "components/sync_driver/data_type_error_handler.h" #include "components/sync_driver/model_associator.h" #include "sync/internal_api/public/util/unrecoverable_error_handler.h" -class Profile; class GURL; namespace bookmarks { @@ -34,6 +34,10 @@ struct UserShare; class WriteTransaction; } +namespace sync_driver { +class SyncClient; +} + namespace browser_sync { // Contains all model association related logic: @@ -50,7 +54,7 @@ class BookmarkModelAssociator // Should be set to true only by mobile clients. BookmarkModelAssociator( bookmarks::BookmarkModel* bookmark_model, - Profile* profile_, + sync_driver::SyncClient* sync_client, syncer::UserShare* user_share, sync_driver::DataTypeErrorHandler* unrecoverable_error_handler, bool expect_mobile_bookmarks_folder); @@ -298,8 +302,9 @@ class BookmarkModelAssociator // the native model has a newer transaction verison. syncer::SyncError CheckModelSyncState(Context* context) const; + base::ThreadChecker thread_checker_; bookmarks::BookmarkModel* bookmark_model_; - Profile* profile_; + sync_driver::SyncClient* sync_client_; syncer::UserShare* user_share_; sync_driver::DataTypeErrorHandler* unrecoverable_error_handler_; const bool expect_mobile_bookmarks_folder_; diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.cc b/chrome/browser/sync/profile_sync_components_factory_impl.cc index 098bb8a35d61f3..76907fc8193454 100644 --- a/chrome/browser/sync/profile_sync_components_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_components_factory_impl.cc @@ -531,16 +531,11 @@ sync_driver::SyncApiComponentFactory::SyncComponents #else const bool kExpectMobileBookmarksFolder = false; #endif - BookmarkModelAssociator* model_associator = - new BookmarkModelAssociator(bookmark_model, - profile_, - user_share, - error_handler, - kExpectMobileBookmarksFolder); - BookmarkChangeProcessor* change_processor = - new BookmarkChangeProcessor(profile_, - model_associator, - error_handler); + BookmarkModelAssociator* model_associator = new BookmarkModelAssociator( + bookmark_model, sync_service->GetSyncClient(), user_share, error_handler, + kExpectMobileBookmarksFolder); + BookmarkChangeProcessor* change_processor = new BookmarkChangeProcessor( + sync_service->GetSyncClient(), model_associator, error_handler); return SyncComponents(model_associator, change_processor); } diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index d62419ca5085a4..17d508f9fae46c 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -1847,6 +1847,10 @@ syncer::ModelTypeSet ProfileSyncService::GetActiveDataTypes() const { return Difference(preferred_types, failed_types); } +sync_driver::SyncClient* ProfileSyncService::GetSyncClient() const { + return sync_client_.get(); +} + syncer::ModelTypeSet ProfileSyncService::GetPreferredDataTypes() const { const syncer::ModelTypeSet registered_types = GetRegisteredDataTypes(); const syncer::ModelTypeSet preferred_types = @@ -2660,10 +2664,6 @@ void ProfileSyncService::FlushDirectory() const { backend_->FlushDirectory(); } -sync_driver::SyncClient* ProfileSyncService::GetSyncClient() const { - return sync_client_.get(); -} - base::FilePath ProfileSyncService::GetDirectoryPathForTest() const { return directory_path_; } diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index 7d40410a00f397..8a29d5748ff418 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -249,6 +249,7 @@ class ProfileSyncService : public sync_driver::SyncService, void RequestStop(SyncStopDataFate data_fate) override; void RequestStart() override; syncer::ModelTypeSet GetActiveDataTypes() const override; + sync_driver::SyncClient* GetSyncClient() const override; syncer::ModelTypeSet GetPreferredDataTypes() const override; void OnUserChoseDatatypes(bool sync_everything, syncer::ModelTypeSet chosen_types) override; @@ -561,9 +562,6 @@ class ProfileSyncService : public sync_driver::SyncService, // killed in the near future. void FlushDirectory() const; - // Returns the SyncClient associated with this profile. - sync_driver::SyncClient* GetSyncClient() const; - // Needed to test whether the directory is deleted properly. base::FilePath GetDirectoryPathForTest() const; diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc index 9de252988cbb37..226c9657ac720c 100644 --- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc @@ -26,6 +26,8 @@ #include "chrome/browser/bookmarks/chrome_bookmark_client.h" #include "chrome/browser/bookmarks/chrome_bookmark_client_factory.h" #include "chrome/browser/bookmarks/managed_bookmark_service_factory.h" +#include "chrome/browser/favicon/favicon_service_factory.h" +#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/sync/glue/bookmark_change_processor.h" #include "chrome/browser/sync/glue/bookmark_model_associator.h" #include "chrome/common/chrome_switches.h" @@ -36,6 +38,7 @@ #include "components/bookmarks/test/bookmark_test_helpers.h" #include "components/sync_driver/data_type_error_handler.h" #include "components/sync_driver/data_type_error_handler_mock.h" +#include "components/sync_driver/fake_sync_client.h" #include "content/public/test/test_browser_thread_bundle.h" #include "sync/api/sync_error.h" #include "sync/api/sync_merge_result.h" @@ -88,6 +91,28 @@ void MakeServerUpdate(syncer::WriteTransaction* trans, int64 id) { MakeServerUpdate(trans, &node); } +class TestSyncClient : public sync_driver::FakeSyncClient { + public: + explicit TestSyncClient(Profile* profile) : profile_(profile) {} + + bookmarks::BookmarkModel* GetBookmarkModel() override { + return BookmarkModelFactory::GetForProfile(profile_); + } + + favicon::FaviconService* GetFaviconService() override { + return FaviconServiceFactory::GetForProfile( + profile_, ServiceAccessType::EXPLICIT_ACCESS); + } + + history::HistoryService* GetHistoryService() override { + return HistoryServiceFactory::GetForProfile( + profile_, ServiceAccessType::EXPLICIT_ACCESS); + } + + private: + Profile* profile_; +}; + // FakeServerChange constructs a list of syncer::ChangeRecords while modifying // the sync model, and can pass the ChangeRecord list to a // syncer::SyncObserver (i.e., the ProfileSyncService) to test the client @@ -350,7 +375,8 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { enum SaveOption { SAVE_TO_STORAGE, DONT_SAVE_TO_STORAGE }; ProfileSyncServiceBookmarkTest() - : model_(NULL), + : sync_client_(&profile_), + model_(NULL), local_merge_result_(syncer::BOOKMARKS), syncer_merge_result_(syncer::BOOKMARKS) {} @@ -522,10 +548,8 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { // Set up model associator. model_associator_.reset(new BookmarkModelAssociator( - BookmarkModelFactory::GetForProfile(&profile_), - &profile_, - test_user_share_.user_share(), - &mock_error_handler_, + BookmarkModelFactory::GetForProfile(&profile_), &sync_client_, + test_user_share_.user_share(), &mock_error_handler_, kExpectMobileBookmarks)); local_merge_result_ = syncer::SyncMergeResult(syncer::BOOKMARKS); @@ -568,10 +592,8 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { ASSERT_TRUE(AssociateModels()); // Set up change processor. - change_processor_.reset( - new BookmarkChangeProcessor(&profile_, - model_associator_.get(), - &mock_error_handler_)); + change_processor_.reset(new BookmarkChangeProcessor( + &sync_client_, model_associator_.get(), &mock_error_handler_)); change_processor_->Start(test_user_share_.user_share()); } @@ -781,6 +803,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test { protected: TestingProfile profile_; + TestSyncClient sync_client_; BookmarkModel* model_; syncer::TestUserShare test_user_share_; scoped_ptr change_processor_; @@ -2569,7 +2592,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, UpdateThenAdd) { // Recreate the change processor then update that bookmark. Sync should // receive the update call and gracefully treat that as if it were an add. change_processor_.reset(new BookmarkChangeProcessor( - &profile_, model_associator_.get(), &mock_error_handler_)); + &sync_client_, model_associator_.get(), &mock_error_handler_)); change_processor_->Start(test_user_share_.user_share()); model_->SetTitle(node, base::ASCIIToUTF16("title2")); ExpectModelMatch(); diff --git a/chrome/browser/sync/test/integration/bookmarks_helper.cc b/chrome/browser/sync/test/integration/bookmarks_helper.cc index 8526cac0e11802..28fca47243a999 100644 --- a/chrome/browser/sync/test/integration/bookmarks_helper.cc +++ b/chrome/browser/sync/test/integration/bookmarks_helper.cc @@ -25,6 +25,8 @@ #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/glue/bookmark_change_processor.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/sync/test/integration/await_match_status_change_checker.h" #include "chrome/browser/sync/test/integration/multi_client_status_change_checker.h" #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h" @@ -260,8 +262,10 @@ void SetFaviconImpl(Profile* profile, favicon_service->SetFavicons( node->url(), icon_url, favicon_base::FAVICON, image); } else { + ProfileSyncService* pss = + ProfileSyncServiceFactory::GetForProfile(profile); browser_sync::BookmarkChangeProcessor::ApplyBookmarkFavicon( - node, profile, icon_url, image.As1xPNGBytes()); + node, pss->GetSyncClient(), icon_url, image.As1xPNGBytes()); } // Wait for the favicon for |node| to be invalidated. diff --git a/components/sync_driver/fake_sync_client.cc b/components/sync_driver/fake_sync_client.cc index 04708715d59755..b85cc6bf10fec4 100644 --- a/components/sync_driver/fake_sync_client.cc +++ b/components/sync_driver/fake_sync_client.cc @@ -34,6 +34,10 @@ bookmarks::BookmarkModel* FakeSyncClient::GetBookmarkModel() { return nullptr; } +favicon::FaviconService* FakeSyncClient::GetFaviconService() { + return nullptr; +} + history::HistoryService* FakeSyncClient::GetHistoryService() { return nullptr; } @@ -52,6 +56,10 @@ FakeSyncClient::GetWebDataService() { return scoped_refptr(); } +BookmarkUndoService* FakeSyncClient::GetBookmarkUndoServiceIfExists() { + return nullptr; +} + base::WeakPtr FakeSyncClient::GetSyncableServiceForType(syncer::ModelType type) { return base::WeakPtr(); diff --git a/components/sync_driver/fake_sync_client.h b/components/sync_driver/fake_sync_client.h index 20071918af0aea..e87b116e936949 100644 --- a/components/sync_driver/fake_sync_client.h +++ b/components/sync_driver/fake_sync_client.h @@ -22,10 +22,12 @@ class FakeSyncClient : public SyncClient { SyncService* GetSyncService() override; PrefService* GetPrefService() override; bookmarks::BookmarkModel* GetBookmarkModel() override; + favicon::FaviconService* GetFaviconService() override; history::HistoryService* GetHistoryService() override; scoped_refptr GetPasswordStore() override; autofill::PersonalDataManager* GetPersonalDataManager() override; scoped_refptr GetWebDataService() override; + BookmarkUndoService* GetBookmarkUndoServiceIfExists() override; base::WeakPtr GetSyncableServiceForType( syncer::ModelType type) override; SyncApiComponentFactory* GetSyncApiComponentFactory() override; diff --git a/components/sync_driver/fake_sync_service.cc b/components/sync_driver/fake_sync_service.cc index 3c2c5e3aebaa9f..f8d37194fb9cea 100644 --- a/components/sync_driver/fake_sync_service.cc +++ b/components/sync_driver/fake_sync_service.cc @@ -34,6 +34,10 @@ syncer::ModelTypeSet FakeSyncService::GetActiveDataTypes() const { return syncer::ModelTypeSet(); } +SyncClient* FakeSyncService::GetSyncClient() const { + return nullptr; +} + void FakeSyncService::AddObserver(SyncServiceObserver* observer) { } diff --git a/components/sync_driver/fake_sync_service.h b/components/sync_driver/fake_sync_service.h index 4611544c92e722..dfd0e2c197c467 100644 --- a/components/sync_driver/fake_sync_service.h +++ b/components/sync_driver/fake_sync_service.h @@ -28,6 +28,7 @@ class FakeSyncService : public sync_driver::SyncService { bool IsSyncAllowed() const override; bool IsSyncActive() const override; syncer::ModelTypeSet GetActiveDataTypes() const override; + SyncClient* GetSyncClient() const override; void AddObserver(SyncServiceObserver* observer) override; void RemoveObserver(SyncServiceObserver* observer) override; bool HasObserver(const SyncServiceObserver* observer) const override; diff --git a/components/sync_driver/sync_client.h b/components/sync_driver/sync_client.h index a4788f7ea452db..870decf76dccff 100644 --- a/components/sync_driver/sync_client.h +++ b/components/sync_driver/sync_client.h @@ -10,6 +10,7 @@ #include "base/memory/weak_ptr.h" #include "sync/internal_api/public/base/model_type.h" +class BookmarkUndoService; class PrefService; namespace autofill { @@ -22,6 +23,10 @@ namespace bookmarks { class BookmarkModel; } // namespace bookmarks +namespace favicon { +class FaviconService; +} // namespace favicon + namespace history { class HistoryService; } // namespace history @@ -63,11 +68,13 @@ class SyncClient { // DataType specific service getters. virtual bookmarks::BookmarkModel* GetBookmarkModel() = 0; + virtual favicon::FaviconService* GetFaviconService() = 0; virtual history::HistoryService* GetHistoryService() = 0; virtual scoped_refptr GetPasswordStore() = 0; virtual autofill::PersonalDataManager* GetPersonalDataManager() = 0; virtual scoped_refptr GetWebDataService() = 0; + virtual BookmarkUndoService* GetBookmarkUndoServiceIfExists() = 0; // Returns a weak pointer to the syncable service specified by |type|. // Weak pointer may be unset if service is already destroyed. diff --git a/components/sync_driver/sync_service.h b/components/sync_driver/sync_service.h index 4b2de8d6c730fd..bdb75233691d4b 100644 --- a/components/sync_driver/sync_service.h +++ b/components/sync_driver/sync_service.h @@ -43,6 +43,7 @@ namespace sync_driver { class DataTypeController; class LocalDeviceInfoProvider; class OpenTabsUIDelegate; +class SyncClient; class SyncService : public DataTypeEncryptionHandler { public: @@ -109,6 +110,9 @@ class SyncService : public DataTypeEncryptionHandler { // be updated. virtual syncer::ModelTypeSet GetActiveDataTypes() const = 0; + // Returns the SyncClient instance associated with this service. + virtual SyncClient* GetSyncClient() const = 0; + // Adds/removes an observer. SyncService does not take ownership of the // observer. virtual void AddObserver(SyncServiceObserver* observer) = 0;