diff options
50 files changed, 670 insertions, 145 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc index 6452f2f..e1dc676 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc @@ -233,7 +233,8 @@ class AutocompleteEditViewTest : public InProcessBrowserTest, Time t = Time::Now() - TimeDelta::FromHours(i + 1); history_service->AddPageWithDetails(url, UTF8ToUTF16(cur.title), cur.visit_count, - cur.typed_count, t, false); + cur.typed_count, t, false, + history::SOURCE_BROWSED); history_service->SetPageContents(url, UTF8ToUTF16(cur.body)); if (cur.starred) { bookmark_model->SetURLStarred(url, string16(), true); diff --git a/chrome/browser/autocomplete/history_contents_provider_unittest.cc b/chrome/browser/autocomplete/history_contents_provider_unittest.cc index 9212bce..21e91fa 100644 --- a/chrome/browser/autocomplete/history_contents_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_contents_provider_unittest.cc @@ -73,7 +73,8 @@ class HistoryContentsProviderTest : public testing::Test, Time t = Time::Now() - TimeDelta::FromDays(arraysize(test_entries) + i); history_service->AddPage(url, t, id_scope, i, GURL(), - PageTransition::LINK, history::RedirectList(), false); + PageTransition::LINK, history::RedirectList(), + history::SOURCE_BROWSED, false); history_service->SetPageTitle(url, UTF8ToUTF16(test_entries[i].title)); history_service->SetPageContents(url, UTF8ToUTF16(test_entries[i].body)); } diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc index d3ab9c3..393cd62 100644 --- a/chrome/browser/autocomplete/history_url_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc @@ -175,7 +175,8 @@ void HistoryURLProviderTest::FillData() { const GURL current_url(cur.url); history_service_->AddPageWithDetails(current_url, UTF8ToUTF16(cur.title), cur.visit_count, cur.typed_count, - visit_time, false); + visit_time, false, + history::SOURCE_BROWSED); } } @@ -315,7 +316,8 @@ TEST_F(HistoryURLProviderTest, CullRedirects) { history_service_->AddPageWithDetails(GURL(redirect[i].url), UTF8ToUTF16("Title"), redirect[i].count, redirect[i].count, - Time::Now(), false); + Time::Now(), false, + history::SOURCE_BROWSED); } // Create a B->C->A redirect chain, but set the visit counts such that they @@ -327,7 +329,8 @@ TEST_F(HistoryURLProviderTest, CullRedirects) { redirects_to_a.push_back(GURL(redirect[2].url)); redirects_to_a.push_back(GURL(redirect[0].url)); history_service_->AddPage(GURL(redirect[0].url), NULL, 0, GURL(), - PageTransition::TYPED, redirects_to_a, true); + PageTransition::TYPED, redirects_to_a, + history::SOURCE_BROWSED, true); // Because all the results are part of a redirect chain with other results, // all but the first one (A) should be culled. We should get the default diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 0062657b..da2c67e 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -109,7 +109,8 @@ void SearchProviderTest::SetUp() { term1_url_ = GURL(default_t_url_->url()->ReplaceSearchTerms( *default_t_url_, UTF16ToWide(term1_), 0, std::wstring())); history->AddPageWithDetails(term1_url_, string16(), 1, 1, - base::Time::Now(), false); + base::Time::Now(), false, + history::SOURCE_BROWSED); history->SetKeywordSearchTermsForURL(term1_url_, default_t_url_->id(), term1_); @@ -126,7 +127,8 @@ void SearchProviderTest::SetUp() { keyword_url_ = GURL(keyword_t_url_->url()->ReplaceSearchTerms( *keyword_t_url_, UTF16ToWide(keyword_term_), 0, std::wstring())); history->AddPageWithDetails(keyword_url_, string16(), 1, 1, - base::Time::Now(), false); + base::Time::Now(), false, + history::SOURCE_BROWSED); history->SetKeywordSearchTermsForURL(keyword_url_, keyword_t_url_->id(), keyword_term_); diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index 6ba2605..46558ab 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -1836,6 +1836,7 @@ void AutomationProvider::AddHistoryItem(Browser* browser, GURL(), PageTransition::LINK, history::RedirectList(), + history::SOURCE_BROWSED, false); if (title.length()) hs->SetPageTitle(gurl, title); diff --git a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc index 40f5110..90ad16b 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc @@ -193,7 +193,8 @@ TEST_F(BookmarkHTMLWriterTest, Test) { const BookmarkNode* f1 = model->AddGroup( model->GetBookmarkBarNode(), 0, f1_title); model->AddURLWithCreationTime(f1, 0, url1_title, url1, t1); - profile.GetHistoryService(Profile::EXPLICIT_ACCESS)->AddPage(url1); + profile.GetHistoryService(Profile::EXPLICIT_ACCESS)->AddPage(url1, + history::SOURCE_BROWSED); profile.GetFaviconService(Profile::EXPLICIT_ACCESS)->SetFavicon(url1, url1_favicon, icon_data); diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc index f588068..c21104c 100644 --- a/chrome/browser/bookmarks/bookmark_model_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc @@ -948,7 +948,7 @@ TEST_F(BookmarkModelTestWithProfile2, RemoveNotification) { profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)->AddPage( url, NULL, 1, GURL(), PageTransition::TYPED, - history::RedirectList(), false); + history::RedirectList(), history::SOURCE_BROWSED, false); // This won't actually delete the URL, rather it'll empty out the visits. // This triggers blocking on the BookmarkModel. diff --git a/chrome/browser/extensions/extension_history_api.cc b/chrome/browser/extensions/extension_history_api.cc index 95d3acb..3df0509 100644 --- a/chrome/browser/extensions/extension_history_api.cc +++ b/chrome/browser/extensions/extension_history_api.cc @@ -304,7 +304,7 @@ bool AddUrlHistoryFunction::RunImpl() { return false; HistoryService* hs = profile()->GetHistoryService(Profile::EXPLICIT_ACCESS); - hs->AddPage(url); + hs->AddPage(url, history::SOURCE_EXTENSION); SendResponse(true); return true; diff --git a/chrome/browser/history/archived_database.cc b/chrome/browser/history/archived_database.cc index 777ee4d..72fa998 100644 --- a/chrome/browser/history/archived_database.cc +++ b/chrome/browser/history/archived_database.cc @@ -13,7 +13,7 @@ namespace history { namespace { -static const int kCurrentVersionNumber = 2; +static const int kCurrentVersionNumber = 3; static const int kCompatibleVersionNumber = 2; } // namespace @@ -110,6 +110,12 @@ sql::InitStatus ArchivedDatabase::EnsureCurrentVersion() { std::min(cur_version, kCompatibleVersionNumber)); } + if (cur_version == 2) { + // This is the version prior to adding visit_source table. + ++cur_version; + meta_table_.SetVersionNumber(cur_version); + } + // Put future migration cases here. // When the version is too old, we just try to continue anyway, there should diff --git a/chrome/browser/history/expire_history_backend.cc b/chrome/browser/history/expire_history_backend.cc index bd471ad..294c0e6 100644 --- a/chrome/browser/history/expire_history_backend.cc +++ b/chrome/browser/history/expire_history_backend.cc @@ -512,7 +512,7 @@ void ExpireHistoryBackend::ExpireURLsForVisits( void ExpireHistoryBackend::ArchiveURLsAndVisits( const VisitVector& visits, DeleteDependencies* dependencies) { - if (!archived_db_) + if (!archived_db_ || !main_db_) return; // Make sure all unique URL rows are added to the dependency list and the @@ -539,6 +539,12 @@ void ExpireHistoryBackend::ArchiveURLsAndVisits( } } + // Retrieve the sources for all the archived visits before archiving. + // The returned visit_sources vector should contain the source for each visit + // from visits at the same index. + VisitSourceMap visit_sources; + main_db_->GetVisitsSource(visits, &visit_sources); + // Now archive the visits since we know the URL ID to make them reference. // The source visit list should still reference the visits in the main DB, but // we will update it to reflect only the visits that were successfully @@ -550,7 +556,9 @@ void ExpireHistoryBackend::ArchiveURLsAndVisits( VisitRow cur_visit(visits[i]); cur_visit.url_id = main_id_to_archived_id[cur_visit.url_id]; cur_visit.referring_visit = 0; - archived_db_->AddVisit(&cur_visit); + VisitSourceMap::iterator iter = visit_sources.find(visits[i].visit_id); + archived_db_->AddVisit(&cur_visit, + iter == visit_sources.end() ? SOURCE_BROWSED : iter->second); // Ignore failures, we will delete it from the main DB no matter what. } } diff --git a/chrome/browser/history/expire_history_backend.h b/chrome/browser/history/expire_history_backend.h index 4bd18b0..96e2e9a 100644 --- a/chrome/browser/history/expire_history_backend.h +++ b/chrome/browser/history/expire_history_backend.h @@ -106,6 +106,7 @@ class ExpireHistoryBackend { FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, DeleteFaviconsIfPossible); FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, ArchiveSomeOldHistory); FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, ExpiringVisitsReader); + FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, ArchiveSomeOldHistoryWithSource); friend class ::TestingProfile; struct DeleteDependencies; diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc index ac3e834..694c2e0 100644 --- a/chrome/browser/history/expire_history_backend_unittest.cc +++ b/chrome/browser/history/expire_history_backend_unittest.cc @@ -56,6 +56,8 @@ class ExpireHistoryTest : public testing::Test, protected: // Called by individual tests when they want data populated. void AddExampleData(URLID url_ids[3], Time visit_times[4]); + // Add visits with source information. + void AddExampleSourceData(const GURL& url, URLID* id); // Returns true if the given favicon/thumanil has an entry in the DB. bool HasFavIcon(FavIconID favicon_id); @@ -227,26 +229,26 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3], Time visit_times[4]) { visit_row1.url_id = url_ids[0]; visit_row1.visit_time = visit_times[0]; visit_row1.is_indexed = true; - main_db_->AddVisit(&visit_row1); + main_db_->AddVisit(&visit_row1, SOURCE_BROWSED); VisitRow visit_row2; visit_row2.url_id = url_ids[1]; visit_row2.visit_time = visit_times[1]; visit_row2.is_indexed = true; - main_db_->AddVisit(&visit_row2); + main_db_->AddVisit(&visit_row2, SOURCE_BROWSED); VisitRow visit_row3; visit_row3.url_id = url_ids[1]; visit_row3.visit_time = visit_times[2]; visit_row3.is_indexed = true; visit_row3.transition = PageTransition::TYPED; - main_db_->AddVisit(&visit_row3); + main_db_->AddVisit(&visit_row3, SOURCE_BROWSED); VisitRow visit_row4; visit_row4.url_id = url_ids[2]; visit_row4.visit_time = visit_times[3]; visit_row4.is_indexed = true; - main_db_->AddVisit(&visit_row4); + main_db_->AddVisit(&visit_row4, SOURCE_BROWSED); // Full text index for each visit. text_db_->AddPageData(url_row1.url(), visit_row1.url_id, visit_row1.visit_id, @@ -267,6 +269,35 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3], Time visit_times[4]) { UTF8ToUTF16("goats body")); } +void ExpireHistoryTest::AddExampleSourceData(const GURL& url, URLID* id) { + if (!main_db_.get()) + return; + + Time last_visit_time = Time::Now(); + // Add one URL. + URLRow url_row1(url); + url_row1.set_last_visit(last_visit_time); + url_row1.set_visit_count(4); + URLID url_id = main_db_->AddURL(url_row1); + *id = url_id; + + // Four times for each visit. + VisitRow visit_row1(url_id, last_visit_time - TimeDelta::FromDays(4), 0, + PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row1, SOURCE_SYNCED); + + VisitRow visit_row2(url_id, last_visit_time - TimeDelta::FromDays(3), 0, + PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row2, SOURCE_BROWSED); + + VisitRow visit_row3(url_id, last_visit_time - TimeDelta::FromDays(2), 0, + PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row3, SOURCE_EXTENSION); + + VisitRow visit_row4(url_id, last_visit_time, 0, PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row4, SOURCE_FIREFOX_IMPORTED); +} + bool ExpireHistoryTest::HasFavIcon(FavIconID favicon_id) { if (!thumb_db_.get()) return false; @@ -815,6 +846,50 @@ TEST_F(ExpireHistoryTest, ExpiringVisitsReader) { EXPECT_EQ(1U, visits.size()); } +// Tests how ArchiveSomeOldHistory treats source information. +TEST_F(ExpireHistoryTest, ArchiveSomeOldHistoryWithSource) { + const GURL url("www.testsource.com"); + URLID url_id; + AddExampleSourceData(url, &url_id); + const ExpiringVisitsReader* reader = expirer_.GetAllVisitsReader(); + + // Archiving all the visits we added. + ASSERT_FALSE(expirer_.ArchiveSomeOldHistory(Time::Now(), reader, 10)); + + URLRow archived_row; + ASSERT_TRUE(archived_db_->GetRowForURL(url, &archived_row)); + VisitVector archived_visits; + archived_db_->GetVisitsForURL(archived_row.id(), &archived_visits); + ASSERT_EQ(4U, archived_visits.size()); + VisitSourceMap sources; + archived_db_->GetVisitsSource(archived_visits, &sources); + ASSERT_EQ(3U, sources.size()); + int result = 0; + VisitSourceMap::iterator iter; + for (int i = 0; i < 4; i++) { + iter = sources.find(archived_visits[i].visit_id); + if (iter == sources.end()) + continue; + switch (iter->second) { + case history::SOURCE_EXTENSION: + result |= 0x1; + break; + case history::SOURCE_FIREFOX_IMPORTED: + result |= 0x2; + break; + case history::SOURCE_SYNCED: + result |= 0x4; + default: + break; + } + } + EXPECT_EQ(0x7, result); + main_db_->GetVisitsSource(archived_visits, &sources); + EXPECT_EQ(0U, sources.size()); + main_db_->GetVisitsForURL(url_id, &archived_visits); + EXPECT_EQ(0U, archived_visits.size()); +} + // TODO(brettw) add some visits with no URL to make sure everything is updated // properly. Have the visits also refer to nonexistant FTS rows. // diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc index 156f9da..eb27b97 100644 --- a/chrome/browser/history/history.cc +++ b/chrome/browser/history/history.cc @@ -301,9 +301,10 @@ void HistoryService::AddPage(const GURL& url, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry) { AddPage(url, Time::Now(), id_scope, page_id, referrer, transition, redirects, - did_replace_entry); + visit_source, did_replace_entry); } void HistoryService::AddPage(const GURL& url, @@ -313,6 +314,7 @@ void HistoryService::AddPage(const GURL& url, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry) { DCHECK(thread_) << "History service being called after cleanup"; @@ -340,8 +342,8 @@ void HistoryService::AddPage(const GURL& url, } scoped_refptr<history::HistoryAddPageArgs> request( - new history::HistoryAddPageArgs(url, time, id_scope, page_id, - referrer, redirects, transition, + new history::HistoryAddPageArgs(url, time, id_scope, page_id, referrer, + redirects, transition, visit_source, did_replace_entry)); ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::AddPage, request); } @@ -356,7 +358,8 @@ void HistoryService::AddPageWithDetails(const GURL& url, int visit_count, int typed_count, Time last_visit, - bool hidden) { + bool hidden, + history::VisitSource visit_source) { // Filter out unwanted URLs. if (!CanAddURL(url)) return; @@ -377,11 +380,12 @@ void HistoryService::AddPageWithDetails(const GURL& url, rows.push_back(row); ScheduleAndForget(PRIORITY_NORMAL, - &HistoryBackend::AddPagesWithDetails, rows); + &HistoryBackend::AddPagesWithDetails, rows, visit_source); } void HistoryService::AddPagesWithDetails( - const std::vector<history::URLRow>& info) { + const std::vector<history::URLRow>& info, + history::VisitSource visit_source) { // Add to the visited links system. VisitedLinkMaster* visited_links; @@ -397,7 +401,7 @@ void HistoryService::AddPagesWithDetails( } ScheduleAndForget(PRIORITY_NORMAL, - &HistoryBackend::AddPagesWithDetails, info); + &HistoryBackend::AddPagesWithDetails, info, visit_source); } void HistoryService::SetPageContents(const GURL& url, diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h index c142c97..5bc3b7f 100644 --- a/chrome/browser/history/history.h +++ b/chrome/browser/history/history.h @@ -185,6 +185,7 @@ class HistoryService : public CancelableRequestProvider, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry); // For adding pages to history with a specific time. This is for testing @@ -196,12 +197,13 @@ class HistoryService : public CancelableRequestProvider, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry); // For adding pages to history where no tracking information can be done. - void AddPage(const GURL& url) { - AddPage(url, NULL, 0, GURL(), PageTransition::LINK, history::RedirectList(), - false); + void AddPage(const GURL& url, history::VisitSource visit_source) { + AddPage(url, NULL, 0, GURL(), PageTransition::LINK, + history::RedirectList(), visit_source, false); } // Sets the title for the given page. The page should be in history. If it @@ -535,10 +537,12 @@ class HistoryService : public CancelableRequestProvider, int visit_count, int typed_count, base::Time last_visit, - bool hidden); + bool hidden, + history::VisitSource visit_source); // The same as AddPageWithDetails() but takes a vector. - void AddPagesWithDetails(const std::vector<history::URLRow>& info); + void AddPagesWithDetails(const std::vector<history::URLRow>& info, + history::VisitSource visit_source); // Starts the TopSites migration in the HistoryThread. Called by the // BackendDelegate. diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc index 91aba75..847dc6e 100644 --- a/chrome/browser/history/history_backend.cc +++ b/chrome/browser/history/history_backend.cc @@ -405,7 +405,7 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) { // No redirect case (one element means just the page itself). last_ids = AddPageVisit(request->url, last_recorded_time_, - last_ids.second, t); + last_ids.second, t, request->visit_source); // Update the segment for this visit. KEYWORD_GENERATED visits should not // result in changing most visited, so we don't update segments (most @@ -471,7 +471,8 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) { // them anyway, and if we ever decide to, we can reconstruct their order // from the redirect chain. last_ids = AddPageVisit(request->redirects[redirect_index], - last_recorded_time_, last_ids.second, t); + last_recorded_time_, last_ids.second, + t, request->visit_source); if (t & PageTransition::CHAIN_START) { // Update the segment for this visit. UpdateSegments(request->redirects[redirect_index], @@ -651,7 +652,8 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( const GURL& url, Time time, VisitID referring_visit, - PageTransition::Type transition) { + PageTransition::Type transition, + VisitSource visit_source) { // Top-level frame navigations are visible, everything else is hidden bool new_hidden = !PageTransition::IsMainFrame(transition); @@ -708,7 +710,7 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( // Add the visit with the time to the database. VisitRow visit_info(url_id, time, referring_visit, transition, 0); - VisitID visit_id = db_->AddVisit(&visit_info); + VisitID visit_id = db_->AddVisit(&visit_info, visit_source); if (visit_info.visit_time < first_recorded_time_) first_recorded_time_ = visit_info.visit_time; @@ -727,7 +729,8 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( return std::make_pair(url_id, visit_id); } -void HistoryBackend::AddPagesWithDetails(const std::vector<URLRow>& urls) { +void HistoryBackend::AddPagesWithDetails(const std::vector<URLRow>& urls, + VisitSource visit_source) { if (!db_.get()) return; @@ -789,7 +792,7 @@ void HistoryBackend::AddPagesWithDetails(const std::vector<URLRow>& urls) { PageTransition::LINK | PageTransition::CHAIN_START | PageTransition::CHAIN_END, 0); visit_info.is_indexed = has_indexed; - if (!visit_database->AddVisit(&visit_info)) { + if (!visit_database->AddVisit(&visit_info, visit_source)) { NOTREACHED() << "Adding visit failed."; return; } @@ -906,11 +909,12 @@ bool HistoryBackend::UpdateURL(URLID id, const history::URLRow& url) { } bool HistoryBackend::AddVisits(const GURL& url, - const std::vector<base::Time>& visits) { + const std::vector<base::Time>& visits, + VisitSource visit_source) { if (db_.get()) { for (std::vector<base::Time>::const_iterator visit = visits.begin(); visit != visits.end(); ++visit) { - if (!AddPageVisit(url, *visit, 0, 0).first) { + if (!AddPageVisit(url, *visit, 0, 0, visit_source).first) { return false; } } diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h index 4d333f3..1965bc5 100644 --- a/chrome/browser/history/history_backend.h +++ b/chrome/browser/history/history_backend.h @@ -258,8 +258,10 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, virtual bool UpdateURL(URLID id, const history::URLRow& url); + // While adding visits in batch, the source needs to be provided. virtual bool AddVisits(const GURL& url, - const std::vector<base::Time>& visits); + const std::vector<base::Time>& visits, + VisitSource visit_source); virtual bool RemoveVisits(const VisitVector& visits); @@ -292,7 +294,9 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // Adds the given rows to the database if it doesn't exist. A visit will be // added for each given URL at the last visit time in the URLRow. - void AddPagesWithDetails(const std::vector<URLRow>& info); + // Each visit will have the visit_source type set. + void AddPagesWithDetails(const std::vector<URLRow>& info, + VisitSource visit_source); #if defined(UNIT_TEST) HistoryDatabase* db() const { return db_.get(); } @@ -312,6 +316,11 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, URLsNoLongerBookmarked); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, StripUsernamePasswordTest); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteThumbnailsDatabaseTest); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageArgsSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddVisitsSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, RemoveVisitsSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, MigrationVisitSource); friend class ::TestingProfile; // Computes the name of the specified database on disk. @@ -339,7 +348,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, std::pair<URLID, VisitID> AddPageVisit(const GURL& url, base::Time time, VisitID referring_visit, - PageTransition::Type transition); + PageTransition::Type transition, + VisitSource visit_source); // Returns a redirect chain in |redirects| for the VisitID // |cur_visit|. |cur_visit| is assumed to be valid. Assumes that diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc index fe36c9d..568fd4f 100644 --- a/chrome/browser/history/history_backend_unittest.cc +++ b/chrome/browser/history/history_backend_unittest.cc @@ -15,6 +15,8 @@ #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/in_memory_history_backend.h" #include "chrome/browser/history/in_memory_database.h" +#include "chrome/common/chrome_constants.h" +#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/notification_service.h" #include "chrome/common/thumbnail_score.h" @@ -75,7 +77,7 @@ class HistoryBackendTest : public testing::Test { scoped_refptr<history::HistoryAddPageArgs> request( new history::HistoryAddPageArgs( redirects.back(), Time::Now(), scope, page_id, GURL(), - redirects, PageTransition::LINK, true)); + redirects, PageTransition::LINK, history::SOURCE_BROWSED, true)); backend_->AddPage(request); } @@ -95,7 +97,8 @@ class HistoryBackendTest : public testing::Test { redirects.push_back(url2); scoped_refptr<HistoryAddPageArgs> request( new HistoryAddPageArgs(url2, base::Time(), dummy_scope, 0, url1, - redirects, PageTransition::CLIENT_REDIRECT, did_replace)); + redirects, PageTransition::CLIENT_REDIRECT, + history::SOURCE_BROWSED, did_replace)); backend_->AddPage(request); *transition1 = getTransition(url1); @@ -112,6 +115,10 @@ class HistoryBackendTest : public testing::Test { return visits[0].transition; } + FilePath getTestDir() { + return test_dir_; + } + BookmarkModel bookmark_model_; protected: @@ -215,7 +222,7 @@ TEST_F(HistoryBackendTest, DeleteAll) { std::vector<URLRow> rows; rows.push_back(row2); // Reversed order for the same reason as favicons. rows.push_back(row1); - backend_->AddPagesWithDetails(rows); + backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL); URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL); @@ -345,7 +352,7 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { std::vector<URLRow> rows; rows.push_back(row2); // Reversed order for the same reason as favicons. rows.push_back(row1); - backend_->AddPagesWithDetails(rows); + backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL); URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL); @@ -454,7 +461,8 @@ TEST_F(HistoryBackendTest, KeywordGenerated) { scoped_refptr<HistoryAddPageArgs> request( new HistoryAddPageArgs(url, visit_time, NULL, 0, GURL(), history::RedirectList(), - PageTransition::KEYWORD_GENERATED, false)); + PageTransition::KEYWORD_GENERATED, + history::SOURCE_BROWSED, false)); backend_->AddPage(request); // A row should have been added for the url. @@ -537,7 +545,7 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) { std::vector<URLRow> rows; rows.push_back(row1); rows.push_back(row2); - backend_->AddPagesWithDetails(rows); + backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); URLRow url_row1, url_row2; EXPECT_FALSE(backend_->db_->GetRowForURL(row1.url(), &url_row1) == 0); EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), &url_row2) == 0); @@ -592,7 +600,8 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) { // Visit the url with username, password. backend_->AddPageVisit(url, base::Time::Now(), 0, - PageTransition::GetQualifier(PageTransition::TYPED)); + PageTransition::GetQualifier(PageTransition::TYPED), + history::SOURCE_BROWSED); // Fetch the row information about stripped url from history db. VisitVector visits; @@ -612,4 +621,214 @@ TEST_F(HistoryBackendTest, DeleteThumbnailsDatabaseTest) { EXPECT_FALSE(backend_->thumbnail_db_->NeedsMigrationToTopSites()); } +TEST_F(HistoryBackendTest, AddPageVisitSource) { + ASSERT_TRUE(backend_.get()); + + GURL url("http://www.google.com"); + + // Clear all history. + backend_->DeleteAllHistory(); + + // Assume visiting the url from an externsion. + backend_->AddPageVisit(url, base::Time::Now(), 0, PageTransition::TYPED, + history::SOURCE_EXTENSION); + // Assume the url is imported from Firefox. + backend_->AddPageVisit(url, base::Time::Now(), 0, PageTransition::TYPED, + history::SOURCE_FIREFOX_IMPORTED); + // Assume this url is also synced. + backend_->AddPageVisit(url, base::Time::Now(), 0, PageTransition::TYPED, + history::SOURCE_SYNCED); + + // Fetch the row information about the url from history db. + VisitVector visits; + URLID row_id = backend_->db_->GetRowForURL(url, NULL); + backend_->db_->GetVisitsForURL(row_id, &visits); + + // Check if all the visits to the url are stored in database. + ASSERT_EQ(3U, visits.size()); + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(3U, visit_sources.size()); + int sources = 0; + for (int i = 0; i < 3; i++) { + switch (visit_sources[visits[i].visit_id]) { + case history::SOURCE_EXTENSION: + sources |= 0x1; + break; + case history::SOURCE_FIREFOX_IMPORTED: + sources |= 0x2; + break; + case history::SOURCE_SYNCED: + sources |= 0x4; + default: + break; + } + } + EXPECT_EQ(0x7, sources); +} + +TEST_F(HistoryBackendTest, AddPageArgsSource) { + ASSERT_TRUE(backend_.get()); + + GURL url("http://testpageargs.com"); + + // Assume this page is browsed by user. + scoped_refptr<HistoryAddPageArgs> request1( + new HistoryAddPageArgs(url, base::Time::Now(), NULL, 0, GURL(), + history::RedirectList(), + PageTransition::KEYWORD_GENERATED, + history::SOURCE_BROWSED, false)); + backend_->AddPage(request1); + // Assume this page is synced. + scoped_refptr<HistoryAddPageArgs> request2( + new HistoryAddPageArgs(url, base::Time::Now(), NULL, 0, GURL(), + history::RedirectList(), + PageTransition::LINK, + history::SOURCE_SYNCED, false)); + backend_->AddPage(request2); + // Assume this page is browsed again. + scoped_refptr<HistoryAddPageArgs> request3( + new HistoryAddPageArgs(url, base::Time::Now(), NULL, 0, GURL(), + history::RedirectList(), + PageTransition::TYPED, + history::SOURCE_BROWSED, false)); + backend_->AddPage(request3); + + // Three visits should be added with proper sources. + VisitVector visits; + URLRow row; + URLID id = backend_->db()->GetRowForURL(url, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(3U, visits.size()); + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(1U, visit_sources.size()); + EXPECT_EQ(history::SOURCE_SYNCED, visit_sources.begin()->second); +} + +TEST_F(HistoryBackendTest, AddVisitsSource) { + ASSERT_TRUE(backend_.get()); + + GURL url1("http://www.cnn.com"); + std::vector<base::Time> visits1; + visits1.push_back(Time::Now() - base::TimeDelta::FromDays(5)); + visits1.push_back(Time::Now() - base::TimeDelta::FromDays(1)); + visits1.push_back(Time::Now()); + + GURL url2("http://www.example.com"); + std::vector<base::Time> visits2; + visits2.push_back(Time::Now() - base::TimeDelta::FromDays(10)); + visits2.push_back(Time::Now()); + + // Clear all history. + backend_->DeleteAllHistory(); + + // Add the visits. + backend_->AddVisits(url1, visits1, history::SOURCE_IE_IMPORTED); + backend_->AddVisits(url2, visits2, history::SOURCE_SYNCED); + + // Verify the visits were added with their sources. + VisitVector visits; + URLRow row; + URLID id = backend_->db()->GetRowForURL(url1, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(3U, visits.size()); + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(3U, visit_sources.size()); + for (int i = 0; i < 3; i++) + EXPECT_EQ(history::SOURCE_IE_IMPORTED, visit_sources[visits[i].visit_id]); + id = backend_->db()->GetRowForURL(url2, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(2U, visits.size()); + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(2U, visit_sources.size()); + for (int i = 0; i < 2; i++) + EXPECT_EQ(history::SOURCE_SYNCED, visit_sources[visits[i].visit_id]); +} + +TEST_F(HistoryBackendTest, RemoveVisitsSource) { + ASSERT_TRUE(backend_.get()); + + GURL url1("http://www.cnn.com"); + std::vector<base::Time> visits1; + visits1.push_back(Time::Now() - base::TimeDelta::FromDays(5)); + visits1.push_back(Time::Now()); + + GURL url2("http://www.example.com"); + std::vector<base::Time> visits2; + visits2.push_back(Time::Now() - base::TimeDelta::FromDays(10)); + visits2.push_back(Time::Now()); + + // Clear all history. + backend_->DeleteAllHistory(); + + // Add the visits. + backend_->AddVisits(url1, visits1, history::SOURCE_IE_IMPORTED); + backend_->AddVisits(url2, visits2, history::SOURCE_SYNCED); + + // Verify the visits of url1 were added. + VisitVector visits; + URLRow row; + URLID id = backend_->db()->GetRowForURL(url1, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(2U, visits.size()); + // Remove these visits. + ASSERT_TRUE(backend_->RemoveVisits(visits)); + + // Now check only url2's source in visit_source table. + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(0U, visit_sources.size()); + id = backend_->db()->GetRowForURL(url2, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(2U, visits.size()); + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(2U, visit_sources.size()); + for (int i = 0; i < 2; i++) + EXPECT_EQ(history::SOURCE_SYNCED, visit_sources[visits[i].visit_id]); +} + +// Test for migration of adding visit_source table. +TEST_F(HistoryBackendTest, MigrationVisitSource) { + ASSERT_TRUE(backend_.get()); + + FilePath old_history_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &old_history_path)); + old_history_path = old_history_path.AppendASCII("History"); + old_history_path = old_history_path.AppendASCII("HistoryNoSource"); + + // Copy history database file to current directory so that it will be deleted + // in Teardown. + FilePath new_history_path(getTestDir()); + file_util::Delete(new_history_path, true); + file_util::CreateDirectory(new_history_path); + FilePath new_history_file = new_history_path.Append(chrome::kHistoryFilename); + ASSERT_TRUE(file_util::CopyFile(old_history_path, new_history_file)); + + backend_->Closing(); + backend_ = new HistoryBackend(new_history_path, + new HistoryBackendTestDelegate(this), + &bookmark_model_); + backend_->Init(false); + + // Now the database should already be migrated. + // Check version first. + int cur_version = HistoryDatabase::GetCurrentVersion(); + sql::Connection db; + ASSERT_TRUE(db.Open(new_history_file)); + sql::Statement s(db.GetUniqueStatement( + "SELECT value FROM meta WHERE key = 'version'")); + ASSERT_TRUE(s.Step()); + int file_version = s.ColumnInt(0); + EXPECT_EQ(cur_version, file_version); + + // Check visit_source table is created and empty. + s.Assign(db.GetUniqueStatement( + "SELECT name FROM sqlite_master WHERE name=\"visit_source\"")); + ASSERT_TRUE(s.Step()); + s.Assign(db.GetUniqueStatement("SELECT * FROM visit_source LIMIT 10")); + EXPECT_FALSE(s.Step()); +} + } // namespace history diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc index 7869fcc..4fab291 100644 --- a/chrome/browser/history/history_database.cc +++ b/chrome/browser/history/history_database.cc @@ -26,7 +26,7 @@ namespace { // Current version number. We write databases at the "current" version number, // but any previous version that can read the "compatible" one can make do with // or database without *too* many bad effects. -static const int kCurrentVersionNumber = 18; +static const int kCurrentVersionNumber = 19; static const int kCompatibleVersionNumber = 16; static const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold"; @@ -134,7 +134,7 @@ void HistoryDatabase::BeginExclusiveMode() { // static int HistoryDatabase::GetCurrentVersion() { - // If we don't use TopSites, we are one version number behind. + // If we don't use TopSites, we stay at the last pre-TopSites version. if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) return 17; // Last pre-TopSites version. else @@ -284,12 +284,19 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion( needs_version_18_migration_ = true; if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites) && - cur_version == 18) { + (cur_version == 18 || cur_version == 19)) { // Set DB version back to pre-top sites. cur_version = 17; meta_table_.SetVersionNumber(cur_version); } + if (needs_version_18_migration_ || cur_version == 18) { + // This is the version prior to adding url_source column. We need to + // migrate the database. + cur_version = 19; + meta_table_.SetVersionNumber(cur_version); + } + // When the version is too old, we just try to continue anyway, there should // not be a released product that makes a database too old for us to handle. LOG_IF(WARNING, (cur_version < GetCurrentVersion() && @@ -325,9 +332,13 @@ void HistoryDatabase::MigrateTimeEpoch() { #endif void HistoryDatabase::MigrationToTopSitesDone() { - // We should be migrating from 17 to 18. - DCHECK_EQ(17, meta_table_.GetVersionNumber()); - meta_table_.SetVersionNumber(18); + // Migration should only happen for version 17 and 19. + int version = meta_table_.GetVersionNumber(); + DCHECK(17 == version || 19 == version); + if (17 == version) { + // We should be migrating from 17 to 18. + meta_table_.SetVersionNumber(18); + } needs_version_18_migration_ = false; } diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h index 64cde29..e746d20 100644 --- a/chrome/browser/history/history_marshaling.h +++ b/chrome/browser/history/history_marshaling.h @@ -30,6 +30,7 @@ class HistoryAddPageArgs const GURL& arg_referrer, const history::RedirectList& arg_redirects, PageTransition::Type arg_transition, + VisitSource arg_source, bool arg_did_replace_entry) : url(arg_url), time(arg_time), @@ -38,6 +39,7 @@ class HistoryAddPageArgs referrer(arg_referrer), redirects(arg_redirects), transition(arg_transition), + visit_source(arg_source), did_replace_entry(arg_did_replace_entry) { } @@ -50,6 +52,7 @@ class HistoryAddPageArgs GURL referrer; history::RedirectList redirects; PageTransition::Type transition; + VisitSource visit_source; bool did_replace_entry; private: diff --git a/chrome/browser/history/history_querying_unittest.cc b/chrome/browser/history/history_querying_unittest.cc index 7512786..7dbf5d1 100644 --- a/chrome/browser/history/history_querying_unittest.cc +++ b/chrome/browser/history/history_querying_unittest.cc @@ -111,7 +111,7 @@ class HistoryQueryTest : public testing::Test { history_->AddPage(url, test_entries[i].time, id_scope, page_id, GURL(), PageTransition::LINK, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history_->SetPageTitle(url, UTF8ToUTF16(test_entries[i].title)); history_->SetPageContents(url, UTF8ToUTF16(test_entries[i].body)); } @@ -314,7 +314,7 @@ TEST_F(HistoryQueryTest, FTSArchived) { row2.set_last_visit(Time::Now()); urls_to_add.push_back(row2); - history_->AddPagesWithDetails(urls_to_add); + history_->AddPagesWithDetails(urls_to_add, history::SOURCE_BROWSED); QueryOptions options; QueryResults results; diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h index 21e8067..de4b88c 100644 --- a/chrome/browser/history/history_types.h +++ b/chrome/browser/history/history_types.h @@ -176,9 +176,27 @@ class URLRow { // We support the implicit copy constuctor and operator=. }; -// VisitRow ------------------------------------------------------------------- +// The enumeration of all possible sources of visits is listed below. +// The source will be propogated along with a URL or a visit item +// and eventually be stored in the history database, +// visit_source table specifically. +// Different from page transition types, they describe the origins of visits. +// (Warning): Please don't change any existing values while it is ok to add +// new values when needed. +typedef enum { + SOURCE_SYNCED = 0, // Synchronized from somewhere else. + SOURCE_BROWSED = 1, // User browsed. + SOURCE_EXTENSION = 2, // Added by an externsion. + SOURCE_FIREFOX_IMPORTED = 3, + SOURCE_IE_IMPORTED = 4, + SOURCE_SAFARI_IMPORTED = 5, +} VisitSource; typedef int64 VisitID; +// Structure to hold the mapping between each visit's id and its source. +typedef std::map<VisitID, VisitSource> VisitSourceMap; + +// VisitRow ------------------------------------------------------------------- // Holds all information associated with a specific visit. A visit holds time // and referrer information for one time a URL is visited. diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc index 063414f..255e466 100644 --- a/chrome/browser/history/history_unittest.cc +++ b/chrome/browser/history/history_unittest.cc @@ -415,7 +415,8 @@ TEST_F(HistoryTest, AddPage) { const GURL test_url("http://www.google.com/"); history->AddPage(test_url, NULL, 0, GURL(), PageTransition::MANUAL_SUBFRAME, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); @@ -423,7 +424,8 @@ TEST_F(HistoryTest, AddPage) { // Add the page once from the main frame (should unhide it). history->AddPage(test_url, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); EXPECT_EQ(2, query_url_row_.visit_count()); // Added twice. EXPECT_EQ(0, query_url_row_.typed_count()); // Never typed. @@ -446,14 +448,16 @@ TEST_F(HistoryTest, AddPageSameTimes) { // additions have different timestamps. history->AddPage(test_urls[0], now, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_urls[0])); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_TRUE(now == query_url_row_.last_visit()); // gtest doesn't like Time history->AddPage(test_urls[1], now, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_urls[1])); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_TRUE(now + TimeDelta::FromMicroseconds(1) == @@ -463,7 +467,8 @@ TEST_F(HistoryTest, AddPageSameTimes) { history->AddPage(test_urls[2], now + TimeDelta::FromMinutes(1), NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_urls[2])); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_TRUE(now + TimeDelta::FromMinutes(1) == @@ -486,7 +491,8 @@ TEST_F(HistoryTest, AddRedirect) { // Add the sequence of pages as a server with no referrer. Note that we need // to have a non-NULL page ID scope. history->AddPage(first_redirects.back(), MakeFakeHost(1), 0, GURL(), - PageTransition::LINK, first_redirects, true); + PageTransition::LINK, first_redirects, + history::SOURCE_BROWSED, true); // The first page should be added once with a link visit type (because we set // LINK when we added the original URL, and a referrer of nowhere (0). @@ -524,7 +530,7 @@ TEST_F(HistoryTest, AddRedirect) { second_redirects[0], static_cast<PageTransition::Type>(PageTransition::LINK | PageTransition::CLIENT_REDIRECT), - second_redirects, true); + second_redirects, history::SOURCE_BROWSED, true); // The last page (source of the client redirect) should NOT have an // additional visit added, because it was a client redirect (normally it @@ -549,7 +555,8 @@ TEST_F(HistoryTest, Typed) { // Add the page once as typed. const GURL test_url("http://www.google.com/"); history->AddPage(test_url, NULL, 0, GURL(), PageTransition::TYPED, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // We should have the same typed & visit count. @@ -558,7 +565,8 @@ TEST_F(HistoryTest, Typed) { // Add the page again not typed. history->AddPage(test_url, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // The second time should not have updated the typed count. @@ -568,7 +576,7 @@ TEST_F(HistoryTest, Typed) { // Add the page again as a generated URL. history->AddPage(test_url, NULL, 0, GURL(), PageTransition::GENERATED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // This should have worked like a link click. @@ -578,7 +586,7 @@ TEST_F(HistoryTest, Typed) { // Add the page again as a reload. history->AddPage(test_url, NULL, 0, GURL(), PageTransition::RELOAD, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // This should not have incremented any visit counts. @@ -593,7 +601,7 @@ TEST_F(HistoryTest, SetTitle) { // Add a URL. const GURL existing_url("http://www.google.com/"); - history->AddPage(existing_url); + history->AddPage(existing_url, history::SOURCE_BROWSED); // Set some title. const string16 existing_title = UTF8ToUTF16("Google"); @@ -628,7 +636,7 @@ TEST_F(HistoryTest, Segments) { const GURL existing_url("http://www.google.com/"); history->AddPage(existing_url, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Make sure a segment was created. history->QuerySegmentUsageSince( @@ -647,7 +655,7 @@ TEST_F(HistoryTest, Segments) { const GURL link_url("http://yahoo.com/"); history->AddPage(link_url, scope, 0, GURL(), PageTransition::LINK, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Query again history->QuerySegmentUsageSince( @@ -665,7 +673,7 @@ TEST_F(HistoryTest, Segments) { // Add a page linked from existing_url. history->AddPage(GURL("http://www.google.com/foo"), scope, 3, existing_url, PageTransition::LINK, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Query again history->QuerySegmentUsageSince( @@ -699,7 +707,8 @@ TEST_F(HistoryTest, Thumbnails) { static const double boringness = 0.25; const GURL url("http://www.google.com/thumbnail_test/"); - history->AddPage(url); // Must be visited before adding a thumbnail. + // Must be visited before adding a thumbnail. + history->AddPage(url, history::SOURCE_BROWSED); history->SetPageThumbnail(url, *thumbnail, ThumbnailScore(boringness, true, true)); @@ -766,10 +775,10 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Add two pages. history->AddPage(url0, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->AddPage(url1, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -782,7 +791,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Add another page. history->AddPage(url2, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -796,7 +805,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Revisit url2, making it the top URL. history->AddPage(url2, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -810,7 +819,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Revisit url1, making it the top URL. history->AddPage(url1, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -829,7 +838,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Visit url4 using redirects. history->AddPage(url4, scope, 0, GURL(), PageTransition::TYPED, redirects, - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -887,7 +896,8 @@ HistoryAddPageArgs* MakeAddArgs(const GURL& url) { 0, GURL(), history::RedirectList(), - PageTransition::TYPED, false); + PageTransition::TYPED, + history::SOURCE_BROWSED, false); } // Convenience version of the above to convert a char string. diff --git a/chrome/browser/history/text_database_manager_unittest.cc b/chrome/browser/history/text_database_manager_unittest.cc index 8e7f27e..1ec7744 100644 --- a/chrome/browser/history/text_database_manager_unittest.cc +++ b/chrome/browser/history/text_database_manager_unittest.cc @@ -79,7 +79,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, visit_row.transition = 0; visit_row.segment_id = 0; visit_row.is_indexed = false; - VisitID visit_id = visit_db->AddVisit(&visit_row); + VisitID visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL1), visit_row.url_id, visit_row.visit_id, @@ -89,7 +89,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL2), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle2), @@ -98,7 +98,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL3), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle3), @@ -108,7 +108,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL4), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle4), @@ -117,7 +117,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL5), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle5), @@ -127,7 +127,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL1), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle1), @@ -242,7 +242,7 @@ TEST_F(TextDatabaseManagerTest, InsertCompleteVisit) { visit.transition = PageTransition::LINK; visit.segment_id = 0; visit.is_indexed = false; - visit_db.AddVisit(&visit); + visit_db.AddVisit(&visit, SOURCE_BROWSED); // Add a full text indexed entry for that visit. const GURL url(kURL2); @@ -335,7 +335,7 @@ TEST_F(TextDatabaseManagerTest, PartialComplete) { VisitRow visit_row; visit_row.url_id = url_id; visit_row.visit_time = added_time; - visit_db.AddVisit(&visit_row); + visit_db.AddVisit(&visit_row, SOURCE_BROWSED); // Add a URL with no title or body, and say that it expired. manager.AddPageURL(url, 0, 0, added_time); diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc index 979accf..ecc7a3c 100644 --- a/chrome/browser/history/thumbnail_database.cc +++ b/chrome/browser/history/thumbnail_database.cc @@ -499,7 +499,8 @@ bool ThumbnailDatabase::RenameAndDropThumbnails(const FilePath& old_db_file, favicons.Close(); // Can't attach within a transaction. - CommitTransaction(); + if (transaction_nesting()) + CommitTransaction(); // Attach new DB. { diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc index 75c91ee..d0156c2 100644 --- a/chrome/browser/history/visit_database.cc +++ b/chrome/browser/history/visit_database.cc @@ -11,6 +11,7 @@ #include "app/sql/statement.h" #include "base/logging.h" +#include "base/string_number_conversions.h" #include "chrome/browser/history/url_database.h" #include "chrome/common/page_transition_types.h" #include "chrome/common/url_constants.h" @@ -50,6 +51,17 @@ bool VisitDatabase::InitVisitTable() { return false; } + // Visit source table contains the source information for all the visits. To + // save space, we do not record those user browsed visits which would be the + // majority in this table. Only other sources are recorded. + // Due to the tight relationship between visit_source and visits table, they + // should be created and dropped at the same time. + if (!GetDB().DoesTableExist("visit_source")) { + if (!GetDB().Execute("CREATE TABLE visit_source(" + "id INTEGER PRIMARY KEY,source INTEGER NOT NULL)")) + return false; + } + // Index over url so we can quickly find visits for a page. This will just // fail if it already exists and we'll ignore it. GetDB().Execute("CREATE INDEX visits_url_index ON visits (url)"); @@ -67,6 +79,7 @@ bool VisitDatabase::InitVisitTable() { } bool VisitDatabase::DropVisitTable() { + GetDB().Execute("DROP TABLE visit_source"); // This will also drop the indices over the table. return GetDB().Execute("DROP TABLE visits"); } @@ -93,7 +106,7 @@ void VisitDatabase::FillVisitVector(sql::Statement& statement, } } -VisitID VisitDatabase::AddVisit(VisitRow* visit) { +VisitID VisitDatabase::AddVisit(VisitRow* visit, VisitSource source) { sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, "INSERT INTO visits " "(url, visit_time, from_visit, transition, segment_id, is_indexed) " @@ -111,6 +124,20 @@ VisitID VisitDatabase::AddVisit(VisitRow* visit) { return 0; visit->visit_id = GetDB().GetLastInsertRowId(); + + if (source != SOURCE_BROWSED) { + // Record the source of this visit when it is not browsed. + sql::Statement statement1(GetDB().GetCachedStatement(SQL_FROM_HERE, + "INSERT INTO visit_source (id, source) VALUES (?,?)")); + if (!statement1.is_valid()) + return 0; + + statement1.BindInt64(0, visit->visit_id); + statement1.BindInt64(1, source); + if (!statement1.Run()) + return 0; + } + return visit->visit_id; } @@ -132,6 +159,16 @@ void VisitDatabase::DeleteVisit(const VisitRow& visit) { return; del.BindInt64(0, visit.visit_id); del.Run(); + + // Try to delete the entry in visit_source table as well. + // If the visit was browsed, there is no corresponding entry in visit_source + // table, and nothing will be deleted. + del.Assign(GetDB().GetCachedStatement(SQL_FROM_HERE, + "DELETE FROM visit_source WHERE id=?")); + if (!del.is_valid()) + return; + del.BindInt64(0, visit.visit_id); + del.Run(); } bool VisitDatabase::GetRowForVisit(VisitID visit_id, VisitRow* out_visit) { @@ -436,4 +473,42 @@ bool VisitDatabase::GetStartDate(base::Time* first_visit) { return true; } +void VisitDatabase::GetVisitsSource(const VisitVector& visits, + VisitSourceMap* sources) { + DCHECK(sources); + sources->clear(); + + // We query the source in batch. Here defines the batch size. + const size_t batch_size = 500; + size_t visits_size = visits.size(); + + size_t start_index = 0, end_index = 0; + while (end_index < visits_size) { + start_index = end_index; + end_index = end_index + batch_size < visits_size ? end_index + batch_size + : visits_size; + + // Compose the sql statement with a list of ids. + std::string sql = "SELECT id,source FROM visit_source "; + sql.append("WHERE id IN ("); + // Append all the ids in the statement. + for (size_t j = start_index; j < end_index; j++) { + if (j != start_index) + sql.push_back(','); + sql.append(base::Int64ToString(visits[j].visit_id)); + } + sql.append(") ORDER BY id"); + sql::Statement statement(GetDB().GetUniqueStatement(sql.c_str())); + if (!statement) + return; + + // Get the source entries out of the query result. + while (statement.Step()) { + std::pair<VisitID, VisitSource> source_entry(statement.ColumnInt64(0), + static_cast<VisitSource>(statement.ColumnInt(1))); + sources->insert(source_entry); + } + } +} + } // namespace history diff --git a/chrome/browser/history/visit_database.h b/chrome/browser/history/visit_database.h index 3731c4a..0e97cac 100644 --- a/chrome/browser/history/visit_database.h +++ b/chrome/browser/history/visit_database.h @@ -34,8 +34,9 @@ class VisitDatabase { // Adds a line to the visit database with the given information, returning // the added row ID on success, 0 on failure. The given visit is updated with - // the new row ID on success. - VisitID AddVisit(VisitRow* visit); + // the new row ID on success. In addition, adds its source into visit_source + // table. + VisitID AddVisit(VisitRow* visit, VisitSource source); // Deletes the given visit from the database. If a visit with the given ID // doesn't exist, it will not do anything. @@ -144,6 +145,10 @@ class VisitDatabase { // Get the time of the first item in our database. bool GetStartDate(base::Time* first_visit); + // Get the source information about the given visits. + void GetVisitsSource(const VisitVector& visits, + VisitSourceMap* sources); + protected: // Returns the database for the functions in this interface. virtual sql::Connection& GetDB() = 0; diff --git a/chrome/browser/history/visit_database_unittest.cc b/chrome/browser/history/visit_database_unittest.cc index ebc2e1b..28a0e15 100644 --- a/chrome/browser/history/visit_database_unittest.cc +++ b/chrome/browser/history/visit_database_unittest.cc @@ -73,19 +73,19 @@ class VisitDatabaseTest : public PlatformTest, TEST_F(VisitDatabaseTest, Add) { // Add one visit. VisitRow visit_info1(1, Time::Now(), 0, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info1)); + EXPECT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); // Add second visit for the same page. VisitRow visit_info2(visit_info1.url_id, visit_info1.visit_time + TimeDelta::FromSeconds(1), 1, PageTransition::TYPED, 0); - EXPECT_TRUE(AddVisit(&visit_info2)); + EXPECT_TRUE(AddVisit(&visit_info2, SOURCE_BROWSED)); // Add third visit for a different page. VisitRow visit_info3(2, visit_info1.visit_time + TimeDelta::FromSeconds(2), 0, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info3)); + EXPECT_TRUE(AddVisit(&visit_info3, SOURCE_BROWSED)); // Query the first two. std::vector<VisitRow> matches; @@ -104,17 +104,17 @@ TEST_F(VisitDatabaseTest, Delete) { static const int kTime1 = 1000; VisitRow visit_info1(1, Time::FromInternalValue(kTime1), 0, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info1)); + EXPECT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); static const int kTime2 = kTime1 + 1; VisitRow visit_info2(1, Time::FromInternalValue(kTime2), visit_info1.visit_id, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info2)); + EXPECT_TRUE(AddVisit(&visit_info2, SOURCE_BROWSED)); static const int kTime3 = kTime2 + 1; VisitRow visit_info3(1, Time::FromInternalValue(kTime3), visit_info2.visit_id, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info3)); + EXPECT_TRUE(AddVisit(&visit_info3, SOURCE_BROWSED)); // First make sure all the visits are there. std::vector<VisitRow> matches; @@ -140,7 +140,7 @@ TEST_F(VisitDatabaseTest, Delete) { TEST_F(VisitDatabaseTest, Update) { // Make something in the database. VisitRow original(1, Time::Now(), 23, 22, 19); - AddVisit(&original); + AddVisit(&original, SOURCE_BROWSED); // Mutate that row. VisitRow modification(original); @@ -167,7 +167,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info1.visit_id = 1; - EXPECT_TRUE(AddVisit(&visit_info1)); + EXPECT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); // Add second visit for the same page. VisitRow visit_info2(visit_info1.url_id, @@ -177,7 +177,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info2.visit_id = 2; - EXPECT_TRUE(AddVisit(&visit_info2)); + EXPECT_TRUE(AddVisit(&visit_info2, SOURCE_BROWSED)); // Add third visit for a different page. VisitRow visit_info3(2, @@ -186,7 +186,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_START), 0); visit_info3.visit_id = 3; - EXPECT_TRUE(AddVisit(&visit_info3)); + EXPECT_TRUE(AddVisit(&visit_info3, SOURCE_BROWSED)); // Add a redirect visit from the last page. VisitRow visit_info4(3, @@ -195,7 +195,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info4.visit_id = 4; - EXPECT_TRUE(AddVisit(&visit_info4)); + EXPECT_TRUE(AddVisit(&visit_info4, SOURCE_BROWSED)); // Add a subframe visit. VisitRow visit_info5(4, @@ -205,7 +205,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info5.visit_id = 5; - EXPECT_TRUE(AddVisit(&visit_info5)); + EXPECT_TRUE(AddVisit(&visit_info5, SOURCE_BROWSED)); // Query the visits for all time, we should not get the first (duplicate of // the second) or the redirect or subframe visits. @@ -227,4 +227,37 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { ASSERT_EQ(static_cast<size_t>(1), results.size()); EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info4)); } + +TEST_F(VisitDatabaseTest, VisitSource) { + // Add visits. + VisitRow visit_info1(111, Time::Now(), 0, PageTransition::LINK, 0); + ASSERT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); + + VisitRow visit_info2(112, Time::Now(), 1, PageTransition::TYPED, 0); + ASSERT_TRUE(AddVisit(&visit_info2, SOURCE_SYNCED)); + + VisitRow visit_info3(113, Time::Now(), 0, PageTransition::TYPED, 0); + ASSERT_TRUE(AddVisit(&visit_info3, SOURCE_EXTENSION)); + + // Query each visit. + std::vector<VisitRow> matches; + ASSERT_TRUE(GetVisitsForURL(111, &matches)); + ASSERT_EQ(1U, matches.size()); + VisitSourceMap sources; + GetVisitsSource(matches, &sources); + EXPECT_EQ(0U, sources.size()); + + ASSERT_TRUE(GetVisitsForURL(112, &matches)); + ASSERT_EQ(1U, matches.size()); + GetVisitsSource(matches, &sources); + ASSERT_EQ(1U, sources.size()); + EXPECT_EQ(SOURCE_SYNCED, sources[matches[0].visit_id]); + + ASSERT_TRUE(GetVisitsForURL(113, &matches)); + ASSERT_EQ(1U, matches.size()); + GetVisitsSource(matches, &sources); + ASSERT_EQ(1U, sources.size()); + EXPECT_EQ(SOURCE_EXTENSION, sources[matches[0].visit_id]); +} + } // namespace history diff --git a/chrome/browser/importer/firefox3_importer.cc b/chrome/browser/importer/firefox3_importer.cc index 4bac89a..76613b6 100644 --- a/chrome/browser/importer/firefox3_importer.cc +++ b/chrome/browser/importer/firefox3_importer.cc @@ -119,7 +119,7 @@ void Firefox3Importer::ImportHistory() { rows.push_back(row); } if (!rows.empty() && !cancelled()) { - bridge_->SetHistoryItems(rows); + bridge_->SetHistoryItems(rows, history::SOURCE_FIREFOX_IMPORTED); } } diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc index ab0a61c..5f0adca 100644 --- a/chrome/browser/importer/ie_importer.cc +++ b/chrome/browser/importer/ie_importer.cc @@ -330,7 +330,7 @@ void IEImporter::ImportHistory() { } if (!rows.empty() && !cancelled()) { - bridge_->SetHistoryItems(rows); + bridge_->SetHistoryItems(rows, history::SOURCE_IE_IMPORTED); } } } diff --git a/chrome/browser/importer/importer.cc b/chrome/browser/importer/importer.cc index 4217a80..3c840f0 100644 --- a/chrome/browser/importer/importer.cc +++ b/chrome/browser/importer/importer.cc @@ -505,14 +505,16 @@ void ExternalProcessImporterClient::OnHistoryImportStart( } void ExternalProcessImporterClient::OnHistoryImportGroup( - const std::vector<history::URLRow> &history_rows_group) { + const std::vector<history::URLRow>& history_rows_group, + int visit_source) { if (cancelled_) return; history_rows_.insert(history_rows_.end(), history_rows_group.begin(), history_rows_group.end()); if (history_rows_.size() == total_history_rows_count_) - bridge_->SetHistoryItems(history_rows_); + bridge_->SetHistoryItems(history_rows_, + static_cast<history::VisitSource>(visit_source)); } void ExternalProcessImporterClient::OnHomePageImportReady( diff --git a/chrome/browser/importer/importer.h b/chrome/browser/importer/importer.h index 8dc42fd..e9197ad 100644 --- a/chrome/browser/importer/importer.h +++ b/chrome/browser/importer/importer.h @@ -344,8 +344,10 @@ class ExternalProcessImporterClient virtual void OnHistoryImportStart(size_t total_history_rows_count); // Called when a group of URLRows has been received. + // The source is passed with history::VisitSource type. virtual void OnHistoryImportGroup( - const std::vector<history::URLRow> &history_rows_group); + const std::vector<history::URLRow> &history_rows_group, + int visit_source); // Called when the home page has been received. virtual void OnHomePageImportReady(const GURL& home_page); diff --git a/chrome/browser/importer/importer_bridge.cc b/chrome/browser/importer/importer_bridge.cc index 5ab94da..079ebe3 100644 --- a/chrome/browser/importer/importer_bridge.cc +++ b/chrome/browser/importer/importer_bridge.cc @@ -61,10 +61,12 @@ void InProcessImporterBridge::SetFavIcons( } void InProcessImporterBridge::SetHistoryItems( - const std::vector<history::URLRow> &rows) { + const std::vector<history::URLRow> &rows, + history::VisitSource visit_source) { ChromeThread::PostTask( ChromeThread::UI, FROM_HERE, - NewRunnableMethod(writer_, &ProfileWriter::AddHistoryPage, rows)); + NewRunnableMethod(writer_, &ProfileWriter::AddHistoryPage, + rows, visit_source)); } void InProcessImporterBridge::SetKeywords( @@ -150,8 +152,9 @@ void ExternalProcessImporterBridge::SetFavIcons( } void ExternalProcessImporterBridge::SetHistoryItems( - const std::vector<history::URLRow> &rows) { - profile_import_thread_->NotifyHistoryImportReady(rows); + const std::vector<history::URLRow> &rows, + history::VisitSource visit_source) { + profile_import_thread_->NotifyHistoryImportReady(rows, visit_source); } void ExternalProcessImporterBridge::SetKeywords( diff --git a/chrome/browser/importer/importer_bridge.h b/chrome/browser/importer/importer_bridge.h index 3d7d62b..8b57ab0 100644 --- a/chrome/browser/importer/importer_bridge.h +++ b/chrome/browser/importer/importer_bridge.h @@ -38,7 +38,8 @@ class ImporterBridge : public base::RefCountedThreadSafe<ImporterBridge> { virtual void SetFavIcons( const std::vector<history::ImportedFavIconUsage>& fav_icons) = 0; - virtual void SetHistoryItems(const std::vector<history::URLRow> &rows) = 0; + virtual void SetHistoryItems(const std::vector<history::URLRow> &rows, + history::VisitSource visit_source) = 0; virtual void SetKeywords(const std::vector<TemplateURL*> &template_urls, int default_keyword_index, bool unique_on_host_and_path) = 0; @@ -95,7 +96,8 @@ class InProcessImporterBridge : public ImporterBridge { virtual void SetFavIcons( const std::vector<history::ImportedFavIconUsage>& fav_icons); - virtual void SetHistoryItems(const std::vector<history::URLRow> &rows); + virtual void SetHistoryItems(const std::vector<history::URLRow> &rows, + history::VisitSource visit_source); virtual void SetKeywords(const std::vector<TemplateURL*>& template_urls, int default_keyword_index, bool unique_on_host_and_path); @@ -140,7 +142,8 @@ class ExternalProcessImporterBridge : public ImporterBridge { virtual void SetFavIcons( const std::vector<history::ImportedFavIconUsage>& fav_icons); - virtual void SetHistoryItems(const std::vector<history::URLRow> &rows); + virtual void SetHistoryItems(const std::vector<history::URLRow> &rows, + history::VisitSource visit_source); virtual void SetKeywords(const std::vector<TemplateURL*>& template_urls, int default_keyword_index, bool unique_on_host_and_path); diff --git a/chrome/browser/importer/importer_messages_internal.h b/chrome/browser/importer/importer_messages_internal.h index b247237..980e397b 100644 --- a/chrome/browser/importer/importer_messages_internal.h +++ b/chrome/browser/importer/importer_messages_internal.h @@ -49,8 +49,10 @@ IPC_BEGIN_MESSAGES(ProfileImportProcessHost) IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyHistoryImportStart, int /* total number of history::URLRow items */) - IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyHistoryImportGroup, - std::vector<history::URLRow>) + IPC_MESSAGE_CONTROL2(ProfileImportProcessHostMsg_NotifyHistoryImportGroup, + std::vector<history::URLRow>, + int /* the source of URLs as in history::VisitSource.*/ + /* To simplify IPC call, pass as an integer */) IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyHomePageImportReady, GURL /* GURL of home page */) diff --git a/chrome/browser/importer/importer_unittest.cc b/chrome/browser/importer/importer_unittest.cc index 930d57d..879d7cb 100644 --- a/chrome/browser/importer/importer_unittest.cc +++ b/chrome/browser/importer/importer_unittest.cc @@ -259,12 +259,15 @@ class TestObserver : public ProfileWriter, ++password_count_; } - virtual void AddHistoryPage(const std::vector<history::URLRow>& page) { + virtual void AddHistoryPage(const std::vector<history::URLRow>& page, + history::VisitSource visit_source) { // Importer should read the specified URL. - for (size_t i = 0; i < page.size(); ++i) + for (size_t i = 0; i < page.size(); ++i) { if (page[i].title() == kIEIdentifyTitle && page[i].url() == GURL(kIEIdentifyUrl)) ++history_count_; + } + EXPECT_EQ(history::SOURCE_IE_IMPORTED, visit_source); } virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark, @@ -610,10 +613,12 @@ class FirefoxObserver : public ProfileWriter, ++password_count_; } - virtual void AddHistoryPage(const std::vector<history::URLRow>& page) { + virtual void AddHistoryPage(const std::vector<history::URLRow>& page, + history::VisitSource visit_source) { ASSERT_EQ(1U, page.size()); EXPECT_EQ("http://en-us.www.mozilla.com/", page[0].url().spec()); EXPECT_EQ(ASCIIToUTF16("Firefox Updated"), page[0].title()); + EXPECT_EQ(history::SOURCE_FIREFOX_IMPORTED, visit_source); ++history_count_; } @@ -806,7 +811,8 @@ class Firefox3Observer : public ProfileWriter, ++password_count_; } - virtual void AddHistoryPage(const std::vector<history::URLRow>& page) { + virtual void AddHistoryPage(const std::vector<history::URLRow>& page, + history::VisitSource visit_source) { ASSERT_EQ(3U, page.size()); EXPECT_EQ("http://www.google.com/", page[0].url().spec()); EXPECT_EQ(ASCIIToUTF16("Google"), page[0].title()); @@ -815,6 +821,7 @@ class Firefox3Observer : public ProfileWriter, EXPECT_EQ("http://www.cs.unc.edu/~jbs/resources/perl/perl-cgi/programs/form1-POST.html", page[2].url().spec()); EXPECT_EQ(ASCIIToUTF16("example form (POST)"), page[2].title()); + EXPECT_EQ(history::SOURCE_FIREFOX_IMPORTED, visit_source); ++history_count_; } diff --git a/chrome/browser/importer/mork_reader.cc b/chrome/browser/importer/mork_reader.cc index 4207ad6..ce1ffe8 100644 --- a/chrome/browser/importer/mork_reader.cc +++ b/chrome/browser/importer/mork_reader.cc @@ -584,5 +584,5 @@ void ImportHistoryFromFirefox2(const FilePath& file, ImporterBridge* bridge) { for (MorkReader::iterator i = reader.begin(); i != reader.end(); ++i) AddToHistory(i->second, data, &rows); if (!rows.empty()) - bridge->SetHistoryItems(rows); + bridge->SetHistoryItems(rows, history::SOURCE_FIREFOX_IMPORTED); } diff --git a/chrome/browser/importer/profile_writer.cc b/chrome/browser/importer/profile_writer.cc index 32f183d..0cd9a77 100644 --- a/chrome/browser/importer/profile_writer.cc +++ b/chrome/browser/importer/profile_writer.cc @@ -36,9 +36,10 @@ void ProfileWriter::AddIE7PasswordInfo(const IE7PasswordInfo& info) { } #endif -void ProfileWriter::AddHistoryPage(const std::vector<history::URLRow>& page) { +void ProfileWriter::AddHistoryPage(const std::vector<history::URLRow>& page, + history::VisitSource visit_source) { profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)-> - AddPagesWithDetails(page); + AddPagesWithDetails(page, visit_source); } void ProfileWriter::AddHomepage(const GURL& home_page) { diff --git a/chrome/browser/importer/profile_writer.h b/chrome/browser/importer/profile_writer.h index 44d4a87..a8571cb 100644 --- a/chrome/browser/importer/profile_writer.h +++ b/chrome/browser/importer/profile_writer.h @@ -12,6 +12,7 @@ #include "base/ref_counted.h" #include "base/time.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" +#include "chrome/browser/history/history_types.h" #include "googleurl/src/gurl.h" class Profile; @@ -72,7 +73,8 @@ class ProfileWriter : public base::RefCountedThreadSafe<ProfileWriter> { #if defined(OS_WIN) virtual void AddIE7PasswordInfo(const IE7PasswordInfo& info); #endif - virtual void AddHistoryPage(const std::vector<history::URLRow>& page); + virtual void AddHistoryPage(const std::vector<history::URLRow>& page, + history::VisitSource visit_source); virtual void AddHomepage(const GURL& homepage); // Adds the bookmarks to the BookmarkModel. // |options| is a bitmask of BookmarkOptions and dictates how and diff --git a/chrome/browser/importer/safari_importer.mm b/chrome/browser/importer/safari_importer.mm index 43e7c28..3ff4676 100644 --- a/chrome/browser/importer/safari_importer.mm +++ b/chrome/browser/importer/safari_importer.mm @@ -317,7 +317,7 @@ void SafariImporter::ImportHistory() { ParseHistoryItems(&rows); if (!rows.empty() && !cancelled()) { - bridge_->SetHistoryItems(rows); + bridge_->SetHistoryItems(rows, history::SOURCE_SAFARI_IMPORTED); } } diff --git a/chrome/browser/profile_import_process_host.h b/chrome/browser/profile_import_process_host.h index 72754c6..8b0d9a3 100644 --- a/chrome/browser/profile_import_process_host.h +++ b/chrome/browser/profile_import_process_host.h @@ -51,7 +51,8 @@ class ProfileImportProcessHost : public BrowserChildProcessHost { // the external process to the process host client. virtual void OnHistoryImportStart(size_t total_history_rows_count) {} virtual void OnHistoryImportGroup( - const std::vector<history::URLRow> &history_rows_group) {} + const std::vector<history::URLRow> &history_rows_group, + int visit_source) {} // visit_source has history::VisitSource type. virtual void OnHomePageImportReady( const GURL& home_page) {} diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc index d777225..5f2d5fb 100644 --- a/chrome/browser/search_engines/template_url_model.cc +++ b/chrome/browser/search_engines/template_url_model.cc @@ -1009,7 +1009,7 @@ void TemplateURLModel::AddTabToSearchVisit(const TemplateURL& t_url) { // autocompleted even if the user doesn't type the url in directly. history->AddPage(url, NULL, 0, GURL(), PageTransition::KEYWORD_GENERATED, - history::RedirectList(), false); + history::RedirectList(), history::SOURCE_BROWSED, false); } // static diff --git a/chrome/browser/search_engines/template_url_model_unittest.cc b/chrome/browser/search_engines/template_url_model_unittest.cc index baee15a..5e174e3 100644 --- a/chrome/browser/search_engines/template_url_model_unittest.cc +++ b/chrome/browser/search_engines/template_url_model_unittest.cc @@ -689,7 +689,7 @@ TEST_F(TemplateURLModelTest, GenerateVisitOnKeyword) { GURL(t_url->url()->ReplaceSearchTerms(*t_url, L"blah", 0, std::wstring())), NULL, 0, GURL(), PageTransition::KEYWORD, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Wait for history to finish processing the request. profile_->BlockUntilHistoryProcessesPendingRequests(); diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc index 7ebd8bf..00ed6a2 100644 --- a/chrome/browser/sync/glue/bookmark_change_processor.cc +++ b/chrome/browser/sync/glue/bookmark_change_processor.cc @@ -520,7 +520,7 @@ bool BookmarkChangeProcessor::SetBookmarkFavicon( FaviconService* favicon_service = profile->GetFaviconService(Profile::EXPLICIT_ACCESS); - history->AddPage(bookmark_node->GetURL()); + history->AddPage(bookmark_node->GetURL(), history::SOURCE_SYNCED); favicon_service->SetFavicon(bookmark_node->GetURL(), fake_icon_url, icon_bytes_vector); diff --git a/chrome/browser/sync/glue/typed_url_model_associator.cc b/chrome/browser/sync/glue/typed_url_model_associator.cc index 3cca88a..d5bf3f1 100644 --- a/chrome/browser/sync/glue/typed_url_model_associator.cc +++ b/chrome/browser/sync/glue/typed_url_model_associator.cc @@ -280,7 +280,7 @@ bool TypedUrlModelAssociator::WriteToHistoryBackend( } } if (new_urls) { - history_backend_->AddPagesWithDetails(*new_urls); + history_backend_->AddPagesWithDetails(*new_urls, history::SOURCE_SYNCED); } if (updated_urls) { for (TypedUrlUpdateVector::const_iterator url = updated_urls->begin(); @@ -294,7 +294,8 @@ bool TypedUrlModelAssociator::WriteToHistoryBackend( if (new_visits) { for (TypedUrlVisitVector::const_iterator visits = new_visits->begin(); visits != new_visits->end(); ++visits) { - if (!history_backend_->AddVisits(visits->first, visits->second)) { + if (!history_backend_->AddVisits(visits->first, visits->second, + history::SOURCE_SYNCED)) { LOG(ERROR) << "Could not add visits."; return false; } 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 331f466..17137ea4 100644 --- a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc @@ -78,8 +78,9 @@ class HistoryBackendMock : public HistoryBackend { MOCK_METHOD2(GetVisitsForURL, bool(history::URLID id, history::VisitVector* visits)); MOCK_METHOD2(UpdateURL, bool(history::URLID id, const history::URLRow& url)); - MOCK_METHOD2(AddVisits, bool(const GURL& url, - const std::vector<base::Time>& visits)); + MOCK_METHOD3(AddVisits, bool(const GURL& url, + const std::vector<base::Time>& visits, + history::VisitSource visit_source)); MOCK_METHOD1(RemoveVisits, bool(const history::VisitVector& visits)); MOCK_METHOD2(GetURL, bool(const GURL& url_id, history::URLRow* url_row)); MOCK_METHOD2(SetPageTitle, void(const GURL& url, const std::wstring& title)); @@ -340,8 +341,8 @@ TEST_F(ProfileSyncServiceTypedUrlTest, HasNativeHasSyncNoMerge) { WillOnce(DoAll(SetArgumentPointee<0>(native_entries), Return(true))); EXPECT_CALL((*history_backend_.get()), GetVisitsForURL(_, _)). WillRepeatedly(DoAll(SetArgumentPointee<1>(native_visits), Return(true))); - EXPECT_CALL((*history_backend_.get()), AddVisits(_, _)). - WillRepeatedly(Return(true)); + EXPECT_CALL((*history_backend_.get()), + AddVisits(_, _, history::SOURCE_SYNCED)).WillRepeatedly(Return(true)); std::vector<history::URLRow> sync_entries; sync_entries.push_back(sync_entry); @@ -385,8 +386,8 @@ TEST_F(ProfileSyncServiceTypedUrlTest, HasNativeHasSyncMerge) { WillOnce(DoAll(SetArgumentPointee<0>(native_entries), Return(true))); EXPECT_CALL((*history_backend_.get()), GetVisitsForURL(_, _)). WillRepeatedly(DoAll(SetArgumentPointee<1>(native_visits), Return(true))); - EXPECT_CALL((*history_backend_.get()), AddVisits(_, _)). - WillRepeatedly(Return(true)); + EXPECT_CALL((*history_backend_.get()), + AddVisits(_, _, history::SOURCE_SYNCED)). WillRepeatedly(Return(true)); std::vector<history::URLRow> sync_entries; sync_entries.push_back(sync_entry); diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index c8e212f..6c54fb0 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -1745,10 +1745,11 @@ void TabContents::UpdateHistoryForNavigation( if (!redirects.empty()) redirects.back() = virtual_url; hs->AddPage(virtual_url, this, params.page_id, params.referrer, - params.transition, redirects, details.did_replace_entry); + params.transition, redirects, history::SOURCE_BROWSED, + details.did_replace_entry); } else { hs->AddPage(params.url, this, params.page_id, params.referrer, - params.transition, params.redirects, + params.transition, params.redirects, history::SOURCE_BROWSED, details.did_replace_entry); } } diff --git a/chrome/browser/visitedlink_unittest.cc b/chrome/browser/visitedlink_unittest.cc index c69c745..b534c41 100644 --- a/chrome/browser/visitedlink_unittest.cc +++ b/chrome/browser/visitedlink_unittest.cc @@ -371,7 +371,7 @@ TEST_F(VisitedLinkTest, Rebuild) { // initialize the visited link DB. int history_count = g_test_count / 2; for (int i = 0; i < history_count; i++) - history_service_->AddPage(TestURL(i)); + history_service_->AddPage(TestURL(i), history::SOURCE_BROWSED); // Initialize the visited link DB. Since the visited links file doesn't exist // and we don't suppress history rebuilding, this will load from history. diff --git a/chrome/profile_import/profile_import_thread.cc b/chrome/profile_import/profile_import_thread.cc index e9ba2a5..6f097b3 100644 --- a/chrome/profile_import/profile_import_thread.cc +++ b/chrome/profile_import/profile_import_thread.cc @@ -105,7 +105,8 @@ void ProfileImportThread::NotifyEnded() { } void ProfileImportThread::NotifyHistoryImportReady( - const std::vector<history::URLRow> &rows) { + const std::vector<history::URLRow> &rows, + history::VisitSource visit_source) { Send(new ProfileImportProcessHostMsg_NotifyHistoryImportStart(rows.size())); std::vector<history::URLRow>::const_iterator it; @@ -117,7 +118,8 @@ void ProfileImportThread::NotifyHistoryImportReady( it + kNumHistoryRowsToSend : rows.end(); row_group.assign(it, end_group); - Send(new ProfileImportProcessHostMsg_NotifyHistoryImportGroup(row_group)); + Send(new ProfileImportProcessHostMsg_NotifyHistoryImportGroup(row_group, + visit_source)); } } diff --git a/chrome/profile_import/profile_import_thread.h b/chrome/profile_import/profile_import_thread.h index 47f2015..f780cc5 100644 --- a/chrome/profile_import/profile_import_thread.h +++ b/chrome/profile_import/profile_import_thread.h @@ -42,7 +42,8 @@ class ProfileImportThread : public ChildThread { void NotifyEnded(); // Bridging methods that move data back across the process boundary. - void NotifyHistoryImportReady(const std::vector<history::URLRow> &rows); + void NotifyHistoryImportReady(const std::vector<history::URLRow> &rows, + history::VisitSource visit_source); void NotifyHomePageImportReady(const GURL& home_page); void NotifyBookmarksImportReady( const std::vector<ProfileWriter::BookmarkEntry>& bookmarks, diff --git a/chrome/tools/profiles/generate_profile.cc b/chrome/tools/profiles/generate_profile.cc index 2900c28..9dc7512 100644 --- a/chrome/tools/profiles/generate_profile.cc +++ b/chrome/tools/profiles/generate_profile.cc @@ -152,7 +152,7 @@ void InsertURLBatch(const std::wstring& profile_dir, int page_id, history_service->AddPage(url, id_scope, page_id, previous_url, transition, - redirects, true); + redirects, history::SOURCE_BROWSED, true); ThumbnailScore score(0.75, false, false); history_service->SetPageTitle(url, ConstructRandomTitle()); if (!history_only) { |