diff options
Diffstat (limited to 'components/history')
4 files changed, 254 insertions, 66 deletions
diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc index 6937454..542e952 100644 --- a/components/history/core/browser/history_backend.cc +++ b/components/history/core/browser/history_backend.cc @@ -2528,28 +2528,26 @@ void HistoryBackend::NotifyURLVisited(ui::PageTransition transition, const URLRow& row, const RedirectList& redirects, base::Time visit_time) { - URLRow url_info(row); if (typed_url_syncable_service_) - typed_url_syncable_service_->OnUrlVisited(transition, &url_info); + typed_url_syncable_service_->OnURLVisited(this, transition, row, redirects, + visit_time); - FOR_EACH_OBSERVER( - HistoryBackendObserver, observers_, - OnURLVisited(this, transition, url_info, redirects, visit_time)); + FOR_EACH_OBSERVER(HistoryBackendObserver, observers_, + OnURLVisited(this, transition, row, redirects, visit_time)); if (delegate_) - delegate_->NotifyURLVisited(transition, url_info, redirects, visit_time); + delegate_->NotifyURLVisited(transition, row, redirects, visit_time); } void HistoryBackend::NotifyURLsModified(const URLRows& rows) { - URLRows changed_urls(rows); if (typed_url_syncable_service_) - typed_url_syncable_service_->OnUrlsModified(&changed_urls); + typed_url_syncable_service_->OnURLsModified(this, rows); FOR_EACH_OBSERVER(HistoryBackendObserver, observers_, - OnURLsModified(this, changed_urls)); + OnURLsModified(this, rows)); if (delegate_) - delegate_->NotifyURLsModified(changed_urls); + delegate_->NotifyURLsModified(rows); } void HistoryBackend::NotifyURLsDeleted(bool all_history, @@ -2558,8 +2556,8 @@ void HistoryBackend::NotifyURLsDeleted(bool all_history, const std::set<GURL>& favicon_urls) { URLRows copied_rows(rows); if (typed_url_syncable_service_) { - typed_url_syncable_service_->OnUrlsDeleted(all_history, expired, - &copied_rows); + typed_url_syncable_service_->OnURLsDeleted(this, all_history, expired, + copied_rows, favicon_urls); } FOR_EACH_OBSERVER( diff --git a/components/history/core/browser/typed_url_syncable_service.cc b/components/history/core/browser/typed_url_syncable_service.cc index 932498c..7fb5410 100644 --- a/components/history/core/browser/typed_url_syncable_service.cc +++ b/components/history/core/browser/typed_url_syncable_service.cc @@ -263,9 +263,10 @@ syncer::SyncError TypedUrlSyncableService::ProcessSyncChanges( return syncer::SyncError(); } -void TypedUrlSyncableService::OnUrlsModified(URLRows* changed_urls) { +void TypedUrlSyncableService::OnURLsModified( + history::HistoryBackend* history_backend, + const history::URLRows& changed_urls) { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(changed_urls); if (processing_syncer_changes_) return; // These are changes originating from us, ignore. @@ -275,13 +276,12 @@ void TypedUrlSyncableService::OnUrlsModified(URLRows* changed_urls) { // Create SyncChangeList. syncer::SyncChangeList changes; - for (URLRows::iterator url = changed_urls->begin(); - url != changed_urls->end(); ++url) { + for (const auto& row : changed_urls) { // Only care if the modified URL is typed. - if (url->typed_count() > 0) { + if (row.typed_count() >= 0) { // If there were any errors updating the sync node, just ignore them and // continue on to process the next URL. - CreateOrUpdateSyncNode(*url, &changes); + CreateOrUpdateSyncNode(row, &changes); } } @@ -290,31 +290,37 @@ void TypedUrlSyncableService::OnUrlsModified(URLRows* changed_urls) { sync_processor_->ProcessSyncChanges(FROM_HERE, changes); } -void TypedUrlSyncableService::OnUrlVisited(ui::PageTransition transition, - URLRow* row) { +void TypedUrlSyncableService::OnURLVisited( + history::HistoryBackend* history_backend, + ui::PageTransition transition, + const history::URLRow& row, + const history::RedirectList& redirects, + base::Time visit_time) { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(row); if (processing_syncer_changes_) return; // These are changes originating from us, ignore. if (!sync_processor_.get()) return; // Sync processor not yet initialized, don't sync. - if (!ShouldSyncVisit(transition, row)) + if (!ShouldSyncVisit(row.typed_count(), transition)) return; // Create SyncChangeList. syncer::SyncChangeList changes; - CreateOrUpdateSyncNode(*row, &changes); + CreateOrUpdateSyncNode(row, &changes); // Send SyncChangeList to server if there are any changes. if (changes.size() > 0) sync_processor_->ProcessSyncChanges(FROM_HERE, changes); } -void TypedUrlSyncableService::OnUrlsDeleted(bool all_history, - bool expired, - URLRows* rows) { +void TypedUrlSyncableService::OnURLsDeleted( + history::HistoryBackend* history_backend, + bool all_history, + bool expired, + const history::URLRows& deleted_rows, + const std::set<GURL>& favicon_urls) { DCHECK(thread_checker_.CalledOnValidThread()); if (processing_syncer_changes_) @@ -335,27 +341,24 @@ void TypedUrlSyncableService::OnUrlsDeleted(bool all_history, if (all_history) { // Delete all synced typed urls. - for (std::set<GURL>::const_iterator url = synced_typed_urls_.begin(); - url != synced_typed_urls_.end(); ++url) { + for (const auto& url : synced_typed_urls_) { VisitVector visits; - URLRow row(*url); + URLRow row(url); AddTypedUrlToChangeList(syncer::SyncChange::ACTION_DELETE, row, visits, - url->spec(), &changes); + url.spec(), &changes); } // Clear cache of server state. synced_typed_urls_.clear(); } else { - DCHECK(rows); // Delete rows. - for (URLRows::const_iterator row = rows->begin(); row != rows->end(); - ++row) { + for (const auto& row : deleted_rows) { // Add specifics to change list for all synced urls that were deleted. - if (synced_typed_urls_.find(row->url()) != synced_typed_urls_.end()) { + if (synced_typed_urls_.find(row.url()) != synced_typed_urls_.end()) { VisitVector visits; - AddTypedUrlToChangeList(syncer::SyncChange::ACTION_DELETE, *row, visits, - row->url().spec(), &changes); + AddTypedUrlToChangeList(syncer::SyncChange::ACTION_DELETE, row, visits, + row.url().spec(), &changes); // Delete typed url from cache. - synced_typed_urls_.erase(row->url()); + synced_typed_urls_.erase(row.url()); } } } @@ -763,15 +766,8 @@ bool TypedUrlSyncableService::ShouldIgnoreVisits( return true; } -bool TypedUrlSyncableService::ShouldSyncVisit( - ui::PageTransition page_transition, - URLRow* row) { - if (!row) - return false; - int typed_count = row->typed_count(); - ui::PageTransition transition = ui::PageTransitionFromInt( - page_transition & ui::PAGE_TRANSITION_CORE_MASK); - +bool TypedUrlSyncableService::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 @@ -779,7 +775,8 @@ bool TypedUrlSyncableService::ShouldSyncVisit( // 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 (transition == ui::PAGE_TRANSITION_TYPED && typed_count > 0 && + return (ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_TYPED) && + typed_count >= 0 && (typed_count < kTypedUrlVisitThrottleThreshold || (typed_count % kTypedUrlVisitThrottleMultiple) == 0)); } @@ -787,7 +784,7 @@ bool TypedUrlSyncableService::ShouldSyncVisit( bool TypedUrlSyncableService::CreateOrUpdateSyncNode( URLRow url, syncer::SyncChangeList* changes) { - DCHECK_GT(url.typed_count(), 0); + DCHECK_GE(url.typed_count(), 0); if (ShouldIgnoreUrl(url.url())) return true; @@ -798,6 +795,15 @@ bool TypedUrlSyncableService::CreateOrUpdateSyncNode( DLOG(ERROR) << "Could not load visits for url: " << url.url(); return false; } + + if (std::find_if(visit_vector.begin(), visit_vector.end(), + [](const history::VisitRow& visit) { + return ui::PageTransitionCoreTypeIs( + visit.transition, ui::PAGE_TRANSITION_TYPED); + }) == visit_vector.end()) + // This URL has no TYPED visits, don't sync it + return false; + DCHECK(!visit_vector.empty()); std::string title = url.url().spec(); @@ -956,6 +962,13 @@ bool TypedUrlSyncableService::FixupURLAndGetVisits(URLRow* url, // 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; + } + VisitRow visit(url->id(), url->last_visit(), 0, ui::PAGE_TRANSITION_TYPED, 0); visits->push_back(visit); diff --git a/components/history/core/browser/typed_url_syncable_service.h b/components/history/core/browser/typed_url_syncable_service.h index 6c6e943..ed7d492 100644 --- a/components/history/core/browser/typed_url_syncable_service.h +++ b/components/history/core/browser/typed_url_syncable_service.h @@ -9,6 +9,7 @@ #include <vector> #include "base/threading/thread_checker.h" +#include "components/history/core/browser/history_backend_observer.h" #include "components/history/core/browser/history_types.h" #include "sync/api/sync_change.h" #include "sync/api/sync_data.h" @@ -33,7 +34,8 @@ class HistoryBackend; class TypedUrlSyncableServiceTest; class URLRow; -class TypedUrlSyncableService : public syncer::SyncableService { +class TypedUrlSyncableService : public syncer::SyncableService, + public history::HistoryBackendObserver { public: explicit TypedUrlSyncableService(HistoryBackend* history_backend); ~TypedUrlSyncableService() override; @@ -52,10 +54,19 @@ class TypedUrlSyncableService : public syncer::SyncableService { const tracked_objects::Location& from_here, const syncer::SyncChangeList& change_list) override; - // Called directly by HistoryBackend when local url data changes. - void OnUrlsModified(URLRows* changed_urls); - void OnUrlVisited(ui::PageTransition transition, URLRow* row); - void OnUrlsDeleted(bool all_history, bool expired, URLRows* rows); + // 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; private: friend class TypedUrlSyncableServiceTest; @@ -142,7 +153,7 @@ class TypedUrlSyncableService : public syncer::SyncableService { // 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(ui::PageTransition transition, URLRow* row); + bool ShouldSyncVisit(int typed_count, ui::PageTransition transition); // 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 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 47e3c98c..977506d 100644 --- a/components/history/core/browser/typed_url_syncable_service_unittest.cc +++ b/components/history/core/browser/typed_url_syncable_service_unittest.cc @@ -57,6 +57,14 @@ bool URLsEqual(URLRow& row, sync_pb::TypedUrlSpecifics& specifics) { (row.hidden() == specifics.hidden())); } +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()); +} + void AddNewestVisit(ui::PageTransition transition, int64 visit_time, URLRow* url, @@ -118,6 +126,19 @@ URLRow MakeTypedUrlRow(const std::string& url, 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; +} + class TestHistoryBackend; class TestHistoryBackendDelegate : public HistoryBackend::Delegate { @@ -253,6 +274,15 @@ class TypedUrlSyncableServiceTest : public testing::Test { static history::VisitRow CreateVisit(ui::PageTransition type, int64 timestamp); + static const TypedUrlSyncableService::MergeResult DIFF_NONE = + TypedUrlSyncableService::DIFF_NONE; + static const TypedUrlSyncableService::MergeResult DIFF_UPDATE_NODE = + TypedUrlSyncableService::DIFF_UPDATE_NODE; + static const TypedUrlSyncableService::MergeResult DIFF_LOCAL_ROW_CHANGED = + TypedUrlSyncableService::DIFF_LOCAL_ROW_CHANGED; + static const TypedUrlSyncableService::MergeResult DIFF_LOCAL_VISITS_ADDED = + TypedUrlSyncableService::DIFF_LOCAL_VISITS_ADDED; + protected: base::MessageLoop message_loop_; base::FilePath test_dir_; @@ -297,14 +327,15 @@ bool TypedUrlSyncableServiceTest::BuildAndPushLocalChanges( int typed = i < num_typed_urls ? 1 : 0; VisitVector visits; visit_vectors->push_back(visits); - rows->push_back(MakeTypedUrlRow( - urls[i], "pie", typed, i + 3, false, &visit_vectors->back())); + rows->push_back(MakeTypedUrlRow(urls[i], kTitle, typed, i + 3, false, + &visit_vectors->back())); fake_history_backend_->SetVisitsForUrl(rows->back(), visit_vectors->back()); changed_urls.push_back(rows->back()); } - typed_url_sync_service_->OnUrlsModified(&changed_urls); + typed_url_sync_service_->OnURLsModified(fake_history_backend_.get(), + changed_urls); } // Check that communication with sync was successful. @@ -445,7 +476,8 @@ TEST_F(TypedUrlSyncableServiceTest, UpdateLocalTypedUrl) { changed_urls.push_back(url_row); // Notify typed url sync service of the update. - typed_url_sync_service_->OnUrlsModified(&changed_urls); + typed_url_sync_service_->OnURLsModified(fake_history_backend_.get(), + changed_urls); ASSERT_EQ(1U, changes.size()); ASSERT_TRUE(changes[0].IsValid()); @@ -504,7 +536,9 @@ TEST_F(TypedUrlSyncableServiceTest, ReloadVisitLocalTypedUrl) { changed_urls.push_back(url_row); // Notify typed url sync service of the update. - typed_url_sync_service_->OnUrlVisited(ui::PAGE_TRANSITION_RELOAD, &url_row); + typed_url_sync_service_->OnURLVisited( + fake_history_backend_.get(), ui::PAGE_TRANSITION_RELOAD, url_row, + RedirectList(), base::Time::FromInternalValue(7)); ASSERT_EQ(0U, changes.size()); @@ -538,7 +572,9 @@ TEST_F(TypedUrlSyncableServiceTest, LinkVisitLocalTypedUrl) { ui::PageTransition transition = ui::PAGE_TRANSITION_LINK; // Notify typed url sync service of non-typed visit, expect no change. - typed_url_sync_service_->OnUrlVisited(transition, &url_row); + typed_url_sync_service_->OnURLVisited(fake_history_backend_.get(), transition, + url_row, RedirectList(), + base::Time::FromInternalValue(6)); ASSERT_EQ(0u, changes.size()); } @@ -566,7 +602,9 @@ TEST_F(TypedUrlSyncableServiceTest, TypedVisitLocalTypedUrl) { // Notify typed url sync service of typed visit. ui::PageTransition transition = ui::PAGE_TRANSITION_TYPED; - typed_url_sync_service_->OnUrlVisited(transition, &url_row); + typed_url_sync_service_->OnURLVisited(fake_history_backend_.get(), transition, + url_row, RedirectList(), + base::Time::Now()); ASSERT_EQ(1U, changes.size()); ASSERT_TRUE(changes[0].IsValid()); @@ -636,7 +674,8 @@ TEST_F(TypedUrlSyncableServiceTest, DeleteLocalTypedUrl) { } // Notify typed url sync service. - typed_url_sync_service_->OnUrlsDeleted(false, false, &rows); + typed_url_sync_service_->OnURLsDeleted(fake_history_backend_.get(), false, + false, rows, std::set<GURL>()); ASSERT_EQ(3u, changes.size()); for (size_t i = 0; i < changes.size(); ++i) { @@ -686,7 +725,9 @@ TEST_F(TypedUrlSyncableServiceTest, DeleteAllLocalTypedUrl) { bool all_history = true; // Notify typed url sync service. - typed_url_sync_service_->OnUrlsDeleted(all_history, false, NULL); + typed_url_sync_service_->OnURLsDeleted(fake_history_backend_.get(), + all_history, false, URLRows(), + std::set<GURL>()); ASSERT_EQ(4u, changes.size()); for (size_t i = 0; i < changes.size(); ++i) { @@ -729,7 +770,9 @@ TEST_F(TypedUrlSyncableServiceTest, MaxVisitLocalTypedUrl) { // Notify typed url sync service of typed visit. ui::PageTransition transition = ui::PAGE_TRANSITION_TYPED; - typed_url_sync_service_->OnUrlVisited(transition, &url_row); + typed_url_sync_service_->OnURLVisited(fake_history_backend_.get(), transition, + url_row, RedirectList(), + base::Time::Now()); syncer::SyncChangeList& changes = fake_change_processor_->changes(); ASSERT_EQ(1U, changes.size()); @@ -780,7 +823,9 @@ TEST_F(TypedUrlSyncableServiceTest, ThrottleVisitLocalTypedUrl) { // Notify typed url sync service of typed visit. ui::PageTransition transition = ui::PAGE_TRANSITION_TYPED; - typed_url_sync_service_->OnUrlVisited(transition, &url_row); + typed_url_sync_service_->OnURLVisited(fake_history_backend_.get(), transition, + url_row, RedirectList(), + base::Time::Now()); // Should throttle, so sync and local cache should not update. syncer::SyncChangeList& changes = fake_change_processor_->changes(); @@ -795,7 +840,9 @@ TEST_F(TypedUrlSyncableServiceTest, ThrottleVisitLocalTypedUrl) { fake_history_backend_->SetVisitsForUrl(url_row, visits); // Notify typed url sync service of typed visit. - typed_url_sync_service_->OnUrlVisited(transition, &url_row); + typed_url_sync_service_->OnURLVisited(fake_history_backend_.get(), transition, + url_row, RedirectList(), + base::Time::Now()); ASSERT_EQ(1u, changes.size()); ASSERT_TRUE(changes[0].IsValid()); @@ -1282,4 +1329,123 @@ TEST_F(TypedUrlSyncableServiceTest, NoTypedVisits) { ui::PageTransitionFromInt(typed_url.visit_transitions(0))); } +TEST_F(TypedUrlSyncableServiceTest, MergeUrls) { + history::VisitVector visits1; + history::URLRow row1(MakeTypedUrlRow(kURL, kTitle, 2, 3, false, &visits1)); + sync_pb::TypedUrlSpecifics specs1( + MakeTypedUrlSpecifics(kURL, kTitle, 3, false)); + history::URLRow new_row1((GURL(kURL))); + std::vector<history::VisitInfo> new_visits1; + EXPECT_TRUE(TypedUrlSyncableServiceTest::MergeUrls(specs1, row1, &visits1, + &new_row1, &new_visits1) == + TypedUrlSyncableServiceTest::DIFF_NONE); + + history::VisitVector visits2; + history::URLRow row2(MakeTypedUrlRow(kURL, kTitle, 2, 3, false, &visits2)); + sync_pb::TypedUrlSpecifics specs2( + MakeTypedUrlSpecifics(kURL, kTitle, 3, true)); + history::VisitVector expected_visits2; + history::URLRow expected2( + MakeTypedUrlRow(kURL, kTitle, 2, 3, true, &expected_visits2)); + history::URLRow new_row2((GURL(kURL))); + std::vector<history::VisitInfo> new_visits2; + EXPECT_TRUE(TypedUrlSyncableServiceTest::MergeUrls(specs2, row2, &visits2, + &new_row2, &new_visits2) == + TypedUrlSyncableServiceTest::DIFF_LOCAL_ROW_CHANGED); + EXPECT_TRUE(URLsEqual(new_row2, expected2)); + + history::VisitVector visits3; + history::URLRow row3(MakeTypedUrlRow(kURL, kTitle, 2, 3, false, &visits3)); + sync_pb::TypedUrlSpecifics specs3( + MakeTypedUrlSpecifics(kURL, kTitle2, 3, true)); + history::VisitVector expected_visits3; + history::URLRow expected3( + MakeTypedUrlRow(kURL, kTitle2, 2, 3, true, &expected_visits3)); + history::URLRow new_row3((GURL(kURL))); + std::vector<history::VisitInfo> new_visits3; + EXPECT_EQ(TypedUrlSyncableServiceTest::DIFF_LOCAL_ROW_CHANGED | + TypedUrlSyncableServiceTest::DIFF_NONE, + TypedUrlSyncableServiceTest::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(kURL, kTitle, 2, 3, false, &visits4)); + sync_pb::TypedUrlSpecifics specs4( + MakeTypedUrlSpecifics(kURL, kTitle2, 4, false)); + history::VisitVector expected_visits4; + history::URLRow expected4( + MakeTypedUrlRow(kURL, kTitle2, 2, 4, false, &expected_visits4)); + history::URLRow new_row4((GURL(kURL))); + std::vector<history::VisitInfo> new_visits4; + EXPECT_EQ(TypedUrlSyncableServiceTest::DIFF_UPDATE_NODE | + TypedUrlSyncableServiceTest::DIFF_LOCAL_ROW_CHANGED | + TypedUrlSyncableServiceTest::DIFF_LOCAL_VISITS_ADDED, + TypedUrlSyncableServiceTest::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(kURL, kTitle, 1, 4, false, &visits5)); + sync_pb::TypedUrlSpecifics specs5( + MakeTypedUrlSpecifics(kURL, kTitle, 3, false)); + history::VisitVector expected_visits5; + history::URLRow expected5( + MakeTypedUrlRow(kURL, kTitle, 2, 3, false, &expected_visits5)); + history::URLRow new_row5((GURL(kURL))); + std::vector<history::VisitInfo> new_visits5; + + // UPDATE_NODE should be set because row5 has a newer last_visit timestamp. + EXPECT_EQ(TypedUrlSyncableServiceTest::DIFF_UPDATE_NODE | + TypedUrlSyncableServiceTest::DIFF_NONE, + TypedUrlSyncableServiceTest::MergeUrls(specs5, row5, &visits5, + &new_row5, &new_visits5)); + EXPECT_TRUE(URLsEqual(new_row5, expected5)); + EXPECT_EQ(0U, new_visits5.size()); +} + +TEST_F(TypedUrlSyncableServiceTest, 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(kURL, kTitle, 2, 3, false, &history_visits)); + + // Now, create a sync node with visits at timestamps 1, 2, 3, 4. + sync_pb::TypedUrlSpecifics node( + MakeTypedUrlSpecifics(kURL, kTitle, 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( + TypedUrlSyncableServiceTest::DIFF_NONE | + TypedUrlSyncableServiceTest::DIFF_LOCAL_VISITS_ADDED, + TypedUrlSyncableServiceTest::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()); +} + } // namespace history |