diff options
author | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-08-19 20:41:02 +0000 |
---|---|---|
committer | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-08-19 20:41:02 +0000 |
commit | 0326752ddaebd5d2f3835c22839951d337f23923 (patch) | |
tree | c5c1701b28174ef6ebfccc2355991416c97df1ce | |
parent | 88942a2dab7da11dc89b4fe95151fdefa6cef036 (diff) | |
download | chromium_src-0326752ddaebd5d2f3835c22839951d337f23923.zip chromium_src-0326752ddaebd5d2f3835c22839951d337f23923.tar.gz chromium_src-0326752ddaebd5d2f3835c22839951d337f23923.tar.bz2 |
A new field to describe the sources of history urls(visits) is added. This field is recorded in visit_database.
So far, it can tell imported, synchronized, entension added or other(mainly testing) entries from user browsed entries.
In the future, history extension API may allow queries to combinate with this criterion.
BUG=none
TEST=Unit tests are already included. Please test the web browser with history from previous
versions to make sure the migration could be done properly. Also try to import or sync some history and inspect the
sources added to the visit_source table in hitory database are correct.
Original review=http://codereview.chromium.org/2906004/show
Patch by weili@google.com
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56742 0039d316-1c4b-4281-b951-d872f2087c98
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) { |