diff options
29 files changed, 140 insertions, 3321 deletions
diff --git a/chrome/browser/sync/chrome_sync_client.cc b/chrome/browser/sync/chrome_sync_client.cc index b826216..9cf8938 100644 --- a/chrome/browser/sync/chrome_sync_client.cc +++ b/chrome/browser/sync/chrome_sync_client.cc @@ -214,9 +214,7 @@ void ChromeSyncClient::Initialize(sync_driver::SyncService* sync_service) { } sync_driver::SyncService* ChromeSyncClient::GetSyncService() { - // TODO(zea): bring back this DCHECK after Typed URLs are converted to - // SyncableService. - // DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK_CURRENTLY_ON(BrowserThread::UI); return sync_service_; } @@ -354,6 +352,13 @@ ChromeSyncClient::GetSyncableServiceForType(syncer::ModelType type) { return history ? history->AsWeakPtr() : base::WeakPtr<history::HistoryService>(); } + case syncer::TYPED_URLS: { + history::HistoryService* history = HistoryServiceFactory::GetForProfile( + profile_, ServiceAccessType::EXPLICIT_ACCESS); + if (!history) + return base::WeakPtr<history::TypedUrlSyncableService>(); + return history->GetTypedUrlSyncableService()->AsWeakPtr(); + } #if defined(ENABLE_SPELLCHECK) case syncer::DICTIONARY: return SpellcheckServiceFactory::GetForContext(profile_)-> diff --git a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc index 540595a..c21a42a 100644 --- a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc @@ -42,9 +42,7 @@ #include "components/history/core/browser/history_db_task.h" #include "components/history/core/browser/history_service.h" #include "components/history/core/browser/history_types.h" -#include "components/history/core/browser/typed_url_change_processor.h" #include "components/history/core/browser/typed_url_data_type_controller.h" -#include "components/history/core/browser/typed_url_model_associator.h" #include "components/invalidation/impl/profile_invalidation_provider.h" #include "components/invalidation/public/invalidation_service.h" #include "components/keyed_service/core/refcounted_keyed_service.h" @@ -66,11 +64,10 @@ using base::Thread; using base::Time; -using browser_sync::TypedUrlChangeProcessor; using browser_sync::TypedUrlDataTypeController; -using browser_sync::TypedUrlModelAssociator; using history::HistoryBackend; using history::HistoryBackendNotifier; +using history::TypedUrlSyncableService; using history::URLID; using history::URLRow; using syncer::syncable::WriteTransaction; @@ -136,6 +133,8 @@ class HistoryServiceMock : public history::HistoryService { MOCK_METHOD0(Shutdown, void()); + MOCK_CONST_METHOD0(GetTypedUrlSyncableService, TypedUrlSyncableService*()); + void ShutdownBaseService() { history::HistoryService::Shutdown(); } void set_task_runner( @@ -170,13 +169,21 @@ scoped_ptr<KeyedService> BuildHistoryService(content::BrowserContext* profile) { return scoped_ptr<KeyedService>(new HistoryServiceMock); } -class TestTypedUrlModelAssociator : public TypedUrlModelAssociator { +class TestTypedUrlSyncableService : public TypedUrlSyncableService { + // TODO(gangwu): remove TestProfileSyncService or even remove whole test + // suite, and make sure typed_url_syncable_service_unittest.cc and the various + // typed url integration tests. public: - TestTypedUrlModelAssociator( - ProfileSyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) : - TypedUrlModelAssociator(sync_service, history_backend, error_handler) {} + TestTypedUrlSyncableService(history::HistoryBackend* history_backend) + : TypedUrlSyncableService(history_backend) {} + + static void WriteToSyncNode(const history::URLRow& url, + const history::VisitVector& visits, + syncer::WriteNode* node) { + sync_pb::TypedUrlSpecifics typed_url; + WriteToTypedUrlSpecifics(url, visits, &typed_url); + node->SetTypedUrlSpecifics(typed_url); + } protected: // Don't clear error stats - that way we can verify their values in our @@ -189,24 +196,9 @@ ACTION_P2(ShutdownHistoryService, thread, service) { delete thread; } -ACTION_P6(MakeTypedUrlSyncComponents, - profile, - service, - hb, - dtc, - error_handler, - model_associator) { - *model_associator = - new TestTypedUrlModelAssociator(service, hb, error_handler); - - const scoped_refptr<base::SingleThreadTaskRunner> ui_thread = - content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::UI); - TypedUrlChangeProcessor* change_processor = - new TypedUrlChangeProcessor(*model_associator, hb, dtc, ui_thread); - - return sync_driver::SyncApiComponentFactory::SyncComponents(*model_associator, - change_processor); +ACTION_P2(ReturnTypedUrlSyncableService, hb, syncable_service) { + syncable_service->reset(new TestTypedUrlSyncableService(hb)); + return syncable_service->get(); } class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest { @@ -220,7 +212,7 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest { syncer::WriteNode::InitUniqueByCreationResult result = node.InitUniqueByCreation(syncer::TYPED_URLS, tag); ASSERT_EQ(syncer::WriteNode::INIT_SUCCESS, result); - TypedUrlModelAssociator::WriteToSyncNode(url, visits, &node); + TestTypedUrlSyncableService::WriteToSyncNode(url, visits, &node); } protected: @@ -263,8 +255,7 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest { AbstractProfileSyncServiceTest::TearDown(); } - TypedUrlModelAssociator* StartSyncService(const base::Closure& callback) { - TypedUrlModelAssociator* model_associator = NULL; + TypedUrlSyncableService* StartSyncService(const base::Closure& callback) { if (!sync_service_) { std::string account_id = AccountTrackerServiceFactory::GetForProfile(profile_) @@ -273,23 +264,19 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest { signin->SetAuthenticatedAccountInfo("gaia_id", "test"); sync_service_ = TestProfileSyncService::BuildAutoStartAsyncInit(profile_, callback); - TypedUrlDataTypeController* data_type_controller = - new TypedUrlDataTypeController(base::ThreadTaskRunnerHandle::Get(), - base::Bind(&base::DoNothing), - sync_service_->GetSyncClient(), - prefs::kSavingBrowserHistoryDisabled); + data_type_controller = new TypedUrlDataTypeController( + base::ThreadTaskRunnerHandle::Get(), base::Bind(&base::DoNothing), + sync_service_->GetSyncClient(), prefs::kSavingBrowserHistoryDisabled); SyncApiComponentFactoryMock* components = sync_service_->GetSyncApiComponentFactoryMock(); - EXPECT_CALL(*components, CreateTypedUrlSyncComponents(_, _, _)). - WillOnce(MakeTypedUrlSyncComponents(profile_, - sync_service_, - history_backend_.get(), - data_type_controller, - &error_handler_, - &model_associator)); + EXPECT_CALL(*components, CreateDataTypeManager(_, _, _, _, _)). WillOnce(ReturnNewDataTypeManager()); + EXPECT_CALL(*history_service_, GetTypedUrlSyncableService()) + .WillOnce(ReturnTypedUrlSyncableService(history_backend_.get(), + &syncable_service_)); + ProfileOAuth2TokenService* oauth2_token_service = ProfileOAuth2TokenServiceFactory::GetForProfile(profile_); oauth2_token_service->UpdateCredentials(account_id, "oauth2_login_token"); @@ -299,7 +286,7 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest { sync_service_->Initialize(); base::MessageLoop::current()->Run(); } - return model_associator; + return syncable_service_.get(); } void GetTypedUrlsFromSyncDB(history::URLRows* urls) { @@ -416,6 +403,8 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest { scoped_refptr<HistoryBackendMock> history_backend_; HistoryServiceMock* history_service_; sync_driver::DataTypeErrorHandlerMock error_handler_; + TypedUrlDataTypeController* data_type_controller; + scoped_ptr<TestTypedUrlSyncableService> syncable_service_; }; void AddTypedUrlEntries(ProfileSyncServiceTypedUrlTest* test, @@ -437,12 +426,12 @@ TEST_F(ProfileSyncServiceTypedUrlTest, EmptyNativeEmptySync) { WillOnce(Return(true)); SetIdleChangeProcessorExpectations(); CreateRootHelper create_root(this, syncer::TYPED_URLS); - TypedUrlModelAssociator* associator = + TypedUrlSyncableService* syncable_service = StartSyncService(create_root.callback()); history::URLRows sync_entries; GetTypedUrlsFromSyncDB(&sync_entries); EXPECT_EQ(0U, sync_entries.size()); - ASSERT_EQ(0, associator->GetErrorPercentage()); + ASSERT_EQ(0, syncable_service->GetErrorPercentage()); } TEST_F(ProfileSyncServiceTypedUrlTest, HasNativeEmptySync) { @@ -457,13 +446,13 @@ TEST_F(ProfileSyncServiceTypedUrlTest, HasNativeEmptySync) { WillRepeatedly(DoAll(SetArgumentPointee<2>(visits), Return(true))); SetIdleChangeProcessorExpectations(); CreateRootHelper create_root(this, syncer::TYPED_URLS); - TypedUrlModelAssociator* associator = + TypedUrlSyncableService* syncable_service = StartSyncService(create_root.callback()); history::URLRows sync_entries; GetTypedUrlsFromSyncDB(&sync_entries); ASSERT_EQ(1U, sync_entries.size()); EXPECT_TRUE(URLsEqual(entries[0], sync_entries[0])); - ASSERT_EQ(0, associator->GetErrorPercentage()); + ASSERT_EQ(0, syncable_service->GetErrorPercentage()); } TEST_F(ProfileSyncServiceTypedUrlTest, HasNativeErrorReadingVisits) { @@ -954,8 +943,8 @@ TEST_F(ProfileSyncServiceTypedUrlTest, FailWriteToHistoryBackend) { native_entries.push_back(native_entry); EXPECT_CALL((*history_backend_.get()), GetAllTypedURLs(_)). WillOnce(DoAll(SetArgumentPointee<0>(native_entries), Return(true))); - EXPECT_CALL((*history_backend_.get()), GetURL(_, _)). - WillOnce(DoAll(SetArgumentPointee<1>(native_entry), Return(true))); + EXPECT_CALL((*history_backend_.get()), GetURL(_, _)) + .WillOnce(DoAll(SetArgumentPointee<1>(native_entry), Return(false))); EXPECT_CALL((*history_backend_.get()), GetMostRecentVisitsForURL(_, _, _)). WillRepeatedly(DoAll(SetArgumentPointee<2>(native_visits), Return(true))); EXPECT_CALL((*history_backend_.get()), @@ -966,7 +955,7 @@ TEST_F(ProfileSyncServiceTypedUrlTest, FailWriteToHistoryBackend) { EXPECT_CALL((*history_backend_.get()), UpdateURL(_, _)). WillRepeatedly(Return(false)); - TypedUrlModelAssociator* associator = + TypedUrlSyncableService* syncable_service = StartSyncService(base::Bind(&AddTypedUrlEntries, this, sync_entries)); // Errors writing to the DB should be recorded, but should not cause an // unrecoverable error. @@ -975,8 +964,8 @@ TEST_F(ProfileSyncServiceTypedUrlTest, FailWriteToHistoryBackend) { syncer::TYPED_URLS)); // Some calls should have succeeded, so the error percentage should be // somewhere > 0 and < 100. - ASSERT_NE(0, associator->GetErrorPercentage()); - ASSERT_NE(100, associator->GetErrorPercentage()); + ASSERT_NE(0, syncable_service->GetErrorPercentage()); + ASSERT_NE(100, syncable_service->GetErrorPercentage()); } TEST_F(ProfileSyncServiceTypedUrlTest, FailToGetTypedURLs) { @@ -995,12 +984,6 @@ TEST_F(ProfileSyncServiceTypedUrlTest, FailToGetTypedURLs) { history::URLRows sync_entries; sync_entries.push_back(sync_entry); - EXPECT_CALL(error_handler_, CreateAndUploadError(_, _, _)). - WillOnce(Return(syncer::SyncError( - FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Unit test", - syncer::TYPED_URLS))); StartSyncService(base::Bind(&AddTypedUrlEntries, this, sync_entries)); // Errors getting typed URLs will cause an unrecoverable error (since we can // do *nothing* in that case). diff --git a/components/browser_sync/browser/profile_sync_components_factory_impl.cc b/components/browser_sync/browser/profile_sync_components_factory_impl.cc index 1df449d..2ed6c9d 100644 --- a/components/browser_sync/browser/profile_sync_components_factory_impl.cc +++ b/components/browser_sync/browser/profile_sync_components_factory_impl.cc @@ -18,9 +18,7 @@ #include "components/browser_sync/common/browser_sync_switches.h" #include "components/dom_distiller/core/dom_distiller_features.h" #include "components/history/core/browser/history_delete_directives_data_type_controller.h" -#include "components/history/core/browser/typed_url_change_processor.h" #include "components/history/core/browser/typed_url_data_type_controller.h" -#include "components/history/core/browser/typed_url_model_associator.h" #include "components/password_manager/core/browser/password_store.h" #include "components/password_manager/sync/browser/password_data_type_controller.h" #include "components/sync_bookmarks/bookmark_change_processor.h" @@ -55,9 +53,7 @@ using browser_sync::HistoryDeleteDirectivesDataTypeController; using browser_sync::PasswordDataTypeController; using browser_sync::SessionDataTypeController; using browser_sync::SyncBackendHost; -using browser_sync::TypedUrlChangeProcessor; using browser_sync::TypedUrlDataTypeController; -using browser_sync::TypedUrlModelAssociator; using sync_driver::DataTypeController; using sync_driver::DataTypeErrorHandler; using sync_driver::DataTypeManager; @@ -380,20 +376,3 @@ ProfileSyncComponentsFactoryImpl::CreateBookmarkSyncComponents( sync_service->GetSyncClient(), model_associator, error_handler); return SyncComponents(model_associator, change_processor); } - -sync_driver::SyncApiComponentFactory::SyncComponents -ProfileSyncComponentsFactoryImpl::CreateTypedUrlSyncComponents( - sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) { - DCHECK(!ui_thread_->BelongsToCurrentThread()); - - // TODO(zea): Once TypedURLs are converted to SyncableService, remove - // |sync_service_| member, and make GetSyncService require it be called on - // the UI thread. - TypedUrlModelAssociator* model_associator = - new TypedUrlModelAssociator(sync_service, history_backend, error_handler); - TypedUrlChangeProcessor* change_processor = new TypedUrlChangeProcessor( - model_associator, history_backend, error_handler, ui_thread_); - return SyncComponents(model_associator, change_processor); -} diff --git a/components/browser_sync/browser/profile_sync_components_factory_impl.h b/components/browser_sync/browser/profile_sync_components_factory_impl.h index 0da2d98..b3a6356 100644 --- a/components/browser_sync/browser/profile_sync_components_factory_impl.h +++ b/components/browser_sync/browser/profile_sync_components_factory_impl.h @@ -85,11 +85,6 @@ class ProfileSyncComponentsFactoryImpl CreateBookmarkSyncComponents( sync_driver::SyncService* sync_service, sync_driver::DataTypeErrorHandler* error_handler) override; - sync_driver::SyncApiComponentFactory::SyncComponents - CreateTypedUrlSyncComponents( - sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) override; private: // Register data types which are enabled on both desktop and mobile. diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 2ff9375..7105cc6 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -289,7 +289,6 @@ 'history/core/browser/top_sites_cache_unittest.cc', 'history/core/browser/top_sites_database_unittest.cc', 'history/core/browser/top_sites_impl_unittest.cc', - 'history/core/browser/typed_url_model_associator_unittest.cc', 'history/core/browser/typed_url_syncable_service_unittest.cc', 'history/core/browser/url_database_unittest.cc', 'history/core/browser/url_utils_unittest.cc', @@ -719,7 +718,6 @@ 'sync_driver/local_device_info_provider_unittest.cc', 'sync_driver/model_association_manager_unittest.cc', 'sync_driver/non_blocking_data_type_controller_unittest.cc', - 'sync_driver/non_frontend_data_type_controller_unittest.cc', 'sync_driver/non_ui_data_type_controller_unittest.cc', 'sync_driver/profile_sync_auth_provider_unittest.cc', 'sync_driver/shared_change_processor_unittest.cc', diff --git a/components/history.gypi b/components/history.gypi index 1252f59..00f6882 100644 --- a/components/history.gypi +++ b/components/history.gypi @@ -95,12 +95,8 @@ 'history/core/browser/top_sites_impl.cc', 'history/core/browser/top_sites_impl.h', 'history/core/browser/top_sites_observer.h', - 'history/core/browser/typed_url_change_processor.cc', - 'history/core/browser/typed_url_change_processor.h', 'history/core/browser/typed_url_data_type_controller.cc', 'history/core/browser/typed_url_data_type_controller.h', - 'history/core/browser/typed_url_model_associator.cc', - 'history/core/browser/typed_url_model_associator.h', 'history/core/browser/typed_url_syncable_service.cc', 'history/core/browser/typed_url_syncable_service.h', 'history/core/browser/url_database.cc', diff --git a/components/history/core/browser/BUILD.gn b/components/history/core/browser/BUILD.gn index e894ec6..0d88839 100644 --- a/components/history/core/browser/BUILD.gn +++ b/components/history/core/browser/BUILD.gn @@ -62,12 +62,8 @@ static_library("browser") { "top_sites_impl.cc", "top_sites_impl.h", "top_sites_observer.h", - "typed_url_change_processor.cc", - "typed_url_change_processor.h", "typed_url_data_type_controller.cc", "typed_url_data_type_controller.h", - "typed_url_model_associator.cc", - "typed_url_model_associator.h", "typed_url_syncable_service.cc", "typed_url_syncable_service.h", "url_database.cc", @@ -154,7 +150,6 @@ source_set("unit_tests") { "top_sites_cache_unittest.cc", "top_sites_database_unittest.cc", "top_sites_impl_unittest.cc", - "typed_url_model_associator_unittest.cc", "typed_url_syncable_service_unittest.cc", "url_database_unittest.cc", "url_utils_unittest.cc", diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc index 0982fcd..0779add 100644 --- a/components/history/core/browser/history_backend.cc +++ b/components/history/core/browser/history_backend.cc @@ -2551,10 +2551,6 @@ void HistoryBackend::NotifyURLVisited(ui::PageTransition transition, const URLRow& row, const RedirectList& redirects, base::Time visit_time) { - if (typed_url_syncable_service_) - typed_url_syncable_service_->OnURLVisited(this, transition, row, redirects, - visit_time); - FOR_EACH_OBSERVER(HistoryBackendObserver, observers_, OnURLVisited(this, transition, row, redirects, visit_time)); @@ -2563,9 +2559,6 @@ void HistoryBackend::NotifyURLVisited(ui::PageTransition transition, } void HistoryBackend::NotifyURLsModified(const URLRows& rows) { - if (typed_url_syncable_service_) - typed_url_syncable_service_->OnURLsModified(this, rows); - FOR_EACH_OBSERVER(HistoryBackendObserver, observers_, OnURLsModified(this, rows)); @@ -2578,11 +2571,6 @@ void HistoryBackend::NotifyURLsDeleted(bool all_history, const URLRows& rows, const std::set<GURL>& favicon_urls) { URLRows copied_rows(rows); - if (typed_url_syncable_service_) { - typed_url_syncable_service_->OnURLsDeleted(this, all_history, expired, - copied_rows, favicon_urls); - } - FOR_EACH_OBSERVER( HistoryBackendObserver, observers_, OnURLsDeleted(this, all_history, expired, copied_rows, favicon_urls)); diff --git a/components/history/core/browser/history_service.h b/components/history/core/browser/history_service.h index aa36e9e..c535158 100644 --- a/components/history/core/browser/history_service.h +++ b/components/history/core/browser/history_service.h @@ -146,7 +146,7 @@ class HistoryService : public syncer::SyncableService, public KeyedService { // Returns a pointer to the TypedUrlSyncableService owned by HistoryBackend. // This method should only be called from the history thread, because the // returned service is intended to be accessed only via the history thread. - TypedUrlSyncableService* GetTypedUrlSyncableService() const; + virtual TypedUrlSyncableService* GetTypedUrlSyncableService() const; // KeyedService: void Shutdown() override; diff --git a/components/history/core/browser/typed_url_change_processor.cc b/components/history/core/browser/typed_url_change_processor.cc deleted file mode 100644 index 4395e80..0000000 --- a/components/history/core/browser/typed_url_change_processor.cc +++ /dev/null @@ -1,356 +0,0 @@ -// Copyright (c) 2012 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 "components/history/core/browser/typed_url_change_processor.h" - -#include "base/location.h" -#include "base/metrics/histogram.h" -#include "base/single_thread_task_runner.h" -#include "base/strings/string_util.h" -#include "base/strings/utf_string_conversions.h" -#include "base/thread_task_runner_handle.h" -#include "components/history/core/browser/history_backend.h" -#include "components/history/core/browser/typed_url_model_associator.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" -#include "sync/internal_api/public/write_transaction.h" -#include "sync/protocol/typed_url_specifics.pb.h" -#include "sync/syncable/entry.h" // TODO(tim): Investigating bug 121587. - -namespace browser_sync { - -// This is the threshold at which we start throttling sync updates for typed -// URLs - any URLs with a typed_count >= this threshold will be throttled. -static const int kTypedUrlVisitThrottleThreshold = 10; - -// This is the multiple we use when throttling sync updates. If the multiple is -// N, we sync up every Nth update (i.e. when typed_count % N == 0). -static const int kTypedUrlVisitThrottleMultiple = 10; - -TypedUrlChangeProcessor::TypedUrlChangeProcessor( - TypedUrlModelAssociator* model_associator, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler, - const scoped_refptr<base::SingleThreadTaskRunner> ui_thread) - : sync_driver::ChangeProcessor(error_handler), - model_associator_(model_associator), - history_backend_(history_backend), - ui_thread_(ui_thread), - backend_thread_(base::ThreadTaskRunnerHandle::Get()), - disconnected_(false), - history_backend_observer_(this) { - DCHECK(model_associator); - DCHECK(history_backend); - DCHECK(error_handler); -} - -TypedUrlChangeProcessor::~TypedUrlChangeProcessor() { - DCHECK(backend_thread_->BelongsToCurrentThread()); - DCHECK(history_backend_); - history_backend_->RemoveObserver(this); -} - -void TypedUrlChangeProcessor::OnURLVisited( - history::HistoryBackend* history_backend, - ui::PageTransition transition, - const history::URLRow& row, - const history::RedirectList& redirects, - base::Time visit_time) { - DCHECK(backend_thread_->BelongsToCurrentThread()); - - base::AutoLock al(disconnect_lock_); - if (disconnected_) - return; - - DVLOG(1) << "Observed typed_url change."; - if (ShouldSyncVisit(row.typed_count(), transition)) { - VisitsToSync visits_to_sync; - if (FixupURLAndGetVisitsToSync(row, &visits_to_sync)) { - syncer::WriteTransaction trans(FROM_HERE, share_handle()); - CreateOrUpdateSyncNode(&trans, visits_to_sync.row, visits_to_sync.visits); - } - } - UMA_HISTOGRAM_PERCENTAGE("Sync.TypedUrlChangeProcessorErrors", - model_associator_->GetErrorPercentage()); -} - -void TypedUrlChangeProcessor::OnURLsModified( - history::HistoryBackend* history_backend, - const history::URLRows& changed_urls) { - DCHECK(backend_thread_->BelongsToCurrentThread()); - - base::AutoLock al(disconnect_lock_); - if (disconnected_) - return; - - std::vector<VisitsToSync> visits_to_sync_vector; - - DVLOG(1) << "Observed typed_url change."; - for (const auto& row : changed_urls) { - if (row.typed_count() >= 0) { - VisitsToSync visits_to_sync; - if (FixupURLAndGetVisitsToSync(row, &visits_to_sync)) { - visits_to_sync_vector.push_back(visits_to_sync); - } - } - } - - if (!visits_to_sync_vector.empty()) { - syncer::WriteTransaction trans(FROM_HERE, share_handle()); - for (const auto& visits_to_sync : visits_to_sync_vector) { - // If there were any errors updating the sync node, just ignore them and - // continue on to process the next URL. - CreateOrUpdateSyncNode(&trans, visits_to_sync.row, visits_to_sync.visits); - } - } - - UMA_HISTOGRAM_PERCENTAGE("Sync.TypedUrlChangeProcessorErrors", - model_associator_->GetErrorPercentage()); -} - -void TypedUrlChangeProcessor::OnURLsDeleted( - history::HistoryBackend* history_backend, - bool all_history, - bool expired, - const history::URLRows& deleted_rows, - const std::set<GURL>& favicon_urls) { - DCHECK(backend_thread_->BelongsToCurrentThread()); - - base::AutoLock al(disconnect_lock_); - if (disconnected_) - return; - - DVLOG(1) << "Observed typed_url change."; - - syncer::WriteTransaction trans(FROM_HERE, share_handle()); - - // Ignore archivals (we don't want to sync them as deletions, to avoid - // extra traffic up to the server, and also to make sure that a client with - // a bad clock setting won't go on an archival rampage and delete all - // history from every client). The server will gracefully age out the sync DB - // entries when they've been idle for long enough. - if (expired) - return; - - if (all_history) { - if (!model_associator_->DeleteAllNodes(&trans)) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to delete local nodes.", - syncer::TYPED_URLS); - error_handler()->OnSingleDataTypeUnrecoverableError(error); - return; - } - } else { - for (const auto& row : deleted_rows) { - syncer::WriteNode sync_node(&trans); - // The deleted URL could have been non-typed, so it might not be found - // in the sync DB. - if (sync_node.InitByClientTagLookup(syncer::TYPED_URLS, - row.url().spec()) == - syncer::BaseNode::INIT_OK) { - sync_node.Tombstone(); - } - } - } - UMA_HISTOGRAM_PERCENTAGE("Sync.TypedUrlChangeProcessorErrors", - model_associator_->GetErrorPercentage()); -} - -TypedUrlChangeProcessor::VisitsToSync::VisitsToSync() {} - -TypedUrlChangeProcessor::VisitsToSync::~VisitsToSync() {} - -bool TypedUrlChangeProcessor::FixupURLAndGetVisitsToSync( - const history::URLRow& url, - VisitsToSync* visits_to_sync) { - DCHECK_GE(url.typed_count(), 0); - - // Get the visits for this node. - visits_to_sync->row = url; - if (!model_associator_->FixupURLAndGetVisits(&visits_to_sync->row, - &visits_to_sync->visits)) { - DLOG(ERROR) << "Could not load visits for url: " << url.url(); - return false; - } - - if (std::find_if(visits_to_sync->visits.begin(), visits_to_sync->visits.end(), - [](const history::VisitRow& visit) { - return ui::PageTransitionCoreTypeIs( - visit.transition, ui::PAGE_TRANSITION_TYPED); - }) == visits_to_sync->visits.end()) { - // This URL has no TYPED visits, don't sync it. - return false; - } - - if (model_associator_->ShouldIgnoreUrl(visits_to_sync->row.url())) - return false; - - return true; -} - -void TypedUrlChangeProcessor::CreateOrUpdateSyncNode( - syncer::WriteTransaction* trans, - const history::URLRow& url, - const history::VisitVector& visit_vector) { - DCHECK_GE(url.typed_count(), 0); - DCHECK(!visit_vector.empty()); - - std::string tag = url.url().spec(); - syncer::WriteNode update_node(trans); - syncer::BaseNode::InitByLookupResult result = - update_node.InitByClientTagLookup(syncer::TYPED_URLS, tag); - if (result == syncer::BaseNode::INIT_OK) { - model_associator_->WriteToSyncNode(url, visit_vector, &update_node); - } else if (result == syncer::BaseNode::INIT_FAILED_DECRYPT_IF_NECESSARY) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to decrypt.", syncer::TYPED_URLS); - error_handler()->OnSingleDataTypeUnrecoverableError(error); - return; - } else { - syncer::WriteNode create_node(trans); - syncer::WriteNode::InitUniqueByCreationResult result = - create_node.InitUniqueByCreation(syncer::TYPED_URLS, tag); - if (result != syncer::WriteNode::INIT_SUCCESS) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to create sync node", syncer::TYPED_URLS); - error_handler()->OnSingleDataTypeUnrecoverableError(error); - return; - } - - create_node.SetTitle(tag); - model_associator_->WriteToSyncNode(url, visit_vector, &create_node); - } -} - -bool TypedUrlChangeProcessor::ShouldSyncVisit(int typed_count, - ui::PageTransition transition) { - // Just use an ad-hoc criteria to determine whether to ignore this - // notification. For most users, the distribution of visits is roughly a bell - // curve with a long tail - there are lots of URLs with < 5 visits so we want - // to make sure we sync up every visit to ensure the proper ordering of - // suggestions. But there are relatively few URLs with > 10 visits, and those - // tend to be more broadly distributed such that there's no need to sync up - // every visit to preserve their relative ordering. - return (ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_TYPED) && - typed_count >= 0 && - (typed_count < kTypedUrlVisitThrottleThreshold || - (typed_count % kTypedUrlVisitThrottleMultiple) == 0)); -} - -void TypedUrlChangeProcessor::ApplyChangesFromSyncModel( - const syncer::BaseTransaction* trans, - int64 model_version, - const syncer::ImmutableChangeRecordList& changes) { - DCHECK(backend_thread_->BelongsToCurrentThread()); - - base::AutoLock al(disconnect_lock_); - if (disconnected_) - return; - - syncer::ReadNode typed_url_root(trans); - if (typed_url_root.InitTypeRoot(syncer::TYPED_URLS) != - syncer::BaseNode::INIT_OK) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to init type root.", syncer::TYPED_URLS); - error_handler()->OnSingleDataTypeUnrecoverableError(error); - return; - } - - DCHECK(pending_new_urls_.empty() && pending_new_visits_.empty() && - pending_deleted_visits_.empty() && pending_updated_urls_.empty() && - pending_deleted_urls_.empty()); - - for (syncer::ChangeRecordList::const_iterator it = changes.Get().begin(); - it != changes.Get().end(); ++it) { - if (syncer::ChangeRecord::ACTION_DELETE == it->action) { - DCHECK(it->specifics.has_typed_url()) - << "Typed URL delete change does not have necessary specifics."; - GURL url(it->specifics.typed_url().url()); - pending_deleted_urls_.push_back(url); - continue; - } - - syncer::ReadNode sync_node(trans); - if (sync_node.InitByIdLookup(it->id) != syncer::BaseNode::INIT_OK) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to init sync node.", syncer::TYPED_URLS); - error_handler()->OnSingleDataTypeUnrecoverableError(error); - return; - } - - DCHECK(syncer::TYPED_URLS == sync_node.GetModelType()); - - const sync_pb::TypedUrlSpecifics& typed_url( - sync_node.GetTypedUrlSpecifics()); - DCHECK(typed_url.visits_size()); - - if (model_associator_->ShouldIgnoreUrl(GURL(typed_url.url()))) - continue; - - sync_pb::TypedUrlSpecifics filtered_url = - model_associator_->FilterExpiredVisits(typed_url); - if (!filtered_url.visits_size()) { - continue; - } - - model_associator_->UpdateFromSyncDB( - filtered_url, &pending_new_visits_, &pending_deleted_visits_, - &pending_updated_urls_, &pending_new_urls_); - } -} - -void TypedUrlChangeProcessor::CommitChangesFromSyncModel() { - DCHECK(backend_thread_->BelongsToCurrentThread()); - - base::AutoLock al(disconnect_lock_); - if (disconnected_) - return; - - // Make sure we stop listening for changes while we're modifying the backend, - // so we don't try to re-apply these changes to the sync DB. - ScopedStopObserving<TypedUrlChangeProcessor> stop_observing(this); - if (!pending_deleted_urls_.empty()) - history_backend_->DeleteURLs(pending_deleted_urls_); - - model_associator_->WriteToHistoryBackend( - &pending_new_urls_, &pending_updated_urls_, &pending_new_visits_, - &pending_deleted_visits_); - - pending_new_urls_.clear(); - pending_updated_urls_.clear(); - pending_new_visits_.clear(); - pending_deleted_visits_.clear(); - pending_deleted_urls_.clear(); - UMA_HISTOGRAM_PERCENTAGE("Sync.TypedUrlChangeProcessorErrors", - model_associator_->GetErrorPercentage()); -} - -void TypedUrlChangeProcessor::Disconnect() { - base::AutoLock al(disconnect_lock_); - disconnected_ = true; -} - -void TypedUrlChangeProcessor::StartImpl() { - DCHECK(history_backend_); - DCHECK(backend_thread_); - DCHECK(ui_thread_->BelongsToCurrentThread()); - backend_thread_->PostTask(FROM_HERE, - base::Bind(&TypedUrlChangeProcessor::StartObserving, - base::Unretained(this))); -} - -void TypedUrlChangeProcessor::StartObserving() { - DCHECK(backend_thread_->BelongsToCurrentThread()); - DCHECK(history_backend_); - history_backend_observer_.Add(history_backend_); -} - -void TypedUrlChangeProcessor::StopObserving() { - DCHECK(backend_thread_->BelongsToCurrentThread()); - DCHECK(history_backend_); - history_backend_observer_.RemoveAll(); -} - -} // namespace browser_sync diff --git a/components/history/core/browser/typed_url_change_processor.h b/components/history/core/browser/typed_url_change_processor.h deleted file mode 100644 index e397d30..0000000 --- a/components/history/core/browser/typed_url_change_processor.h +++ /dev/null @@ -1,137 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_HISTORY_CORE_BROWSER_TYPED_URL_CHANGE_PROCESSOR_H_ -#define COMPONENTS_HISTORY_CORE_BROWSER_TYPED_URL_CHANGE_PROCESSOR_H_ - -#include "components/sync_driver/change_processor.h" - -#include "base/basictypes.h" -#include "base/compiler_specific.h" -#include "base/memory/scoped_ptr.h" -#include "base/scoped_observer.h" -#include "components/history/core/browser/history_backend_observer.h" -#include "components/history/core/browser/typed_url_model_associator.h" -#include "components/sync_driver/data_type_error_handler.h" - -namespace base { -class MessageLoop; -} - -namespace history { -class HistoryBackend; -class URLRow; -}; - -namespace browser_sync { - -class DataTypeErrorHandler; - -// This class is responsible for taking changes from the history backend and -// applying them to the sync API 'syncable' model, and vice versa. All -// operations and use of this class are from the UI thread. -class TypedUrlChangeProcessor : public sync_driver::ChangeProcessor, - public history::HistoryBackendObserver { - public: - TypedUrlChangeProcessor( - TypedUrlModelAssociator* model_associator, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler, - const scoped_refptr<base::SingleThreadTaskRunner> ui_thread); - ~TypedUrlChangeProcessor() override; - - // sync API model -> WebDataService change application. - void ApplyChangesFromSyncModel( - const syncer::BaseTransaction* trans, - int64 model_version, - const syncer::ImmutableChangeRecordList& changes) override; - - // Commit changes here, after we've released the transaction lock to avoid - // jank. - void CommitChangesFromSyncModel() override; - - // Stop processing changes and wait for being destroyed. - void Disconnect(); - - protected: - void StartImpl() override; - - private: - friend class ScopedStopObserving<TypedUrlChangeProcessor>; - void StartObserving(); - void StopObserving(); - - // history::HistoryBackendObserver: - void OnURLVisited(history::HistoryBackend* history_backend, - ui::PageTransition transition, - const history::URLRow& row, - const history::RedirectList& redirects, - base::Time visit_time) override; - void OnURLsModified(history::HistoryBackend* history_backend, - const history::URLRows& changed_urls) override; - void OnURLsDeleted(history::HistoryBackend* history_backend, - bool all_history, - bool expired, - const history::URLRows& deleted_rows, - const std::set<GURL>& favicon_urls) override; - - // Returns true if the caller should sync as a result of the passed visit - // notification. We use this to throttle the number of sync changes we send - // to the server so we don't hit the server for every - // single typed URL visit. - bool ShouldSyncVisit(int typed_count, ui::PageTransition transition); - - // This struct is used to return result from FixupURLAndGetVisitsToSync - // function below. - struct VisitsToSync { - VisitsToSync(); - ~VisitsToSync(); - - history::URLRow row; - history::VisitVector visits; - }; - - // Utility function that fixes the URL and retrieves visits to be used to - // update sync node for the given |url|. Returns false if the data can't be - // retrieved from History backend or if there are no typed visits to sync. - bool FixupURLAndGetVisitsToSync(const history::URLRow& row, - VisitsToSync* visits_to_sync); - - // Utility routine that either updates an existing sync node or creates a - // new one for the passed |typed_url| if one does not already exist. Returns - // false and sets an unrecoverable error if the operation failed. - void CreateOrUpdateSyncNode(syncer::WriteTransaction* transaction, - const history::URLRow& typed_url, - const history::VisitVector& visit_vector); - - // The two models should be associated according to this ModelAssociator. - TypedUrlModelAssociator* model_associator_; - - // The model we are processing changes from. This is owned by the - // WebDataService which is kept alive by our data type controller - // holding a reference. - history::HistoryBackend* history_backend_; - scoped_refptr<base::SingleThreadTaskRunner> ui_thread_; - scoped_refptr<base::SingleThreadTaskRunner> backend_thread_; - - // The set of pending changes that will be written out on the next - // CommitChangesFromSyncModel() call. - history::URLRows pending_new_urls_; - history::URLRows pending_updated_urls_; - std::vector<GURL> pending_deleted_urls_; - TypedUrlModelAssociator::TypedUrlVisitVector pending_new_visits_; - history::VisitVector pending_deleted_visits_; - - bool disconnected_; - base::Lock disconnect_lock_; - - ScopedObserver<history::HistoryBackend, history::HistoryBackendObserver> - history_backend_observer_; - - DISALLOW_COPY_AND_ASSIGN(TypedUrlChangeProcessor); -}; - -} // namespace browser_sync - -#endif // COMPONENTS_HISTORY_CORE_BROWSER_TYPED_URL_CHANGE_PROCESSOR_H_ diff --git a/components/history/core/browser/typed_url_data_type_controller.cc b/components/history/core/browser/typed_url_data_type_controller.cc index 717fbef..462569f 100644 --- a/components/history/core/browser/typed_url_data_type_controller.cc +++ b/components/history/core/browser/typed_url_data_type_controller.cc @@ -10,7 +10,6 @@ #include "base/prefs/pref_service.h" #include "components/history/core/browser/history_db_task.h" #include "components/history/core/browser/history_service.h" -#include "components/history/core/browser/typed_url_change_processor.h" #include "components/sync_driver/sync_client.h" namespace browser_sync { @@ -57,9 +56,10 @@ TypedUrlDataTypeController::TypedUrlDataTypeController( const base::Closure& error_callback, sync_driver::SyncClient* sync_client, const char* history_disabled_pref_name) - : NonFrontendDataTypeController(ui_thread, error_callback, sync_client), + : NonUIDataTypeController(ui_thread, error_callback, sync_client), history_disabled_pref_name_(history_disabled_pref_name), - backend_(NULL) { + backend_(NULL), + sync_client_(sync_client) { pref_registrar_.Init(sync_client->GetPrefService()); pref_registrar_.Add( history_disabled_pref_name_, @@ -78,7 +78,7 @@ syncer::ModelSafeGroup TypedUrlDataTypeController::model_safe_group() const { bool TypedUrlDataTypeController::ReadyForStart() const { DCHECK(ui_thread()->BelongsToCurrentThread()); - return !sync_client()->GetPrefService()->GetBoolean( + return !sync_client_->GetPrefService()->GetBoolean( history_disabled_pref_name_); } @@ -89,16 +89,18 @@ void TypedUrlDataTypeController::SetBackend(history::HistoryBackend* backend) { void TypedUrlDataTypeController::OnSavingBrowserHistoryDisabledChanged() { DCHECK(ui_thread()->BelongsToCurrentThread()); - if (sync_client()->GetPrefService()->GetBoolean( - history_disabled_pref_name_)) { + if (sync_client_->GetPrefService()->GetBoolean(history_disabled_pref_name_)) { // We've turned off history persistence, so if we are running, // generate an unrecoverable error. This can be fixed by restarting // Chrome (on restart, typed urls will not be a registered type). if (state() != NOT_RUNNING && state() != STOPPING) { - syncer::SyncError error( - FROM_HERE, syncer::SyncError::DATATYPE_POLICY_ERROR, - "History saving is now disabled by policy.", syncer::TYPED_URLS); - DisableImpl(error); + PostTaskOnBackendThread( + FROM_HERE, + base::Bind(&DataTypeController::OnSingleDataTypeUnrecoverableError, + this, + syncer::SyncError( + FROM_HERE, syncer::SyncError::DATATYPE_POLICY_ERROR, + "History saving is now disabled by policy.", type()))); } } } @@ -107,7 +109,7 @@ bool TypedUrlDataTypeController::PostTaskOnBackendThread( const tracked_objects::Location& from_here, const base::Closure& task) { DCHECK(ui_thread()->BelongsToCurrentThread()); - history::HistoryService* history = sync_client()->GetHistoryService(); + history::HistoryService* history = sync_client_->GetHistoryService(); if (history) { history->ScheduleDBTask(scoped_ptr<history::HistoryDBTask>( new RunTaskOnHistoryThread(task, this)), @@ -120,22 +122,6 @@ bool TypedUrlDataTypeController::PostTaskOnBackendThread( } } -sync_driver::SyncApiComponentFactory::SyncComponents -TypedUrlDataTypeController::CreateSyncComponents() { - DCHECK(!ui_thread()->BelongsToCurrentThread()); - DCHECK_EQ(state(), ASSOCIATING); - DCHECK(backend_); - return sync_client() - ->GetSyncApiComponentFactory() - ->CreateTypedUrlSyncComponents(sync_client()->GetSyncService(), backend_, - this); -} - -void TypedUrlDataTypeController::DisconnectProcessor( - sync_driver::ChangeProcessor* processor) { - static_cast<TypedUrlChangeProcessor*>(processor)->Disconnect(); -} - TypedUrlDataTypeController::~TypedUrlDataTypeController() {} } // namespace browser_sync diff --git a/components/history/core/browser/typed_url_data_type_controller.h b/components/history/core/browser/typed_url_data_type_controller.h index ab0e46b..58c4e8d 100644 --- a/components/history/core/browser/typed_url_data_type_controller.h +++ b/components/history/core/browser/typed_url_data_type_controller.h @@ -12,7 +12,7 @@ #include "base/memory/ref_counted.h" #include "base/prefs/pref_change_registrar.h" #include "base/task/cancelable_task_tracker.h" -#include "components/sync_driver/non_frontend_data_type_controller.h" +#include "components/sync_driver/non_ui_data_type_controller.h" #include "components/sync_driver/sync_api_component_factory.h" namespace history { @@ -24,7 +24,7 @@ namespace browser_sync { class ControlTask; // A class that manages the startup and shutdown of typed_url sync. -class TypedUrlDataTypeController : public NonFrontendDataTypeController { +class TypedUrlDataTypeController : public sync_driver::NonUIDataTypeController { public: explicit TypedUrlDataTypeController( const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread, @@ -32,7 +32,7 @@ class TypedUrlDataTypeController : public NonFrontendDataTypeController { sync_driver::SyncClient* sync_client, const char* history_disabled_pref_name); - // NonFrontendDataTypeController implementation + // NonUIDataTypeController implementation syncer::ModelType type() const override; syncer::ModelSafeGroup model_safe_group() const override; bool ReadyForStart() const override; @@ -42,12 +42,9 @@ class TypedUrlDataTypeController : public NonFrontendDataTypeController { void SetBackend(history::HistoryBackend* backend); protected: - // NonFrontendDataTypeController interface. + // NonUIDataTypeController interface. bool PostTaskOnBackendThread(const tracked_objects::Location& from_here, const base::Closure& task) override; - sync_driver::SyncApiComponentFactory::SyncComponents CreateSyncComponents() - override; - void DisconnectProcessor(sync_driver::ChangeProcessor* processor) override; private: ~TypedUrlDataTypeController() override; @@ -64,6 +61,8 @@ class TypedUrlDataTypeController : public NonFrontendDataTypeController { // thread. base::CancelableTaskTracker task_tracker_; + sync_driver::SyncClient* const sync_client_; + DISALLOW_COPY_AND_ASSIGN(TypedUrlDataTypeController); }; diff --git a/components/history/core/browser/typed_url_model_associator.cc b/components/history/core/browser/typed_url_model_associator.cc deleted file mode 100644 index 31a3979..0000000 --- a/components/history/core/browser/typed_url_model_associator.cc +++ /dev/null @@ -1,862 +0,0 @@ -// Copyright 2012 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 "components/history/core/browser/typed_url_model_associator.h" - -#include <algorithm> -#include <set> - -#include "base/location.h" -#include "base/logging.h" -#include "base/metrics/histogram.h" -#include "base/strings/utf_string_conversions.h" -#include "components/history/core/browser/history_backend.h" -#include "components/sync_driver/sync_service.h" -#include "net/base/net_util.h" -#include "sync/api/sync_error.h" -#include "sync/api/sync_merge_result.h" -#include "sync/internal_api/public/read_node.h" -#include "sync/internal_api/public/read_transaction.h" -#include "sync/internal_api/public/write_node.h" -#include "sync/internal_api/public/write_transaction.h" -#include "sync/protocol/typed_url_specifics.pb.h" - -namespace browser_sync { - -// The server backend can't handle arbitrarily large node sizes, so to keep -// the size under control we limit the visit array. -static const int kMaxTypedUrlVisits = 100; - -// There's no limit on how many visits the history DB could have for a given -// typed URL, so we limit how many we fetch from the DB to avoid crashes due to -// running out of memory (http://crbug.com/89793). This value is different -// from kMaxTypedUrlVisits, as some of the visits fetched from the DB may be -// RELOAD visits, which will be stripped. -static const int kMaxVisitsToFetch = 1000; - -static bool CheckVisitOrdering(const history::VisitVector& visits) { - int64 previous_visit_time = 0; - for (history::VisitVector::const_iterator visit = visits.begin(); - visit != visits.end(); ++visit) { - if (visit != visits.begin()) { - // We allow duplicate visits here - they shouldn't really be allowed, but - // they still seem to show up sometimes and we haven't figured out the - // source, so we just log an error instead of failing an assertion. - // (http://crbug.com/91473). - if (previous_visit_time == visit->visit_time.ToInternalValue()) - DVLOG(1) << "Duplicate visit time encountered"; - else if (previous_visit_time > visit->visit_time.ToInternalValue()) - return false; - } - - previous_visit_time = visit->visit_time.ToInternalValue(); - } - return true; -} - -TypedUrlModelAssociator::TypedUrlModelAssociator( - sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) - : sync_service_(sync_service), - history_backend_(history_backend), - expected_loop_(base::MessageLoop::current()), - abort_requested_(false), - error_handler_(error_handler), - num_db_accesses_(0), - num_db_errors_(0) { - DCHECK(sync_service_); - // history_backend_ may be null for unit tests (since it's not mockable). -} - -TypedUrlModelAssociator::~TypedUrlModelAssociator() {} - - -bool TypedUrlModelAssociator::FixupURLAndGetVisits( - history::URLRow* url, - history::VisitVector* visits) { - ++num_db_accesses_; - CHECK(history_backend_); - if (!history_backend_->GetMostRecentVisitsForURL( - url->id(), kMaxVisitsToFetch, visits)) { - ++num_db_errors_; - return false; - } - - // Sometimes (due to a bug elsewhere in the history or sync code, or due to - // a crash between adding a URL to the history database and updating the - // visit DB) the visit vector for a URL can be empty. If this happens, just - // create a new visit whose timestamp is the same as the last_visit time. - // This is a workaround for http://crbug.com/84258. - if (visits->empty()) { - DVLOG(1) << "Found empty visits for URL: " << url->url(); - - if (url->last_visit().is_null()) { - // If modified URL is bookmarked, history backend treats it as modified - // even if all its visits are deleted. Return false to stop further - // processing because sync expects valid visit time for modified entry. - return false; - } - - history::VisitRow visit( - url->id(), url->last_visit(), 0, ui::PAGE_TRANSITION_TYPED, 0); - visits->push_back(visit); - } - - // GetMostRecentVisitsForURL() returns the data in the opposite order that - // we need it, so reverse it. - std::reverse(visits->begin(), visits->end()); - - // Sometimes, the last_visit field in the URL doesn't match the timestamp of - // the last visit in our visit array (they come from different tables, so - // crashes/bugs can cause them to mismatch), so just set it here. - url->set_last_visit(visits->back().visit_time); - DCHECK(CheckVisitOrdering(*visits)); - return true; -} - -bool TypedUrlModelAssociator::ShouldIgnoreUrl(const GURL& url) { - // Ignore empty URLs. Not sure how this can happen (maybe import from other - // busted browsers, or misuse of the history API, or just plain bugs) but we - // can't deal with them. - if (url.spec().empty()) - return true; - - // Ignore local file URLs. - if (url.SchemeIsFile()) - return true; - - // Ignore localhost URLs. - if (net::IsLocalhost(url.host())) - return true; - - return false; -} - -bool TypedUrlModelAssociator::ShouldIgnoreVisits( - const history::VisitVector& visits) { - // We ignore URLs that were imported, but have never been visited by - // chromium. - static const int kLastImportedSource = history::SOURCE_EXTENSION; - history::VisitSourceMap map; - if (!history_backend_->GetVisitsSource(visits, &map)) - return false; // If we can't read the visit, assume it's not imported. - - // Walk the list of visits and look for a non-imported item. - for (history::VisitVector::const_iterator it = visits.begin(); - it != visits.end(); ++it) { - if (map.count(it->visit_id) == 0 || - map[it->visit_id] <= kLastImportedSource) { - return false; - } - } - // We only saw imported visits, so tell the caller to ignore them. - return true; -} - -syncer::SyncError TypedUrlModelAssociator::AssociateModels( - syncer::SyncMergeResult* local_merge_result, - syncer::SyncMergeResult* syncer_merge_result) { - ClearErrorStats(); - syncer::SyncError error = - DoAssociateModels(local_merge_result, syncer_merge_result); - UMA_HISTOGRAM_PERCENTAGE("Sync.TypedUrlModelAssociationErrors", - GetErrorPercentage()); - ClearErrorStats(); - return error; -} - -void TypedUrlModelAssociator::ClearErrorStats() { - num_db_accesses_ = 0; - num_db_errors_ = 0; -} - -int TypedUrlModelAssociator::GetErrorPercentage() const { - return num_db_accesses_ ? (100 * num_db_errors_ / num_db_accesses_) : 0; -} - -syncer::SyncError TypedUrlModelAssociator::DoAssociateModels( - syncer::SyncMergeResult* local_merge_result, - syncer::SyncMergeResult* syncer_merge_result) { - DVLOG(1) << "Associating TypedUrl Models"; - DCHECK(expected_loop_ == base::MessageLoop::current()); - - history::URLRows typed_urls; - ++num_db_accesses_; - bool query_succeeded = - history_backend_ && history_backend_->GetAllTypedURLs(&typed_urls); - - history::URLRows new_urls; - history::URLRows updated_urls; - TypedUrlVisitVector new_visits; - { - base::AutoLock au(abort_lock_); - if (abort_requested_) { - return syncer::SyncError(FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Association was aborted.", - model_type()); - } - - // Must lock and check first to make sure |error_handler_| is valid. - if (!query_succeeded) { - ++num_db_errors_; - return error_handler_->CreateAndUploadError( - FROM_HERE, - "Could not get the typed_url entries.", - model_type()); - } - local_merge_result->set_num_items_before_association(typed_urls.size()); - - // Get all the visits. - std::map<history::URLID, history::VisitVector> visit_vectors; - history::URLRows::iterator new_end = typed_urls.end(); - for (history::URLRows::iterator ix = typed_urls.begin(); - ix != new_end;) { - DCHECK_EQ(0U, visit_vectors.count(ix->id())); - if (!FixupURLAndGetVisits(&(*ix), &(visit_vectors[ix->id()])) || - ShouldIgnoreUrl(ix->url()) || - ShouldIgnoreVisits(visit_vectors[ix->id()])) { - // Ignore this URL if we couldn't load the visits or if there's some - // other problem with it (it was empty, or imported and never visited). - --new_end; - if (ix != new_end) - *ix = *new_end; - } else { - ++ix; - } - } - typed_urls.erase(new_end, typed_urls.end()); - - syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare()); - syncer::ReadNode typed_url_root(&trans); - if (typed_url_root.InitTypeRoot(syncer::TYPED_URLS) != - syncer::BaseNode::INIT_OK) { - return error_handler_->CreateAndUploadError( - FROM_HERE, - "Server did not create the top-level typed_url node. We " - "might be running against an out-of-date server.", - model_type()); - } - syncer_merge_result->set_num_items_before_association( - typed_url_root.GetTotalNodeCount()); - - std::set<std::string> current_urls; - for (history::URLRows::iterator ix = typed_urls.begin(); - ix != typed_urls.end(); ++ix) { - std::string tag = ix->url().spec(); - // Empty URLs should be filtered out by ShouldIgnoreUrl() previously. - DCHECK(!tag.empty()); - history::VisitVector& visits = visit_vectors[ix->id()]; - - syncer::ReadNode node(&trans); - if (node.InitByClientTagLookup(syncer::TYPED_URLS, tag) == - syncer::BaseNode::INIT_OK) { - // Same URL exists in sync data and in history data - compare the - // entries to see if there's any difference. - sync_pb::TypedUrlSpecifics typed_url( - FilterExpiredVisits(node.GetTypedUrlSpecifics())); - DCHECK_EQ(tag, typed_url.url()); - - // Initialize fields in |new_url| to the same values as the fields in - // the existing URLRow in the history DB. This is needed because we - // overwrite the existing value below in WriteToHistoryBackend(), but - // some of the values in that structure are not synced (like - // typed_count). - history::URLRow new_url(*ix); - - std::vector<history::VisitInfo> added_visits; - MergeResult difference = - MergeUrls(typed_url, *ix, &visits, &new_url, &added_visits); - if (difference & DIFF_UPDATE_NODE) { - syncer::WriteNode write_node(&trans); - if (write_node.InitByClientTagLookup(syncer::TYPED_URLS, tag) != - syncer::BaseNode::INIT_OK) { - return error_handler_->CreateAndUploadError( - FROM_HERE, - "Failed to edit typed_url sync node.", - model_type()); - } - // We don't want to resurrect old visits that have been aged out by - // other clients, so remove all visits that are older than the - // earliest existing visit in the sync node. - if (typed_url.visits_size() > 0) { - base::Time earliest_visit = - base::Time::FromInternalValue(typed_url.visits(0)); - for (history::VisitVector::iterator it = visits.begin(); - it != visits.end() && it->visit_time < earliest_visit; ) { - it = visits.erase(it); - } - // Should never be possible to delete all the items, since the - // visit vector contains all the items in typed_url.visits. - DCHECK_GT(visits.size(), 0u); - } - DCHECK_EQ(new_url.last_visit().ToInternalValue(), - visits.back().visit_time.ToInternalValue()); - WriteToSyncNode(new_url, visits, &write_node); - syncer_merge_result->set_num_items_modified( - syncer_merge_result->num_items_modified() + 1); - } - if (difference & DIFF_LOCAL_ROW_CHANGED) { - DCHECK_EQ(ix->id(), new_url.id()); - updated_urls.push_back(new_url); - } - if (difference & DIFF_LOCAL_VISITS_ADDED) { - new_visits.push_back( - std::pair<GURL, std::vector<history::VisitInfo> >(ix->url(), - added_visits)); - } - } else { - // Sync has never seen this URL before. - syncer::WriteNode node(&trans); - syncer::WriteNode::InitUniqueByCreationResult result = - node.InitUniqueByCreation(syncer::TYPED_URLS, tag); - if (result != syncer::WriteNode::INIT_SUCCESS) { - return error_handler_->CreateAndUploadError( - FROM_HERE, - "Failed to create typed_url sync node: " + tag, - model_type()); - } - - node.SetTitle(tag); - WriteToSyncNode(*ix, visits, &node); - syncer_merge_result->set_num_items_added( - syncer_merge_result->num_items_added() + 1); - } - - current_urls.insert(tag); - } - - // Now walk the sync nodes and detect any URLs that exist there, but not in - // the history DB, so we can add them to our local history DB. - std::vector<int64> obsolete_nodes; - std::vector<int64> sync_ids; - typed_url_root.GetChildIds(&sync_ids); - - for (std::vector<int64>::const_iterator it = sync_ids.begin(); - it != sync_ids.end(); ++it) { - syncer::ReadNode sync_child_node(&trans); - if (sync_child_node.InitByIdLookup(*it) != syncer::BaseNode::INIT_OK) { - return error_handler_->CreateAndUploadError( - FROM_HERE, - "Failed to fetch child node.", - model_type()); - } - const sync_pb::TypedUrlSpecifics& typed_url( - sync_child_node.GetTypedUrlSpecifics()); - - // Ignore old sync nodes that don't have any transition data stored with - // them, or transition data that does not match the visit data (will be - // deleted below). - if (typed_url.visit_transitions_size() == 0 || - typed_url.visit_transitions_size() != typed_url.visits_size()) { - // Generate a debug assertion to help track down http://crbug.com/91473, - // even though we gracefully handle this case by throwing away this - // node. - DCHECK_EQ(typed_url.visits_size(), typed_url.visit_transitions_size()); - DVLOG(1) << "Deleting obsolete sync node with no visit " - << "transition info."; - obsolete_nodes.push_back(sync_child_node.GetId()); - continue; - } - - if (typed_url.url().empty()) { - DVLOG(1) << "Ignoring empty URL in sync DB"; - continue; - } - - // Now, get rid of the expired visits, and if there are no un-expired - // visits left, just ignore this node. - sync_pb::TypedUrlSpecifics filtered_url = FilterExpiredVisits(typed_url); - if (filtered_url.visits_size() == 0) { - DVLOG(1) << "Ignoring expired URL in sync DB: " << filtered_url.url(); - continue; - } - - if (current_urls.find(filtered_url.url()) == current_urls.end()) { - // Update the local DB from the sync DB. Since we are doing our - // initial model association, we don't want to remove any of the - // existing visits (pass NULL as |visits_to_remove|). - UpdateFromSyncDB(filtered_url, - &new_visits, - NULL, - &updated_urls, - &new_urls); - local_merge_result->set_num_items_added( - local_merge_result->num_items_added() + 1); - } - } - - // If we encountered any obsolete nodes, remove them so they don't hang - // around and confuse people looking at the sync node browser. - if (!obsolete_nodes.empty()) { - for (std::vector<int64>::const_iterator it = obsolete_nodes.begin(); - it != obsolete_nodes.end(); - ++it) { - syncer::WriteNode sync_node(&trans); - if (sync_node.InitByIdLookup(*it) != syncer::BaseNode::INIT_OK) { - return error_handler_->CreateAndUploadError( - FROM_HERE, - "Failed to fetch obsolete node.", - model_type()); - } - sync_node.Tombstone(); - } - } - syncer_merge_result->set_num_items_after_association( - typed_url_root.GetTotalNodeCount()); - } - - // Since we're on the history thread, we don't have to worry about updating - // the history database after closing the write transaction, since - // this is the only thread that writes to the database. We also don't have - // to worry about the sync model getting out of sync, because changes are - // propagated to the ChangeProcessor on this thread. - WriteToHistoryBackend(&new_urls, &updated_urls, &new_visits, NULL); - local_merge_result->set_num_items_modified(updated_urls.size()); - local_merge_result->set_num_items_after_association( - local_merge_result->num_items_before_association() + - local_merge_result->num_items_added()); - return syncer::SyncError(); -} - -void TypedUrlModelAssociator::UpdateFromSyncDB( - const sync_pb::TypedUrlSpecifics& typed_url, - TypedUrlVisitVector* visits_to_add, - history::VisitVector* visits_to_remove, - history::URLRows* updated_urls, - history::URLRows* new_urls) { - history::URLRow new_url(GURL(typed_url.url())); - history::VisitVector existing_visits; - bool existing_url = history_backend_->GetURL(new_url.url(), &new_url); - if (existing_url) { - // This URL already exists locally - fetch the visits so we can - // merge them below. - if (!FixupURLAndGetVisits(&new_url, &existing_visits)) { - // Couldn't load the visits for this URL due to some kind of DB error. - // Don't bother writing this URL to the history DB (if we ignore the - // error and continue, we might end up duplicating existing visits). - DLOG(ERROR) << "Could not load visits for url: " << new_url.url(); - return; - } - } - visits_to_add->push_back(std::pair<GURL, std::vector<history::VisitInfo> >( - new_url.url(), std::vector<history::VisitInfo>())); - - // Update the URL with information from the typed URL. - UpdateURLRowFromTypedUrlSpecifics(typed_url, &new_url); - - // Figure out which visits we need to add. - DiffVisits(existing_visits, typed_url, &visits_to_add->back().second, - visits_to_remove); - - if (existing_url) { - updated_urls->push_back(new_url); - } else { - new_urls->push_back(new_url); - } -} - -sync_pb::TypedUrlSpecifics TypedUrlModelAssociator::FilterExpiredVisits( - const sync_pb::TypedUrlSpecifics& source) { - // Make a copy of the source, then regenerate the visits. - sync_pb::TypedUrlSpecifics specifics(source); - specifics.clear_visits(); - specifics.clear_visit_transitions(); - for (int i = 0; i < source.visits_size(); ++i) { - base::Time time = base::Time::FromInternalValue(source.visits(i)); - if (!history_backend_->IsExpiredVisitTime(time)) { - specifics.add_visits(source.visits(i)); - specifics.add_visit_transitions(source.visit_transitions(i)); - } - } - DCHECK(specifics.visits_size() == specifics.visit_transitions_size()); - return specifics; -} - -bool TypedUrlModelAssociator::DeleteAllNodes( - syncer::WriteTransaction* trans) { - DCHECK(expected_loop_ == base::MessageLoop::current()); - - // Just walk through all our child nodes and delete them. - syncer::ReadNode typed_url_root(trans); - if (typed_url_root.InitTypeRoot(syncer::TYPED_URLS) != - syncer::BaseNode::INIT_OK) { - LOG(ERROR) << "Could not lookup root node"; - return false; - } - - std::vector<int64> sync_ids; - typed_url_root.GetChildIds(&sync_ids); - - for (std::vector<int64>::const_iterator it = sync_ids.begin(); - it != sync_ids.end(); ++it) { - syncer::WriteNode sync_child_node(trans); - if (sync_child_node.InitByIdLookup(*it) != syncer::BaseNode::INIT_OK) { - LOG(ERROR) << "Typed url node lookup failed."; - return false; - } - sync_child_node.Tombstone(); - } - return true; -} - -syncer::SyncError TypedUrlModelAssociator::DisassociateModels() { - return syncer::SyncError(); -} - -void TypedUrlModelAssociator::AbortAssociation() { - base::AutoLock lock(abort_lock_); - abort_requested_ = true; -} - -bool TypedUrlModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) { - DCHECK(has_nodes); - *has_nodes = false; - syncer::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare()); - syncer::ReadNode sync_node(&trans); - if (sync_node.InitTypeRoot(syncer::TYPED_URLS) != syncer::BaseNode::INIT_OK) { - LOG(ERROR) << "Server did not create the top-level typed_url node. We " - << "might be running against an out-of-date server."; - return false; - } - - // The sync model has user created nodes if the typed_url folder has any - // children. - *has_nodes = sync_node.HasChildren(); - return true; -} - -void TypedUrlModelAssociator::WriteToHistoryBackend( - const history::URLRows* new_urls, - const history::URLRows* updated_urls, - const TypedUrlVisitVector* new_visits, - const history::VisitVector* deleted_visits) { - if (new_urls) { - history_backend_->AddPagesWithDetails(*new_urls, history::SOURCE_SYNCED); - } - if (updated_urls) { - ++num_db_accesses_; - // These are existing entries in the URL database. We don't verify the - // visit_count or typed_count values here, because either one (or both) - // could be zero in the case of bookmarks, or in the case of a URL - // transitioning from non-typed to typed as a result of this sync. - // In the field we sometimes run into errors on specific URLs. It's OK to - // just continue, as we can try writing again on the next model association. - size_t num_successful_updates = history_backend_->UpdateURLs(*updated_urls); - num_db_errors_ += updated_urls->size() - num_successful_updates; - } - if (new_visits) { - for (TypedUrlVisitVector::const_iterator visits = new_visits->begin(); - visits != new_visits->end(); ++visits) { - // If there are no visits to add, just skip this. - if (visits->second.empty()) - continue; - ++num_db_accesses_; - if (!history_backend_->AddVisits(visits->first, visits->second, - history::SOURCE_SYNCED)) { - ++num_db_errors_; - DLOG(ERROR) << "Could not add visits."; - } - } - } - if (deleted_visits) { - ++num_db_accesses_; - if (!history_backend_->RemoveVisits(*deleted_visits)) { - ++num_db_errors_; - DLOG(ERROR) << "Could not remove visits."; - // This is bad news, since it means we may end up resurrecting history - // entries on the next reload. It's unavoidable so we'll just keep on - // syncing. - } - } -} - -// static -TypedUrlModelAssociator::MergeResult TypedUrlModelAssociator::MergeUrls( - const sync_pb::TypedUrlSpecifics& node, - const history::URLRow& url, - history::VisitVector* visits, - history::URLRow* new_url, - std::vector<history::VisitInfo>* new_visits) { - DCHECK(new_url); - DCHECK(!node.url().compare(url.url().spec())); - DCHECK(!node.url().compare(new_url->url().spec())); - DCHECK(visits->size()); - CHECK_EQ(node.visits_size(), node.visit_transitions_size()); - - // If we have an old-format node (before we added the visits and - // visit_transitions arrays to the protobuf) or else the node only contained - // expired visits, so just overwrite it with our local history data. - if (node.visits_size() == 0) - return DIFF_UPDATE_NODE; - - // Convert these values only once. - base::string16 node_title(base::UTF8ToUTF16(node.title())); - base::Time node_last_visit = base::Time::FromInternalValue( - node.visits(node.visits_size() - 1)); - - // This is a bitfield representing what we'll need to update with the output - // value. - MergeResult different = DIFF_NONE; - - // Check if the non-incremented values changed. - if ((node_title.compare(url.title()) != 0) || - (node.hidden() != url.hidden())) { - // Use the values from the most recent visit. - if (node_last_visit >= url.last_visit()) { - new_url->set_title(node_title); - new_url->set_hidden(node.hidden()); - different |= DIFF_LOCAL_ROW_CHANGED; - } else { - new_url->set_title(url.title()); - new_url->set_hidden(url.hidden()); - different |= DIFF_UPDATE_NODE; - } - } else { - // No difference. - new_url->set_title(url.title()); - new_url->set_hidden(url.hidden()); - } - - size_t node_num_visits = node.visits_size(); - size_t history_num_visits = visits->size(); - size_t node_visit_index = 0; - size_t history_visit_index = 0; - base::Time earliest_history_time = (*visits)[0].visit_time; - // Walk through the two sets of visits and figure out if any new visits were - // added on either side. - while (node_visit_index < node_num_visits || - history_visit_index < history_num_visits) { - // Time objects are initialized to "earliest possible time". - base::Time node_time, history_time; - if (node_visit_index < node_num_visits) - node_time = base::Time::FromInternalValue(node.visits(node_visit_index)); - if (history_visit_index < history_num_visits) - history_time = (*visits)[history_visit_index].visit_time; - if (node_visit_index >= node_num_visits || - (history_visit_index < history_num_visits && - node_time > history_time)) { - // We found a visit in the history DB that doesn't exist in the sync DB, - // so mark the node as modified so the caller will update the sync node. - different |= DIFF_UPDATE_NODE; - ++history_visit_index; - } else if (history_visit_index >= history_num_visits || - node_time < history_time) { - // Found a visit in the sync node that doesn't exist in the history DB, so - // add it to our list of new visits and set the appropriate flag so the - // caller will update the history DB. - // If the node visit is older than any existing visit in the history DB, - // don't re-add it - this keeps us from resurrecting visits that were - // aged out locally. - if (node_time > earliest_history_time) { - different |= DIFF_LOCAL_VISITS_ADDED; - new_visits->push_back(history::VisitInfo( - node_time, - ui::PageTransitionFromInt( - node.visit_transitions(node_visit_index)))); - } - // This visit is added to visits below. - ++node_visit_index; - } else { - // Same (already synced) entry found in both DBs - no need to do anything. - ++node_visit_index; - ++history_visit_index; - } - } - - DCHECK(CheckVisitOrdering(*visits)); - if (different & DIFF_LOCAL_VISITS_ADDED) { - // Insert new visits into the apropriate place in the visits vector. - history::VisitVector::iterator visit_ix = visits->begin(); - for (std::vector<history::VisitInfo>::iterator new_visit = - new_visits->begin(); - new_visit != new_visits->end(); ++new_visit) { - while (visit_ix != visits->end() && - new_visit->first > visit_ix->visit_time) { - ++visit_ix; - } - visit_ix = visits->insert(visit_ix, - history::VisitRow(url.id(), new_visit->first, - 0, new_visit->second, 0)); - ++visit_ix; - } - } - DCHECK(CheckVisitOrdering(*visits)); - - new_url->set_last_visit(visits->back().visit_time); - return different; -} - -// static -void TypedUrlModelAssociator::WriteToSyncNode( - const history::URLRow& url, - const history::VisitVector& visits, - syncer::WriteNode* node) { - sync_pb::TypedUrlSpecifics typed_url; - WriteToTypedUrlSpecifics(url, visits, &typed_url); - node->SetTypedUrlSpecifics(typed_url); -} - -void TypedUrlModelAssociator::WriteToTypedUrlSpecifics( - const history::URLRow& url, - const history::VisitVector& visits, - sync_pb::TypedUrlSpecifics* typed_url) { - - DCHECK(!url.last_visit().is_null()); - DCHECK(!visits.empty()); - DCHECK_EQ(url.last_visit().ToInternalValue(), - visits.back().visit_time.ToInternalValue()); - - typed_url->set_url(url.url().spec()); - typed_url->set_title(base::UTF16ToUTF8(url.title())); - typed_url->set_hidden(url.hidden()); - - DCHECK(CheckVisitOrdering(visits)); - - bool only_typed = false; - int skip_count = 0; - - if (visits.size() > static_cast<size_t>(kMaxTypedUrlVisits)) { - int typed_count = 0; - int total = 0; - // Walk the passed-in visit vector and count the # of typed visits. - for (history::VisitVector::const_iterator visit = visits.begin(); - visit != visits.end(); ++visit) { - ui::PageTransition transition = - ui::PageTransitionStripQualifier(visit->transition); - // We ignore reload visits. - if (transition == ui::PAGE_TRANSITION_RELOAD) - continue; - ++total; - if (transition == ui::PAGE_TRANSITION_TYPED) - ++typed_count; - } - // We should have at least one typed visit. This can sometimes happen if - // the history DB has an inaccurate count for some reason (there's been - // bugs in the history code in the past which has left users in the wild - // with incorrect counts - http://crbug.com/84258). - DCHECK_GT(typed_count, 0); - - if (typed_count > kMaxTypedUrlVisits) { - only_typed = true; - skip_count = typed_count - kMaxTypedUrlVisits; - } else if (total > kMaxTypedUrlVisits) { - skip_count = total - kMaxTypedUrlVisits; - } - } - - - for (history::VisitVector::const_iterator visit = visits.begin(); - visit != visits.end(); ++visit) { - ui::PageTransition transition = - ui::PageTransitionStripQualifier(visit->transition); - // Skip reload visits. - if (transition == ui::PAGE_TRANSITION_RELOAD) - continue; - - // If we only have room for typed visits, then only add typed visits. - if (only_typed && transition != ui::PAGE_TRANSITION_TYPED) - continue; - - if (skip_count > 0) { - // We have too many entries to fit, so we need to skip the oldest ones. - // Only skip typed URLs if there are too many typed URLs to fit. - if (only_typed || transition != ui::PAGE_TRANSITION_TYPED) { - --skip_count; - continue; - } - } - typed_url->add_visits(visit->visit_time.ToInternalValue()); - typed_url->add_visit_transitions(visit->transition); - } - DCHECK_EQ(skip_count, 0); - - if (typed_url->visits_size() == 0) { - // If we get here, it's because we don't actually have any TYPED visits - // even though the visit's typed_count > 0 (corrupted typed_count). So - // let's go ahead and add a RELOAD visit at the most recent visit since - // it's not legal to have an empty visit array (yet another workaround - // for http://crbug.com/84258). - typed_url->add_visits(url.last_visit().ToInternalValue()); - typed_url->add_visit_transitions(ui::PAGE_TRANSITION_RELOAD); - } - CHECK_GT(typed_url->visits_size(), 0); - CHECK_LE(typed_url->visits_size(), kMaxTypedUrlVisits); - CHECK_EQ(typed_url->visits_size(), typed_url->visit_transitions_size()); -} - -// static -void TypedUrlModelAssociator::DiffVisits( - const history::VisitVector& old_visits, - const sync_pb::TypedUrlSpecifics& new_url, - std::vector<history::VisitInfo>* new_visits, - history::VisitVector* removed_visits) { - DCHECK(new_visits); - size_t old_visit_count = old_visits.size(); - size_t new_visit_count = new_url.visits_size(); - size_t old_index = 0; - size_t new_index = 0; - while (old_index < old_visit_count && new_index < new_visit_count) { - base::Time new_visit_time = - base::Time::FromInternalValue(new_url.visits(new_index)); - if (old_visits[old_index].visit_time < new_visit_time) { - if (new_index > 0 && removed_visits) { - // If there are visits missing from the start of the node, that - // means that they were probably clipped off due to our code that - // limits the size of the sync nodes - don't delete them from our - // local history. - removed_visits->push_back(old_visits[old_index]); - } - ++old_index; - } else if (old_visits[old_index].visit_time > new_visit_time) { - new_visits->push_back(history::VisitInfo( - new_visit_time, - ui::PageTransitionFromInt( - new_url.visit_transitions(new_index)))); - ++new_index; - } else { - ++old_index; - ++new_index; - } - } - - if (removed_visits) { - for ( ; old_index < old_visit_count; ++old_index) { - removed_visits->push_back(old_visits[old_index]); - } - } - - for ( ; new_index < new_visit_count; ++new_index) { - new_visits->push_back(history::VisitInfo( - base::Time::FromInternalValue(new_url.visits(new_index)), - ui::PageTransitionFromInt(new_url.visit_transitions(new_index)))); - } -} - - -// static -void TypedUrlModelAssociator::UpdateURLRowFromTypedUrlSpecifics( - const sync_pb::TypedUrlSpecifics& typed_url, history::URLRow* new_url) { - DCHECK_GT(typed_url.visits_size(), 0); - CHECK_EQ(typed_url.visit_transitions_size(), typed_url.visits_size()); - new_url->set_title(base::UTF8ToUTF16(typed_url.title())); - new_url->set_hidden(typed_url.hidden()); - // Only provide the initial value for the last_visit field - after that, let - // the history code update the last_visit field on its own. - if (new_url->last_visit().is_null()) { - new_url->set_last_visit(base::Time::FromInternalValue( - typed_url.visits(typed_url.visits_size() - 1))); - } -} - -bool TypedUrlModelAssociator::CryptoReadyIfNecessary() { - // We only access the cryptographer while holding a transaction. - syncer::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare()); - const syncer::ModelTypeSet encrypted_types = trans.GetEncryptedTypes(); - return !encrypted_types.Has(syncer::TYPED_URLS) || - sync_service_->IsCryptographerReady(&trans); -} - -} // namespace browser_sync diff --git a/components/history/core/browser/typed_url_model_associator.h b/components/history/core/browser/typed_url_model_associator.h deleted file mode 100644 index 35db2eb..0000000 --- a/components/history/core/browser/typed_url_model_associator.h +++ /dev/null @@ -1,208 +0,0 @@ -// Copyright 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_HISTORY_CORE_BROWSER_TYPED_URL_MODEL_ASSOCIATOR_H_ -#define COMPONENTS_HISTORY_CORE_BROWSER_TYPED_URL_MODEL_ASSOCIATOR_H_ - -#include <map> -#include <string> -#include <utility> -#include <vector> - -#include "base/basictypes.h" -#include "base/compiler_specific.h" -#include "base/strings/string16.h" -#include "components/history/core/browser/history_types.h" -#include "components/sync_driver/data_type_error_handler.h" -#include "components/sync_driver/model_associator.h" -#include "sync/protocol/typed_url_specifics.pb.h" - -class GURL; - -namespace base { -class MessageLoop; -} - -namespace history { -class HistoryBackend; -class URLRow; -}; - -namespace syncer { -class WriteNode; -class WriteTransaction; -}; - -namespace sync_driver { -class SyncService; -} - -namespace browser_sync { - -// Contains all model association related logic: -// * Algorithm to associate typed_url model and sync model. -// * Persisting model associations and loading them back. -// We do not check if we have local data before this run; we always -// merge and sync. -class TypedUrlModelAssociator : public sync_driver::AssociatorInterface { - public: - typedef std::vector<std::pair<GURL, std::vector<history::VisitInfo> > > - TypedUrlVisitVector; - - static syncer::ModelType model_type() { return syncer::TYPED_URLS; } - TypedUrlModelAssociator(sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler); - ~TypedUrlModelAssociator() override; - - // AssociatorInterface implementation. - // - // Iterates through the sync model looking for matched pairs of items. - syncer::SyncError AssociateModels( - syncer::SyncMergeResult* local_merge_result, - syncer::SyncMergeResult* syncer_merge_result) override; - - // Clears all associations. - syncer::SyncError DisassociateModels() override; - - // Called from the main thread, to abort the currently active model - // association (for example, if we are shutting down). - void AbortAssociation() override; - - // The has_nodes out param is true if the sync model has nodes other - // than the permanent tagged nodes. - bool SyncModelHasUserCreatedNodes(bool* has_nodes) override; - - bool CryptoReadyIfNecessary() override; - - // Delete all typed url nodes. - bool DeleteAllNodes(syncer::WriteTransaction* trans); - - void WriteToHistoryBackend(const history::URLRows* new_urls, - const history::URLRows* updated_urls, - const TypedUrlVisitVector* new_visits, - const history::VisitVector* deleted_visits); - - // Given a typed URL in the sync DB, looks for an existing entry in the - // local history DB and generates a list of visits to add to the - // history DB to bring it up to date (avoiding duplicates). - // Updates the passed |visits_to_add| and |visits_to_remove| vectors with the - // visits to add to/remove from the history DB, and adds a new entry to either - // |updated_urls| or |new_urls| depending on whether the URL already existed - // in the history DB. - void UpdateFromSyncDB(const sync_pb::TypedUrlSpecifics& typed_url, - TypedUrlVisitVector* visits_to_add, - history::VisitVector* visits_to_remove, - history::URLRows* updated_urls, - history::URLRows* new_urls); - - // Given a TypedUrlSpecifics object, removes all visits that are older than - // the current expiration time. Note that this can result in having no visits - // at all. - sync_pb::TypedUrlSpecifics FilterExpiredVisits( - const sync_pb::TypedUrlSpecifics& specifics); - - // Returns the percentage of DB accesses that have resulted in an error. - int GetErrorPercentage() const; - - // Bitfield returned from MergeUrls to specify the result of the merge. - typedef uint32 MergeResult; - static const MergeResult DIFF_NONE = 0; - static const MergeResult DIFF_UPDATE_NODE = 1 << 0; - static const MergeResult DIFF_LOCAL_ROW_CHANGED = 1 << 1; - static const MergeResult DIFF_LOCAL_VISITS_ADDED = 1 << 2; - - // Merges the URL information in |typed_url| with the URL information from the - // history database in |url| and |visits|, and returns a bitmask with the - // results of the merge: - // DIFF_UPDATE_NODE - changes have been made to |new_url| and |visits| which - // should be persisted to the sync node. - // DIFF_LOCAL_ROW_CHANGED - The history data in |new_url| should be persisted - // to the history DB. - // DIFF_LOCAL_VISITS_ADDED - |new_visits| contains a list of visits that - // should be written to the history DB for this URL. Deletions are not - // written to the DB - each client is left to age out visits on their own. - static MergeResult MergeUrls(const sync_pb::TypedUrlSpecifics& typed_url, - const history::URLRow& url, - history::VisitVector* visits, - history::URLRow* new_url, - std::vector<history::VisitInfo>* new_visits); - static void WriteToSyncNode(const history::URLRow& url, - const history::VisitVector& visits, - syncer::WriteNode* node); - - // Diffs the set of visits between the history DB and the sync DB, using the - // sync DB as the canonical copy. Result is the set of |new_visits| and - // |removed_visits| that can be applied to the history DB to make it match - // the sync DB version. |removed_visits| can be null if the caller does not - // care about which visits to remove. - static void DiffVisits(const history::VisitVector& old_visits, - const sync_pb::TypedUrlSpecifics& new_url, - std::vector<history::VisitInfo>* new_visits, - history::VisitVector* removed_visits); - - // Converts the passed URL information to a TypedUrlSpecifics structure for - // writing to the sync DB - static void WriteToTypedUrlSpecifics(const history::URLRow& url, - const history::VisitVector& visits, - sync_pb::TypedUrlSpecifics* specifics); - - // Fetches visits from the history DB corresponding to the passed URL. This - // function compensates for the fact that the history DB has rather poor data - // integrity (duplicate visits, visit timestamps that don't match the - // last_visit timestamp, huge data sets that exhaust memory when fetched, - // etc) by modifying the passed |url| object and |visits| vector. - // Returns false if we could not fetch the visits for the passed URL, and - // tracks DB error statistics internally for reporting via UMA. - bool FixupURLAndGetVisits(history::URLRow* url, - history::VisitVector* visits); - - // Updates the passed |url_row| based on the values in |specifics|. Fields - // that are not contained in |specifics| (such as typed_count) are left - // unchanged. - static void UpdateURLRowFromTypedUrlSpecifics( - const sync_pb::TypedUrlSpecifics& specifics, history::URLRow* url_row); - - // Helper function that determines if we should ignore a URL for the purposes - // of sync, because it contains invalid data. - bool ShouldIgnoreUrl(const GURL& url); - - protected: - // Helper function that clears our error counters (used to reset stats after - // model association so we can track model association errors separately). - // Overridden by tests. - virtual void ClearErrorStats(); - - private: - // Helper routine that actually does the work of associating models. - syncer::SyncError DoAssociateModels( - syncer::SyncMergeResult* local_merge_result, - syncer::SyncMergeResult* syncer_merge_result); - - // Helper function that determines if we should ignore a URL for the purposes - // of sync, based on the visits the URL had. - bool ShouldIgnoreVisits(const history::VisitVector& visits); - - sync_driver::SyncService* sync_service_; - history::HistoryBackend* history_backend_; - - base::MessageLoop* expected_loop_; - - bool abort_requested_; - base::Lock abort_lock_; - - // Guaranteed to outlive datatypes. - sync_driver::DataTypeErrorHandler* error_handler_; - - // Statistics for the purposes of tracking the percentage of DB accesses that - // fail for each client via UMA. - int num_db_accesses_; - int num_db_errors_; - - DISALLOW_COPY_AND_ASSIGN(TypedUrlModelAssociator); -}; - -} // namespace browser_sync - -#endif // COMPONENTS_HISTORY_CORE_BROWSER_TYPED_URL_MODEL_ASSOCIATOR_H_ diff --git a/components/history/core/browser/typed_url_model_associator_unittest.cc b/components/history/core/browser/typed_url_model_associator_unittest.cc deleted file mode 100644 index ab8fec4..0000000 --- a/components/history/core/browser/typed_url_model_associator_unittest.cc +++ /dev/null @@ -1,432 +0,0 @@ -// Copyright (c) 2012 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 "base/basictypes.h" -#include "base/bind.h" -#include "base/strings/string_piece.h" -#include "base/strings/utf_string_conversions.h" -#include "base/synchronization/waitable_event.h" -#include "base/test/test_timeouts.h" -#include "base/threading/thread.h" -#include "base/time/time.h" -#include "components/history/core/browser/history_types.h" -#include "components/history/core/browser/typed_url_model_associator.h" -#include "components/sync_driver/fake_sync_service.h" -#include "sync/protocol/typed_url_specifics.pb.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "url/gurl.h" - -using browser_sync::TypedUrlModelAssociator; - -namespace { -class SyncTypedUrlModelAssociatorTest : public testing::Test { - public: - static history::URLRow MakeTypedUrlRow(const char* url, - const char* title, - int typed_count, - int64 last_visit, - bool hidden, - history::VisitVector* visits) { - GURL gurl(url); - history::URLRow history_url(gurl); - history_url.set_title(base::UTF8ToUTF16(title)); - history_url.set_typed_count(typed_count); - history_url.set_last_visit( - base::Time::FromInternalValue(last_visit)); - history_url.set_hidden(hidden); - visits->push_back(history::VisitRow( - history_url.id(), history_url.last_visit(), 0, - ui::PAGE_TRANSITION_RELOAD, 0)); - history_url.set_visit_count(visits->size()); - return history_url; - } - - static sync_pb::TypedUrlSpecifics MakeTypedUrlSpecifics(const char* url, - const char* title, - int64 last_visit, - bool hidden) { - sync_pb::TypedUrlSpecifics typed_url; - typed_url.set_url(url); - typed_url.set_title(title); - typed_url.set_hidden(hidden); - typed_url.add_visits(last_visit); - typed_url.add_visit_transitions(ui::PAGE_TRANSITION_TYPED); - return typed_url; - } - - static bool URLsEqual(history::URLRow& lhs, history::URLRow& rhs) { - // Only compare synced fields (ignore typed_count and visit_count as those - // are maintained by the history subsystem). - return (lhs.url().spec().compare(rhs.url().spec()) == 0) && - (lhs.title().compare(rhs.title()) == 0) && - (lhs.hidden() == rhs.hidden()); - } -}; - -static void CreateModelAssociatorAsync(base::WaitableEvent* startup, - base::WaitableEvent* aborted, - base::WaitableEvent* done, - TypedUrlModelAssociator** associator, - sync_driver::SyncService* service) { - // Grab the done lock - when we exit, this will be released and allow the - // test to finish. - *associator = new TypedUrlModelAssociator(service, NULL, NULL); - - // Signal frontend to call AbortAssociation and proceed after it's called. - startup->Signal(); - aborted->Wait(); - syncer::SyncError error = (*associator)->AssociateModels(NULL, NULL); - EXPECT_TRUE(error.IsSet()); - EXPECT_EQ("Association was aborted.", error.message()); - delete *associator; - done->Signal(); -} - -} // namespace - -TEST_F(SyncTypedUrlModelAssociatorTest, MergeUrls) { - history::VisitVector visits1; - history::URLRow row1(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, false, &visits1)); - sync_pb::TypedUrlSpecifics specs1(MakeTypedUrlSpecifics("http://pie.com/", - "pie", - 3, false)); - history::URLRow new_row1(GURL("http://pie.com/")); - std::vector<history::VisitInfo> new_visits1; - EXPECT_TRUE(TypedUrlModelAssociator::MergeUrls(specs1, row1, &visits1, - &new_row1, &new_visits1) == TypedUrlModelAssociator::DIFF_NONE); - - history::VisitVector visits2; - history::URLRow row2(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, false, &visits2)); - sync_pb::TypedUrlSpecifics specs2(MakeTypedUrlSpecifics("http://pie.com/", - "pie", - 3, true)); - history::VisitVector expected_visits2; - history::URLRow expected2(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, true, &expected_visits2)); - history::URLRow new_row2(GURL("http://pie.com/")); - std::vector<history::VisitInfo> new_visits2; - EXPECT_TRUE(TypedUrlModelAssociator::MergeUrls(specs2, row2, &visits2, - &new_row2, &new_visits2) == - TypedUrlModelAssociator::DIFF_LOCAL_ROW_CHANGED); - EXPECT_TRUE(URLsEqual(new_row2, expected2)); - - history::VisitVector visits3; - history::URLRow row3(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, false, &visits3)); - sync_pb::TypedUrlSpecifics specs3(MakeTypedUrlSpecifics("http://pie.com/", - "pie2", - 3, true)); - history::VisitVector expected_visits3; - history::URLRow expected3(MakeTypedUrlRow("http://pie.com/", "pie2", - 2, 3, true, &expected_visits3)); - history::URLRow new_row3(GURL("http://pie.com/")); - std::vector<history::VisitInfo> new_visits3; - EXPECT_EQ(TypedUrlModelAssociator::DIFF_LOCAL_ROW_CHANGED | - TypedUrlModelAssociator::DIFF_NONE, - TypedUrlModelAssociator::MergeUrls(specs3, row3, &visits3, - &new_row3, &new_visits3)); - EXPECT_TRUE(URLsEqual(new_row3, expected3)); - - // Create one node in history DB with timestamp of 3, and one node in sync - // DB with timestamp of 4. Result should contain one new item (4). - history::VisitVector visits4; - history::URLRow row4(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, false, &visits4)); - sync_pb::TypedUrlSpecifics specs4(MakeTypedUrlSpecifics("http://pie.com/", - "pie2", - 4, false)); - history::VisitVector expected_visits4; - history::URLRow expected4(MakeTypedUrlRow("http://pie.com/", "pie2", - 2, 4, false, &expected_visits4)); - history::URLRow new_row4(GURL("http://pie.com/")); - std::vector<history::VisitInfo> new_visits4; - EXPECT_EQ(TypedUrlModelAssociator::DIFF_UPDATE_NODE | - TypedUrlModelAssociator::DIFF_LOCAL_ROW_CHANGED | - TypedUrlModelAssociator::DIFF_LOCAL_VISITS_ADDED, - TypedUrlModelAssociator::MergeUrls(specs4, row4, &visits4, - &new_row4, &new_visits4)); - EXPECT_EQ(1U, new_visits4.size()); - EXPECT_EQ(specs4.visits(0), new_visits4[0].first.ToInternalValue()); - EXPECT_TRUE(URLsEqual(new_row4, expected4)); - EXPECT_EQ(2U, visits4.size()); - - history::VisitVector visits5; - history::URLRow row5(MakeTypedUrlRow("http://pie.com/", "pie", - 1, 4, false, &visits5)); - sync_pb::TypedUrlSpecifics specs5(MakeTypedUrlSpecifics("http://pie.com/", - "pie", - 3, false)); - history::VisitVector expected_visits5; - history::URLRow expected5(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, false, &expected_visits5)); - history::URLRow new_row5(GURL("http://pie.com/")); - std::vector<history::VisitInfo> new_visits5; - - // UPDATE_NODE should be set because row5 has a newer last_visit timestamp. - EXPECT_EQ(TypedUrlModelAssociator::DIFF_UPDATE_NODE | - TypedUrlModelAssociator::DIFF_NONE, - TypedUrlModelAssociator::MergeUrls(specs5, row5, &visits5, - &new_row5, &new_visits5)); - EXPECT_TRUE(URLsEqual(new_row5, expected5)); - EXPECT_EQ(0U, new_visits5.size()); -} - -TEST_F(SyncTypedUrlModelAssociatorTest, MergeUrlsAfterExpiration) { - // Tests to ensure that we don't resurrect expired URLs (URLs that have been - // deleted from the history DB but still exist in the sync DB). - - // First, create a history row that has two visits, with timestamps 2 and 3. - history::VisitVector(history_visits); - history_visits.push_back(history::VisitRow( - 0, base::Time::FromInternalValue(2), 0, ui::PAGE_TRANSITION_TYPED, - 0)); - history::URLRow history_url(MakeTypedUrlRow("http://pie.com/", "pie", - 2, 3, false, &history_visits)); - - // Now, create a sync node with visits at timestamps 1, 2, 3, 4. - sync_pb::TypedUrlSpecifics node(MakeTypedUrlSpecifics("http://pie.com/", - "pie", 1, false)); - node.add_visits(2); - node.add_visits(3); - node.add_visits(4); - node.add_visit_transitions(2); - node.add_visit_transitions(3); - node.add_visit_transitions(4); - history::URLRow new_history_url(history_url.url()); - std::vector<history::VisitInfo> new_visits; - EXPECT_EQ(TypedUrlModelAssociator::DIFF_NONE | - TypedUrlModelAssociator::DIFF_LOCAL_VISITS_ADDED, - TypedUrlModelAssociator::MergeUrls( - node, history_url, &history_visits, &new_history_url, - &new_visits)); - EXPECT_TRUE(URLsEqual(history_url, new_history_url)); - EXPECT_EQ(1U, new_visits.size()); - EXPECT_EQ(4U, new_visits[0].first.ToInternalValue()); - // We should not sync the visit with timestamp #1 since it is earlier than - // any other visit for this URL in the history DB. But we should sync visit - // #4. - EXPECT_EQ(3U, history_visits.size()); - EXPECT_EQ(2U, history_visits[0].visit_time.ToInternalValue()); - EXPECT_EQ(3U, history_visits[1].visit_time.ToInternalValue()); - EXPECT_EQ(4U, history_visits[2].visit_time.ToInternalValue()); -} - -TEST_F(SyncTypedUrlModelAssociatorTest, DiffVisitsSame) { - history::VisitVector old_visits; - sync_pb::TypedUrlSpecifics new_url; - - const int64 visits[] = { 1024, 2065, 65534, 1237684 }; - - for (size_t c = 0; c < arraysize(visits); ++c) { - old_visits.push_back(history::VisitRow( - 0, base::Time::FromInternalValue(visits[c]), 0, - ui::PAGE_TRANSITION_TYPED, 0)); - new_url.add_visits(visits[c]); - new_url.add_visit_transitions(ui::PAGE_TRANSITION_TYPED); - } - - std::vector<history::VisitInfo> new_visits; - history::VisitVector removed_visits; - - TypedUrlModelAssociator::DiffVisits(old_visits, new_url, - &new_visits, &removed_visits); - EXPECT_TRUE(new_visits.empty()); - EXPECT_TRUE(removed_visits.empty()); -} - -TEST_F(SyncTypedUrlModelAssociatorTest, DiffVisitsRemove) { - history::VisitVector old_visits; - sync_pb::TypedUrlSpecifics new_url; - - const int64 visits_left[] = { 1, 2, 1024, 1500, 2065, 6000, - 65534, 1237684, 2237684 }; - const int64 visits_right[] = { 1024, 2065, 65534, 1237684 }; - - // DiffVisits will not remove the first visit, because we never delete visits - // from the start of the array (since those visits can get truncated by the - // size-limiting code). - const int64 visits_removed[] = { 1500, 6000, 2237684 }; - - for (size_t c = 0; c < arraysize(visits_left); ++c) { - old_visits.push_back(history::VisitRow( - 0, base::Time::FromInternalValue(visits_left[c]), 0, - ui::PAGE_TRANSITION_TYPED, 0)); - } - - for (size_t c = 0; c < arraysize(visits_right); ++c) { - new_url.add_visits(visits_right[c]); - new_url.add_visit_transitions(ui::PAGE_TRANSITION_TYPED); - } - - std::vector<history::VisitInfo> new_visits; - history::VisitVector removed_visits; - - TypedUrlModelAssociator::DiffVisits(old_visits, new_url, - &new_visits, &removed_visits); - EXPECT_TRUE(new_visits.empty()); - ASSERT_EQ(removed_visits.size(), arraysize(visits_removed)); - for (size_t c = 0; c < arraysize(visits_removed); ++c) { - EXPECT_EQ(removed_visits[c].visit_time.ToInternalValue(), - visits_removed[c]); - } -} - -TEST_F(SyncTypedUrlModelAssociatorTest, DiffVisitsAdd) { - history::VisitVector old_visits; - sync_pb::TypedUrlSpecifics new_url; - - const int64 visits_left[] = { 1024, 2065, 65534, 1237684 }; - const int64 visits_right[] = { 1, 1024, 1500, 2065, 6000, - 65534, 1237684, 2237684 }; - - const int64 visits_added[] = { 1, 1500, 6000, 2237684 }; - - for (size_t c = 0; c < arraysize(visits_left); ++c) { - old_visits.push_back(history::VisitRow( - 0, base::Time::FromInternalValue(visits_left[c]), 0, - ui::PAGE_TRANSITION_TYPED, 0)); - } - - for (size_t c = 0; c < arraysize(visits_right); ++c) { - new_url.add_visits(visits_right[c]); - new_url.add_visit_transitions(ui::PAGE_TRANSITION_TYPED); - } - - std::vector<history::VisitInfo> new_visits; - history::VisitVector removed_visits; - - TypedUrlModelAssociator::DiffVisits(old_visits, new_url, - &new_visits, &removed_visits); - EXPECT_TRUE(removed_visits.empty()); - ASSERT_TRUE(new_visits.size() == arraysize(visits_added)); - for (size_t c = 0; c < arraysize(visits_added); ++c) { - EXPECT_EQ(new_visits[c].first.ToInternalValue(), - visits_added[c]); - EXPECT_EQ(new_visits[c].second, ui::PAGE_TRANSITION_TYPED); - } -} - -static history::VisitRow CreateVisit(ui::PageTransition type, - int64 timestamp) { - return history::VisitRow(0, base::Time::FromInternalValue(timestamp), 0, - type, 0); -} - -TEST_F(SyncTypedUrlModelAssociatorTest, WriteTypedUrlSpecifics) { - history::VisitVector visits; - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_TYPED, 1)); - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_RELOAD, 2)); - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_LINK, 3)); - - history::URLRow url(MakeTypedUrlRow("http://pie.com/", "pie", - 1, 100, false, &visits)); - sync_pb::TypedUrlSpecifics typed_url; - TypedUrlModelAssociator::WriteToTypedUrlSpecifics(url, visits, &typed_url); - // RELOAD visits should be removed. - EXPECT_EQ(2, typed_url.visits_size()); - EXPECT_EQ(typed_url.visit_transitions_size(), typed_url.visits_size()); - EXPECT_EQ(1, typed_url.visits(0)); - EXPECT_EQ(3, typed_url.visits(1)); - EXPECT_EQ(ui::PAGE_TRANSITION_TYPED, - ui::PageTransitionFromInt(typed_url.visit_transitions(0))); - EXPECT_EQ(ui::PAGE_TRANSITION_LINK, - ui::PageTransitionFromInt(typed_url.visit_transitions(1))); -} - -TEST_F(SyncTypedUrlModelAssociatorTest, TooManyVisits) { - history::VisitVector visits; - int64 timestamp = 1000; - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_TYPED, timestamp++)); - for (int i = 0 ; i < 100; ++i) { - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_LINK, timestamp++)); - } - history::URLRow url(MakeTypedUrlRow("http://pie.com/", "pie", - 1, timestamp++, false, &visits)); - sync_pb::TypedUrlSpecifics typed_url; - TypedUrlModelAssociator::WriteToTypedUrlSpecifics(url, visits, &typed_url); - // # visits should be capped at 100. - EXPECT_EQ(100, typed_url.visits_size()); - EXPECT_EQ(typed_url.visit_transitions_size(), typed_url.visits_size()); - EXPECT_EQ(1000, typed_url.visits(0)); - // Visit with timestamp of 1001 should be omitted since we should have - // skipped that visit to stay under the cap. - EXPECT_EQ(1002, typed_url.visits(1)); - EXPECT_EQ(ui::PAGE_TRANSITION_TYPED, - ui::PageTransitionFromInt(typed_url.visit_transitions(0))); - EXPECT_EQ(ui::PAGE_TRANSITION_LINK, - ui::PageTransitionFromInt(typed_url.visit_transitions(1))); -} - -TEST_F(SyncTypedUrlModelAssociatorTest, TooManyTypedVisits) { - history::VisitVector visits; - int64 timestamp = 1000; - for (int i = 0 ; i < 102; ++i) { - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_TYPED, timestamp++)); - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_LINK, timestamp++)); - visits.push_back(CreateVisit(ui::PAGE_TRANSITION_RELOAD, timestamp++)); - } - history::URLRow url(MakeTypedUrlRow("http://pie.com/", "pie", - 1, timestamp++, false, &visits)); - sync_pb::TypedUrlSpecifics typed_url; - TypedUrlModelAssociator::WriteToTypedUrlSpecifics(url, visits, &typed_url); - // # visits should be capped at 100. - EXPECT_EQ(100, typed_url.visits_size()); - EXPECT_EQ(typed_url.visit_transitions_size(), typed_url.visits_size()); - // First two typed visits should be skipped. - EXPECT_EQ(1006, typed_url.visits(0)); - - // Ensure there are no non-typed visits since that's all that should fit. - for (int i = 0; i < typed_url.visits_size(); ++i) { - EXPECT_EQ(ui::PAGE_TRANSITION_TYPED, - ui::PageTransitionFromInt(typed_url.visit_transitions(i))); - } -} - -TEST_F(SyncTypedUrlModelAssociatorTest, NoTypedVisits) { - history::VisitVector visits; - history::URLRow url(MakeTypedUrlRow("http://pie.com/", "pie", - 1, 1000, false, &visits)); - sync_pb::TypedUrlSpecifics typed_url; - TypedUrlModelAssociator::WriteToTypedUrlSpecifics(url, visits, &typed_url); - // URLs with no typed URL visits should be translated to a URL with one - // reload visit. - EXPECT_EQ(1, typed_url.visits_size()); - EXPECT_EQ(typed_url.visit_transitions_size(), typed_url.visits_size()); - // First two typed visits should be skipped. - EXPECT_EQ(1000, typed_url.visits(0)); - EXPECT_EQ(ui::PAGE_TRANSITION_RELOAD, - ui::PageTransitionFromInt(typed_url.visit_transitions(0))); -} - -// This test verifies that we can abort model association from the UI thread. -// We start up the model associator on the DB thread, block until we abort the -// association on the UI thread, then ensure that AssociateModels() returns -// false. -TEST_F(SyncTypedUrlModelAssociatorTest, TestAbort) { - base::Thread db_thread("DB_Thread"); - db_thread.Start(); - - base::WaitableEvent startup(false, false); - base::WaitableEvent aborted(false, false); - base::WaitableEvent done(false, false); - sync_driver::FakeSyncService service; - TypedUrlModelAssociator* associator(NULL); - // Fire off to the DB thread to create the model associator and start - // model association. - base::Closure callback = base::Bind( - &CreateModelAssociatorAsync, &startup, &aborted, &done, &associator, - &service); - db_thread.task_runner()->PostTask(FROM_HERE, callback); - // Wait for the model associator to get created and start assocation. - ASSERT_TRUE(startup.TimedWait(TestTimeouts::action_timeout())); - // Abort the model assocation - this should be callable from any thread. - associator->AbortAssociation(); - // Tell the remote thread to continue. - aborted.Signal(); - // Block until CreateModelAssociator() exits. - ASSERT_TRUE(done.TimedWait(TestTimeouts::action_timeout())); - db_thread.Stop(); -} diff --git a/components/history/core/browser/typed_url_syncable_service.cc b/components/history/core/browser/typed_url_syncable_service.cc index 7fb5410..1aaaeec 100644 --- a/components/history/core/browser/typed_url_syncable_service.cc +++ b/components/history/core/browser/typed_url_syncable_service.cc @@ -61,7 +61,11 @@ static bool CheckVisitOrdering(const VisitVector& visits) { TypedUrlSyncableService::TypedUrlSyncableService( HistoryBackend* history_backend) - : history_backend_(history_backend), processing_syncer_changes_(false) { + : history_backend_(history_backend), + processing_syncer_changes_(false), + num_db_accesses_(0), + num_db_errors_(0), + history_backend_observer_(this) { DCHECK(history_backend_); DCHECK(thread_checker_.CalledOnValidThread()); } @@ -88,16 +92,6 @@ syncer::SyncMergeResult TypedUrlSyncableService::MergeDataAndStartSyncing( DVLOG(1) << "Associating TypedUrl: MergeDataAndStartSyncing"; - // Get all the typed urls from the history db. - history::URLRows typed_urls; - ++num_db_accesses_; - if (!history_backend_->GetAllTypedURLs(&typed_urls)) { - ++num_db_errors_; - sync_error_handler_->CreateAndUploadError( - FROM_HERE, "Could not get the typed_url entries."); - return merge_result; - } - // Create a mapping of all local data by URLID. These will be narrowed down // by CreateOrUpdateUrl() to include only the entries different from sync // server data. @@ -105,20 +99,33 @@ syncer::SyncMergeResult TypedUrlSyncableService::MergeDataAndStartSyncing( // Get all the visits and map the URLRows by URL. UrlVisitVectorMap visit_vectors; - for (history::URLRows::iterator iter = typed_urls.begin(); - iter != typed_urls.end();) { - DCHECK_EQ(0U, visit_vectors.count(iter->url())); - if (!FixupURLAndGetVisits(&(*iter), &(visit_vectors[iter->url()])) || - ShouldIgnoreUrl(iter->url()) || - ShouldIgnoreVisits(visit_vectors[iter->url()])) { - // Ignore this URL if we couldn't load the visits or if there's some - // other problem with it (it was empty, or imported and never visited). - iter = typed_urls.erase(iter); - } else { - // Add url to map. - new_db_urls[iter->url()] = - std::make_pair(syncer::SyncChange::ACTION_ADD, iter); - ++iter; + + { + // Get all the typed urls from the history db. + history::URLRows typed_urls; + ++num_db_accesses_; + if (!history_backend_->GetAllTypedURLs(&typed_urls)) { + ++num_db_errors_; + merge_result.set_error(sync_error_handler_->CreateAndUploadError( + FROM_HERE, "Could not get the typed_url entries.")); + return merge_result; + } + + for (history::URLRows::iterator iter = typed_urls.begin(); + iter != typed_urls.end();) { + DCHECK_EQ(0U, visit_vectors.count(iter->url())); + if (!FixupURLAndGetVisits(&(*iter), &(visit_vectors[iter->url()])) || + ShouldIgnoreUrl(iter->url()) || + ShouldIgnoreVisits(visit_vectors[iter->url()])) { + // Ignore this URL if we couldn't load the visits or if there's some + // other problem with it (it was empty, or imported and never visited). + iter = typed_urls.erase(iter); + } else { + // Add url to map. + new_db_urls[iter->url()] = + std::make_pair(syncer::SyncChange::ACTION_ADD, *iter); + ++iter; + } } } @@ -164,15 +171,14 @@ syncer::SyncMergeResult TypedUrlSyncableService::MergeDataAndStartSyncing( continue; } - CreateOrUpdateUrl(typed_url, &typed_urls, &new_db_urls, &visit_vectors, - &new_synced_urls, &new_synced_visits, - &updated_synced_urls); + CreateOrUpdateUrl(typed_url, &new_db_urls, &visit_vectors, &new_synced_urls, + &new_synced_visits, &updated_synced_urls); } for (TypedUrlMap::iterator i = new_db_urls.begin(); i != new_db_urls.end(); ++i) { std::string tag = i->first.spec(); - AddTypedUrlToChangeList(i->second.first, *(i->second.second), + AddTypedUrlToChangeList(i->second.first, i->second.second, visit_vectors[i->first], tag, &new_changes); // Add url to cache of sync state, if not already cached @@ -188,6 +194,8 @@ syncer::SyncMergeResult TypedUrlSyncableService::MergeDataAndStartSyncing( &new_synced_visits, NULL); } + history_backend_observer_.Add(history_backend_); + UMA_HISTOGRAM_PERCENTAGE("Sync.TypedUrlMergeAndStartSyncingErrors", GetErrorPercentage()); ClearErrorStats(); @@ -202,6 +210,8 @@ void TypedUrlSyncableService::StopSyncing(syncer::ModelType type) { // Clear cache of server state. synced_typed_urls_.clear(); + history_backend_observer_.RemoveAll(); + ClearErrorStats(); sync_processor_.reset(); @@ -370,7 +380,6 @@ void TypedUrlSyncableService::OnURLsDeleted( void TypedUrlSyncableService::CreateOrUpdateUrl( const sync_pb::TypedUrlSpecifics& typed_url, - history::URLRows* typed_urls, TypedUrlMap* loaded_data, UrlVisitVectorMap* visit_vectors, history::URLRows* new_synced_urls, @@ -405,10 +414,6 @@ void TypedUrlSyncableService::CreateOrUpdateUrl( bool is_existing_url = history_backend_->GetURL(untyped_url.url(), &untyped_url); if (is_existing_url) { - // This URL already exists locally, but was not grabbed earlier - // because |typed_count| is 0 - DCHECK_EQ(untyped_url.typed_count(), 0); - // Add a new entry to |loaded_data|, and set the iterator to it. history::VisitVector untyped_visits; if (!FixupURLAndGetVisits(&untyped_url, &untyped_visits)) { @@ -421,12 +426,9 @@ void TypedUrlSyncableService::CreateOrUpdateUrl( (*visit_vectors)[untyped_url.url()] = untyped_visits; // Store row info that will be used to update sync's visits. - history::URLRows::iterator ri = - typed_urls->insert(typed_urls->end(), untyped_url); (*loaded_data)[untyped_url.url()] = - std::pair<syncer::SyncChange::SyncChangeType, - history::URLRows::iterator>( - syncer::SyncChange::ACTION_UPDATE, ri); + std::pair<syncer::SyncChange::SyncChangeType, history::URLRow>( + syncer::SyncChange::ACTION_UPDATE, untyped_url); // Set iterator |it| to point to this entry. it = loaded_data->find(untyped_url.url()); @@ -463,21 +465,21 @@ void TypedUrlSyncableService::CreateOrUpdateUrl( std::vector<history::VisitInfo> added_visits; // Empty URLs should be filtered out by ShouldIgnoreUrl() previously. - DCHECK(!it->second.second->url().spec().empty()); + DCHECK(!it->second.second.url().spec().empty()); // Initialize fields in |new_url| to the same values as the fields in // the existing URLRow in the history DB. This is needed because we // overwrite the existing value in WriteToHistoryBackend(), but some of // the values in that structure are not synced (like typed_count). - history::URLRow new_url(*(it->second.second)); + history::URLRow new_url(it->second.second); - MergeResult difference = MergeUrls(sync_url, *(it->second.second), &visits, - &new_url, &added_visits); + MergeResult difference = + MergeUrls(sync_url, it->second.second, &visits, &new_url, &added_visits); if (difference != DIFF_NONE) { + it->second.second = new_url; if (difference & DIFF_UPDATE_NODE) { // Edit map entry to reflect update to sync. - *(it->second.second) = new_url; it->second.first = syncer::SyncChange::ACTION_UPDATE; // We don't want to resurrect old visits that have been aged out by // other clients, so remove all visits that are older than the @@ -504,7 +506,7 @@ void TypedUrlSyncableService::CreateOrUpdateUrl( } if (difference & DIFF_LOCAL_ROW_CHANGED) { // Add entry to updated_synced_urls to update the local db. - DCHECK_EQ(it->second.second->id(), new_url.id()); + DCHECK_EQ(it->second.second.id(), new_url.id()); updated_synced_urls->push_back(new_url); } if (difference & DIFF_LOCAL_VISITS_ADDED) { @@ -525,23 +527,14 @@ sync_pb::TypedUrlSpecifics TypedUrlSyncableService::FilterExpiredVisits( sync_pb::TypedUrlSpecifics specifics(source); specifics.clear_visits(); specifics.clear_visit_transitions(); - int typed_count = 0; for (int i = 0; i < source.visits_size(); ++i) { base::Time time = base::Time::FromInternalValue(source.visits(i)); if (!history_backend_->IsExpiredVisitTime(time)) { specifics.add_visits(source.visits(i)); specifics.add_visit_transitions(source.visit_transitions(i)); - if (source.visit_transitions(i) == ui::PAGE_TRANSITION_TYPED) - ++typed_count; } } DCHECK(specifics.visits_size() == specifics.visit_transitions_size()); - // Treat specifics with no non-expired typed visits as though they have no - // non-expired visits of any kind - if (typed_count == 0) { - specifics.clear_visits(); - specifics.clear_visit_transitions(); - } return specifics; } diff --git a/components/history/core/browser/typed_url_syncable_service.h b/components/history/core/browser/typed_url_syncable_service.h index ed7d492..da396dc 100644 --- a/components/history/core/browser/typed_url_syncable_service.h +++ b/components/history/core/browser/typed_url_syncable_service.h @@ -8,6 +8,7 @@ #include <set> #include <vector> +#include "base/scoped_observer.h" #include "base/threading/thread_checker.h" #include "components/history/core/browser/history_backend_observer.h" #include "components/history/core/browser/history_types.h" @@ -68,6 +69,15 @@ class TypedUrlSyncableService : public syncer::SyncableService, const history::URLRows& deleted_rows, const std::set<GURL>& favicon_urls) override; + // Returns the percentage of DB accesses that have resulted in an error. + int GetErrorPercentage() const; + + // Converts the passed URL information to a TypedUrlSpecifics structure for + // writing to the sync DB. + static void WriteToTypedUrlSpecifics(const URLRow& url, + const VisitVector& visits, + sync_pb::TypedUrlSpecifics* specifics); + private: friend class TypedUrlSyncableServiceTest; @@ -77,9 +87,8 @@ class TypedUrlSyncableService : public syncer::SyncableService, // This is a helper map used only in Merge/Process* functions. The lifetime // of the iterator is longer than the map object. - typedef std::map<GURL, - std::pair<syncer::SyncChange::SyncChangeType, - URLRows::iterator>> TypedUrlMap; + typedef std::map<GURL, std::pair<syncer::SyncChange::SyncChangeType, URLRow>> + TypedUrlMap; // This is a helper map used to associate visit vectors from the history db // to the typed urls in the above map. @@ -93,17 +102,17 @@ class TypedUrlSyncableService : public syncer::SyncableService, static const MergeResult DIFF_LOCAL_VISITS_ADDED = 1 << 2; // Helper method for getting the set of synced urls. + // Set it as virtual for testing. void GetSyncedUrls(std::set<GURL>* urls) const; // Helper function that clears our error counters (used to reset stats after // model association so we can track model association errors separately). - void ClearErrorStats(); + virtual void ClearErrorStats(); // Compares |typed_url| from the server against local history to decide how // to merge any existing data, and updates appropriate data containers to // write to server and backend. void CreateOrUpdateUrl(const sync_pb::TypedUrlSpecifics& typed_url, - history::URLRows* typed_urls, TypedUrlMap* loaded_data, UrlVisitVectorMap* visit_vectors, history::URLRows* new_synced_urls, @@ -138,9 +147,6 @@ class TypedUrlSyncableService : public syncer::SyncableService, const TypedUrlVisitVector* new_visits, const history::VisitVector* deleted_visits); - // Returns the percentage of DB accesses that have resulted in an error. - int GetErrorPercentage() const; - // Helper function that determines if we should ignore a URL for the purposes // of sync, because it contains invalid data. bool ShouldIgnoreUrl(const GURL& url); @@ -168,12 +174,6 @@ class TypedUrlSyncableService : public syncer::SyncableService, std::string title, syncer::SyncChangeList* change_list); - // Converts the passed URL information to a TypedUrlSpecifics structure for - // writing to the sync DB. - static void WriteToTypedUrlSpecifics(const URLRow& url, - const VisitVector& visits, - sync_pb::TypedUrlSpecifics* specifics); - // Fills |new_url| with formatted data from |typed_url|. static void UpdateURLRowFromTypedUrlSpecifics( const sync_pb::TypedUrlSpecifics& typed_url, @@ -234,6 +234,9 @@ class TypedUrlSyncableService : public syncer::SyncableService, base::ThreadChecker thread_checker_; + ScopedObserver<history::HistoryBackend, history::HistoryBackendObserver> + history_backend_observer_; + DISALLOW_COPY_AND_ASSIGN(TypedUrlSyncableService); }; diff --git a/components/history/core/browser/typed_url_syncable_service_unittest.cc b/components/history/core/browser/typed_url_syncable_service_unittest.cc index fdd7fce..a3405da 100644 --- a/components/history/core/browser/typed_url_syncable_service_unittest.cc +++ b/components/history/core/browser/typed_url_syncable_service_unittest.cc @@ -210,7 +210,7 @@ class TestHistoryBackend : public HistoryBackend { class TypedUrlSyncableServiceTest : public testing::Test { public: - TypedUrlSyncableServiceTest() {} + TypedUrlSyncableServiceTest() : typed_url_sync_service_(NULL) {} ~TypedUrlSyncableServiceTest() override {} void SetUp() override { @@ -219,8 +219,8 @@ class TypedUrlSyncableServiceTest : public testing::Test { fake_history_backend_->Init( std::string(), false, TestHistoryDatabaseParamsForPath(test_dir_.path())); - typed_url_sync_service_.reset( - new TypedUrlSyncableService(fake_history_backend_.get())); + typed_url_sync_service_ = + fake_history_backend_->GetTypedUrlSyncableService(); fake_change_processor_.reset(new syncer::FakeSyncChangeProcessor); } @@ -288,7 +288,7 @@ class TypedUrlSyncableServiceTest : public testing::Test { base::MessageLoop message_loop_; base::ScopedTempDir test_dir_; scoped_refptr<TestHistoryBackend> fake_history_backend_; - scoped_ptr<TypedUrlSyncableService> typed_url_sync_service_; + TypedUrlSyncableService* typed_url_sync_service_; scoped_ptr<syncer::FakeSyncChangeProcessor> fake_change_processor_; }; @@ -305,6 +305,7 @@ void TypedUrlSyncableServiceTest::StartSyncing( fake_change_processor_.get())), scoped_ptr<syncer::SyncErrorFactory>( new syncer::SyncErrorFactoryMock())); + typed_url_sync_service_->history_backend_observer_.RemoveAll(); EXPECT_FALSE(result.error().IsSet()) << result.error().message(); } @@ -316,7 +317,7 @@ bool TypedUrlSyncableServiceTest::BuildAndPushLocalChanges( std::vector<VisitVector>* visit_vectors) { unsigned int total_urls = num_typed_urls + num_reload_urls; DCHECK(urls.size() >= total_urls); - if (!typed_url_sync_service_.get()) + if (!typed_url_sync_service_) return false; if (total_urls) { diff --git a/components/sync_driver.gypi b/components/sync_driver.gypi index fa4aeb3..d5daf40 100644 --- a/components/sync_driver.gypi +++ b/components/sync_driver.gypi @@ -91,8 +91,6 @@ 'sync_driver/model_associator.h', 'sync_driver/non_blocking_data_type_controller.cc', 'sync_driver/non_blocking_data_type_controller.h', - 'sync_driver/non_frontend_data_type_controller.cc', - 'sync_driver/non_frontend_data_type_controller.h', 'sync_driver/non_ui_data_type_controller.cc', 'sync_driver/non_ui_data_type_controller.h', 'sync_driver/pref_names.cc', @@ -212,8 +210,6 @@ 'sync_driver/local_device_info_provider_mock.h', 'sync_driver/model_associator_mock.cc', 'sync_driver/model_associator_mock.h', - 'sync_driver/non_frontend_data_type_controller_mock.cc', - 'sync_driver/non_frontend_data_type_controller_mock.h', 'sync_driver/non_ui_data_type_controller_mock.cc', 'sync_driver/non_ui_data_type_controller_mock.h', 'sync_driver/sync_api_component_factory_mock.cc', diff --git a/components/sync_driver/BUILD.gn b/components/sync_driver/BUILD.gn index ddbe6f4..49debde 100644 --- a/components/sync_driver/BUILD.gn +++ b/components/sync_driver/BUILD.gn @@ -72,8 +72,6 @@ source_set("sync_driver") { "model_associator.h", "non_blocking_data_type_controller.cc", "non_blocking_data_type_controller.h", - "non_frontend_data_type_controller.cc", - "non_frontend_data_type_controller.h", "non_ui_data_type_controller.cc", "non_ui_data_type_controller.h", "pref_names.cc", @@ -179,8 +177,6 @@ source_set("test_support") { "local_device_info_provider_mock.h", "model_associator_mock.cc", "model_associator_mock.h", - "non_frontend_data_type_controller_mock.cc", - "non_frontend_data_type_controller_mock.h", "non_ui_data_type_controller_mock.cc", "non_ui_data_type_controller_mock.h", "sync_api_component_factory_mock.cc", @@ -221,7 +217,6 @@ source_set("unit_tests") { "local_device_info_provider_unittest.cc", "model_association_manager_unittest.cc", "non_blocking_data_type_controller_unittest.cc", - "non_frontend_data_type_controller_unittest.cc", "non_ui_data_type_controller_unittest.cc", "profile_sync_auth_provider_unittest.cc", "shared_change_processor_unittest.cc", diff --git a/components/sync_driver/generic_change_processor_unittest.cc b/components/sync_driver/generic_change_processor_unittest.cc index 63e6187..3d75db1 100644 --- a/components/sync_driver/generic_change_processor_unittest.cc +++ b/components/sync_driver/generic_change_processor_unittest.cc @@ -110,12 +110,6 @@ class MockSyncApiComponentFactory : public SyncApiComponentFactory { sync_driver::DataTypeErrorHandler* error_handler) override { return SyncComponents(nullptr, nullptr); } - SyncComponents CreateTypedUrlSyncComponents( - sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) override { - return SyncComponents(nullptr, nullptr); - } scoped_ptr<syncer::AttachmentService> CreateAttachmentService( scoped_ptr<syncer::AttachmentStoreForSync> attachment_store, diff --git a/components/sync_driver/non_frontend_data_type_controller.cc b/components/sync_driver/non_frontend_data_type_controller.cc deleted file mode 100644 index 02742cb..0000000 --- a/components/sync_driver/non_frontend_data_type_controller.cc +++ /dev/null @@ -1,445 +0,0 @@ -// Copyright (c) 2012 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 "components/sync_driver/non_frontend_data_type_controller.h" - -#include "base/bind.h" -#include "base/callback.h" -#include "base/logging.h" -#include "base/thread_task_runner_handle.h" -#include "components/sync_driver/change_processor.h" -#include "components/sync_driver/model_associator.h" -#include "components/sync_driver/sync_client.h" -#include "components/sync_driver/sync_service.h" -#include "sync/api/sync_error.h" -#include "sync/internal_api/public/base/model_type.h" -#include "sync/internal_api/public/util/weak_handle.h" -#include "sync/util/data_type_histogram.h" - -namespace browser_sync { - -class NonFrontendDataTypeController::BackendComponentsContainer { - public: - explicit BackendComponentsContainer( - NonFrontendDataTypeController* controller); - ~BackendComponentsContainer(); - void Run(); - void Disconnect(); - - private: - bool CreateComponents(); - void Associate(); - - // For creating components. - NonFrontendDataTypeController* controller_; - base::Lock controller_lock_; - - syncer::ModelType type_; - - // For returning association results to controller on UI. - syncer::WeakHandle<NonFrontendDataTypeController> controller_handle_; - - scoped_ptr<sync_driver::AssociatorInterface> model_associator_; - scoped_ptr<sync_driver::ChangeProcessor> change_processor_; -}; - -NonFrontendDataTypeController:: -BackendComponentsContainer::BackendComponentsContainer( - NonFrontendDataTypeController* controller) - : controller_(controller), - type_(controller->type()) { - controller_handle_ = - syncer::MakeWeakHandle(controller_->weak_ptr_factory_.GetWeakPtr()); -} - -NonFrontendDataTypeController:: -BackendComponentsContainer::~BackendComponentsContainer() { - if (model_associator_) - model_associator_->DisassociateModels(); -} - -void NonFrontendDataTypeController::BackendComponentsContainer::Run() { - DCHECK(controller_->IsOnBackendThread()); - if (CreateComponents()) - Associate(); -} - -bool -NonFrontendDataTypeController::BackendComponentsContainer::CreateComponents() { - base::AutoLock al(controller_lock_); - if (!controller_) { - DVLOG(1) << "Controller was stopped before sync components are created."; - return false; - } - - sync_driver::SyncApiComponentFactory::SyncComponents sync_components = - controller_->CreateSyncComponents(); - model_associator_.reset(sync_components.model_associator); - change_processor_.reset(sync_components.change_processor); - return true; -} - -void NonFrontendDataTypeController::BackendComponentsContainer::Associate() { - CHECK(model_associator_); - - bool succeeded = false; - - browser_sync::NonFrontendDataTypeController::AssociationResult result(type_); - if (!model_associator_->CryptoReadyIfNecessary()) { - result.needs_crypto = true; - } else { - base::TimeTicks start_time = base::TimeTicks::Now(); - - if (!model_associator_->SyncModelHasUserCreatedNodes( - &result.sync_has_nodes)) { - result.unrecoverable_error = true; - result.error = syncer::SyncError(FROM_HERE, - syncer::SyncError::UNRECOVERABLE_ERROR, - "Failed to load sync nodes", - type_); - } else { - result.error = model_associator_->AssociateModels( - &result.local_merge_result, &result.syncer_merge_result); - - // Return components to frontend when no error. - if (!result.error.IsSet()) { - succeeded = true; - result.change_processor = change_processor_.get(); - result.model_associator = model_associator_.get(); - } - } - result.association_time = base::TimeTicks::Now() - start_time; - } - result.local_merge_result.set_error(result.error); - - // Destroy processor/associator on backend on failure. - if (!succeeded) { - base::AutoLock al(controller_lock_); - model_associator_->DisassociateModels(); - change_processor_.reset(); - model_associator_.reset(); - } - - controller_handle_.Call( - FROM_HERE, - &browser_sync::NonFrontendDataTypeController::AssociationCallback, - result); -} - -void NonFrontendDataTypeController::BackendComponentsContainer::Disconnect() { - base::AutoLock al(controller_lock_); - CHECK(controller_); - - if (change_processor_) - controller_->DisconnectProcessor(change_processor_.get()); - if (model_associator_) - model_associator_->AbortAssociation(); - - controller_ = NULL; -} - -NonFrontendDataTypeController::AssociationResult::AssociationResult( - syncer::ModelType type) - : needs_crypto(false), - unrecoverable_error(false), - sync_has_nodes(false), - local_merge_result(type), - syncer_merge_result(type), - change_processor(NULL), - model_associator(NULL) {} - -NonFrontendDataTypeController::AssociationResult::~AssociationResult() {} - -NonFrontendDataTypeController::NonFrontendDataTypeController( - const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread, - const base::Closure& error_callback, - sync_driver::SyncClient* sync_client) - : DirectoryDataTypeController(ui_thread, error_callback), - state_(NOT_RUNNING), - sync_client_(sync_client), - model_associator_(NULL), - change_processor_(NULL), - weak_ptr_factory_(this) { - DCHECK(this->ui_thread()->RunsTasksOnCurrentThread()); - DCHECK(sync_client_); -} - -void NonFrontendDataTypeController::LoadModels( - const ModelLoadCallback& model_load_callback) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - model_load_callback_ = model_load_callback; - if (state_ != NOT_RUNNING) { - model_load_callback.Run(type(), - syncer::SyncError(FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Model already loaded", - type())); - return; - } - - state_ = MODEL_STARTING; - if (!StartModels()) { - // We failed to start the models. There is no point in waiting. - // Note: This code is deprecated. The only 2 datatypes here, - // passwords and typed urls, dont have any special loading. So if we - // get a false it means they failed. - DCHECK(state_ == NOT_RUNNING || state_ == MODEL_STARTING - || state_ == DISABLED); - model_load_callback.Run(type(), - syncer::SyncError(FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Failed loading", - type())); - return; - } - state_ = MODEL_LOADED; - - model_load_callback.Run(type(), syncer::SyncError()); -} - -void NonFrontendDataTypeController::StartAssociating( - const StartCallback& start_callback) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - DCHECK(!start_callback.is_null()); - DCHECK(!components_container_); - DCHECK_EQ(state_, MODEL_LOADED); - - // Kick off association on the thread the datatype resides on. - state_ = ASSOCIATING; - start_callback_ = start_callback; - - components_container_.reset(new BackendComponentsContainer(this)); - - if (!PostTaskOnBackendThread( - FROM_HERE, - base::Bind(&BackendComponentsContainer::Run, - base::Unretained(components_container_.get())))) { - syncer::SyncError error( - FROM_HERE, - syncer::SyncError::DATATYPE_ERROR, - "Failed to post StartAssociation", type()); - syncer::SyncMergeResult local_merge_result(type()); - local_merge_result.set_error(error); - StartDone(ASSOCIATION_FAILED, - local_merge_result, - syncer::SyncMergeResult(type())); - } -} - -void DestroyComponentsInBackend( - NonFrontendDataTypeController::BackendComponentsContainer *container) { - delete container; -} - -void NonFrontendDataTypeController::Stop() { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - - if (state_ == NOT_RUNNING) - return; - - // Ignore association callback. - weak_ptr_factory_.InvalidateWeakPtrs(); - - // Disconnect on UI and post task to destroy on backend. - if (components_container_) { - components_container_->Disconnect(); - PostTaskOnBackendThread( - FROM_HERE, - base::Bind(&DestroyComponentsInBackend, - components_container_.release())); - model_associator_ = NULL; - change_processor_ = NULL; - } - - state_ = NOT_RUNNING; -} - -std::string NonFrontendDataTypeController::name() const { - // For logging only. - return syncer::ModelTypeToString(type()); -} - -sync_driver::DataTypeController::State NonFrontendDataTypeController::state() - const { - return state_; -} - -void NonFrontendDataTypeController::OnSingleDataTypeUnrecoverableError( - const syncer::SyncError& error) { - DCHECK(IsOnBackendThread()); - DCHECK_EQ(type(), error.model_type()); - RecordUnrecoverableError(error.location(), error.message()); - ui_thread()->PostTask(error.location(), - base::Bind(&NonFrontendDataTypeController::DisableImpl, - this, - error)); -} - -NonFrontendDataTypeController::NonFrontendDataTypeController() - : DirectoryDataTypeController(base::ThreadTaskRunnerHandle::Get(), - base::Closure()), - state_(NOT_RUNNING), - sync_client_(NULL), - model_associator_(NULL), - change_processor_(NULL), - weak_ptr_factory_(this) {} - -NonFrontendDataTypeController::~NonFrontendDataTypeController() { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - DCHECK(!change_processor_); - DCHECK(!model_associator_); -} - -bool NonFrontendDataTypeController::StartModels() { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - DCHECK_EQ(state_, MODEL_STARTING); - // By default, no additional services need to be started before we can proceed - // with model association, so do nothing. - return true; -} - -bool NonFrontendDataTypeController::IsOnBackendThread() { - return !ui_thread()->RunsTasksOnCurrentThread(); -} - -void NonFrontendDataTypeController::StartDone( - DataTypeController::ConfigureResult start_result, - const syncer::SyncMergeResult& local_merge_result, - const syncer::SyncMergeResult& syncer_merge_result) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - DataTypeController::State new_state; - - if (IsSuccessfulResult(start_result)) { - new_state = RUNNING; - } else { - new_state = (start_result == ASSOCIATION_FAILED ? DISABLED : NOT_RUNNING); - if (IsUnrecoverableResult(start_result)) - RecordUnrecoverableError(FROM_HERE, "StartFailed"); - } - - StartDoneImpl(start_result, new_state, local_merge_result, - syncer_merge_result); -} - -void NonFrontendDataTypeController::StartDoneImpl( - DataTypeController::ConfigureResult start_result, - DataTypeController::State new_state, - const syncer::SyncMergeResult& local_merge_result, - const syncer::SyncMergeResult& syncer_merge_result) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - - state_ = new_state; - if (state_ != RUNNING) { - // Start failed. - RecordStartFailure(start_result); - } - - start_callback_.Run(start_result, local_merge_result, syncer_merge_result); -} - -void NonFrontendDataTypeController::DisableImpl( - const syncer::SyncError& error) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - if (!model_load_callback_.is_null()) { - model_load_callback_.Run(type(), error); - } -} - -void NonFrontendDataTypeController::RecordAssociationTime( - base::TimeDelta time) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_TIMES("Sync." type_str "AssociationTime", time); - SYNC_DATA_TYPE_HISTOGRAM(type()); -#undef PER_DATA_TYPE_MACRO -} - -void NonFrontendDataTypeController::RecordStartFailure(ConfigureResult result) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - UMA_HISTOGRAM_ENUMERATION("Sync.DataTypeStartFailures", - ModelTypeToHistogramInt(type()), - syncer::MODEL_TYPE_COUNT); -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_ENUMERATION("Sync." type_str "ConfigureFailure", result, \ - MAX_CONFIGURE_RESULT); - SYNC_DATA_TYPE_HISTOGRAM(type()); -#undef PER_DATA_TYPE_MACRO -} - -void NonFrontendDataTypeController::RecordUnrecoverableError( - const tracked_objects::Location& from_here, - const std::string& message) { - DVLOG(1) << "Datatype Controller failed for type " - << ModelTypeToString(type()) << " " - << message << " at location " - << from_here.ToString(); - UMA_HISTOGRAM_ENUMERATION("Sync.DataTypeRunFailures", - ModelTypeToHistogramInt(type()), - syncer::MODEL_TYPE_COUNT); - - if (!error_callback_.is_null()) - error_callback_.Run(); -} - - -sync_driver::SyncClient* NonFrontendDataTypeController::sync_client() const { - return sync_client_; -} - -void NonFrontendDataTypeController::set_start_callback( - const StartCallback& callback) { - start_callback_ = callback; -} - -void NonFrontendDataTypeController::set_state(State state) { - state_ = state; -} - -sync_driver::AssociatorInterface* NonFrontendDataTypeController::associator() - const { - return model_associator_; -} - -sync_driver::ChangeProcessor* -NonFrontendDataTypeController::GetChangeProcessor() const { - return change_processor_; -} - -void NonFrontendDataTypeController::AssociationCallback( - AssociationResult result) { - DCHECK(ui_thread()->RunsTasksOnCurrentThread()); - - if (result.needs_crypto) { - StartDone(NEEDS_CRYPTO, - result.local_merge_result, - result.syncer_merge_result); - return; - } - - if (result.unrecoverable_error) { - StartDone(UNRECOVERABLE_ERROR, - result.local_merge_result, - result.syncer_merge_result); - return; - } - - RecordAssociationTime(result.association_time); - if (result.error.IsSet()) { - StartDone(ASSOCIATION_FAILED, - result.local_merge_result, - result.syncer_merge_result); - return; - } - - CHECK(result.change_processor); - CHECK(result.model_associator); - change_processor_ = result.change_processor; - model_associator_ = result.model_associator; - - StartDone(!result.sync_has_nodes ? OK_FIRST_RUN : OK, - result.local_merge_result, - result.syncer_merge_result); -} - -} // namespace browser_sync diff --git a/components/sync_driver/non_frontend_data_type_controller.h b/components/sync_driver/non_frontend_data_type_controller.h deleted file mode 100644 index b75a5c9..0000000 --- a/components/sync_driver/non_frontend_data_type_controller.h +++ /dev/null @@ -1,186 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_SYNC_DRIVER_NON_FRONTEND_DATA_TYPE_CONTROLLER_H__ -#define COMPONENTS_SYNC_DRIVER_NON_FRONTEND_DATA_TYPE_CONTROLLER_H__ - -#include <string> - -#include "base/basictypes.h" -#include "base/callback_forward.h" -#include "base/compiler_specific.h" -#include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" -#include "base/synchronization/waitable_event.h" -#include "components/sync_driver/data_type_error_handler.h" -#include "components/sync_driver/directory_data_type_controller.h" -#include "components/sync_driver/sync_api_component_factory.h" - -namespace base { -class TimeDelta; -class SingleThreadTaskRunner; -} - -namespace syncer { -class SyncError; -} - -namespace sync_driver { -class AssociatorInterface; -class ChangeProcessor; -class SyncClient; -} - -namespace browser_sync { - -// Implementation for datatypes that do not reside on the frontend thread -// (UI thread). This is the same thread we perform initialization -// on, so we don't have to worry about thread safety. The main start/stop -// functionality is implemented by default. Derived classes must implement: -// type() -// model_safe_group() -// PostTaskOnBackendThread() -// CreateSyncComponents() -class NonFrontendDataTypeController - : public sync_driver::DirectoryDataTypeController { - public: - // For creating non-frontend processor/associator and associating on backend. - class BackendComponentsContainer; - - NonFrontendDataTypeController( - const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread, - const base::Closure& error_callback, - sync_driver::SyncClient* sync_client); - - // DataTypeController interface. - void LoadModels(const ModelLoadCallback& model_load_callback) override; - void StartAssociating(const StartCallback& start_callback) override; - void Stop() override; - syncer::ModelType type() const override = 0; - syncer::ModelSafeGroup model_safe_group() const override = 0; - std::string name() const override; - State state() const override; - - // DataTypeErrorHandler interface. - // Note: this is performed on the datatype's thread. - void OnSingleDataTypeUnrecoverableError( - const syncer::SyncError& error) override; - - // Callback to receive background association results. - struct AssociationResult { - explicit AssociationResult(syncer::ModelType type); - ~AssociationResult(); - bool needs_crypto; - bool unrecoverable_error; - bool sync_has_nodes; - syncer::SyncError error; - syncer::SyncMergeResult local_merge_result; - syncer::SyncMergeResult syncer_merge_result; - base::TimeDelta association_time; - sync_driver::ChangeProcessor* change_processor; - sync_driver::AssociatorInterface* model_associator; - }; - void AssociationCallback(AssociationResult result); - - protected: - // For testing only. - NonFrontendDataTypeController(); - - ~NonFrontendDataTypeController() override; - - // Start any dependent services that need to be running before we can - // associate models. The default implementation is a no-op. - // Return value: - // True - if models are ready and association can proceed. - // False - if models are not ready. StartAssociationAsync should be called - // when the models are ready. - // Note: this is performed on the frontend (UI) thread. - virtual bool StartModels(); - - // Posts the given task to the backend thread, i.e. the thread the - // datatype lives on. Return value: True if task posted successfully, - // false otherwise. - // NOTE: The StopAssociationAsync() implementation relies on the fact that - // implementations of this API do not hold any references to the DTC while - // the task is executing. See http://crbug.com/127706. - virtual bool PostTaskOnBackendThread( - const tracked_objects::Location& from_here, - const base::Closure& task) = 0; - - // Returns true if the current thread is the backend thread, i.e. the same - // thread used by |PostTaskOnBackendThread|. The default implementation just - // checks that the current thread is not the UI thread, but subclasses should - // override it appropriately. - virtual bool IsOnBackendThread(); - - // Datatype specific creation of sync components. - // Note: this is performed on the datatype's thread. - virtual sync_driver::SyncApiComponentFactory::SyncComponents - CreateSyncComponents() = 0; - - // Called on UI thread during shutdown to effectively disable processing - // any changes. - virtual void DisconnectProcessor(sync_driver::ChangeProcessor* processor) = 0; - - // Start up complete, update the state and invoke the callback. - // Note: this is performed on the datatype's thread. - virtual void StartDone( - DataTypeController::ConfigureResult start_result, - const syncer::SyncMergeResult& local_merge_result, - const syncer::SyncMergeResult& syncer_merge_result); - - // UI thread implementation of StartDone. - virtual void StartDoneImpl( - DataTypeController::ConfigureResult start_result, - DataTypeController::State new_state, - const syncer::SyncMergeResult& local_merge_result, - const syncer::SyncMergeResult& syncer_merge_result); - - // The actual implementation of Disabling the datatype. This happens - // on the UI thread. - virtual void DisableImpl(const syncer::SyncError& error); - - // Record association time. Called on Datatype's thread. - virtual void RecordAssociationTime(base::TimeDelta time); - // Record causes of start failure. Called on UI thread. - virtual void RecordStartFailure(ConfigureResult result); - - // Handles the reporting of unrecoverable error. It records stuff in - // UMA and reports to breakpad. - // Virtual for testing purpose. - virtual void RecordUnrecoverableError( - const tracked_objects::Location& from_here, - const std::string& message); - - // Accessors and mutators used by derived classes. - sync_driver::SyncClient* sync_client() const; - void set_start_callback(const StartCallback& callback); - void set_state(State state); - - virtual sync_driver::AssociatorInterface* associator() const; - sync_driver::ChangeProcessor* GetChangeProcessor() const override; - - State state_; - StartCallback start_callback_; - ModelLoadCallback model_load_callback_; - - private: - friend class BackendComponentsContainer; - sync_driver::SyncClient* const sync_client_; - - // Created on UI thread and passed to backend to create processor/associator - // and associate model. Released on backend. - scoped_ptr<BackendComponentsContainer> components_container_; - - sync_driver::AssociatorInterface* model_associator_; - sync_driver::ChangeProcessor* change_processor_; - - base::WeakPtrFactory<NonFrontendDataTypeController> weak_ptr_factory_; - - DISALLOW_COPY_AND_ASSIGN(NonFrontendDataTypeController); -}; - -} // namespace browser_sync - -#endif // COMPONENTS_SYNC_DRIVER_NON_FRONTEND_DATA_TYPE_CONTROLLER_H__ diff --git a/components/sync_driver/non_frontend_data_type_controller_mock.cc b/components/sync_driver/non_frontend_data_type_controller_mock.cc deleted file mode 100644 index c14b9ab..0000000 --- a/components/sync_driver/non_frontend_data_type_controller_mock.cc +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) 2011 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 "components/sync_driver/non_frontend_data_type_controller_mock.h" - -namespace browser_sync { - -NonFrontendDataTypeControllerMock::NonFrontendDataTypeControllerMock() {} - -NonFrontendDataTypeControllerMock::~NonFrontendDataTypeControllerMock() {} - -} // namespace browser_sync diff --git a/components/sync_driver/non_frontend_data_type_controller_mock.h b/components/sync_driver/non_frontend_data_type_controller_mock.h deleted file mode 100644 index 1867541..0000000 --- a/components/sync_driver/non_frontend_data_type_controller_mock.h +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_SYNC_DRIVER_NON_FRONTEND_DATA_TYPE_CONTROLLER_MOCK_H__ -#define COMPONENTS_SYNC_DRIVER_NON_FRONTEND_DATA_TYPE_CONTROLLER_MOCK_H__ - -#include "components/sync_driver/non_frontend_data_type_controller.h" -#include "sync/api/sync_error.h" -#include "testing/gmock/include/gmock/gmock.h" - -namespace browser_sync { - -class NonFrontendDataTypeControllerMock : public NonFrontendDataTypeController { - public: - NonFrontendDataTypeControllerMock(); - - // DataTypeController mocks. - MOCK_METHOD1(StartAssociating, - void(const StartCallback& start_callback)); - MOCK_METHOD1(LoadModels, void(const ModelLoadCallback& model_load_callback)); - - MOCK_METHOD0(Stop, void()); - MOCK_METHOD0(enabled, bool()); - MOCK_CONST_METHOD0(type, syncer::ModelType()); - MOCK_CONST_METHOD0(name, std::string()); - MOCK_CONST_METHOD0(model_safe_group, syncer::ModelSafeGroup()); - MOCK_CONST_METHOD0(state, State()); - MOCK_METHOD2(OnUnrecoverableError, void(const tracked_objects::Location&, - const std::string&)); - - // NonFrontendDataTypeController mocks. - MOCK_METHOD0(StartModels, bool()); - MOCK_METHOD2(PostTaskOnBackendThread, - bool(const tracked_objects::Location&, - const base::Closure&)); - MOCK_METHOD0(StartAssociation, void()); - MOCK_METHOD0(CreateSyncComponents, - sync_driver::SyncApiComponentFactory::SyncComponents()); - MOCK_METHOD3(StartDone, - void(DataTypeController::ConfigureResult result, - const syncer::SyncMergeResult& local_merge_result, - const syncer::SyncMergeResult& syncer_merge_result)); - MOCK_METHOD4(StartDoneImpl, - void(DataTypeController::ConfigureResult result, - DataTypeController::State new_state, - const syncer::SyncMergeResult& local_merge_result, - const syncer::SyncMergeResult& syncer_merge_result)); - MOCK_METHOD1(DisconnectProcessor, void(sync_driver::ChangeProcessor*)); - MOCK_METHOD2(OnUnrecoverableErrorImpl, void(const tracked_objects::Location&, - const std::string&)); - MOCK_METHOD2(RecordUnrecoverableError, void(const tracked_objects::Location&, - const std::string&)); - MOCK_METHOD1(RecordAssociationTime, void(base::TimeDelta time)); - MOCK_METHOD1(RecordStartFailure, void(ConfigureResult result)); - - protected: - virtual ~NonFrontendDataTypeControllerMock(); -}; - -} // namespace browser_sync - -#endif // COMPONENTS_SYNC_DRIVER_NON_FRONTEND_DATA_TYPE_CONTROLLER_MOCK_H__ diff --git a/components/sync_driver/non_frontend_data_type_controller_unittest.cc b/components/sync_driver/non_frontend_data_type_controller_unittest.cc deleted file mode 100644 index 3a3d726..0000000 --- a/components/sync_driver/non_frontend_data_type_controller_unittest.cc +++ /dev/null @@ -1,375 +0,0 @@ -// Copyright (c) 2012 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 "components/sync_driver/non_frontend_data_type_controller.h" - -#include "base/bind.h" -#include "base/bind_helpers.h" -#include "base/callback.h" -#include "base/compiler_specific.h" -#include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" -#include "base/synchronization/waitable_event.h" -#include "base/test/test_timeouts.h" -#include "base/thread_task_runner_handle.h" -#include "base/threading/thread.h" -#include "base/tracked_objects.h" -#include "components/sync_driver/change_processor_mock.h" -#include "components/sync_driver/data_type_controller_mock.h" -#include "components/sync_driver/fake_sync_client.h" -#include "components/sync_driver/fake_sync_service.h" -#include "components/sync_driver/model_associator_mock.h" -#include "components/sync_driver/non_frontend_data_type_controller_mock.h" -#include "components/sync_driver/sync_api_component_factory_mock.h" -#include "sync/internal_api/public/engine/model_safe_worker.h" -#include "testing/gtest/include/gtest/gtest.h" - -using base::WaitableEvent; -using syncer::GROUP_DB; -using browser_sync::NonFrontendDataTypeController; -using browser_sync::NonFrontendDataTypeControllerMock; -using sync_driver::ChangeProcessorMock; -using sync_driver::DataTypeController; -using sync_driver::ModelAssociatorMock; -using sync_driver::ModelLoadCallbackMock; -using sync_driver::StartCallbackMock; -using testing::_; -using testing::DoAll; -using testing::InvokeWithoutArgs; -using testing::Return; -using testing::SetArgumentPointee; -using testing::StrictMock; - -namespace { - -ACTION_P(WaitOnEvent, event) { - event->Wait(); -} - -ACTION_P(SignalEvent, event) { - event->Signal(); -} - -class NonFrontendDataTypeControllerFake : public NonFrontendDataTypeController { - public: - NonFrontendDataTypeControllerFake( - const scoped_refptr<base::TaskRunner> background_task_runner, - sync_driver::SyncClient* sync_client, - NonFrontendDataTypeControllerMock* mock) - : NonFrontendDataTypeController(base::ThreadTaskRunnerHandle::Get(), - base::Closure(), - sync_client), - background_task_runner_(background_task_runner), - mock_(mock), - sync_client_(sync_client) {} - - syncer::ModelType type() const override { return syncer::BOOKMARKS; } - syncer::ModelSafeGroup model_safe_group() const override { - return syncer::GROUP_DB; - } - - private: - ~NonFrontendDataTypeControllerFake() override {} - - sync_driver::SyncApiComponentFactory::SyncComponents CreateSyncComponents() - override { - return sync_client_->GetSyncApiComponentFactory() - ->CreateBookmarkSyncComponents(nullptr, this); - } - - bool PostTaskOnBackendThread(const tracked_objects::Location& from_here, - const base::Closure& task) override { - return background_task_runner_->PostTask(from_here, task); - } - - // We mock the following methods because their default implementations do - // nothing, but we still want to make sure they're called appropriately. - bool StartModels() override { return mock_->StartModels(); } - void RecordUnrecoverableError(const tracked_objects::Location& from_here, - const std::string& message) override { - mock_->RecordUnrecoverableError(from_here, message); - } - void RecordAssociationTime(base::TimeDelta time) override { - mock_->RecordAssociationTime(time); - } - void RecordStartFailure(DataTypeController::ConfigureResult result) override { - mock_->RecordStartFailure(result); - } - void DisconnectProcessor(sync_driver::ChangeProcessor* processor) override { - mock_->DisconnectProcessor(processor); - } - - private: - const scoped_refptr<base::TaskRunner> background_task_runner_; - NonFrontendDataTypeControllerMock* mock_; - sync_driver::SyncClient* sync_client_; -}; - -class SyncNonFrontendDataTypeControllerTest - : public testing::Test, - public sync_driver::FakeSyncClient { - public: - SyncNonFrontendDataTypeControllerTest() - : sync_driver::FakeSyncClient(&profile_sync_factory_), - db_thread_("DB_Thread"), - model_associator_(NULL), - change_processor_(NULL) {} - - // FakeSyncClient overrides. - sync_driver::SyncService* GetSyncService() override { return &service_; } - - void SetUp() override { - db_thread_.Start(); - - // All of these are refcounted, so don't need to be released. - dtc_mock_ = new StrictMock<NonFrontendDataTypeControllerMock>(); - non_frontend_dtc_ = new NonFrontendDataTypeControllerFake( - db_thread_.task_runner(), this, dtc_mock_.get()); - } - - void TearDown() override { - if (non_frontend_dtc_->state() != - NonFrontendDataTypeController::NOT_RUNNING) { - non_frontend_dtc_->Stop(); - } - db_thread_.Stop(); - } - - protected: - void SetStartExpectations() { - EXPECT_CALL(*dtc_mock_.get(), StartModels()).WillOnce(Return(true)); - EXPECT_CALL(model_load_callback_, Run(_, _)); - model_associator_ = new ModelAssociatorMock(); - change_processor_ = new ChangeProcessorMock(); - EXPECT_CALL(profile_sync_factory_, CreateBookmarkSyncComponents(_, _)) - .WillOnce(Return(sync_driver::SyncApiComponentFactory::SyncComponents( - model_associator_, change_processor_))); - } - - void SetAssociateExpectations() { - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillOnce(Return(true)); - EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)) - .WillOnce(DoAll(SetArgumentPointee<0>(true), Return(true))); - EXPECT_CALL(*model_associator_, AssociateModels(_, _)) - .WillOnce(Return(syncer::SyncError())); - EXPECT_CALL(*dtc_mock_.get(), RecordAssociationTime(_)); - } - - void SetActivateExpectations(DataTypeController::ConfigureResult result) { - EXPECT_CALL(start_callback_, Run(result, _, _)); - } - - void SetStopExpectations() { - EXPECT_CALL(*dtc_mock_.get(), DisconnectProcessor(_)); - EXPECT_CALL(*model_associator_, DisassociateModels()) - .WillOnce(Return(syncer::SyncError())); - } - - void SetStartFailExpectations(DataTypeController::ConfigureResult result) { - if (DataTypeController::IsUnrecoverableResult(result)) - EXPECT_CALL(*dtc_mock_.get(), RecordUnrecoverableError(_, _)); - if (model_associator_) { - EXPECT_CALL(*model_associator_, DisassociateModels()) - .WillOnce(Return(syncer::SyncError())); - } - EXPECT_CALL(*dtc_mock_.get(), RecordStartFailure(result)); - EXPECT_CALL(start_callback_, Run(result, _, _)); - } - - static void SignalDone(WaitableEvent* done) { done->Signal(); } - - void WaitForDTC() { - WaitableEvent done(true, false); - db_thread_.task_runner()->PostTask( - FROM_HERE, - base::Bind(&SyncNonFrontendDataTypeControllerTest::SignalDone, &done)); - done.TimedWait(TestTimeouts::action_timeout()); - if (!done.IsSignaled()) { - ADD_FAILURE() << "Timed out waiting for DB thread to finish."; - } - base::MessageLoop::current()->RunUntilIdle(); - } - - void Start() { - non_frontend_dtc_->LoadModels(base::Bind( - &ModelLoadCallbackMock::Run, base::Unretained(&model_load_callback_))); - non_frontend_dtc_->StartAssociating(base::Bind( - &StartCallbackMock::Run, base::Unretained(&start_callback_))); - } - - base::MessageLoop message_loop_; - base::Thread db_thread_; - scoped_refptr<NonFrontendDataTypeControllerFake> non_frontend_dtc_; - scoped_refptr<NonFrontendDataTypeControllerMock> dtc_mock_; - sync_driver::FakeSyncService service_; - SyncApiComponentFactoryMock profile_sync_factory_; - ModelAssociatorMock* model_associator_; - ChangeProcessorMock* change_processor_; - StartCallbackMock start_callback_; - ModelLoadCallbackMock model_load_callback_; -}; - -TEST_F(SyncNonFrontendDataTypeControllerTest, StartOk) { - SetStartExpectations(); - SetAssociateExpectations(); - SetActivateExpectations(DataTypeController::OK); - SetStopExpectations(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::RUNNING, non_frontend_dtc_->state()); -} - -TEST_F(SyncNonFrontendDataTypeControllerTest, StartFirstRun) { - SetStartExpectations(); - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillOnce(Return(true)); - EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)) - .WillOnce(DoAll(SetArgumentPointee<0>(false), Return(true))); - EXPECT_CALL(*model_associator_, AssociateModels(_, _)) - .WillOnce(Return(syncer::SyncError())); - EXPECT_CALL(*dtc_mock_.get(), RecordAssociationTime(_)); - SetActivateExpectations(DataTypeController::OK_FIRST_RUN); - SetStopExpectations(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::RUNNING, non_frontend_dtc_->state()); -} - -TEST_F(SyncNonFrontendDataTypeControllerTest, StartAssociationFailed) { - SetStartExpectations(); - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillOnce(Return(true)); - EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)) - .WillOnce(DoAll(SetArgumentPointee<0>(true), Return(true))); - EXPECT_CALL(*model_associator_, AssociateModels(_, _)) - .WillOnce( - Return(syncer::SyncError(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Error", syncer::BOOKMARKS))); - EXPECT_CALL(*dtc_mock_.get(), RecordAssociationTime(_)); - SetStartFailExpectations(DataTypeController::ASSOCIATION_FAILED); - // Set up association to fail with an association failed error. - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::DISABLED, non_frontend_dtc_->state()); -} - -TEST_F(SyncNonFrontendDataTypeControllerTest, - StartAssociationTriggersUnrecoverableError) { - SetStartExpectations(); - SetStartFailExpectations(DataTypeController::UNRECOVERABLE_ERROR); - // Set up association to fail with an unrecoverable error. - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillRepeatedly(Return(true)); - EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)) - .WillRepeatedly(DoAll(SetArgumentPointee<0>(false), Return(false))); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); -} - -TEST_F(SyncNonFrontendDataTypeControllerTest, StartAssociationCryptoNotReady) { - SetStartExpectations(); - SetStartFailExpectations(DataTypeController::NEEDS_CRYPTO); - // Set up association to fail with a NEEDS_CRYPTO error. - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillRepeatedly(Return(false)); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); -} - -// Trigger a Stop() call when we check if the model associator has user created -// nodes. -TEST_F(SyncNonFrontendDataTypeControllerTest, AbortDuringAssociationInactive) { - WaitableEvent wait_for_db_thread_pause(false, false); - WaitableEvent pause_db_thread(false, false); - - SetStartExpectations(); - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillOnce(Return(true)); - EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)) - .WillOnce(DoAll(SignalEvent(&wait_for_db_thread_pause), - WaitOnEvent(&pause_db_thread), - SetArgumentPointee<0>(true), Return(true))); - EXPECT_CALL(*model_associator_, AbortAssociation()) - .WillOnce(SignalEvent(&pause_db_thread)); - EXPECT_CALL(*model_associator_, AssociateModels(_, _)) - .WillOnce(Return(syncer::SyncError())); - SetStopExpectations(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - wait_for_db_thread_pause.Wait(); - non_frontend_dtc_->Stop(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); -} - -// Same as above but abort during the Activate call. -TEST_F(SyncNonFrontendDataTypeControllerTest, AbortDuringAssociationActivated) { - WaitableEvent wait_for_association_starts(false, false); - WaitableEvent wait_for_dtc_stop(false, false); - - SetStartExpectations(); - EXPECT_CALL(*model_associator_, CryptoReadyIfNecessary()) - .WillOnce(Return(true)); - EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)) - .WillOnce(DoAll(SetArgumentPointee<0>(true), Return(true))); - EXPECT_CALL(*model_associator_, AbortAssociation()); - EXPECT_CALL(*model_associator_, AssociateModels(_, _)) - .WillOnce(DoAll(SignalEvent(&wait_for_association_starts), - WaitOnEvent(&wait_for_dtc_stop), - Return(syncer::SyncError()))); - SetStopExpectations(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - wait_for_association_starts.Wait(); - non_frontend_dtc_->Stop(); - wait_for_dtc_stop.Signal(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); -} - -TEST_F(SyncNonFrontendDataTypeControllerTest, Stop) { - SetStartExpectations(); - SetAssociateExpectations(); - SetActivateExpectations(DataTypeController::OK); - SetStopExpectations(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::RUNNING, non_frontend_dtc_->state()); - non_frontend_dtc_->Stop(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); -} - -// Disabled due to http://crbug.com/388367 -TEST_F(SyncNonFrontendDataTypeControllerTest, - DISABLED_OnSingleDataTypeUnrecoverableError) { - SetStartExpectations(); - SetAssociateExpectations(); - SetActivateExpectations(DataTypeController::OK); - EXPECT_CALL(*dtc_mock_.get(), RecordUnrecoverableError(_, "Test")); - SetStopExpectations(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); - Start(); - WaitForDTC(); - EXPECT_EQ(DataTypeController::RUNNING, non_frontend_dtc_->state()); - // This should cause non_frontend_dtc_->Stop() to be called. - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, "error", - non_frontend_dtc_->type()); - db_thread_.task_runner()->PostTask( - FROM_HERE, base::Bind(&NonFrontendDataTypeControllerFake:: - OnSingleDataTypeUnrecoverableError, - non_frontend_dtc_.get(), error)); - WaitForDTC(); - EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); -} - -} // namespace diff --git a/components/sync_driver/shared_change_processor_unittest.cc b/components/sync_driver/shared_change_processor_unittest.cc index d277a3b..5979488 100644 --- a/components/sync_driver/shared_change_processor_unittest.cc +++ b/components/sync_driver/shared_change_processor_unittest.cc @@ -69,12 +69,6 @@ class TestSyncApiComponentFactory : public SyncApiComponentFactory { sync_driver::DataTypeErrorHandler* error_handler) override { return SyncApiComponentFactory::SyncComponents(nullptr, nullptr); } - SyncApiComponentFactory::SyncComponents CreateTypedUrlSyncComponents( - sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) override { - return SyncApiComponentFactory::SyncComponents(nullptr, nullptr); - } scoped_ptr<syncer::AttachmentService> CreateAttachmentService( scoped_ptr<syncer::AttachmentStoreForSync> attachment_store, const syncer::UserShare& user_share, diff --git a/components/sync_driver/sync_api_component_factory.h b/components/sync_driver/sync_api_component_factory.h index d64bba1..0631a3b 100644 --- a/components/sync_driver/sync_api_component_factory.h +++ b/components/sync_driver/sync_api_component_factory.h @@ -116,10 +116,6 @@ class SyncApiComponentFactory { virtual SyncComponents CreateBookmarkSyncComponents( sync_driver::SyncService* sync_service, sync_driver::DataTypeErrorHandler* error_handler) = 0; - virtual SyncComponents CreateTypedUrlSyncComponents( - sync_driver::SyncService* sync_service, - history::HistoryBackend* history_backend, - sync_driver::DataTypeErrorHandler* error_handler) = 0; // Creates attachment service. // Note: Should only be called from the model type thread. |