diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-01-29 16:35:13 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-02-01 15:33:35 +0000 |
commit | c8c2d1901aec01e934adf561a9fdf0cc776cdef8 (patch) | |
tree | 9157c3d9815e5870799e070b113813bec53e0535 /chromium/components/history | |
parent | abefd5095b41dac94ca451d784ab6e27372e981a (diff) | |
download | qtwebengine-chromium-c8c2d1901aec01e934adf561a9fdf0cc776cdef8.tar.gz |
BASELINE: Update Chromium to 64.0.3282.139
Change-Id: I1cae68fe9c94ff7608b26b8382fc19862cdb293a
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Diffstat (limited to 'chromium/components/history')
48 files changed, 1610 insertions, 1017 deletions
diff --git a/chromium/components/history/core/browser/BUILD.gn b/chromium/components/history/core/browser/BUILD.gn index 3e4bfbc7fb3..f4af57bf61a 100644 --- a/chromium/components/history/core/browser/BUILD.gn +++ b/chromium/components/history/core/browser/BUILD.gn @@ -7,6 +7,8 @@ static_library("browser") { "browsing_history_driver.h", "browsing_history_service.cc", "browsing_history_service.h", + "default_top_sites_provider.cc", + "default_top_sites_provider.h", "delete_directive_handler.cc", "delete_directive_handler.h", "download_constants.h", @@ -65,6 +67,7 @@ static_library("browser") { "top_sites_impl.cc", "top_sites_impl.h", "top_sites_observer.h", + "top_sites_provider.h", "typed_url_data_type_controller.cc", "typed_url_data_type_controller.h", "typed_url_model_type_controller.cc", diff --git a/chromium/components/history/core/browser/OWNERS b/chromium/components/history/core/browser/OWNERS index a09694a874b..f782f95e7cf 100644 --- a/chromium/components/history/core/browser/OWNERS +++ b/chromium/components/history/core/browser/OWNERS @@ -1,3 +1,5 @@ +per-file typed_url_data_type_controller*=gangwu@chromium.org +per-file typed_url_model_type_controller*=gangwu@chromium.org per-file typed_url_sync_bridge*=gangwu@chromium.org per-file typed_url_syncable_service*=gangwu@chromium.org per-file typed_url_syncable_service*=zea@chromium.org diff --git a/chromium/components/history/core/browser/android/android_history_types.cc b/chromium/components/history/core/browser/android/android_history_types.cc index 1cf91664418..4f304469268 100644 --- a/chromium/components/history/core/browser/android/android_history_types.cc +++ b/chromium/components/history/core/browser/android/android_history_types.cc @@ -96,8 +96,7 @@ HistoryAndBookmarkRow::ColumnID HistoryAndBookmarkRow::GetColumnID( BookmarkIDMapping::const_iterator i = g_bookmark_id_mapping->find(name); if (i == g_bookmark_id_mapping->end()) return HistoryAndBookmarkRow::COLUMN_END; - else - return i->second; + return i->second; } SearchRow::SearchRow() : id_(0), keyword_id_(0) { @@ -119,8 +118,7 @@ SearchRow::ColumnID SearchRow::GetColumnID(const std::string& name) { SearchIDMapping::const_iterator i = g_search_id_mapping->find(name); if (i == g_search_id_mapping->end()) return SearchRow::COLUMN_END; - else - return i->second; + return i->second; } AndroidURLRow::AndroidURLRow() : id(0), url_id(0) { diff --git a/chromium/components/history/core/browser/android/favicon_sql_handler.cc b/chromium/components/history/core/browser/android/favicon_sql_handler.cc index b630bae4ea3..28205cf07ba 100644 --- a/chromium/components/history/core/browser/android/favicon_sql_handler.cc +++ b/chromium/components/history/core/browser/android/favicon_sql_handler.cc @@ -39,8 +39,8 @@ bool FaviconSQLHandler::Update(const HistoryAndBookmarkRow& row, // icon is already in database, just create a new favicon. // TODO(pkotwicz): Pass in real pixel size. favicon_base::FaviconID favicon_id = thumbnail_db_->AddFavicon( - GURL(), favicon_base::FAVICON, row.favicon(), FaviconBitmapType::ON_VISIT, - Time::Now(), gfx::Size()); + GURL(), favicon_base::IconType::kFavicon, row.favicon(), + FaviconBitmapType::ON_VISIT, Time::Now(), gfx::Size()); if (!favicon_id) return false; @@ -48,10 +48,10 @@ bool FaviconSQLHandler::Update(const HistoryAndBookmarkRow& row, std::vector<favicon_base::FaviconID> favicon_ids; for (TableIDRows::const_iterator i = ids_set.begin(); i != ids_set.end(); ++i) { - // Remove all icon mappings to favicons of type FAVICON. + // Remove all icon mappings to favicons of type kFavicon. std::vector<IconMapping> icon_mappings; thumbnail_db_->GetIconMappingsForPageURL( - i->url, favicon_base::FAVICON, &icon_mappings); + i->url, {favicon_base::IconType::kFavicon}, &icon_mappings); for (std::vector<IconMapping>::const_iterator m = icon_mappings.begin(); m != icon_mappings.end(); ++m) { if (!thumbnail_db_->DeleteIconMapping(m->mapping_id)) @@ -105,7 +105,7 @@ bool FaviconSQLHandler::Insert(HistoryAndBookmarkRow* row) { // Is it a problem to give a empty URL? // TODO(pkotwicz): Pass in real pixel size. favicon_base::FaviconID id = thumbnail_db_->AddFavicon( - GURL(), favicon_base::FAVICON, row->favicon(), + GURL(), favicon_base::IconType::kFavicon, row->favicon(), FaviconBitmapType::ON_VISIT, Time::Now(), gfx::Size()); if (!id) return false; diff --git a/chromium/components/history/core/browser/browsing_history_service.cc b/chromium/components/history/core/browser/browsing_history_service.cc index 8041ce6a165..b4f36713228 100644 --- a/chromium/components/history/core/browser/browsing_history_service.cc +++ b/chromium/components/history/core/browser/browsing_history_service.cc @@ -73,6 +73,21 @@ bool CanRetry(QuerySourceStatus status) { return status == MORE_RESULTS || status == FAILURE || status == TIMED_OUT; } +base::Time OldestTime( + const std::vector<BrowsingHistoryService::HistoryEntry>& entries) { + // If the vector is empty, then there is no oldest, so we use the oldest + // possible time instead. + if (entries.empty()) { + return base::Time(); + } + + base::Time best = base::Time::Max(); + for (const BrowsingHistoryService::HistoryEntry& entry : entries) { + best = std::min(best, entry.time); + } + return best; +} + } // namespace struct BrowsingHistoryService::QueryHistoryState @@ -421,14 +436,10 @@ void BrowsingHistoryService::MergeDuplicateResults( DCHECK(results); DCHECK(results->empty()); - // Will be used later to decide if we need to hold back results. This requires - // results to be sorted. - base::Time youngest_local = state->local_results.empty() - ? base::Time() - : state->local_results.rbegin()->time; - base::Time youngest_remote = state->remote_results.empty() - ? base::Time() - : state->remote_results.rbegin()->time; + // Will be used later to decide if we need to hold back results. This iterates + // through each entry and makes no assumptions about their ordering. + base::Time oldest_local = OldestTime(state->local_results); + base::Time oldest_remote = OldestTime(state->remote_results); std::vector<HistoryEntry> sorted; sorted.assign(std::make_move_iterator(state->local_results.begin()), @@ -481,25 +492,25 @@ void BrowsingHistoryService::MergeDuplicateResults( // held back until the former source catches up. This only send the UI history // entries in the correct order. Subsequent continuation requests will get the // delayed entries. - base::Time youngest_allowed = base::Time(); + base::Time oldest_allowed = base::Time(); if (state->local_status == MORE_RESULTS) { - youngest_allowed = std::max(youngest_allowed, youngest_local); - state->local_end_time_for_continuation = youngest_local; + oldest_allowed = std::max(oldest_allowed, oldest_local); + state->local_end_time_for_continuation = oldest_local; } if (state->remote_status == MORE_RESULTS) { - youngest_allowed = std::max(youngest_allowed, youngest_remote); - state->remote_end_time_for_continuation = youngest_remote; + oldest_allowed = std::max(oldest_allowed, oldest_remote); + state->remote_end_time_for_continuation = oldest_remote; } else if (CanRetry(state->remote_status)) { // TODO(skym): It is unclear if this is the best behavior. The UI is going // to behave incorrectly if out of order results are received. So to - // guarantee that doesn't happen, use |youngest_local| for continuation + // guarantee that doesn't happen, use |oldest_local| for continuation // calls. This will result in missing history entries for the failed calls. // crbug.com/685866 is related to this problem. - state->remote_end_time_for_continuation = youngest_local; + state->remote_end_time_for_continuation = oldest_local; } HistoryEntry search_entry; - search_entry.time = youngest_allowed; + search_entry.time = oldest_allowed; auto threshold_iter = std::upper_bound(deduped.begin(), deduped.end(), search_entry, HistoryEntry::SortByTimeDescending); @@ -554,24 +565,25 @@ void BrowsingHistoryService::ReturnResultsToDriver( // call to Web History is successful, we shouldn't be able to have empty sync // results at the same time as we have pending local. if (state->remote_results.size()) { - int local_result_count = state->local_results.size(); MergeDuplicateResults(state.get(), &results); - if (local_result_count) { - // In the best case, we expect that all local results are duplicated on - // the server. Keep track of how many are missing. - int missing_count = 0; - for (const HistoryEntry& entry : results) { - missing_count += - entry.entry_type == - BrowsingHistoryService::HistoryEntry::LOCAL_ENTRY - ? entry.all_timestamps.size() - : 0; - } - DCHECK_GE(local_result_count, missing_count); + // In the best case, we expect that all local results are duplicated on + // the server. Keep track of how many are missing. + int combined_count = 0; + int local_count = 0; + for (const HistoryEntry& entry : results) { + if (entry.entry_type == HistoryEntry::LOCAL_ENTRY) + ++local_count; + else if (entry.entry_type == HistoryEntry::COMBINED_ENTRY) + ++combined_count; + } + + int local_and_combined = combined_count + local_count; + if (local_and_combined > 0) { UMA_HISTOGRAM_PERCENTAGE("WebHistory.LocalResultMissingOnServer", - missing_count * 100.0 / local_result_count); + local_count * 100.0 / local_and_combined); } + } else { // TODO(skym): Is the optimization to skip merge on local only results worth // the complexity increase here? diff --git a/chromium/components/history/core/browser/browsing_history_service_unittest.cc b/chromium/components/history/core/browser/browsing_history_service_unittest.cc index f20b5d40e98..cfb147c20f5 100644 --- a/chromium/components/history/core/browser/browsing_history_service_unittest.cc +++ b/chromium/components/history/core/browser/browsing_history_service_unittest.cc @@ -16,7 +16,6 @@ #include "base/timer/mock_timer.h" #include "base/values.h" #include "components/history/core/browser/browsing_history_driver.h" -#include "components/history/core/browser/history_db_task.h" #include "components/history/core/browser/history_service.h" #include "components/history/core/test/fake_web_history_service.h" #include "components/history/core/test/history_service_test_util.h" @@ -57,23 +56,6 @@ struct TestResult { // Used to bind a callback. void DoNothing(bool ignored) {} -class QuittingHistoryDBTask : public HistoryDBTask { - public: - explicit QuittingHistoryDBTask(base::OnceClosure done_closure) - : done_closure_(std::move(done_closure)) {} - - // HistoryDBTask implementation. - bool RunOnDBThread(history::HistoryBackend* backend, - history::HistoryDatabase* db) override { - return true; - } - void DoneRunOnMainThread() override { std::move(done_closure_).Run(); } - - private: - base::OnceClosure done_closure_; - DISALLOW_COPY_AND_ASSIGN(QuittingHistoryDBTask); -}; - class TestSyncService : public syncer::FakeSyncService { public: int GetObserverCount() { return observer_count_; } @@ -168,6 +150,20 @@ class TestWebHistoryService : public FakeWebHistoryService { }; }; +class ReversedWebHistoryService : public TestWebHistoryService { + private: + std::vector<FakeWebHistoryService::Visit> GetVisitsBetween( + base::Time begin, + base::Time end, + size_t count, + bool* more_results_left) override { + auto result = FakeWebHistoryService::GetVisitsBetween(begin, end, count, + more_results_left); + std::reverse(result.begin(), result.end()); + return result; + } +}; + class TimeoutWebHistoryService : public TestWebHistoryService { private: // WebHistoryService implementation. @@ -218,33 +214,32 @@ class BrowsingHistoryServiceTest : public ::testing::Test { } void BlockUntilHistoryProcessesPendingRequests() { - base::CancelableTaskTracker tracker; - base::RunLoop run_loop; - local_history_->ScheduleDBTask( - std::make_unique<QuittingHistoryDBTask>(run_loop.QuitWhenIdleClosure()), - &tracker); - run_loop.Run(); + history::BlockUntilHistoryProcessesPendingRequests(local_history()); } Time OffsetToTime(int64_t hour_offset) { return baseline_time_ + TimeDelta::FromHours(hour_offset); } - void AddHistory(const std::vector<TestResult>& data) { + void AddHistory(const std::vector<TestResult>& data, + TestWebHistoryService* web_history) { for (const TestResult& entry : data) { if (entry.type == kLocal) { local_history()->AddPage(GURL(entry.url), OffsetToTime(entry.hour_offset), VisitSource::SOURCE_BROWSED); } else if (entry.type == kRemote) { - web_history()->AddSyncedVisit(entry.url, - OffsetToTime(entry.hour_offset)); + web_history->AddSyncedVisit(entry.url, OffsetToTime(entry.hour_offset)); } else { NOTREACHED(); } } } + void AddHistory(const std::vector<TestResult>& data) { + AddHistory(data, web_history()); + } + void VerifyEntry(const TestResult& expected, const HistoryEntry& actual) { EXPECT_EQ(GURL(expected.url), actual.url); EXPECT_EQ(OffsetToTime(expected.hour_offset), actual.time); @@ -537,9 +532,8 @@ TEST_F(BrowsingHistoryServiceTest, QueryHistoryFullLocalPending) { {{kUrl3, 3, kRemote}}, QueryHistory(1)); local_history()->DeleteURL(GURL(kUrl1)); - VerifyQueryResult( - /*reached_beginning*/ true, /*has_synced_results*/ true, - {{kUrl2, 2, kRemote}, {kUrl1, 1, kLocal}}, ContinueQuery()); + VerifyQueryResult(/*reached_beginning*/ true, /*has_synced_results*/ true, + {{kUrl2, 2, kRemote}, {kUrl1, 1, kLocal}}, ContinueQuery()); } // Part of a request worth of local results will sit in pending, resulting in us @@ -693,6 +687,30 @@ TEST_F(BrowsingHistoryServiceTest, ObservingWebHistoryDelayedWeb) { EXPECT_EQ(1, driver()->GetHistoryDeletedCount()); } +TEST_F(BrowsingHistoryServiceTest, IncorrectlyOrderedRemoteResults) { + // Created from crbug.com/787928, where suspected MergeDuplicateResults did + // not start with sorted data. This case originally hit a NOTREACHED. + ReversedWebHistoryService reversed; + driver()->SetWebHistory(&reversed); + ResetService(driver(), local_history(), sync()); + AddHistory({{kUrl1, 1, kRemote}, + {kUrl1, 2, kLocal}, + {kUrl3, 3, kLocal}, + {kUrl5, 4, kRemote}, + {kUrl5, 5, kLocal}, + {kUrl6, 6, kRemote}}, + &reversed); + VerifyQueryResult( + /*reached_beginning*/ false, /*has_synced_results*/ true, + {{kUrl6, 6, kRemote}, {kUrl5, 5, kBoth}}, QueryHistory(2)); + + // WebHistoryService will DCHECK if we destroy it before the observer in + // BrowsingHistoryService is removed, so reset our first + // BrowsingHistoryService before |reversed| goes out of scope. + driver()->SetWebHistory(nullptr); + ResetService(driver(), nullptr, nullptr); +} + } // namespace } // namespace history diff --git a/chromium/components/history/core/browser/default_top_sites_provider.cc b/chromium/components/history/core/browser/default_top_sites_provider.cc new file mode 100644 index 00000000000..42278ef7fa6 --- /dev/null +++ b/chromium/components/history/core/browser/default_top_sites_provider.cc @@ -0,0 +1,29 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/history/core/browser/default_top_sites_provider.h" + +namespace { + +const int kDaysOfHistory = 90; + +} // namespace + +namespace history { + +DefaultTopSitesProvider::DefaultTopSitesProvider( + HistoryService* history_service) + : history_service_(history_service) {} + +DefaultTopSitesProvider::~DefaultTopSitesProvider() {} + +void DefaultTopSitesProvider::ProvideTopSites( + int result_count, + const HistoryService::QueryMostVisitedURLsCallback& callback, + base::CancelableTaskTracker* tracker) { + history_service_->QueryMostVisitedURLs(result_count, kDaysOfHistory, callback, + tracker); +} + +} // namespace history diff --git a/chromium/components/history/core/browser/default_top_sites_provider.h b/chromium/components/history/core/browser/default_top_sites_provider.h new file mode 100644 index 00000000000..8331d8c3efc --- /dev/null +++ b/chromium/components/history/core/browser/default_top_sites_provider.h @@ -0,0 +1,33 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_HISTORY_CORE_BROWSER_DEFAULT_TOP_SITES_PROVIDER_H_ +#define COMPONENTS_HISTORY_CORE_BROWSER_DEFAULT_TOP_SITES_PROVIDER_H_ + +#include "base/macros.h" +#include "components/history/core/browser/top_sites_provider.h" + +namespace history { + +// Queries the history service for most frequently recently visited sites and +// provides them for TopSites. +class DefaultTopSitesProvider : public TopSitesProvider { + public: + DefaultTopSitesProvider(HistoryService* history_service); + ~DefaultTopSitesProvider() override; + + void ProvideTopSites( + int result_count, + const HistoryService::QueryMostVisitedURLsCallback& callback, + base::CancelableTaskTracker* tracker) override; + + private: + HistoryService* history_service_; + + DISALLOW_COPY_AND_ASSIGN(DefaultTopSitesProvider); +}; + +} // namespace history + +#endif // COMPONENTS_HISTORY_CORE_BROWSER_DEFAULT_TOP_SITES_PROVIDER_H_ diff --git a/chromium/components/history/core/browser/expire_history_backend_unittest.cc b/chromium/components/history/core/browser/expire_history_backend_unittest.cc index cdc1a82ec4d..46a4615e617 100644 --- a/chromium/components/history/core/browser/expire_history_backend_unittest.cc +++ b/chromium/components/history/core/browser/expire_history_backend_unittest.cc @@ -23,6 +23,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/test/scoped_feature_list.h" #include "base/test/scoped_task_environment.h" +#include "components/history/core/browser/default_top_sites_provider.h" #include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_backend_notifier.h" #include "components/history/core/browser/history_constants.h" @@ -145,9 +146,11 @@ class ExpireHistoryTest : public testing::Test, public HistoryBackendNotifier { TopSitesImpl::RegisterPrefs(pref_service_->registry()); expirer_.SetDatabases(main_db_.get(), thumb_db_.get()); - top_sites_ = new TopSitesImpl(pref_service_.get(), nullptr, - PrepopulatedPageList(), - base::Bind(MockCanAddURLToHistory)); + top_sites_ = new TopSitesImpl( + pref_service_.get(), nullptr, + std::make_unique<history::DefaultTopSitesProvider>( + /*history_service=*/nullptr), + PrepopulatedPageList(), base::Bind(MockCanAddURLToHistory)); WaitTopSitesLoadedObserver wait_top_sites_observer(top_sites_); top_sites_->Init(path().Append(kTopSitesFilename)); wait_top_sites_observer.Run(); @@ -213,10 +216,10 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3], // Two favicons. The first two URLs will share the same one, while the last // one will have a unique favicon. - favicon_base::FaviconID favicon1 = - thumb_db_->AddFavicon(GURL("http://favicon/url1"), favicon_base::FAVICON); - favicon_base::FaviconID favicon2 = - thumb_db_->AddFavicon(GURL("http://favicon/url2"), favicon_base::FAVICON); + favicon_base::FaviconID favicon1 = thumb_db_->AddFavicon( + GURL("http://favicon/url1"), favicon_base::IconType::kFavicon); + favicon_base::FaviconID favicon2 = thumb_db_->AddFavicon( + GURL("http://favicon/url2"), favicon_base::IconType::kFavicon); // Three URLs. URLRow url_row1(GURL("http://www.google.com/1")); @@ -310,7 +313,7 @@ favicon_base::FaviconID ExpireHistoryTest::GetFavicon( const GURL& page_url, favicon_base::IconType icon_type) { std::vector<IconMapping> icon_mappings; - if (thumb_db_->GetIconMappingsForPageURL(page_url, icon_type, + if (thumb_db_->GetIconMappingsForPageURL(page_url, {icon_type}, &icon_mappings)) { return icon_mappings[0].icon_id; } @@ -381,7 +384,7 @@ TEST_F(ExpireHistoryTest, DeleteFaviconsIfPossible) { // Add a favicon record. const GURL favicon_url("http://www.google.com/favicon.ico"); favicon_base::FaviconID icon_id = - thumb_db_->AddFavicon(favicon_url, favicon_base::FAVICON); + thumb_db_->AddFavicon(favicon_url, favicon_base::IconType::kFavicon); EXPECT_TRUE(icon_id); EXPECT_TRUE(HasFavicon(icon_id)); @@ -396,7 +399,8 @@ TEST_F(ExpireHistoryTest, DeleteFaviconsIfPossible) { } // Add back the favicon. - icon_id = thumb_db_->AddFavicon(favicon_url, favicon_base::TOUCH_ICON); + icon_id = + thumb_db_->AddFavicon(favicon_url, favicon_base::IconType::kTouchIcon); EXPECT_TRUE(icon_id); EXPECT_TRUE(HasFavicon(icon_id)); @@ -436,7 +440,7 @@ TEST_F(ExpireHistoryTest, DISABLED_DeleteURLAndFavicon) { URLRow last_row; ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &last_row)); favicon_base::FaviconID favicon_id = - GetFavicon(last_row.url(), favicon_base::FAVICON); + GetFavicon(last_row.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(url_ids[2])); @@ -450,7 +454,7 @@ TEST_F(ExpireHistoryTest, DISABLED_DeleteURLAndFavicon) { // All the normal data + the favicon should be gone. EnsureURLInfoGone(last_row, false); - EXPECT_FALSE(GetFavicon(last_row.url(), favicon_base::FAVICON)); + EXPECT_FALSE(GetFavicon(last_row.url(), favicon_base::IconType::kFavicon)); EXPECT_FALSE(HasFavicon(favicon_id)); } @@ -465,7 +469,7 @@ TEST_F(ExpireHistoryTest, DeleteURLWithoutFavicon) { URLRow last_row; ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &last_row)); favicon_base::FaviconID favicon_id = - GetFavicon(last_row.url(), favicon_base::FAVICON); + GetFavicon(last_row.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(url_ids[1])); @@ -504,7 +508,8 @@ TEST_F(ExpireHistoryTest, DeleteStarredVisitedURL) { EnsureURLInfoGone(url_row, false); // Yet the favicon should exist. - favicon_base::FaviconID favicon_id = GetFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID favicon_id = + GetFavicon(url, favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // Should still have the thumbnail. @@ -516,8 +521,8 @@ TEST_F(ExpireHistoryTest, DeleteStarredVisitedURL) { TEST_F(ExpireHistoryTest, DeleteStarredUnvisitedURL) { // Create a bookmark associated with a favicon. const GURL url("http://www.google.com/starred"); - favicon_base::FaviconID favicon = - thumb_db_->AddFavicon(GURL("http://favicon/url1"), favicon_base::FAVICON); + favicon_base::FaviconID favicon = thumb_db_->AddFavicon( + GURL("http://favicon/url1"), favicon_base::IconType::kFavicon); thumb_db_->AddIconMapping(url, favicon); StarURL(url); @@ -525,7 +530,8 @@ TEST_F(ExpireHistoryTest, DeleteStarredUnvisitedURL) { expirer_.DeleteURL(url); // The favicon should exist. - favicon_base::FaviconID favicon_id = GetFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID favicon_id = + GetFavicon(url, favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // Unstar the URL and try again to delete it. @@ -533,7 +539,7 @@ TEST_F(ExpireHistoryTest, DeleteStarredUnvisitedURL) { expirer_.DeleteURL(url); // The favicon should be gone. - favicon_id = GetFavicon(url, favicon_base::FAVICON); + favicon_id = GetFavicon(url, favicon_base::IconType::kFavicon); EXPECT_FALSE(HasFavicon(favicon_id)); } @@ -553,7 +559,8 @@ TEST_F(ExpireHistoryTest, DeleteURLs) { urls.push_back(GURL()); for (size_t i = 0; i < arraysize(rows); ++i) { ASSERT_TRUE(main_db_->GetURLRow(url_ids[i], &rows[i])); - favicon_ids[i] = GetFavicon(rows[i].url(), favicon_base::FAVICON); + favicon_ids[i] = + GetFavicon(rows[i].url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_ids[i])); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(url_ids[i])); @@ -611,14 +618,14 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsUnstarred) { // Verify that the middle URL's favicon and thumbnail is still there. favicon_base::FaviconID favicon_id = - GetFavicon(url_row1.url(), favicon_base::FAVICON); + GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(url_row1.id())); // Verify that the last URL was deleted. favicon_base::FaviconID favicon_id2 = - GetFavicon(url_row2.url(), favicon_base::FAVICON); + GetFavicon(url_row2.url(), favicon_base::IconType::kFavicon); EnsureURLInfoGone(url_row2, false); EXPECT_FALSE(HasFavicon(favicon_id2)); } @@ -661,14 +668,14 @@ TEST_F(ExpireHistoryTest, FlushURLsForTimes) { // Verify that the middle URL's favicon and thumbnail is still there. favicon_base::FaviconID favicon_id = - GetFavicon(url_row1.url(), favicon_base::FAVICON); + GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(url_row1.id())); // Verify that the last URL was deleted. favicon_base::FaviconID favicon_id2 = - GetFavicon(url_row2.url(), favicon_base::FAVICON); + GetFavicon(url_row2.url(), favicon_base::IconType::kFavicon); EnsureURLInfoGone(url_row2, false); EXPECT_FALSE(HasFavicon(favicon_id2)); } @@ -712,7 +719,7 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsUnstarredRestricted) { // Verify that the middle URL's favicon and thumbnail is still there. favicon_base::FaviconID favicon_id = - GetFavicon(url_row1.url(), favicon_base::FAVICON); + GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(url_row1.id())); @@ -764,11 +771,11 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsStarred) { // exists in history, this should not be a privacy problem, we only update // the visit counts in this case for consistency anyway. favicon_base::FaviconID favicon_id = - GetFavicon(url_row1.url(), favicon_base::FAVICON); + GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(new_url_row1.id())); - favicon_id = GetFavicon(url_row1.url(), favicon_base::FAVICON); + favicon_id = GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon); EXPECT_TRUE(HasFavicon(favicon_id)); // TODO(sky): fix this, see comment in HasThumbnail. // EXPECT_TRUE(HasThumbnail(new_url_row2.id())); @@ -913,7 +920,8 @@ TEST_F(ExpireHistoryTest, ClearOldOnDemandFaviconsDoesDeleteUnstarred) { // Icon: old and not bookmarked case. GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon_id = thumb_db_->AddFavicon( - url, favicon_base::FAVICON, favicon, FaviconBitmapType::ON_DEMAND, + url, favicon_base::IconType::kFavicon, favicon, + FaviconBitmapType::ON_DEMAND, GetOldFaviconThreshold() - base::TimeDelta::FromSeconds(1), gfx::Size()); ASSERT_NE(0, icon_id); GURL page_url("http://google.com/"); @@ -941,7 +949,8 @@ TEST_F(ExpireHistoryTest, ClearOldOnDemandFaviconsDoesNotDeleteStarred) { // Icon: old but bookmarked case. GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon_id = thumb_db_->AddFavicon( - url, favicon_base::FAVICON, favicon, FaviconBitmapType::ON_DEMAND, + url, favicon_base::IconType::kFavicon, favicon, + FaviconBitmapType::ON_DEMAND, GetOldFaviconThreshold() - base::TimeDelta::FromSeconds(1), gfx::Size()); ASSERT_NE(0, icon_id); GURL page_url1("http://google.com/1"); @@ -983,7 +992,8 @@ TEST_F(ExpireHistoryTest, ClearOldOnDemandFaviconsDoesDeleteAfterLongDelay) { // Icon: old and not bookmarked case. GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon_id = thumb_db_->AddFavicon( - url, favicon_base::FAVICON, favicon, FaviconBitmapType::ON_DEMAND, + url, favicon_base::IconType::kFavicon, favicon, + FaviconBitmapType::ON_DEMAND, GetOldFaviconThreshold() - base::TimeDelta::FromSeconds(1), gfx::Size()); ASSERT_NE(0, icon_id); GURL page_url("http://google.com/"); @@ -1016,7 +1026,8 @@ TEST_F(ExpireHistoryTest, // Icon: old but bookmarked case. GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon_id = thumb_db_->AddFavicon( - url, favicon_base::FAVICON, favicon, FaviconBitmapType::ON_DEMAND, + url, favicon_base::IconType::kFavicon, favicon, + FaviconBitmapType::ON_DEMAND, GetOldFaviconThreshold() - base::TimeDelta::FromSeconds(1), gfx::Size()); ASSERT_NE(0, icon_id); GURL page_url1("http://google.com/1"); diff --git a/chromium/components/history/core/browser/history_backend.cc b/chromium/components/history/core/browser/history_backend.cc index 6002fa2c56a..dbb225f3545 100644 --- a/chromium/components/history/core/browser/history_backend.cc +++ b/chromium/components/history/core/browser/history_backend.cc @@ -77,6 +77,10 @@ namespace history { const base::Feature kAvoidStrippingRefFromFaviconPageUrls{ "AvoidStrippingRefFromFaviconPageUrls", base::FEATURE_DISABLED_BY_DEFAULT}; +// TODO(crbug.com/759631): Clean this up after impact is measured via Finch. +const base::Feature kPropagateFaviconsAcrossClientRedirects{ + "PropagateFaviconsAcrossClientRedirects", base::FEATURE_ENABLED_BY_DEFAULT}; + namespace { void RunUnlessCanceled( @@ -102,26 +106,6 @@ const int kMaxRedirectCount = 32; // and is deleted. const int kExpireDaysThreshold = 90; -// Converts from PageUsageData to MostVisitedURL. |redirects| is a -// list of redirects for this URL. Empty list means no redirects. -MostVisitedURL MakeMostVisitedURL(const PageUsageData& page_data, - const RedirectList& redirects) { - MostVisitedURL mv; - mv.url = page_data.GetURL(); - mv.title = page_data.GetTitle(); - if (redirects.empty()) { - // Redirects must contain at least the target url. - mv.redirects.push_back(mv.url); - } else { - mv.redirects = redirects; - if (mv.redirects.back() != mv.url) { - // The last url must be the target url. - mv.redirects.push_back(mv.url); - } - } - return mv; -} - QueuedHistoryDBTask::QueuedHistoryDBTask( std::unique_ptr<HistoryDBTask> task, scoped_refptr<base::SingleThreadTaskRunner> origin_loop, @@ -400,7 +384,7 @@ bool HistoryBackend::IsUntypedIntranetHost(const GURL& url) { net::registry_controlled_domains::GetCanonicalHostRegistryLength( host, net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES, net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES); - return (registry_length == 0) && !db_->IsTypedHost(host); + return (registry_length == 0) && !db_->IsTypedHost(host, /*scheme=*/nullptr); } TopHostsList HistoryBackend::TopHosts(size_t num_hosts) const { @@ -499,7 +483,7 @@ void HistoryBackend::AddPage(const HistoryAddPageArgs& request) { // No redirect case (one element means just the page itself). last_ids = AddPageVisit(request.url, request.time, last_ids.second, t, - request.visit_source); + request.hidden, 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 @@ -557,15 +541,20 @@ void HistoryBackend::AddPage(const HistoryAddPageArgs& request) { // can be called a lot, for example, the page cycler, and most of the // time we won't have changed anything. VisitRow visit_row; - if (request.did_replace_entry && - db_->GetRowForVisit(last_ids.second, &visit_row) && - visit_row.transition & ui::PAGE_TRANSITION_CHAIN_END) { - visit_row.transition = ui::PageTransitionFromInt( - visit_row.transition & ~ui::PAGE_TRANSITION_CHAIN_END); - db_->UpdateVisitRow(visit_row); + if (request.did_replace_entry) { + if (db_->GetRowForVisit(last_ids.second, &visit_row) && + visit_row.transition & ui::PAGE_TRANSITION_CHAIN_END) { + visit_row.transition = ui::PageTransitionFromInt( + visit_row.transition & ~ui::PAGE_TRANSITION_CHAIN_END); + db_->UpdateVisitRow(visit_row); + } + + if (base::FeatureList::IsEnabled( + kPropagateFaviconsAcrossClientRedirects)) { + extended_redirect_chain = + GetCachedRecentRedirects(request.referrer); + } } - - GetCachedRecentRedirects(request.referrer, &extended_redirect_chain); } } @@ -582,8 +571,10 @@ void HistoryBackend::AddPage(const HistoryAddPageArgs& request) { // Record all redirect visits with the same timestamp. We don't display // them anyway, and if we ever decide to, we can reconstruct their order // from the redirect chain. - last_ids = AddPageVisit(redirects[redirect_index], request.time, - last_ids.second, t, request.visit_source); + last_ids = + AddPageVisit(redirects[redirect_index], request.time, last_ids.second, + t, request.hidden, request.visit_source); + if (t & ui::PAGE_TRANSITION_CHAIN_START) { if (request.consider_for_ntp_most_visited) { UpdateSegments(redirects[redirect_index], from_visit_id, @@ -776,10 +767,8 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( Time time, VisitID referring_visit, ui::PageTransition transition, + bool hidden, VisitSource visit_source) { - // Top-level frame navigations are visible, everything else is hidden - bool new_hidden = !ui::PageTransitionIsMainFrame(transition); - // NOTE: This code must stay in sync with // ExpireHistoryBackend::ExpireURLsForVisits(). int typed_increment = 0; @@ -808,7 +797,7 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( url_info.set_last_visit(time); // Only allow un-hiding of pages, never hiding. - if (!new_hidden) + if (!hidden) url_info.set_hidden(false); db_->UpdateURLRow(url_id, url_info); @@ -817,7 +806,7 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( url_info.set_visit_count(1); url_info.set_typed_count(typed_increment); url_info.set_last_visit(time); - url_info.set_hidden(new_hidden); + url_info.set_hidden(hidden); url_id = db_->AddURL(url_info); if (!url_id) { @@ -1034,7 +1023,9 @@ bool HistoryBackend::AddVisits(const GURL& url, if (db_) { for (std::vector<VisitInfo>::const_iterator visit = visits.begin(); visit != visits.end(); ++visit) { - if (!AddPageVisit(url, visit->first, 0, visit->second, visit_source) + if (!AddPageVisit(url, visit->first, 0, visit->second, + !ui::PageTransitionIsMainFrame(visit->second), + visit_source) .first) { return false; } @@ -1397,8 +1388,8 @@ void HistoryBackend::QueryMostVisitedURLs(int result_count, for (const std::unique_ptr<PageUsageData>& current_data : data) { RedirectList redirects; QueryRedirectsFrom(current_data->GetURL(), &redirects); - MostVisitedURL url = MakeMostVisitedURL(*current_data, redirects); - result->push_back(url); + result->emplace_back(current_data->GetURL(), current_data->GetTitle(), + redirects); } UMA_HISTOGRAM_TIMES("History.QueryMostVisitedURLsTime", @@ -1476,7 +1467,7 @@ void HistoryBackend::GetFavicon( void HistoryBackend::GetLargestFaviconForURL( const GURL& page_url, - const std::vector<int>& icon_types, + const std::vector<favicon_base::IconTypeSet>& icon_types_list, int minimum_size_in_pixels, favicon_base::FaviconRawBitmapResult* favicon_bitmap_result) { DCHECK(favicon_bitmap_result); @@ -1491,18 +1482,16 @@ void HistoryBackend::GetLargestFaviconForURL( icon_mappings.empty()) return; - int required_icon_types = 0; - for (std::vector<int>::const_iterator i = icon_types.begin(); - i != icon_types.end(); ++i) { - required_icon_types |= *i; - } + favicon_base::IconTypeSet required_icon_types; + for (const favicon_base::IconTypeSet& icon_types : icon_types_list) + required_icon_types.insert(icon_types.begin(), icon_types.end()); // Find the largest bitmap for each IconType placing in // |largest_favicon_bitmaps|. std::map<favicon_base::IconType, FaviconBitmap> largest_favicon_bitmaps; for (std::vector<IconMapping>::const_iterator i = icon_mappings.begin(); i != icon_mappings.end(); ++i) { - if (!(i->icon_type & required_icon_types)) + if (required_icon_types.count(i->icon_type) == 0) continue; std::vector<FaviconBitmapIDSize> bitmap_id_sizes; thumbnail_db_->GetFaviconBitmapIDSizes(i->icon_id, &bitmap_id_sizes); @@ -1525,12 +1514,11 @@ void HistoryBackend::GetLargestFaviconForURL( // Find an icon which is larger than minimum_size_in_pixels in the order of // icon_types. FaviconBitmap largest_icon; - for (std::vector<int>::const_iterator t = icon_types.begin(); - t != icon_types.end(); ++t) { + for (const favicon_base::IconTypeSet& icon_types : icon_types_list) { for (std::map<favicon_base::IconType, FaviconBitmap>::const_iterator f = largest_favicon_bitmaps.begin(); f != largest_favicon_bitmaps.end(); ++f) { - if (f->first & *t && + if (icon_types.count(f->first) != 0 && (largest_icon.bitmap_id == 0 || (largest_icon.pixel_size.height() < f->second.pixel_size.height() && largest_icon.pixel_size.width() < f->second.pixel_size.width()))) { @@ -1572,7 +1560,7 @@ void HistoryBackend::GetLargestFaviconForURL( void HistoryBackend::GetFaviconsForURL( const GURL& page_url, - int icon_types, + const favicon_base::IconTypeSet& icon_types, const std::vector<int>& desired_sizes, std::vector<favicon_base::FaviconRawBitmapResult>* bitmap_results) { TRACE_EVENT0("browser", "HistoryBackend::GetFaviconsForURL"); @@ -1612,6 +1600,24 @@ void HistoryBackend::UpdateFaviconMappingsAndFetch( desired_sizes, bitmap_results); } +void HistoryBackend::DeleteFaviconMappings( + const base::flat_set<GURL>& page_urls, + favicon_base::IconType icon_type) { + if (!thumbnail_db_ || !db_) + return; + + for (const GURL& page_url : page_urls) { + bool mapping_changed = SetFaviconMappingsForPageAndRedirects( + page_url, icon_type, /*icon_id=*/0); + + if (mapping_changed) { + // Notify the UI that this function changed an icon mapping. + SendFaviconChangedNotificationForPageAndRedirects(page_url); + ScheduleCommit(); + } + } +} + void HistoryBackend::MergeFavicon( const GURL& page_url, const GURL& icon_url, @@ -1713,7 +1719,8 @@ void HistoryBackend::MergeFavicon( // modified. std::vector<IconMapping> icon_mappings; - thumbnail_db_->GetIconMappingsForPageURL(page_url, icon_type, &icon_mappings); + thumbnail_db_->GetIconMappingsForPageURL(page_url, {icon_type}, + &icon_mappings); // Copy the favicon bitmaps mapped to |page_url| to the favicon at |icon_url| // till the limit of |kMaxFaviconBitmapsPerIconURL| is reached. @@ -1756,9 +1763,7 @@ void HistoryBackend::MergeFavicon( // Update the favicon mappings such that only |icon_url| is mapped to // |page_url|. if (icon_mappings.size() != 1 || icon_mappings[0].icon_url != icon_url) { - std::vector<favicon_base::FaviconID> favicon_ids; - favicon_ids.push_back(favicon_id); - SetFaviconMappingsForPageAndRedirects(page_url, icon_type, favicon_ids); + SetFaviconMappingsForPageAndRedirects(page_url, icon_type, favicon_id); SendFaviconChangedNotificationForPageAndRedirects(page_url); } @@ -1780,6 +1785,49 @@ void HistoryBackend::SetFavicons(const base::flat_set<GURL>& page_urls, FaviconBitmapType::ON_VISIT); } +void HistoryBackend::CloneFaviconMappingsForPages( + const GURL& page_url_to_read, + const favicon_base::IconTypeSet& icon_types, + const base::flat_set<GURL>& page_urls_to_write) { + if (!db_ || !thumbnail_db_) + return; + + // Update mappings including redirects for each entry in |page_urls_to_write|. + base::flat_set<GURL> page_urls_to_update_mappings; + for (const GURL& update_mappings_for_page : page_urls_to_write) { + RedirectList redirects = GetCachedRecentRedirects(update_mappings_for_page); + page_urls_to_update_mappings.insert(redirects.begin(), redirects.end()); + } + + // No need to update mapping for |page_url_to_read|, because this is where + // we're getting the mappings from. + page_urls_to_update_mappings.erase(page_url_to_read); + + if (page_urls_to_update_mappings.empty()) + return; + + // Get FaviconIDs for |page_url_to_read| and one of |icon_types|. + std::vector<IconMapping> icon_mappings; + thumbnail_db_->GetIconMappingsForPageURL(page_url_to_read, icon_types, + &icon_mappings); + if (icon_mappings.empty()) + return; + + std::set<GURL> changed_page_urls; + for (const IconMapping& icon_mapping : icon_mappings) { + std::vector<GURL> v = SetFaviconMappingsForPages( + page_urls_to_update_mappings, icon_mapping.icon_type, + icon_mapping.icon_id); + changed_page_urls.insert(std::make_move_iterator(v.begin()), + std::make_move_iterator(v.end())); + } + + if (!changed_page_urls.empty()) { + NotifyFaviconsChanged(changed_page_urls, GURL()); + ScheduleCommit(); + } +} + bool HistoryBackend::SetOnDemandFavicons(const GURL& page_url, favicon_base::IconType icon_type, const GURL& icon_url, @@ -1831,13 +1879,13 @@ void HistoryBackend::SetImportedFavicons( for (size_t i = 0; i < favicon_usage.size(); i++) { favicon_base::FaviconID favicon_id = - thumbnail_db_->GetFaviconIDForFaviconURL(favicon_usage[i].favicon_url, - favicon_base::FAVICON); + thumbnail_db_->GetFaviconIDForFaviconURL( + favicon_usage[i].favicon_url, favicon_base::IconType::kFavicon); if (!favicon_id) { // This favicon doesn't exist yet, so we create it using the given data. // TODO(pkotwicz): Pass in real pixel size. favicon_id = thumbnail_db_->AddFavicon( - favicon_usage[i].favicon_url, favicon_base::FAVICON, + favicon_usage[i].favicon_url, favicon_base::IconType::kFavicon, new base::RefCountedBytes(favicon_usage[i].png_data), FaviconBitmapType::ON_VISIT, now, gfx::Size()); } @@ -1864,7 +1912,7 @@ void HistoryBackend::SetImportedFavicons( } } else { if (!thumbnail_db_->GetIconMappingsForPageURL( - *url, favicon_base::FAVICON, nullptr)) { + *url, {favicon_base::IconType::kFavicon}, nullptr)) { // URL is present in history, update the favicon *only* if it is not // set already. thumbnail_db_->AddIconMapping(*url, favicon_id); @@ -1906,10 +1954,9 @@ bool HistoryBackend::SetFaviconsImpl(const base::flat_set<GURL>& page_urls, favicon_data_modified = SetFaviconBitmaps(icon_id, bitmaps, type); } - std::vector<favicon_base::FaviconID> icon_ids(1u, icon_id); for (const GURL& page_url : page_urls) { bool mapping_changed = - SetFaviconMappingsForPageAndRedirects(page_url, icon_type, icon_ids); + SetFaviconMappingsForPageAndRedirects(page_url, icon_type, icon_id); if (mapping_changed) { // Notify the UI that this function changed an icon mapping. @@ -1935,29 +1982,24 @@ void HistoryBackend::UpdateFaviconMappingsAndFetchImpl( std::vector<favicon_base::FaviconRawBitmapResult>* bitmap_results) { bitmap_results->clear(); - if (!thumbnail_db_) { + if (!thumbnail_db_) return; - } - - std::vector<favicon_base::FaviconID> favicon_ids; const favicon_base::FaviconID favicon_id = thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type); - if (favicon_id) - favicon_ids.push_back(favicon_id); - - if (!favicon_ids.empty()) { - for (const GURL& page_url : page_urls) { - bool mappings_updated = SetFaviconMappingsForPageAndRedirects( - page_url, icon_type, favicon_ids); - if (mappings_updated) { - SendFaviconChangedNotificationForPageAndRedirects(page_url); - ScheduleCommit(); - } + if (!favicon_id) + return; + + for (const GURL& page_url : page_urls) { + bool mappings_updated = + SetFaviconMappingsForPageAndRedirects(page_url, icon_type, favicon_id); + if (mappings_updated) { + SendFaviconChangedNotificationForPageAndRedirects(page_url); + ScheduleCommit(); } } - GetFaviconBitmapResultsForBestMatch(favicon_ids, desired_sizes, + GetFaviconBitmapResultsForBestMatch({favicon_id}, desired_sizes, bitmap_results); } @@ -2037,7 +2079,7 @@ bool HistoryBackend::IsFaviconBitmapDataEqual( bool HistoryBackend::GetFaviconsFromDB( const GURL& page_url, - int icon_types, + const favicon_base::IconTypeSet& icon_types, const std::vector<int>& desired_sizes, std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results) { DCHECK(favicon_bitmap_results); @@ -2139,16 +2181,17 @@ bool HistoryBackend::GetFaviconBitmapResultsForBestMatch( bool HistoryBackend::SetFaviconMappingsForPageAndRedirects( const GURL& page_url, favicon_base::IconType icon_type, - const std::vector<favicon_base::FaviconID>& icon_ids) { + favicon_base::FaviconID icon_id) { if (!thumbnail_db_) return false; // Find all the pages whose favicons we should set, we want to set it for // all the pages in the redirect chain if it redirected. - RedirectList redirects; - GetCachedRecentRedirects(page_url, &redirects); - bool mappings_changed = SetFaviconMappingsForPages(redirects, icon_type, - icon_ids); + RedirectList redirects = GetCachedRecentRedirects(page_url); + bool mappings_changed = + !SetFaviconMappingsForPages(base::flat_set<GURL>(redirects), icon_type, + icon_id) + .empty(); if (page_url.has_ref() && !base::FeatureList::IsEnabled(kAvoidStrippingRefFromFaviconPageUrls)) { // Refs often gets added by Javascript, but the redirect chain is keyed to @@ -2158,63 +2201,63 @@ bool HistoryBackend::SetFaviconMappingsForPageAndRedirects( GURL::Replacements replacements; replacements.ClearRef(); GURL page_url_without_ref = page_url.ReplaceComponents(replacements); - GetCachedRecentRedirects(page_url_without_ref, &redirects); - mappings_changed |= SetFaviconMappingsForPages(redirects, icon_type, - icon_ids); + redirects = GetCachedRecentRedirects(page_url_without_ref); + mappings_changed |= + !SetFaviconMappingsForPages(base::flat_set<GURL>(redirects), icon_type, + icon_id) + .empty(); } return mappings_changed; } -bool HistoryBackend::SetFaviconMappingsForPages( - const std::vector<GURL>& page_urls, +std::vector<GURL> HistoryBackend::SetFaviconMappingsForPages( + const base::flat_set<GURL>& page_urls, favicon_base::IconType icon_type, - const std::vector<favicon_base::FaviconID>& icon_ids) { - bool mappings_changed = false; - for (auto i(page_urls.begin()); i != page_urls.end(); ++i) { - mappings_changed |= SetFaviconMappingsForPage(*i, icon_type, icon_ids); + favicon_base::FaviconID icon_id) { + std::vector<GURL> changed_page_urls; + for (const GURL& page_url : page_urls) { + if (SetFaviconMappingsForPage(page_url, icon_type, icon_id)) + changed_page_urls.push_back(page_url); } - return mappings_changed; + return changed_page_urls; } bool HistoryBackend::SetFaviconMappingsForPage( const GURL& page_url, favicon_base::IconType icon_type, - const std::vector<favicon_base::FaviconID>& icon_ids) { - DCHECK_LE(icon_ids.size(), kMaxFaviconsPerPage); + favicon_base::FaviconID icon_id) { bool mappings_changed = false; // Two icon types are considered 'equivalent' if both types are one of - // TOUCH_ICON, TOUCH_PRECOMPOSED_ICON or WEB_MANIFEST_ICON. - const int equivalent_types = favicon_base::TOUCH_ICON | - favicon_base::TOUCH_PRECOMPOSED_ICON | - favicon_base::WEB_MANIFEST_ICON; - - // Sets the icon mappings from |page_url| for |icon_type| to the favicons - // with |icon_ids|. Mappings for |page_url| to favicons of type |icon_type| - // whose FaviconID is not in |icon_ids| are removed. All icon mappings for + // kTouchIcon, kTouchPrecomposedIcon or kWebManifestIcon. + const favicon_base::IconTypeSet equivalent_types = { + favicon_base::IconType::kTouchIcon, + favicon_base::IconType::kTouchPrecomposedIcon, + favicon_base::IconType::kWebManifestIcon}; + + // Sets the icon mappings from |page_url| for |icon_type| to the favicon + // with |icon_id|. Mappings for |page_url| to favicons of type |icon_type| + // with FaviconID other than |icon_id| are removed. All icon mappings for // |page_url| to favicons of a type equivalent to |icon_type| are removed. // Remove any favicons which are orphaned as a result of the removal of the // icon mappings. - std::vector<favicon_base::FaviconID> unmapped_icon_ids = icon_ids; + favicon_base::FaviconID unmapped_icon_id = icon_id; std::vector<IconMapping> icon_mappings; thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings); for (std::vector<IconMapping>::iterator m = icon_mappings.begin(); m != icon_mappings.end(); ++m) { - std::vector<favicon_base::FaviconID>::iterator icon_id_it = std::find( - unmapped_icon_ids.begin(), unmapped_icon_ids.end(), m->icon_id); - - // If the icon mapping already exists, avoid removing it and adding it back. - if (icon_id_it != unmapped_icon_ids.end()) { - unmapped_icon_ids.erase(icon_id_it); + if (unmapped_icon_id == m->icon_id) { + unmapped_icon_id = 0; continue; } - if (icon_type == m->icon_type || ((icon_type & equivalent_types) != 0 && - (m->icon_type & equivalent_types) != 0)) { + if (icon_type == m->icon_type || + (equivalent_types.count(icon_type) != 0 && + equivalent_types.count(m->icon_type) != 0)) { thumbnail_db_->DeleteIconMapping(m->mapping_id); // Removing the icon mapping may have orphaned the associated favicon so @@ -2227,32 +2270,28 @@ bool HistoryBackend::SetFaviconMappingsForPage( } } - for (size_t i = 0; i < unmapped_icon_ids.size(); ++i) { - thumbnail_db_->AddIconMapping(page_url, unmapped_icon_ids[i]); + if (unmapped_icon_id) { + thumbnail_db_->AddIconMapping(page_url, unmapped_icon_id); mappings_changed = true; } return mappings_changed; } -void HistoryBackend::GetCachedRecentRedirects(const GURL& page_url, - RedirectList* redirect_list) { +RedirectList HistoryBackend::GetCachedRecentRedirects(const GURL& page_url) { RedirectCache::iterator iter = recent_redirects_.Get(page_url); if (iter != recent_redirects_.end()) { - *redirect_list = iter->second; - // The redirect chain should have the destination URL as the last item. - DCHECK(!redirect_list->empty()); - DCHECK_EQ(redirect_list->back(), page_url); - } else { - // No known redirects, construct mock redirect chain containing |page_url|. - redirect_list->push_back(page_url); + DCHECK(!iter->second.empty()); + DCHECK_EQ(iter->second.back(), page_url); + return iter->second; } + // No known redirects, construct mock redirect chain containing |page_url|. + return RedirectList{page_url}; } void HistoryBackend::SendFaviconChangedNotificationForPageAndRedirects( const GURL& page_url) { - RedirectList redirect_list; - GetCachedRecentRedirects(page_url, &redirect_list); + RedirectList redirect_list = GetCachedRecentRedirects(page_url); if (!redirect_list.empty()) { std::set<GURL> favicons_changed(redirect_list.begin(), redirect_list.end()); @@ -2488,6 +2527,11 @@ void HistoryBackend::DatabaseErrorCallback(int error, sql::Statement* stmt) { db_diagnostics_ = db_->GetDiagnosticInfo(error, stmt); + // Notify SyncBridge about storage error. It will report failure to sync + // engine and stop accepting remote updates. + if (typed_url_sync_bridge_) + typed_url_sync_bridge_->OnDatabaseError(); + // Don't just do the close/delete here, as we are being called by |db| and // that seems dangerous. // TODO(shess): Consider changing KillHistoryDatabase() to use diff --git a/chromium/components/history/core/browser/history_backend.h b/chromium/components/history/core/browser/history_backend.h index 7dc0043d7c1..5e162627a4f 100644 --- a/chromium/components/history/core/browser/history_backend.h +++ b/chromium/components/history/core/browser/history_backend.h @@ -59,10 +59,6 @@ class TypedUrlSyncableService; class HistoryBackendHelper; class URLDatabase; -// The maximum number of icons URLs per page which can be stored in the -// thumbnail database. -static const size_t kMaxFaviconsPerPage = 8; - // The maximum number of bitmaps for a single icon URL which can be stored in // the thumbnail database. static const size_t kMaxFaviconBitmapsPerIconURL = 8; @@ -297,13 +293,13 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, void GetLargestFaviconForURL( const GURL& page_url, - const std::vector<int>& icon_types, + const std::vector<favicon_base::IconTypeSet>& icon_types_list, int minimum_size_in_pixels, favicon_base::FaviconRawBitmapResult* bitmap_result); void GetFaviconsForURL( const GURL& page_url, - int icon_types, + const favicon_base::IconTypeSet& icon_types, const std::vector<int>& desired_sizes, std::vector<favicon_base::FaviconRawBitmapResult>* bitmap_results); @@ -319,6 +315,9 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, const std::vector<int>& desired_sizes, std::vector<favicon_base::FaviconRawBitmapResult>* bitmap_results); + void DeleteFaviconMappings(const base::flat_set<GURL>& page_urls, + favicon_base::IconType icon_type); + void MergeFavicon(const GURL& page_url, const GURL& icon_url, favicon_base::IconType icon_type, @@ -331,6 +330,11 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, const GURL& icon_url, const std::vector<SkBitmap>& bitmaps); + void CloneFaviconMappingsForPages( + const GURL& page_url_to_read, + const favicon_base::IconTypeSet& icon_types, + const base::flat_set<GURL>& page_urls_to_write); + bool SetOnDemandFavicons(const GURL& page_url, favicon_base::IconType icon_type, const GURL& icon_url, @@ -568,6 +572,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, NoFaviconChangedNotifications); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, UpdateFaviconMappingsAndFetchMultipleIconTypes); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, CloneFaviconMappingsForPages); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconsFromDBEmpty); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconsFromDBNoFaviconBitmaps); @@ -623,6 +628,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, base::Time time, VisitID referring_visit, ui::PageTransition transition, + bool hidden, VisitSource visit_source); // Returns a redirect chain in |redirects| for the VisitID @@ -735,12 +741,12 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // largest favicon bitmap with one of the icon types in |icon_types| is // returned. If |icon_types| contains multiple icon types and there are // several matched icon types in the database, results will only be returned - // for a single icon type in the priority of TOUCH_PRECOMPOSED_ICON, - // TOUCH_ICON, and FAVICON. See the comment for + // for a single icon type in the priority of kTouchPrecomposedIcon, + // kTouchIcon, and kFavicon. See the comment for // GetFaviconResultsForBestMatch() for more details on how // |favicon_bitmap_results| is constructed. bool GetFaviconsFromDB(const GURL& page_url, - int icon_types, + const favicon_base::IconTypeSet& icon_types, const std::vector<int>& desired_sizes, std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results); @@ -760,34 +766,33 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results); - // Maps the favicon ids in |icon_ids| to |page_url| (and all redirects) - // for |icon_type|. + // Maps the favicon ID |icon_id| to |page_url| (and all redirects) for + // |icon_type|. |icon_id| == 0 deletes previously existing mappings. // Returns true if the mappings for the page or any of its redirects were // changed. - bool SetFaviconMappingsForPageAndRedirects( - const GURL& page_url, - favicon_base::IconType icon_type, - const std::vector<favicon_base::FaviconID>& icon_ids); - - // Maps the favicon ids in |icon_ids| to URLs in |page_urls| for |icon_type|. - // Returns true if the function changed at least one of the |page_urls| - // mappings. - bool SetFaviconMappingsForPages( - const std::vector<GURL>& page_urls, + bool SetFaviconMappingsForPageAndRedirects(const GURL& page_url, + favicon_base::IconType icon_type, + favicon_base::FaviconID icon_id); + + // Maps the favicon ID |icon_id| to URLs in |page_urls| for |icon_type|. + // |icon_id| == 0 deletes previously existing mappings. + // Returns page URLs among |page_urls| whose mappings were changed (might be + // empty). + std::vector<GURL> SetFaviconMappingsForPages( + const base::flat_set<GURL>& page_urls, favicon_base::IconType icon_type, - const std::vector<favicon_base::FaviconID>& icon_ids); + favicon_base::FaviconID icon_id); - // Maps the favicon ids in |icon_ids| to |page_url| for |icon_type|. + // Maps the favicon ID |icon_ids| to |page_url| for |icon_type|. + // |icon_id| == 0 deletes previously existing mappings. // Returns true if the function changed at least one of |page_url|'s mappings. - bool SetFaviconMappingsForPage( - const GURL& page_url, - favicon_base::IconType icon_type, - const std::vector<favicon_base::FaviconID>& icon_ids); + bool SetFaviconMappingsForPage(const GURL& page_url, + favicon_base::IconType icon_type, + favicon_base::FaviconID icon_id); // Returns all the page URLs in the redirect chain for |page_url|. If there // are no known redirects for |page_url|, returns a vector with |page_url|. - void GetCachedRecentRedirects(const GURL& page_url, - RedirectList* redirect_list); + RedirectList GetCachedRecentRedirects(const GURL& page_url); // Send notification that the favicon has changed for |page_url| and all its // redirects. This should be called if the mapping between the page URL diff --git a/chromium/components/history/core/browser/history_backend_db_unittest.cc b/chromium/components/history/core/browser/history_backend_db_unittest.cc index 53e128ee7c9..432cb9c5be3 100644 --- a/chromium/components/history/core/browser/history_backend_db_unittest.cc +++ b/chromium/components/history/core/browser/history_backend_db_unittest.cc @@ -38,6 +38,8 @@ #include "components/history/core/browser/page_usage_data.h" #include "components/history/core/test/history_backend_db_base_test.h" #include "components/history/core/test/test_history_database.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" namespace history { namespace { @@ -1330,6 +1332,118 @@ TEST_F(HistoryBackendDBTest, CheckLastCompatibleVersion) { } } +// Tests that visit segment names are recomputed and segments merged when +// migrating to version 37. +TEST_F(HistoryBackendDBTest, MigrateVisitSegmentNames) { + ASSERT_NO_FATAL_FAILURE(CreateDBVersion(32)); + + const SegmentID segment_id1 = 7; + const SegmentID segment_id2 = 8; + const URLID url_id1 = 3; + const URLID url_id2 = 4; + const GURL url1("http://www.foo.com"); + const GURL url2("http://m.foo.com"); + const std::string legacy_segment_name1("http://foo.com/"); + const std::string legacy_segment_name2("http://m.foo.com/"); + const base::string16 title1(base::ASCIIToUTF16("Title1")); + const base::string16 title2(base::ASCIIToUTF16("Title2")); + const base::Time segment_time(base::Time::Now()); + + { + // Open the db for manual manipulation. + sql::Connection db; + ASSERT_TRUE(db.Open(history_dir_.Append(kHistoryFilename))); + + // Add first entry to urls. + { + sql::Statement s( + db.GetUniqueStatement("INSERT INTO urls " + "(id, url, title, last_visit_time) VALUES " + "(?, ?, ?, ?)")); + s.BindInt64(0, url_id1); + s.BindString(1, url1.spec()); + s.BindString16(2, title1); + s.BindInt64(3, segment_time.ToInternalValue()); + ASSERT_TRUE(s.Run()); + } + + // Add first entry to segments. + { + sql::Statement s( + db.GetUniqueStatement("INSERT INTO segments " + "(id, name, url_id) VALUES " + "(?, ?, ?)")); + s.BindInt64(0, segment_id1); + s.BindString(1, legacy_segment_name1); + s.BindInt64(2, url_id1); + ASSERT_TRUE(s.Run()); + } + + // And first to segment_usage. + { + sql::Statement s(db.GetUniqueStatement( + "INSERT INTO segment_usage " + "(id, segment_id, time_slot, visit_count) VALUES " + "(?, ?, ?, ?)")); + s.BindInt64(0, 4); // id. + s.BindInt64(1, segment_id1); + s.BindInt64(2, segment_time.ToInternalValue()); + s.BindInt(3, 11); // visit count. + ASSERT_TRUE(s.Run()); + } + + // Add second entry to urls. + { + sql::Statement s( + db.GetUniqueStatement("INSERT INTO urls " + "(id, url, title, last_visit_time) VALUES " + "(?, ?, ?, ?)")); + s.BindInt64(0, url_id2); + s.BindString(1, url2.spec()); + s.BindString16(2, title2); + s.BindInt64(3, segment_time.ToInternalValue()); + ASSERT_TRUE(s.Run()); + } + + // Add second entry to segments. + { + sql::Statement s( + db.GetUniqueStatement("INSERT INTO segments " + "(id, name, url_id) VALUES " + "(?, ?, ?)")); + s.BindInt64(0, segment_id2); + s.BindString(1, legacy_segment_name2); + s.BindInt64(2, url_id2); + ASSERT_TRUE(s.Run()); + } + + // And second to segment_usage. + { + sql::Statement s(db.GetUniqueStatement( + "INSERT INTO segment_usage " + "(id, segment_id, time_slot, visit_count) VALUES " + "(?, ?, ?, ?)")); + s.BindInt64(0, 5); // id. + s.BindInt64(1, segment_id2); + s.BindInt64(2, segment_time.ToInternalValue()); + s.BindInt(3, 13); // visit count. + ASSERT_TRUE(s.Run()); + } + } + + // Re-open the db, triggering migration. + CreateBackendAndDatabase(); + + std::vector<std::unique_ptr<PageUsageData>> results = + db_->QuerySegmentUsage(segment_time, /*max_result_count=*/10, + base::Callback<bool(const GURL&)>()); + ASSERT_EQ(1u, results.size()); + EXPECT_THAT(results[0]->GetURL(), testing::AnyOf(url1, url2)); + EXPECT_THAT(results[0]->GetTitle(), testing::AnyOf(title1, title2)); + EXPECT_EQ(segment_id1, db_->GetSegmentNamed(legacy_segment_name1)); + EXPECT_EQ(0u, db_->GetSegmentNamed(legacy_segment_name2)); +} + bool FilterURL(const GURL& url) { return url.SchemeIsHTTPOrHTTPS(); } diff --git a/chromium/components/history/core/browser/history_backend_unittest.cc b/chromium/components/history/core/browser/history_backend_unittest.cc index dee14a1639e..5ee081b7ecc 100644 --- a/chromium/components/history/core/browser/history_backend_unittest.cc +++ b/chromium/components/history/core/browser/history_backend_unittest.cc @@ -51,6 +51,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/skia/include/core/SkBitmap.h" +#include "third_party/sqlite/sqlite3.h" #include "ui/gfx/codec/png_codec.h" #include "url/gurl.h" @@ -62,7 +63,10 @@ namespace { using ::testing::ElementsAre; +using ::testing::UnorderedElementsAre; using base::HistogramBase; +using favicon_base::IconType; +using favicon_base::IconTypeSet; const int kTinyEdgeSize = 10; const int kSmallEdgeSize = 16; @@ -266,7 +270,7 @@ class HistoryBackendTestBase : public testing::Test { void TearDown() override { if (backend_.get()) backend_->Closing(); - backend_ = NULL; + backend_ = nullptr; mem_backend_.reset(); base::DeleteFile(test_dir_, true); base::RunLoop().RunUntilIdle(); @@ -352,14 +356,13 @@ class HistoryBackendTest : public HistoryBackendTestBase { ui::PageTransition transition, base::Time time) { history::RedirectList redirects; - for (int i = 0; sequence[i] != NULL; ++i) + for (int i = 0; sequence[i] != nullptr; ++i) redirects.push_back(GURL(sequence[i])); ContextID context_id = reinterpret_cast<ContextID>(1); history::HistoryAddPageArgs request( - redirects.back(), time, context_id, nav_entry_id, GURL(), - redirects, transition, history::SOURCE_BROWSED, - true, true); + redirects.back(), time, context_id, nav_entry_id, GURL(), redirects, + transition, false, history::SOURCE_BROWSED, true, true); backend_->AddPage(request); } @@ -382,10 +385,9 @@ class HistoryBackendTest : public HistoryBackendTestBase { redirects.push_back(url1); if (url2.is_valid()) redirects.push_back(url2); - HistoryAddPageArgs request( - url2, time, dummy_context_id, 0, url1, - redirects, ui::PAGE_TRANSITION_CLIENT_REDIRECT, - history::SOURCE_BROWSED, did_replace, true); + HistoryAddPageArgs request(url2, time, dummy_context_id, 0, url1, redirects, + ui::PAGE_TRANSITION_CLIENT_REDIRECT, false, + history::SOURCE_BROWSED, did_replace, true); backend_->AddPage(request); if (transition1) @@ -414,29 +416,19 @@ class HistoryBackendTest : public HistoryBackendTestBase { } // Returns the number of icon mappings of |icon_type| to |page_url|. - size_t NumIconMappingsForPageURL(const GURL& page_url, - favicon_base::IconType icon_type) { + size_t NumIconMappingsForPageURL(const GURL& page_url, IconType icon_type) { std::vector<IconMapping> icon_mappings; - backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, icon_type, + backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, {icon_type}, &icon_mappings); return icon_mappings.size(); } - // Returns the icon mappings for |page_url| sorted alphabetically by icon - // URL in ascending order. Returns true if there is at least one icon - // mapping. - bool GetSortedIconMappingsForPageURL( - const GURL& page_url, - std::vector<IconMapping>* icon_mappings) { - if (!backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, - icon_mappings)) { - return false; - } - std::sort(icon_mappings->begin(), icon_mappings->end(), - [](const history::IconMapping& a, const history::IconMapping& b) { - return a.icon_url < b.icon_url; - }); - return true; + // Returns the icon mappings for |page_url|. + std::vector<IconMapping> GetIconMappingsForPageURL(const GURL& page_url) { + std::vector<IconMapping> icon_mappings; + backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, + &icon_mappings); + return icon_mappings; } // Returns the favicon bitmaps for |icon_id| sorted by pixel size in @@ -503,8 +495,8 @@ class InMemoryHistoryBackendTest : public HistoryBackendTestBase { protected: void SimulateNotificationURLsDeleted(const URLRow* row1, - const URLRow* row2 = NULL, - const URLRow* row3 = NULL) { + const URLRow* row2 = nullptr, + const URLRow* row3 = nullptr) { URLRows rows; rows.push_back(*row1); if (row2) rows.push_back(*row2); @@ -591,9 +583,9 @@ TEST_F(HistoryBackendTest, DeleteAll) { GURL favicon_url1("http://www.google.com/favicon.ico"); GURL favicon_url2("http://news.google.com/favicon.ico"); favicon_base::FaviconID favicon2 = - backend_->thumbnail_db_->AddFavicon(favicon_url2, favicon_base::FAVICON); + backend_->thumbnail_db_->AddFavicon(favicon_url2, IconType::kFavicon); favicon_base::FaviconID favicon1 = - backend_->thumbnail_db_->AddFavicon(favicon_url1, favicon_base::FAVICON); + backend_->thumbnail_db_->AddFavicon(favicon_url1, IconType::kFavicon); std::vector<unsigned char> data; data.push_back('a'); @@ -631,8 +623,8 @@ TEST_F(HistoryBackendTest, DeleteAll) { rows.push_back(row1); backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); - URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL); - URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL); + URLID row1_id = backend_->db_->GetRowForURL(row1.url(), nullptr); + URLID row2_id = backend_->db_->GetRowForURL(row2.url(), nullptr); // Get the two visits for the URLs we just added. VisitVector visits; @@ -647,7 +639,7 @@ TEST_F(HistoryBackendTest, DeleteAll) { // typed URL. ASSERT_TRUE(mem_backend_.get()); URLRow outrow1; - EXPECT_TRUE(mem_backend_->db_->GetRowForURL(row1.url(), NULL)); + EXPECT_TRUE(mem_backend_->db_->GetRowForURL(row1.url(), nullptr)); // Star row1. history_client_.AddBookmark(row1.url()); @@ -677,7 +669,7 @@ TEST_F(HistoryBackendTest, DeleteAll) { // look them up by favicon URL since the IDs may have changed. favicon_base::FaviconID out_favicon1 = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(favicon_url1, - favicon_base::FAVICON); + IconType::kFavicon); EXPECT_TRUE(out_favicon1); std::vector<FaviconBitmap> favicon_bitmaps; @@ -703,14 +695,14 @@ TEST_F(HistoryBackendTest, DeleteAll) { favicon_base::FaviconID out_favicon2 = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(favicon_url2, - favicon_base::FAVICON); + IconType::kFavicon); EXPECT_FALSE(out_favicon2) << "Favicon not deleted"; // The remaining URL should still reference the same favicon, even if its // ID has changed. std::vector<IconMapping> mappings; EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - outrow1.url(), favicon_base::FAVICON, &mappings)); + outrow1.url(), {IconType::kFavicon}, &mappings)); EXPECT_EQ(1u, mappings.size()); EXPECT_EQ(out_favicon1, mappings[0].icon_id); @@ -744,14 +736,14 @@ TEST_F(HistoryBackendTest, DeleteAllURLPreviouslyDeleted) { std::vector<unsigned char> data; data.push_back('a'); favicon_base::FaviconID favicon = backend_->thumbnail_db_->AddFavicon( - kFaviconURL, favicon_base::FAVICON, new base::RefCountedBytes(data), + kFaviconURL, IconType::kFavicon, new base::RefCountedBytes(data), FaviconBitmapType::ON_VISIT, base::Time::Now(), kSmallSize); backend_->thumbnail_db_->AddIconMapping(row.url(), favicon); history_client_.AddBookmark(kPageURL); // Test initial state. - URLID row_id = backend_->db_->GetRowForURL(kPageURL, NULL); + URLID row_id = backend_->db_->GetRowForURL(kPageURL, nullptr); ASSERT_NE(0, row_id); VisitVector visits; backend_->db_->GetVisitsForURL(row_id, &visits); @@ -759,7 +751,7 @@ TEST_F(HistoryBackendTest, DeleteAllURLPreviouslyDeleted) { std::vector<IconMapping> icon_mappings; ASSERT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - kPageURL, favicon_base::FAVICON, &icon_mappings)); + kPageURL, {IconType::kFavicon}, &icon_mappings)); ASSERT_EQ(1u, icon_mappings.size()); // Delete information for |kPageURL|, then clear all browsing data. @@ -768,11 +760,11 @@ TEST_F(HistoryBackendTest, DeleteAllURLPreviouslyDeleted) { // Test that the entry in the url table for the bookmark is gone but that the // favicon data for the bookmark is still there. - ASSERT_EQ(0, backend_->db_->GetRowForURL(kPageURL, NULL)); + ASSERT_EQ(0, backend_->db_->GetRowForURL(kPageURL, nullptr)); icon_mappings.clear(); EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - kPageURL, favicon_base::FAVICON, &icon_mappings)); + kPageURL, {IconType::kFavicon}, &icon_mappings)); EXPECT_EQ(1u, icon_mappings.size()); } @@ -784,9 +776,9 @@ TEST_F(HistoryBackendTest, DeleteAllThenAddData) { base::Time visit_time = base::Time::Now(); GURL url("http://www.google.com/"); - HistoryAddPageArgs request(url, visit_time, NULL, 0, GURL(), + HistoryAddPageArgs request(url, visit_time, nullptr, 0, GURL(), history::RedirectList(), - ui::PAGE_TRANSITION_KEYWORD_GENERATED, + ui::PAGE_TRANSITION_KEYWORD_GENERATED, false, history::SOURCE_BROWSED, false, true); backend_->AddPage(request); @@ -830,12 +822,12 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { std::vector<unsigned char> data; data.push_back('1'); favicon_base::FaviconID favicon1 = backend_->thumbnail_db_->AddFavicon( - favicon_url1, favicon_base::FAVICON, new base::RefCountedBytes(data), + favicon_url1, IconType::kFavicon, new base::RefCountedBytes(data), FaviconBitmapType::ON_VISIT, base::Time::Now(), gfx::Size()); data[0] = '2'; favicon_base::FaviconID favicon2 = backend_->thumbnail_db_->AddFavicon( - favicon_url2, favicon_base::FAVICON, new base::RefCountedBytes(data), + favicon_url2, IconType::kFavicon, new base::RefCountedBytes(data), FaviconBitmapType::ON_VISIT, base::Time::Now(), gfx::Size()); // First visit two URLs. @@ -855,8 +847,8 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { rows.push_back(row1); backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); - URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL); - URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL); + URLID row1_id = backend_->db_->GetRowForURL(row1.url(), nullptr); + URLID row2_id = backend_->db_->GetRowForURL(row2.url(), nullptr); // Star the two URLs. history_client_.AddBookmark(row1.url()); @@ -864,13 +856,13 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { // Delete url 2. backend_->expirer_.DeleteURL(row2.url()); - EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), NULL)); + EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), nullptr)); VisitVector visits; backend_->db_->GetVisitsForURL(row2_id, &visits); EXPECT_EQ(0U, visits.size()); // The favicon should still be valid. EXPECT_EQ(favicon2, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - favicon_url2, favicon_base::FAVICON)); + favicon_url2, IconType::kFavicon)); // Unstar row2. history_client_.DelBookmark(row2.url()); @@ -882,10 +874,10 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { backend_->URLsNoLongerBookmarked(unstarred_urls); // The URL should still not exist. - EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), NULL)); + EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), nullptr)); // And the favicon should be deleted. EXPECT_EQ(0, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - favicon_url2, favicon_base::FAVICON)); + favicon_url2, IconType::kFavicon)); // Unstar row 1. history_client_.DelBookmark(row1.url()); @@ -897,7 +889,7 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { backend_->URLsNoLongerBookmarked(unstarred_urls); // The URL should still exist (because there were visits). - EXPECT_EQ(row1_id, backend_->db_->GetRowForURL(row1.url(), NULL)); + EXPECT_EQ(row1_id, backend_->db_->GetRowForURL(row1.url(), nullptr)); // There should still be visits. visits.clear(); @@ -906,7 +898,7 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { // The favicon should still be valid. EXPECT_EQ(favicon1, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - favicon_url1, favicon_base::FAVICON)); + favicon_url1, IconType::kFavicon)); } // Tests a handful of assertions for a navigation with a type of @@ -917,9 +909,9 @@ TEST_F(HistoryBackendTest, KeywordGenerated) { GURL url("http://google.com"); base::Time visit_time = base::Time::Now() - base::TimeDelta::FromDays(1); - HistoryAddPageArgs request(url, visit_time, NULL, 0, GURL(), + HistoryAddPageArgs request(url, visit_time, nullptr, 0, GURL(), history::RedirectList(), - ui::PAGE_TRANSITION_KEYWORD_GENERATED, + ui::PAGE_TRANSITION_KEYWORD_GENERATED, false, history::SOURCE_BROWSED, false, true); backend_->AddPage(request); @@ -950,9 +942,9 @@ TEST_F(HistoryBackendTest, KeywordGenerated) { // Going back to the same entry should not increment the typed count. ui::PageTransition back_transition = ui::PageTransitionFromInt( ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FORWARD_BACK); - HistoryAddPageArgs back_request(url, visit_time, NULL, 0, GURL(), + HistoryAddPageArgs back_request(url, visit_time, nullptr, 0, GURL(), history::RedirectList(), back_transition, - history::SOURCE_BROWSED, false, true); + false, history::SOURCE_BROWSED, false, true); backend_->AddPage(back_request); url_id = backend_->db()->GetRowForURL(url, &row); ASSERT_NE(0, url_id); @@ -1173,7 +1165,7 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) { std::vector<unsigned char> data; data.push_back('1'); favicon_base::FaviconID favicon1 = backend_->thumbnail_db_->AddFavicon( - favicon_url1, favicon_base::FAVICON, + favicon_url1, IconType::kFavicon, base::RefCountedBytes::TakeVector(&data), FaviconBitmapType::ON_VISIT, base::Time::Now(), gfx::Size()); URLRow row1(GURL("http://www.google.com/")); @@ -1191,8 +1183,8 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) { 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); - EXPECT_EQ(1u, NumIconMappingsForPageURL(row1.url(), favicon_base::FAVICON)); - EXPECT_EQ(0u, NumIconMappingsForPageURL(row2.url(), favicon_base::FAVICON)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(row1.url(), IconType::kFavicon)); + EXPECT_EQ(0u, NumIconMappingsForPageURL(row2.url(), IconType::kFavicon)); // Now provide one imported favicon for both URLs already in the registry. // The new favicon should only be used with the URL that doesn't already have @@ -1210,14 +1202,14 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) { std::vector<IconMapping> mappings; EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - row1.url(), favicon_base::FAVICON, &mappings)); + row1.url(), {IconType::kFavicon}, &mappings)); EXPECT_EQ(1u, mappings.size()); EXPECT_EQ(favicon1, mappings[0].icon_id); EXPECT_EQ(favicon_url1, mappings[0].icon_url); mappings.clear(); EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - row2.url(), favicon_base::FAVICON, &mappings)); + row2.url(), {IconType::kFavicon}, &mappings)); EXPECT_EQ(1u, mappings.size()); EXPECT_EQ(favicon.favicon_url, mappings[0].icon_url); @@ -1251,14 +1243,15 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) { backend_->DeleteAllHistory(); // Visit the url with username, password. - backend_->AddPageVisit(url, base::Time::Now(), 0, + backend_->AddPageVisit( + url, base::Time::Now(), 0, ui::PageTransitionFromInt( ui::PageTransitionGetQualifier(ui::PAGE_TRANSITION_TYPED)), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); // Fetch the row information about stripped url from history db. VisitVector visits; - URLID row_id = backend_->db_->GetRowForURL(stripped_url, NULL); + URLID row_id = backend_->db_->GetRowForURL(stripped_url, nullptr); backend_->db_->GetVisitsForURL(row_id, &visits); // Check if stripped url is stored in database. @@ -1274,9 +1267,8 @@ TEST_F(HistoryBackendTest, AddPageVisitBackForward) { backend_->DeleteAllHistory(); // Visit the url after typing it. - backend_->AddPageVisit(url, base::Time::Now(), 0, - ui::PAGE_TRANSITION_TYPED, - history::SOURCE_BROWSED); + backend_->AddPageVisit(url, base::Time::Now(), 0, ui::PAGE_TRANSITION_TYPED, + false, history::SOURCE_BROWSED); // Ensure both the typed count and visit count are 1. VisitVector visits; @@ -1287,10 +1279,11 @@ TEST_F(HistoryBackendTest, AddPageVisitBackForward) { EXPECT_EQ(1, row.visit_count()); // Visit the url again via back/forward. - backend_->AddPageVisit(url, base::Time::Now(), 0, - ui::PageTransitionFromInt( - ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FORWARD_BACK), - history::SOURCE_BROWSED); + backend_->AddPageVisit( + url, base::Time::Now(), 0, + ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED | + ui::PAGE_TRANSITION_FORWARD_BACK), + false, history::SOURCE_BROWSED); // Ensure the typed count is still 1 but the visit count is 2. id = backend_->db()->GetRowForURL(url, &row); @@ -1309,13 +1302,13 @@ TEST_F(HistoryBackendTest, AddPageVisitRedirectBackForward) { backend_->DeleteAllHistory(); // Visit a typed URL with a redirect. - backend_->AddPageVisit(url1, base::Time::Now(), 0, - ui::PAGE_TRANSITION_TYPED, - history::SOURCE_BROWSED); - backend_->AddPageVisit(url2, base::Time::Now(), 0, - ui::PageTransitionFromInt( - ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_CLIENT_REDIRECT), - history::SOURCE_BROWSED); + backend_->AddPageVisit(url1, base::Time::Now(), 0, ui::PAGE_TRANSITION_TYPED, + false, history::SOURCE_BROWSED); + backend_->AddPageVisit( + url2, base::Time::Now(), 0, + ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED | + ui::PAGE_TRANSITION_CLIENT_REDIRECT), + false, history::SOURCE_BROWSED); // Ensure the redirected URL does not count as typed. VisitVector visits; @@ -1326,12 +1319,12 @@ TEST_F(HistoryBackendTest, AddPageVisitRedirectBackForward) { EXPECT_EQ(1, row.visit_count()); // Visit the redirected url again via back/forward. - backend_->AddPageVisit(url2, base::Time::Now(), 0, - ui::PageTransitionFromInt( - ui::PAGE_TRANSITION_TYPED | - ui::PAGE_TRANSITION_FORWARD_BACK | - ui::PAGE_TRANSITION_CLIENT_REDIRECT), - history::SOURCE_BROWSED); + backend_->AddPageVisit( + url2, base::Time::Now(), 0, + ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED | + ui::PAGE_TRANSITION_FORWARD_BACK | + ui::PAGE_TRANSITION_CLIENT_REDIRECT), + false, history::SOURCE_BROWSED); // Ensure the typed count is still 1 but the visit count is 2. id = backend_->db()->GetRowForURL(url2, &row); @@ -1349,21 +1342,18 @@ TEST_F(HistoryBackendTest, AddPageVisitSource) { backend_->DeleteAllHistory(); // Assume visiting the url from an externsion. - backend_->AddPageVisit( - url, base::Time::Now(), 0, ui::PAGE_TRANSITION_TYPED, - history::SOURCE_EXTENSION); + backend_->AddPageVisit(url, base::Time::Now(), 0, ui::PAGE_TRANSITION_TYPED, + false, history::SOURCE_EXTENSION); // Assume the url is imported from Firefox. - backend_->AddPageVisit(url, base::Time::Now(), 0, - ui::PAGE_TRANSITION_TYPED, - history::SOURCE_FIREFOX_IMPORTED); + backend_->AddPageVisit(url, base::Time::Now(), 0, ui::PAGE_TRANSITION_TYPED, + false, history::SOURCE_FIREFOX_IMPORTED); // Assume this url is also synced. - backend_->AddPageVisit(url, base::Time::Now(), 0, - ui::PAGE_TRANSITION_TYPED, - history::SOURCE_SYNCED); + backend_->AddPageVisit(url, base::Time::Now(), 0, ui::PAGE_TRANSITION_TYPED, + false, history::SOURCE_SYNCED); // Fetch the row information about the url from history db. VisitVector visits; - URLID row_id = backend_->db_->GetRowForURL(url, NULL); + URLID row_id = backend_->db_->GetRowForURL(url, nullptr); backend_->db_->GetVisitsForURL(row_id, &visits); // Check if all the visits to the url are stored in database. @@ -1403,17 +1393,19 @@ TEST_F(HistoryBackendTest, AddPageVisitNotLastVisit) { base::Time older_time = recent_time - visit_age; // Visit the url with recent time. - backend_->AddPageVisit(url, recent_time, 0, + backend_->AddPageVisit( + url, recent_time, 0, ui::PageTransitionFromInt( ui::PageTransitionGetQualifier(ui::PAGE_TRANSITION_TYPED)), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); // Add to the url a visit with older time (could be syncing from another // client, etc.). - backend_->AddPageVisit(url, older_time, 0, + backend_->AddPageVisit( + url, older_time, 0, ui::PageTransitionFromInt( ui::PageTransitionGetQualifier(ui::PAGE_TRANSITION_TYPED)), - history::SOURCE_SYNCED); + false, history::SOURCE_SYNCED); // Fetch the row information about url from history db. VisitVector visits; @@ -1438,12 +1430,11 @@ TEST_F(HistoryBackendTest, AddPageVisitFiresNotificationWithCorrectDetails) { ClearBroadcastedNotifications(); // Visit two distinct URLs, the second one twice. - backend_->AddPageVisit(url1, base::Time::Now(), 0, - ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED); + backend_->AddPageVisit(url1, base::Time::Now(), 0, ui::PAGE_TRANSITION_LINK, + false, history::SOURCE_BROWSED); for (int i = 0; i < 2; ++i) { backend_->AddPageVisit(url2, base::Time::Now(), 0, - ui::PAGE_TRANSITION_TYPED, + ui::PAGE_TRANSITION_TYPED, false, history::SOURCE_BROWSED); } @@ -1476,22 +1467,20 @@ TEST_F(HistoryBackendTest, AddPageArgsSource) { GURL url("http://testpageargs.com"); // Assume this page is browsed by user. - HistoryAddPageArgs request1(url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_KEYWORD_GENERATED, - history::SOURCE_BROWSED, false, true); + HistoryAddPageArgs request1(url, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), + ui::PAGE_TRANSITION_KEYWORD_GENERATED, false, + history::SOURCE_BROWSED, false, true); backend_->AddPage(request1); // Assume this page is synced. - HistoryAddPageArgs request2(url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_LINK, - history::SOURCE_SYNCED, false, true); + HistoryAddPageArgs request2(url, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + false, history::SOURCE_SYNCED, false, true); backend_->AddPage(request2); // Assume this page is browsed again. - HistoryAddPageArgs request3(url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_TYPED, - history::SOURCE_BROWSED, false, true); + HistoryAddPageArgs request3( + url, base::Time::Now(), nullptr, 0, GURL(), history::RedirectList(), + ui::PAGE_TRANSITION_TYPED, false, history::SOURCE_BROWSED, false, true); backend_->AddPage(request3); // Three visits should be added with proper sources. @@ -1687,7 +1676,7 @@ TEST_F(HistoryBackendTest, RemoveVisitsSource) { TEST_F(HistoryBackendTest, MigrationVisitSource) { ASSERT_TRUE(backend_.get()); backend_->Closing(); - backend_ = NULL; + backend_ = nullptr; base::FilePath old_history_path; ASSERT_TRUE(GetTestDataHistoryDir(&old_history_path)); @@ -1706,7 +1695,7 @@ TEST_F(HistoryBackendTest, MigrationVisitSource) { base::ThreadTaskRunnerHandle::Get()); backend_->Init(false, TestHistoryDatabaseParamsForPath(new_history_path)); backend_->Closing(); - backend_ = NULL; + backend_ = nullptr; // Now the database should already be migrated. // Check version first. @@ -1759,57 +1748,51 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageAndRedirects) { bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); // Add a favicon. - backend_->SetFavicons({url1}, favicon_base::FAVICON, icon_url1, bitmaps); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, favicon_base::FAVICON)); + backend_->SetFavicons({url1}, IconType::kFavicon, icon_url1, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, IconType::kFavicon)); // Add one touch_icon - backend_->SetFavicons({url1}, favicon_base::TOUCH_ICON, icon_url1, bitmaps); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::TOUCH_ICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, favicon_base::TOUCH_ICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); + backend_->SetFavicons({url1}, IconType::kTouchIcon, icon_url1, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kTouchIcon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, IconType::kTouchIcon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); - // Add one TOUCH_PRECOMPOSED_ICON - backend_->SetFavicons({url1}, favicon_base::TOUCH_PRECOMPOSED_ICON, icon_url1, + // Add one kTouchPrecomposedIcon + backend_->SetFavicons({url1}, IconType::kTouchPrecomposedIcon, icon_url1, bitmaps); // The touch_icon was replaced. - EXPECT_EQ(0u, NumIconMappingsForPageURL(url1, favicon_base::TOUCH_ICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - EXPECT_EQ( - 1u, - NumIconMappingsForPageURL(url1, favicon_base::TOUCH_PRECOMPOSED_ICON)); - EXPECT_EQ( - 1u, - NumIconMappingsForPageURL(url2, favicon_base::TOUCH_PRECOMPOSED_ICON)); + EXPECT_EQ(0u, NumIconMappingsForPageURL(url1, IconType::kTouchIcon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + EXPECT_EQ(1u, + NumIconMappingsForPageURL(url1, IconType::kTouchPrecomposedIcon)); + EXPECT_EQ(1u, + NumIconMappingsForPageURL(url2, IconType::kTouchPrecomposedIcon)); // Add a touch_icon. - backend_->SetFavicons({url1}, favicon_base::TOUCH_ICON, icon_url1, bitmaps); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::TOUCH_ICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - // The TOUCH_PRECOMPOSED_ICON was replaced. - EXPECT_EQ( - 0u, - NumIconMappingsForPageURL(url1, favicon_base::TOUCH_PRECOMPOSED_ICON)); + backend_->SetFavicons({url1}, IconType::kTouchIcon, icon_url1, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kTouchIcon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + // The kTouchPrecomposedIcon was replaced. + EXPECT_EQ(0u, + NumIconMappingsForPageURL(url1, IconType::kTouchPrecomposedIcon)); // Add a web manifest_icon. - backend_->SetFavicons({url1}, favicon_base::WEB_MANIFEST_ICON, icon_url2, - bitmaps); - EXPECT_EQ(1u, - NumIconMappingsForPageURL(url1, favicon_base::WEB_MANIFEST_ICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - // The TOUCH_ICON_ICON was replaced. - EXPECT_EQ(0u, NumIconMappingsForPageURL(url1, favicon_base::TOUCH_ICON)); + backend_->SetFavicons({url1}, IconType::kWebManifestIcon, icon_url2, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kWebManifestIcon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + // The kTouchIcon was replaced. + EXPECT_EQ(0u, NumIconMappingsForPageURL(url1, IconType::kTouchIcon)); - // The TOUCH_PRECOMPOSED_ICON was replaced. - EXPECT_EQ(0u, NumIconMappingsForPageURL( - url1, favicon_base::TOUCH_PRECOMPOSED_ICON)); + // The kTouchPrecomposedIcon was replaced. + EXPECT_EQ(0u, + NumIconMappingsForPageURL(url1, IconType::kTouchPrecomposedIcon)); // Add a different favicon. - backend_->SetFavicons({url1}, favicon_base::FAVICON, icon_url2, bitmaps); - EXPECT_EQ(1u, - NumIconMappingsForPageURL(url1, favicon_base::WEB_MANIFEST_ICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, favicon_base::FAVICON)); + backend_->SetFavicons({url1}, IconType::kFavicon, icon_url2, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kWebManifestIcon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, IconType::kFavicon)); } // Test that SetFaviconMappingsForPageAndRedirects correctly updates icon @@ -1852,10 +1835,10 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageAndRedirectsWithFragment) { redirects.push_back(url2); backend_->recent_redirects_.Put(url2, redirects); - backend_->SetFavicons({url1}, favicon_base::FAVICON, icon_url1, bitmaps); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, favicon_base::FAVICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url3, favicon_base::FAVICON)); + backend_->SetFavicons({url1}, IconType::kFavicon, icon_url1, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, IconType::kFavicon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url3, IconType::kFavicon)); // Both page and redirect key have a fragment. redirects.clear(); @@ -1865,10 +1848,10 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageAndRedirectsWithFragment) { backend_->recent_redirects_.Clear(); backend_->recent_redirects_.Put(url1, redirects); - backend_->SetFavicons({url1}, favicon_base::FAVICON, icon_url1, bitmaps); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, favicon_base::FAVICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, favicon_base::FAVICON)); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url3, favicon_base::FAVICON)); + backend_->SetFavicons({url1}, IconType::kFavicon, icon_url1, bitmaps); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, IconType::kFavicon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, IconType::kFavicon)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url3, IconType::kFavicon)); } TEST_F(HistoryBackendTest, @@ -1880,12 +1863,11 @@ TEST_F(HistoryBackendTest, const GURL url_without_ref("http://www.google.com"); const GURL icon_url("http://www.google.com/icon"); backend_->SetFavicons( - {url}, favicon_base::FAVICON, icon_url, + {url}, IconType::kFavicon, icon_url, std::vector<SkBitmap>{CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)}); - EXPECT_EQ(1u, NumIconMappingsForPageURL(url, favicon_base::FAVICON)); - EXPECT_EQ(0u, - NumIconMappingsForPageURL(url_without_ref, favicon_base::FAVICON)); + EXPECT_EQ(1u, NumIconMappingsForPageURL(url, IconType::kFavicon)); + EXPECT_EQ(0u, NumIconMappingsForPageURL(url_without_ref, IconType::kFavicon)); } // Test that |recent_redirects_| stores the full redirect chain in case of @@ -1895,22 +1877,31 @@ TEST_F(HistoryBackendTest, RecentRedirectsForClientRedirects) { GURL server_redirect_url("http://google.com/a"); GURL client_redirect_url("http://google.com/b"); GURL landing_url("http://google.com/c"); + GURL clicked_url("http://google.com/d"); // Page A is browsed by user and server redirects to B. HistoryAddPageArgs request( - client_redirect_url, base::Time::Now(), NULL, 0, GURL(), + client_redirect_url, base::Time::Now(), nullptr, 0, GURL(), /*redirects=*/{server_redirect_url, client_redirect_url}, - ui::PAGE_TRANSITION_TYPED, history::SOURCE_BROWSED, false, true); + ui::PAGE_TRANSITION_TYPED, false, history::SOURCE_BROWSED, false, true); backend_->AddPage(request); - // Client redirect to page C. - AddClientRedirect(client_redirect_url, landing_url, /*did_replace=*/false, + // Client redirect to page C (non-user initiated). + AddClientRedirect(client_redirect_url, landing_url, /*did_replace=*/true, base::Time(), /*transition1=*/nullptr, /*transition2=*/nullptr); EXPECT_THAT( backend_->recent_redirects_.Get(landing_url)->second, ElementsAre(server_redirect_url, client_redirect_url, landing_url)); + + // Navigation to page D (user initiated). + AddClientRedirect(landing_url, clicked_url, /*did_replace=*/false, + base::Time(), /*transition1=*/nullptr, + /*transition2=*/nullptr); + + EXPECT_THAT(backend_->recent_redirects_.Get(clicked_url)->second, + ElementsAre(clicked_url)); } // Test that there is no churn in icon mappings from calling @@ -1923,19 +1914,19 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageDuplicates) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); - backend_->SetFavicons({url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({url}, IconType::kFavicon, icon_url, bitmaps); std::vector<IconMapping> icon_mappings; EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - url, favicon_base::FAVICON, &icon_mappings)); + url, {IconType::kFavicon}, &icon_mappings)); EXPECT_EQ(1u, icon_mappings.size()); IconMappingID mapping_id = icon_mappings[0].mapping_id; - backend_->SetFavicons({url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({url}, IconType::kFavicon, icon_url, bitmaps); icon_mappings.clear(); EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - url, favicon_base::FAVICON, &icon_mappings)); + url, {IconType::kFavicon}, &icon_mappings)); EXPECT_EQ(1u, icon_mappings.size()); // The same row in the icon_mapping table should be used for the mapping as @@ -1953,14 +1944,13 @@ TEST_F(HistoryBackendTest, SetFaviconsDeleteBitmaps) { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); // Test initial state. - std::vector<IconMapping> icon_mappings; - EXPECT_TRUE(GetSortedIconMappingsForPageURL(page_url, &icon_mappings)); - EXPECT_EQ(1u, icon_mappings.size()); + std::vector<IconMapping> icon_mappings = GetIconMappingsForPageURL(page_url); + ASSERT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url, icon_mappings[0].icon_url); - EXPECT_EQ(favicon_base::FAVICON, icon_mappings[0].icon_type); + EXPECT_EQ(IconType::kFavicon, icon_mappings[0].icon_type); favicon_base::FaviconID favicon_id = icon_mappings[0].icon_id; std::vector<FaviconBitmap> favicon_bitmaps; @@ -1979,14 +1969,14 @@ TEST_F(HistoryBackendTest, SetFaviconsDeleteBitmaps) { // the small bitmap is in fact deleted. bitmaps.clear(); bitmaps.push_back(CreateBitmap(SK_ColorWHITE, kLargeEdgeSize)); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); scoped_refptr<base::RefCountedMemory> bitmap_data_out; gfx::Size pixel_size_out; - EXPECT_FALSE(backend_->thumbnail_db_->GetFaviconBitmap(small_bitmap_id, - NULL, NULL, &bitmap_data_out, &pixel_size_out)); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmap(large_bitmap_id, - NULL, NULL, &bitmap_data_out, &pixel_size_out)); + EXPECT_FALSE(backend_->thumbnail_db_->GetFaviconBitmap( + small_bitmap_id, nullptr, nullptr, &bitmap_data_out, &pixel_size_out)); + EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmap( + large_bitmap_id, nullptr, nullptr, &bitmap_data_out, &pixel_size_out)); EXPECT_TRUE(BitmapColorEqual(SK_ColorWHITE, bitmap_data_out)); EXPECT_EQ(kLargeSize, pixel_size_out); @@ -2005,11 +1995,11 @@ TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); // Add bitmap to the database. - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); favicon_base::FaviconID original_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, - favicon_base::FAVICON); + IconType::kFavicon); EXPECT_NE(0, original_favicon_id); FaviconBitmap original_favicon_bitmap; EXPECT_TRUE( @@ -2020,11 +2010,11 @@ TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { // Call SetFavicons() with completely identical data. bitmaps[0] = CreateBitmap(SK_ColorBLUE, kSmallEdgeSize); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); favicon_base::FaviconID updated_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, - favicon_base::FAVICON); + IconType::kFavicon); EXPECT_NE(0, updated_favicon_id); FaviconBitmap updated_favicon_bitmap; EXPECT_TRUE( @@ -2035,10 +2025,10 @@ TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { // Call SetFavicons() with a different bitmap of the same size. bitmaps[0] = CreateBitmap(SK_ColorWHITE, kSmallEdgeSize); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); updated_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url, favicon_base::FAVICON); + icon_url, IconType::kFavicon); EXPECT_NE(0, updated_favicon_id); EXPECT_TRUE( GetOnlyFaviconBitmap(updated_favicon_id, &updated_favicon_bitmap)); @@ -2063,11 +2053,11 @@ TEST_F(HistoryBackendTest, SetFaviconsSameFaviconURLForTwoPages) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url, bitmaps); std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url2}, icon_url, favicon_base::FAVICON, GetEdgeSizesSmallAndLarge(), + {page_url2}, icon_url, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), &bitmap_results); // Check that the same FaviconID is mapped to both page URLs. @@ -2087,8 +2077,7 @@ TEST_F(HistoryBackendTest, SetFaviconsSameFaviconURLForTwoPages) { // Change the icon URL that |page_url1| is mapped to. bitmaps.clear(); bitmaps.push_back(CreateBitmap(SK_ColorWHITE, kSmallEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url_new, - bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url_new, bitmaps); // |page_url1| should map to a new FaviconID and have valid bitmap data. icon_mappings.clear(); @@ -2127,7 +2116,7 @@ TEST_F(HistoryBackendTest, SetFaviconsWithTwoPageURLs) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); - backend_->SetFavicons({page_url1, page_url2}, favicon_base::FAVICON, icon_url, + backend_->SetFavicons({page_url1, page_url2}, IconType::kFavicon, icon_url, bitmaps); std::vector<IconMapping> icon_mappings; @@ -2144,6 +2133,31 @@ TEST_F(HistoryBackendTest, SetFaviconsWithTwoPageURLs) { EXPECT_EQ(favicon_id, icon_mappings[0].icon_id); } +// Test that favicon mappings can be deleted using DeleteFaviconMappings(). +TEST_F(HistoryBackendTest, DeleteFaviconMappings) { + GURL icon_url1("http://www.google.com/favicon.ico"); + GURL icon_url2("http://www.google.com/favicon2.ico"); + GURL page_url("http://www.google.com"); + std::vector<SkBitmap> bitmaps; + bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); + bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); + + // Setup + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url1, bitmaps); + backend_->SetFavicons({page_url}, IconType::kTouchIcon, icon_url2, bitmaps); + ClearBroadcastedNotifications(); + + // Delete one of the two mappings. + backend_->DeleteFaviconMappings({page_url}, IconType::kTouchIcon); + EXPECT_EQ(1u, NumIconMappingsForPageURL(page_url, IconType::kFavicon)); + EXPECT_EQ(0u, NumIconMappingsForPageURL(page_url, IconType::kTouchIcon)); + EXPECT_THAT(favicon_changed_notifications_page_urls(), ElementsAre(page_url)); + + // Delete the second mapping. + backend_->DeleteFaviconMappings({page_url}, IconType::kFavicon); + EXPECT_EQ(0u, NumIconMappingsForPageURL(page_url, IconType::kFavicon)); +} + // Tests calling SetOnDemandFavicons(). Neither |page_url| nor |icon_url| are // known to the database. TEST_F(HistoryBackendTest, SetOnDemandFaviconsForEmptyDB) { @@ -2153,12 +2167,12 @@ TEST_F(HistoryBackendTest, SetOnDemandFaviconsForEmptyDB) { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorRED, kSmallEdgeSize)); - EXPECT_TRUE(backend_->SetOnDemandFavicons(page_url, favicon_base::FAVICON, + EXPECT_TRUE(backend_->SetOnDemandFavicons(page_url, IconType::kFavicon, icon_url, bitmaps)); favicon_base::FaviconID favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, - favicon_base::FAVICON); + IconType::kFavicon); EXPECT_NE(0, favicon_id); FaviconBitmap favicon_bitmap; @@ -2170,9 +2184,9 @@ TEST_F(HistoryBackendTest, SetOnDemandFaviconsForEmptyDB) { // The raw bitmap result is marked as fetched on-demand. favicon_base::FaviconRawBitmapResult result; - backend_->GetLargestFaviconForURL(page_url, - std::vector<int>{favicon_base::FAVICON}, - kSmallEdgeSize, &result); + backend_->GetLargestFaviconForURL( + page_url, std::vector<IconTypeSet>({{IconType::kFavicon}}), + kSmallEdgeSize, &result); EXPECT_FALSE(result.fetched_because_of_page_visit); } @@ -2186,18 +2200,18 @@ TEST_F(HistoryBackendTest, SetOnDemandFaviconsForPageInDB) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); // Add bitmap to the database. - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url1, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url1, bitmaps); favicon_base::FaviconID original_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url1, - favicon_base::FAVICON); + IconType::kFavicon); ASSERT_NE(0, original_favicon_id); // Call SetOnDemandFavicons() with a different icon URL and bitmap data. bitmaps[0] = CreateBitmap(SK_ColorWHITE, kSmallEdgeSize); - EXPECT_FALSE(backend_->SetOnDemandFavicons(page_url, favicon_base::FAVICON, + EXPECT_FALSE(backend_->SetOnDemandFavicons(page_url, IconType::kFavicon, icon_url2, bitmaps)); EXPECT_EQ(0, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url2, favicon_base::FAVICON)); + icon_url2, IconType::kFavicon)); FaviconBitmap favicon_bitmap; ASSERT_TRUE(GetOnlyFaviconBitmap(original_favicon_id, &favicon_bitmap)); @@ -2208,9 +2222,9 @@ TEST_F(HistoryBackendTest, SetOnDemandFaviconsForPageInDB) { // The raw bitmap result is not marked as fetched on-demand. favicon_base::FaviconRawBitmapResult result; - backend_->GetLargestFaviconForURL(page_url, - std::vector<int>{favicon_base::FAVICON}, - kSmallEdgeSize, &result); + backend_->GetLargestFaviconForURL( + page_url, std::vector<IconTypeSet>({{IconType::kFavicon}}), + kSmallEdgeSize, &result); EXPECT_TRUE(result.fetched_because_of_page_visit); } @@ -2224,21 +2238,20 @@ TEST_F(HistoryBackendTest, SetOnDemandFaviconsForIconInDB) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); // Add bitmap to the database. - backend_->SetFavicons({old_page_url}, favicon_base::FAVICON, icon_url, - bitmaps); + backend_->SetFavicons({old_page_url}, IconType::kFavicon, icon_url, bitmaps); favicon_base::FaviconID original_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, - favicon_base::FAVICON); + IconType::kFavicon); ASSERT_NE(0, original_favicon_id); // Call SetOnDemandFavicons() with a different bitmap. bitmaps[0] = CreateBitmap(SK_ColorWHITE, kSmallEdgeSize); - EXPECT_FALSE(backend_->SetOnDemandFavicons(page_url, favicon_base::FAVICON, + EXPECT_FALSE(backend_->SetOnDemandFavicons(page_url, IconType::kFavicon, icon_url, bitmaps)); EXPECT_EQ(original_favicon_id, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url, favicon_base::FAVICON)); + icon_url, IconType::kFavicon)); FaviconBitmap favicon_bitmap; ASSERT_TRUE(GetOnlyFaviconBitmap(original_favicon_id, &favicon_bitmap)); @@ -2249,9 +2262,9 @@ TEST_F(HistoryBackendTest, SetOnDemandFaviconsForIconInDB) { // The raw bitmap result is not marked as fetched on-demand. favicon_base::FaviconRawBitmapResult result; - backend_->GetLargestFaviconForURL(page_url, - std::vector<int>{favicon_base::FAVICON}, - kSmallEdgeSize, &result); + backend_->GetLargestFaviconForURL( + page_url, std::vector<IconTypeSet>({{IconType::kFavicon}}), + kSmallEdgeSize, &result); EXPECT_TRUE(result.fetched_because_of_page_visit); } @@ -2266,8 +2279,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLNotInDB) { scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon( - page_url, icon_url, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, bitmap_data, + kSmallSize); // |page_url| should now be mapped to |icon_url| and the favicon bitmap should // not be expired. @@ -2285,8 +2298,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLNotInDB) { data[0] = 'b'; bitmap_data = new base::RefCountedBytes(data); - backend_->MergeFavicon( - page_url, icon_url, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, bitmap_data, + kSmallSize); // |page_url| should still have a single favicon bitmap. The bitmap data // should be updated. @@ -2310,7 +2323,7 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url1, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url1, bitmaps); // Test initial state. std::vector<IconMapping> icon_mappings; @@ -2330,8 +2343,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { gfx::PNGCodec::EncodeBGRASkBitmap(bitmaps[0], false, &data); scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon( - page_url, icon_url1, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url, icon_url1, IconType::kFavicon, bitmap_data, + kSmallSize); // All the data should stay the same and no notifications should have been // sent. @@ -2350,8 +2363,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { data.clear(); data.push_back('b'); bitmap_data = new base::RefCountedBytes(data); - backend_->MergeFavicon( - page_url, icon_url1, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url, icon_url1, IconType::kFavicon, bitmap_data, + kSmallSize); // The small favicon bitmap at |icon_url1| should be overwritten. icon_mappings.clear(); @@ -2369,8 +2382,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { // no favicon bitmap. data[0] = 'c'; bitmap_data = new base::RefCountedBytes(data); - backend_->MergeFavicon( - page_url, icon_url1, favicon_base::FAVICON, bitmap_data, kTinySize); + backend_->MergeFavicon(page_url, icon_url1, IconType::kFavicon, bitmap_data, + kTinySize); // A new favicon bitmap should be created and the preexisting favicon bitmap // ('b') should be expired. @@ -2394,8 +2407,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { // mapped to page URL. data[0] = 'd'; bitmap_data = new base::RefCountedBytes(data); - backend_->MergeFavicon( - page_url, icon_url2, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url, icon_url2, IconType::kFavicon, bitmap_data, + kSmallSize); // The existing favicon bitmaps should be copied over to the newly created // favicon at |icon_url2|. |page_url| should solely be mapped to |icon_url2|. @@ -2428,7 +2441,7 @@ TEST_F(HistoryBackendTest, MergeFaviconIconURLMappedToDifferentPageURL) { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url, bitmaps); // Test initial state. std::vector<IconMapping> icon_mappings; @@ -2449,12 +2462,12 @@ TEST_F(HistoryBackendTest, MergeFaviconIconURLMappedToDifferentPageURL) { scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon( - page_url2, icon_url, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url2, icon_url, IconType::kFavicon, bitmap_data, + kSmallSize); favicon_base::FaviconID favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, - favicon_base::FAVICON); + IconType::kFavicon); EXPECT_NE(0, favicon_id); EXPECT_TRUE(GetOnlyFaviconBitmap(favicon_id, &favicon_bitmap)); @@ -2467,11 +2480,11 @@ TEST_F(HistoryBackendTest, MergeFaviconIconURLMappedToDifferentPageURL) { data.clear(); data.push_back('b'); bitmap_data = new base::RefCountedBytes(data); - backend_->MergeFavicon( - page_url3, icon_url, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url3, icon_url, IconType::kFavicon, bitmap_data, + kSmallSize); favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url, favicon_base::FAVICON); + icon_url, IconType::kFavicon); EXPECT_NE(0, favicon_id); EXPECT_TRUE(GetOnlyFaviconBitmap(favicon_id, &favicon_bitmap)); @@ -2516,10 +2529,7 @@ TEST_F(HistoryBackendTest, MergeFaviconMaxFaviconBitmapsPerIconURL) { icon_url_string[replace_index] = '0' + i; GURL icon_url(icon_url_string); - backend_->MergeFavicon(page_url, - icon_url, - favicon_base::FAVICON, - bitmap_data, + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, bitmap_data, gfx::Size(pixel_size, pixel_size)); ++pixel_size; } @@ -2547,26 +2557,21 @@ TEST_F(HistoryBackendTest, MergeFaviconShowsUpInGetFaviconsForURLResult) { bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); // Set some preexisting favicons for |page_url|. - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); // Merge small favicon. std::vector<unsigned char> data; data.push_back('c'); scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon(page_url, - merged_icon_url, - favicon_base::FAVICON, - bitmap_data, - kSmallSize); + backend_->MergeFavicon(page_url, merged_icon_url, IconType::kFavicon, + bitmap_data, kSmallSize); // Request favicon bitmaps for both 1x and 2x to simulate request done by // BookmarkModel::GetFavicon(). std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; - backend_->GetFaviconsForURL(page_url, - favicon_base::FAVICON, - GetEdgeSizesSmallAndLarge(), - &bitmap_results); + backend_->GetFaviconsForURL(page_url, {IconType::kFavicon}, + GetEdgeSizesSmallAndLarge(), &bitmap_results); EXPECT_EQ(2u, bitmap_results.size()); const favicon_base::FaviconRawBitmapResult& first_result = bitmap_results[0]; @@ -2590,8 +2595,7 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationNewFavicon) { { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url1, - bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url1, bitmaps); ASSERT_EQ(1u, favicon_changed_notifications_page_urls().size()); EXPECT_EQ(page_url1, favicon_changed_notifications_page_urls()[0]); EXPECT_EQ(0u, favicon_changed_notifications_icon_urls().size()); @@ -2604,8 +2608,8 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationNewFavicon) { data.push_back('a'); scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon( - page_url2, icon_url2, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url2, icon_url2, IconType::kFavicon, + bitmap_data, kSmallSize); ASSERT_EQ(1u, favicon_changed_notifications_page_urls().size()); EXPECT_EQ(page_url2, favicon_changed_notifications_page_urls()[0]); EXPECT_EQ(0u, favicon_changed_notifications_icon_urls().size()); @@ -2624,7 +2628,7 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationBitmapDataChanged) { { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); ClearBroadcastedNotifications(); } @@ -2632,7 +2636,7 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationBitmapDataChanged) { { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorWHITE, kSmallEdgeSize)); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); EXPECT_EQ(0u, favicon_changed_notifications_page_urls().size()); ASSERT_EQ(1u, favicon_changed_notifications_icon_urls().size()); EXPECT_EQ(icon_url, favicon_changed_notifications_icon_urls()[0]); @@ -2645,8 +2649,8 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationBitmapDataChanged) { data.push_back('a'); scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon( - page_url, icon_url, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, bitmap_data, + kSmallSize); EXPECT_EQ(0u, favicon_changed_notifications_page_urls().size()); ASSERT_EQ(1u, favicon_changed_notifications_icon_urls().size()); EXPECT_EQ(icon_url, favicon_changed_notifications_icon_urls()[0]); @@ -2674,29 +2678,27 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationIconMappingChanged) { { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url1, - bitmaps); - backend_->SetFavicons({page_url2}, favicon_base::FAVICON, icon_url2, - bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url1, bitmaps); + backend_->SetFavicons({page_url2}, IconType::kFavicon, icon_url2, bitmaps); // Map |page_url3| to |icon_url1| so that the test does not delete the // favicon at |icon_url1|. std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url3}, icon_url1, favicon_base::FAVICON, - GetEdgeSizesSmallAndLarge(), &bitmap_results); + {page_url3}, icon_url1, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), + &bitmap_results); ClearBroadcastedNotifications(); } // SetFavicons() - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url2, bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url2, bitmaps); EXPECT_THAT(favicon_changed_notifications_page_urls(), ElementsAre(page_url1)); EXPECT_EQ(0u, favicon_changed_notifications_icon_urls().size()); ClearBroadcastedNotifications(); // MergeFavicon() - backend_->MergeFavicon(page_url1, icon_url1, favicon_base::FAVICON, + backend_->MergeFavicon(page_url1, icon_url1, IconType::kFavicon, new base::RefCountedBytes(png_bytes), kSmallSize); EXPECT_THAT(favicon_changed_notifications_page_urls(), ElementsAre(page_url1)); @@ -2707,8 +2709,8 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationIconMappingChanged) { { std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url1}, icon_url2, favicon_base::FAVICON, - GetEdgeSizesSmallAndLarge(), &bitmap_results); + {page_url1}, icon_url2, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), + &bitmap_results); EXPECT_THAT(favicon_changed_notifications_page_urls(), ElementsAre(page_url1)); EXPECT_EQ(0u, favicon_changed_notifications_icon_urls().size()); @@ -2735,8 +2737,7 @@ TEST_F(HistoryBackendTest, { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url4}, favicon_base::FAVICON, icon_url, - bitmaps); + backend_->SetFavicons({page_url4}, IconType::kFavicon, icon_url, bitmaps); ClearBroadcastedNotifications(); } @@ -2744,7 +2745,7 @@ TEST_F(HistoryBackendTest, { std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url1, page_url2}, icon_url, favicon_base::FAVICON, + {page_url1, page_url2}, icon_url, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), &bitmap_results); EXPECT_THAT(favicon_changed_notifications_page_urls(), ElementsAre(page_url1, page_url2)); @@ -2756,7 +2757,7 @@ TEST_F(HistoryBackendTest, { std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url3, page_url4}, icon_url, favicon_base::FAVICON, + {page_url3, page_url4}, icon_url, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), &bitmap_results); EXPECT_THAT(favicon_changed_notifications_page_urls(), ElementsAre(page_url3)); @@ -2780,17 +2781,15 @@ TEST_F(HistoryBackendTest, { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url1, - bitmaps); - backend_->SetFavicons({page_url2}, favicon_base::FAVICON, icon_url2, - bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url1, bitmaps); + backend_->SetFavicons({page_url2}, IconType::kFavicon, icon_url2, bitmaps); // Map |page_url3| to |icon_url1| so that the test does not delete the // favicon at |icon_url1|. std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url3}, icon_url1, favicon_base::FAVICON, - GetEdgeSizesSmallAndLarge(), &bitmap_results); + {page_url3}, icon_url1, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), + &bitmap_results); ClearBroadcastedNotifications(); } @@ -2798,8 +2797,7 @@ TEST_F(HistoryBackendTest, { std::vector<SkBitmap> bitmaps; bitmaps.push_back(CreateBitmap(SK_ColorWHITE, kSmallEdgeSize)); - backend_->SetFavicons({page_url1}, favicon_base::FAVICON, icon_url2, - bitmaps); + backend_->SetFavicons({page_url1}, IconType::kFavicon, icon_url2, bitmaps); ASSERT_EQ(1u, favicon_changed_notifications_page_urls().size()); EXPECT_EQ(page_url1, favicon_changed_notifications_page_urls()[0]); ASSERT_EQ(1u, favicon_changed_notifications_icon_urls().size()); @@ -2813,8 +2811,8 @@ TEST_F(HistoryBackendTest, data.push_back('a'); scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon( - page_url1, icon_url1, favicon_base::FAVICON, bitmap_data, kSmallSize); + backend_->MergeFavicon(page_url1, icon_url1, IconType::kFavicon, + bitmap_data, kSmallSize); ASSERT_EQ(1u, favicon_changed_notifications_page_urls().size()); EXPECT_EQ(page_url1, favicon_changed_notifications_page_urls()[0]); ASSERT_EQ(1u, favicon_changed_notifications_icon_urls().size()); @@ -2836,18 +2834,18 @@ TEST_F(HistoryBackendTest, FaviconChangedNotificationsMergeCopy) { png_bytes2.push_back('b'); // Setup - backend_->MergeFavicon(page_url1, icon_url1, favicon_base::FAVICON, + backend_->MergeFavicon(page_url1, icon_url1, IconType::kFavicon, new base::RefCountedBytes(png_bytes1), kSmallSize); - backend_->MergeFavicon(page_url2, icon_url2, favicon_base::FAVICON, + backend_->MergeFavicon(page_url2, icon_url2, IconType::kFavicon, new base::RefCountedBytes(png_bytes2), kSmallSize); - backend_->MergeFavicon(page_url2, icon_url2, favicon_base::FAVICON, + backend_->MergeFavicon(page_url2, icon_url2, IconType::kFavicon, new base::RefCountedBytes(png_bytes2), kLargeSize); ClearBroadcastedNotifications(); // Calling MergeFavicon() with |page_url2|, |icon_url1|, |png_bytes1| and // |kSmallSize| should cause the large favicon bitmap from |icon_url2| to // be copied to |icon_url1|. - backend_->MergeFavicon(page_url2, icon_url1, favicon_base::FAVICON, + backend_->MergeFavicon(page_url2, icon_url1, IconType::kFavicon, new base::RefCountedBytes(png_bytes1), kSmallSize); ASSERT_EQ(1u, favicon_changed_notifications_page_urls().size()); @@ -2870,10 +2868,7 @@ TEST_F(HistoryBackendTest, MergeIdenticalFaviconDoesNotChangeLastUpdatedTime) { scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes(data)); - backend_->MergeFavicon(page_url, - icon_url, - favicon_base::FAVICON, - bitmap_data, + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, bitmap_data, kSmallSize); // Find the ID of the add favicon bitmap. @@ -2892,10 +2887,7 @@ TEST_F(HistoryBackendTest, MergeIdenticalFaviconDoesNotChangeLastUpdatedTime) { favicon_bitmaps[0].bitmap_id, kLastUpdateTime); // Call MergeFavicon() with identical data. - backend_->MergeFavicon(page_url, - icon_url, - favicon_base::FAVICON, - bitmap_data, + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, bitmap_data, kSmallSize); // Check that the "last updated" time did not change. @@ -2923,22 +2915,22 @@ TEST_F(HistoryBackendTest, NoFaviconChangedNotifications) { ASSERT_TRUE(gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &png_bytes)); // Setup - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); ClearBroadcastedNotifications(); // SetFavicons() - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); // MergeFavicon() - backend_->MergeFavicon(page_url, icon_url, favicon_base::FAVICON, + backend_->MergeFavicon(page_url, icon_url, IconType::kFavicon, new base::RefCountedBytes(png_bytes), kSmallSize); // UpdateFaviconMappingsAndFetch() { std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( - {page_url}, icon_url, favicon_base::FAVICON, - GetEdgeSizesSmallAndLarge(), &bitmap_results); + {page_url}, icon_url, IconType::kFavicon, GetEdgeSizesSmallAndLarge(), + &bitmap_results); } EXPECT_EQ(0u, favicon_changed_notifications_page_urls().size()); @@ -2959,26 +2951,26 @@ TEST_F(HistoryBackendTest, TestGetFaviconsForURLWithIconTypesPriority) { touch_bitmaps.push_back(CreateBitmap(SK_ColorWHITE, 64)); // Set some preexisting favicons for |page_url|. - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, favicon_bitmaps); - backend_->SetFavicons({page_url}, favicon_base::TOUCH_ICON, touch_icon_url, + backend_->SetFavicons({page_url}, IconType::kTouchIcon, touch_icon_url, touch_bitmaps); favicon_base::FaviconRawBitmapResult result; - std::vector<int> icon_types; - icon_types.push_back(favicon_base::FAVICON); - icon_types.push_back(favicon_base::TOUCH_ICON); + std::vector<IconTypeSet> icon_types; + icon_types.push_back({IconType::kFavicon}); + icon_types.push_back({IconType::kTouchIcon}); backend_->GetLargestFaviconForURL(page_url, icon_types, 16, &result); // Verify the result icon is 32x32 favicon. EXPECT_EQ(gfx::Size(32, 32), result.pixel_size); - EXPECT_EQ(favicon_base::FAVICON, result.icon_type); + EXPECT_EQ(IconType::kFavicon, result.icon_type); // Change Minimal size to 32x32 and verify the 64x64 touch icon returned. backend_->GetLargestFaviconForURL(page_url, icon_types, 32, &result); EXPECT_EQ(gfx::Size(64, 64), result.pixel_size); - EXPECT_EQ(favicon_base::TOUCH_ICON, result.icon_type); + EXPECT_EQ(IconType::kTouchIcon, result.icon_type); } // Test the the first types of icon is returned if its size equal to the @@ -2996,27 +2988,27 @@ TEST_F(HistoryBackendTest, TestGetFaviconsForURLReturnFavicon) { touch_bitmaps.push_back(CreateBitmap(SK_ColorWHITE, 32)); // Set some preexisting favicons for |page_url|. - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, favicon_bitmaps); - backend_->SetFavicons({page_url}, favicon_base::TOUCH_ICON, touch_icon_url, + backend_->SetFavicons({page_url}, IconType::kTouchIcon, touch_icon_url, touch_bitmaps); favicon_base::FaviconRawBitmapResult result; - std::vector<int> icon_types; - icon_types.push_back(favicon_base::FAVICON); - icon_types.push_back(favicon_base::TOUCH_ICON); + std::vector<IconTypeSet> icon_types; + icon_types.push_back({IconType::kFavicon}); + icon_types.push_back({IconType::kTouchIcon}); backend_->GetLargestFaviconForURL(page_url, icon_types, 16, &result); // Verify the result icon is 32x32 favicon. EXPECT_EQ(gfx::Size(32, 32), result.pixel_size); - EXPECT_EQ(favicon_base::FAVICON, result.icon_type); + EXPECT_EQ(IconType::kFavicon, result.icon_type); // Change minimal size to 32x32 and verify the 32x32 favicon returned. favicon_base::FaviconRawBitmapResult result1; backend_->GetLargestFaviconForURL(page_url, icon_types, 32, &result1); EXPECT_EQ(gfx::Size(32, 32), result1.pixel_size); - EXPECT_EQ(favicon_base::FAVICON, result1.icon_type); + EXPECT_EQ(IconType::kFavicon, result1.icon_type); } // Test the favicon is returned if its size is smaller than minimal size, @@ -3029,18 +3021,18 @@ TEST_F(HistoryBackendTest, TestGetFaviconsForURLReturnFaviconEvenItSmaller) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, 16)); // Set preexisting favicons for |page_url|. - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); favicon_base::FaviconRawBitmapResult result; - std::vector<int> icon_types; - icon_types.push_back(favicon_base::FAVICON); - icon_types.push_back(favicon_base::TOUCH_ICON); + std::vector<IconTypeSet> icon_types; + icon_types.push_back({IconType::kFavicon}); + icon_types.push_back({IconType::kTouchIcon}); backend_->GetLargestFaviconForURL(page_url, icon_types, 32, &result); // Verify 16x16 icon is returned, even it small than minimal_size. EXPECT_EQ(gfx::Size(16, 16), result.pixel_size); - EXPECT_EQ(favicon_base::FAVICON, result.icon_type); + EXPECT_EQ(IconType::kFavicon, result.icon_type); } // Test the results of GetFaviconsFromDB() when there are no found favicons. @@ -3048,8 +3040,7 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBEmpty) { const GURL page_url("http://www.google.com/"); std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; - EXPECT_FALSE(backend_->GetFaviconsFromDB(page_url, - favicon_base::FAVICON, + EXPECT_FALSE(backend_->GetFaviconsFromDB(page_url, {IconType::kFavicon}, GetEdgeSizesSmallAndLarge(), &bitmap_results)); EXPECT_TRUE(bitmap_results.empty()); @@ -3062,13 +3053,12 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBNoFaviconBitmaps) { const GURL icon_url("http://www.google.com/icon1"); favicon_base::FaviconID icon_id = - backend_->thumbnail_db_->AddFavicon(icon_url, favicon_base::FAVICON); + backend_->thumbnail_db_->AddFavicon(icon_url, IconType::kFavicon); EXPECT_NE(0, icon_id); EXPECT_NE(0, backend_->thumbnail_db_->AddIconMapping(page_url, icon_id)); std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results_out; - EXPECT_FALSE(backend_->GetFaviconsFromDB(page_url, - favicon_base::FAVICON, + EXPECT_FALSE(backend_->GetFaviconsFromDB(page_url, {IconType::kFavicon}, GetEdgeSizesSmallAndLarge(), &bitmap_results_out)); EXPECT_TRUE(bitmap_results_out.empty()); @@ -3084,11 +3074,10 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBSelectClosestMatch) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); bitmaps.push_back(CreateBitmap(SK_ColorRED, kLargeEdgeSize)); - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url, bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url, bitmaps); std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results_out; - EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, - favicon_base::FAVICON, + EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, {IconType::kFavicon}, GetEdgeSizesSmallAndLarge(), &bitmap_results_out)); @@ -3107,13 +3096,13 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBSelectClosestMatch) { BitmapColorEqual(SK_ColorBLUE, bitmap_results_out[0].bitmap_data)); EXPECT_EQ(kSmallSize, bitmap_results_out[0].pixel_size); EXPECT_EQ(icon_url, bitmap_results_out[0].icon_url); - EXPECT_EQ(favicon_base::FAVICON, bitmap_results_out[0].icon_type); + EXPECT_EQ(IconType::kFavicon, bitmap_results_out[0].icon_type); EXPECT_FALSE(bitmap_results_out[1].expired); EXPECT_TRUE(BitmapColorEqual(SK_ColorRED, bitmap_results_out[1].bitmap_data)); EXPECT_EQ(kLargeSize, bitmap_results_out[1].pixel_size); EXPECT_EQ(icon_url, bitmap_results_out[1].icon_url); - EXPECT_EQ(favicon_base::FAVICON, bitmap_results_out[1].icon_type); + EXPECT_EQ(IconType::kFavicon, bitmap_results_out[1].icon_type); } // Test the results of GetFaviconsFromDB() when called with different @@ -3126,28 +3115,25 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBIconType) { bitmaps.push_back(CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)); std::vector<favicon_base::FaviconRawBitmapData> favicon_bitmap_data; - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url1, bitmaps); - backend_->SetFavicons({page_url}, favicon_base::TOUCH_ICON, icon_url2, - bitmaps); + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url1, bitmaps); + backend_->SetFavicons({page_url}, IconType::kTouchIcon, icon_url2, bitmaps); std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results_out; - EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, - favicon_base::FAVICON, + EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, {IconType::kFavicon}, GetEdgeSizesSmallAndLarge(), &bitmap_results_out)); EXPECT_EQ(1u, bitmap_results_out.size()); - EXPECT_EQ(favicon_base::FAVICON, bitmap_results_out[0].icon_type); + EXPECT_EQ(IconType::kFavicon, bitmap_results_out[0].icon_type); EXPECT_EQ(icon_url1, bitmap_results_out[0].icon_url); bitmap_results_out.clear(); - EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, - favicon_base::TOUCH_ICON, + EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, {IconType::kTouchIcon}, GetEdgeSizesSmallAndLarge(), &bitmap_results_out)); EXPECT_EQ(1u, bitmap_results_out.size()); - EXPECT_EQ(favicon_base::TOUCH_ICON, bitmap_results_out[0].icon_type); + EXPECT_EQ(IconType::kTouchIcon, bitmap_results_out[0].icon_type); EXPECT_EQ(icon_url2, bitmap_results_out[0].icon_url); } @@ -3159,9 +3145,9 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBMultipleIconTypes) { const GURL icon_url2("http://www.google.com/icon2.png"); std::vector<favicon_base::FaviconRawBitmapData> favicon_bitmap_data; - backend_->SetFavicons({page_url}, favicon_base::FAVICON, icon_url1, + backend_->SetFavicons({page_url}, IconType::kFavicon, icon_url1, {CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)}); - backend_->SetFavicons({page_url}, favicon_base::TOUCH_ICON, icon_url2, + backend_->SetFavicons({page_url}, IconType::kTouchIcon, icon_url2, {CreateBitmap(SK_ColorBLUE, kLargeEdgeSize)}); struct TestCase { @@ -3172,7 +3158,7 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBMultipleIconTypes) { for (const TestCase& test_case : kTestCases) { std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results_out; backend_->GetFaviconsForURL( - page_url, favicon_base::FAVICON | favicon_base::TOUCH_ICON, + page_url, {IconType::kFavicon, IconType::kTouchIcon}, {test_case.desired_edge_size}, &bitmap_results_out); ASSERT_EQ(1u, bitmap_results_out.size()); @@ -3180,6 +3166,46 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBMultipleIconTypes) { } } +// Test that CloneFaviconMappingsForPages() propagates favicon mappings to the +// provided pages and their redirects. +TEST_F(HistoryBackendTest, CloneFaviconMappingsForPages) { + const GURL landing_page_url1("http://www.google.com/landing"); + const GURL landing_page_url2("http://www.google.ca/landing"); + const GURL redirecting_page_url1("http://www.google.com/redirect"); + const GURL redirecting_page_url2("http://www.google.ca/redirect"); + const GURL icon_url("http://www.google.com/icon.png"); + + // Setup + { + // A mapping exists for |landing_page_url1|. + std::vector<favicon_base::FaviconRawBitmapData> favicon_bitmap_data; + backend_->SetFavicons({landing_page_url1}, IconType::kFavicon, icon_url, + {CreateBitmap(SK_ColorBLUE, kSmallEdgeSize)}); + + // Init recent_redirects_. + backend_->recent_redirects_.Put( + landing_page_url1, + RedirectList{redirecting_page_url1, landing_page_url1}); + backend_->recent_redirects_.Put( + landing_page_url2, + RedirectList{redirecting_page_url2, landing_page_url2}); + ClearBroadcastedNotifications(); + } + + std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results_out; + backend_->CloneFaviconMappingsForPages( + landing_page_url1, {IconType::kFavicon}, + {landing_page_url1, landing_page_url2}); + + EXPECT_THAT(favicon_changed_notifications_page_urls(), + UnorderedElementsAre(redirecting_page_url1, landing_page_url2, + redirecting_page_url2)); + + EXPECT_EQ(1U, GetIconMappingsForPageURL(redirecting_page_url1).size()); + EXPECT_EQ(1U, GetIconMappingsForPageURL(landing_page_url2).size()); + EXPECT_EQ(1U, GetIconMappingsForPageURL(redirecting_page_url2).size()); +} + // Test that GetFaviconsFromDB() correctly sets the expired flag for bitmap // reults. TEST_F(HistoryBackendTest, GetFaviconsFromDBExpired) { @@ -3192,14 +3218,13 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBExpired) { base::RefCountedBytes::TakeVector(&data)); base::Time last_updated = base::Time::FromTimeT(0); favicon_base::FaviconID icon_id = backend_->thumbnail_db_->AddFavicon( - icon_url, favicon_base::FAVICON, bitmap_data, FaviconBitmapType::ON_VISIT, + icon_url, IconType::kFavicon, bitmap_data, FaviconBitmapType::ON_VISIT, last_updated, kSmallSize); EXPECT_NE(0, icon_id); EXPECT_NE(0, backend_->thumbnail_db_->AddIconMapping(page_url, icon_id)); std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results_out; - EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, - favicon_base::FAVICON, + EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, {IconType::kFavicon}, GetEdgeSizesSmallAndLarge(), &bitmap_results_out)); @@ -3215,9 +3240,9 @@ TEST_F(HistoryBackendTest, UpdateFaviconMappingsAndFetchNoDB) { std::vector<favicon_base::FaviconRawBitmapResult> bitmap_results; - backend_->UpdateFaviconMappingsAndFetch( - {GURL()}, GURL(), favicon_base::FAVICON, GetEdgeSizesSmallAndLarge(), - &bitmap_results); + backend_->UpdateFaviconMappingsAndFetch({GURL()}, GURL(), IconType::kFavicon, + GetEdgeSizesSmallAndLarge(), + &bitmap_results); EXPECT_TRUE(bitmap_results.empty()); } @@ -3233,7 +3258,7 @@ TEST_F(HistoryBackendTest, TopHosts) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } EXPECT_THAT(backend_->TopHosts(3), @@ -3252,7 +3277,7 @@ TEST_F(HistoryBackendTest, TopHosts_ElidePortAndScheme) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } EXPECT_THAT(backend_->TopHosts(3), ElementsAre(std::make_pair("cnn.com", 3))); @@ -3269,7 +3294,7 @@ TEST_F(HistoryBackendTest, TopHosts_ElideWWW) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } EXPECT_THAT(backend_->TopHosts(3), @@ -3288,7 +3313,7 @@ TEST_F(HistoryBackendTest, TopHosts_OnlyLast30Days) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } backend_->AddPageVisit( GURL("http://www.oracle.com/"), @@ -3296,7 +3321,7 @@ TEST_F(HistoryBackendTest, TopHosts_OnlyLast30Days) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); EXPECT_THAT(backend_->TopHosts(3), ElementsAre(std::make_pair("cnn.com", 2), @@ -3317,7 +3342,7 @@ TEST_F(HistoryBackendTest, TopHosts_MaxNumHosts) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } EXPECT_THAT(backend_->TopHosts(2), @@ -3342,7 +3367,7 @@ TEST_F(HistoryBackendTest, TopHosts_IgnoreUnusualURLs) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } EXPECT_THAT(backend_->TopHosts(5), ElementsAre(std::make_pair("cnn.com", 3))); @@ -3350,10 +3375,10 @@ TEST_F(HistoryBackendTest, TopHosts_IgnoreUnusualURLs) { TEST_F(HistoryBackendTest, TopHosts_IgnoreRedirects) { const char* redirect1[] = {"http://foo.com/page1.html", - "http://mobile.foo.com/page1.html", NULL}; + "http://mobile.foo.com/page1.html", nullptr}; const char* redirect2[] = {"http://bar.com/page1.html", "https://bar.com/page1.html", - "https://mobile.bar.com/page1.html", NULL}; + "https://mobile.bar.com/page1.html", nullptr}; AddRedirectChain(redirect1, 0); AddRedirectChain(redirect2, 1); EXPECT_THAT(backend_->TopHosts(5), @@ -3372,7 +3397,7 @@ TEST_F(HistoryBackendTest, HostRankIfAvailable) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } EXPECT_EQ(kMaxTopHosts, @@ -3400,7 +3425,7 @@ TEST_F(HistoryBackendTest, RecordTopHostsMetrics) { ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CHAIN_START | ui::PAGE_TRANSITION_CHAIN_END), - history::SOURCE_BROWSED); + false, history::SOURCE_BROWSED); } // Compute host_ranks_ for RecordTopHostsMetrics. @@ -3414,7 +3439,7 @@ TEST_F(HistoryBackendTest, RecordTopHostsMetrics) { urls.push_back(GURL("http://www.unipresse.com/")); for (const GURL& url : urls) { backend_->AddPageVisit(url, base::Time::Now(), 0, - ui::PAGE_TRANSITION_CHAIN_END, + ui::PAGE_TRANSITION_CHAIN_END, false, history::SOURCE_BROWSED); } @@ -3429,17 +3454,23 @@ TEST_F(HistoryBackendTest, GetCountsAndLastVisitForOrigins) { base::Time last_week = now - base::TimeDelta::FromDays(7); backend_->AddPageVisit(GURL("http://cnn.com/intl"), yesterday, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); backend_->AddPageVisit(GURL("http://cnn.com/us"), last_week, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); backend_->AddPageVisit(GURL("http://cnn.com/ny"), now, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); backend_->AddPageVisit(GURL("https://cnn.com/intl"), yesterday, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); backend_->AddPageVisit(GURL("http://cnn.com:8080/path"), yesterday, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); backend_->AddPageVisit(GURL("http://dogtopia.com/pups?q=poods"), now, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); std::set<GURL> origins; origins.insert(GURL("http://cnn.com/")); @@ -3452,7 +3483,8 @@ TEST_F(HistoryBackendTest, GetCountsAndLastVisitForOrigins) { origins.insert(GURL("https://cnn.com/")); origins.insert(GURL("http://notpresent.com/")); backend_->AddPageVisit(GURL("http://cnn.com/"), tomorrow, 0, - ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED); + ui::PAGE_TRANSITION_LINK, false, + history::SOURCE_BROWSED); EXPECT_THAT( backend_->GetCountsAndLastVisitForOrigins(origins), @@ -3520,7 +3552,7 @@ TEST_F(HistoryBackendTest, UpdateVisitDuration) { TEST_F(HistoryBackendTest, MigrationVisitDuration) { ASSERT_TRUE(backend_.get()); backend_->Closing(); - backend_ = NULL; + backend_ = nullptr; base::FilePath old_history_path, old_history; ASSERT_TRUE(GetTestDataHistoryDir(&old_history_path)); @@ -3539,7 +3571,7 @@ TEST_F(HistoryBackendTest, MigrationVisitDuration) { base::ThreadTaskRunnerHandle::Get()); backend_->Init(false, TestHistoryDatabaseParamsForPath(new_history_path)); backend_->Closing(); - backend_ = NULL; + backend_ = nullptr; // Now the history database should already be migrated. @@ -3612,7 +3644,7 @@ TEST_F(HistoryBackendTest, ExpireHistoryForTimes) { // Visits to http://example.com are untouched. VisitVector visit_vector; EXPECT_TRUE(backend_->GetVisitsForURL( - backend_->db_->GetRowForURL(GURL("http://example.com"), NULL), + backend_->db_->GetRowForURL(GURL("http://example.com"), nullptr), &visit_vector)); ASSERT_EQ(5u, visit_vector.size()); EXPECT_EQ(base::Time::FromInternalValue(0), visit_vector[0].visit_time); @@ -3624,7 +3656,7 @@ TEST_F(HistoryBackendTest, ExpireHistoryForTimes) { // Visits to http://example.net between [2,8] are removed. visit_vector.clear(); EXPECT_TRUE(backend_->GetVisitsForURL( - backend_->db_->GetRowForURL(GURL("http://example.net"), NULL), + backend_->db_->GetRowForURL(GURL("http://example.net"), nullptr), &visit_vector)); ASSERT_EQ(2u, visit_vector.size()); EXPECT_EQ(base::Time::FromInternalValue(1), visit_vector[0].visit_time); @@ -3644,7 +3676,7 @@ TEST_F(HistoryBackendTest, ExpireHistory) { // Insert 4 entries into the database. HistoryAddPageArgs args[4]; for (size_t i = 0; i < arraysize(args); ++i) { - args[i].url = GURL("http://example" + base::SizeTToString(i) + ".com"); + args[i].url = GURL("http://example" + base::NumberToString(i) + ".com"); args[i].time = reference_time + base::TimeDelta::FromDays(i); backend_->AddPage(args[i]); } @@ -3747,9 +3779,9 @@ TEST_F(HistoryBackendTest, DeleteMatchingUrlsForKeyword) { // Test that corresponding keyword search terms are deleted for rows 2 & 3, // but not for row 1 - EXPECT_TRUE(backend_->db()->GetKeywordSearchTermRow(url1_id, NULL)); - EXPECT_FALSE(backend_->db()->GetKeywordSearchTermRow(url2_id, NULL)); - EXPECT_FALSE(backend_->db()->GetKeywordSearchTermRow(url3_id, NULL)); + EXPECT_TRUE(backend_->db()->GetKeywordSearchTermRow(url1_id, nullptr)); + EXPECT_FALSE(backend_->db()->GetKeywordSearchTermRow(url2_id, nullptr)); + EXPECT_FALSE(backend_->db()->GetKeywordSearchTermRow(url3_id, nullptr)); } // Simple test that removes a bookmark. This test exercises the code paths in @@ -3766,9 +3798,8 @@ TEST_F(HistoryBackendTest, RemoveNotification) { EXPECT_TRUE(service->Init( TestHistoryDatabaseParamsForPath(scoped_temp_dir.GetPath()))); - service->AddPage( - url, base::Time::Now(), NULL, 1, GURL(), RedirectList(), - ui::PAGE_TRANSITION_TYPED, SOURCE_BROWSED, false); + service->AddPage(url, base::Time::Now(), nullptr, 1, GURL(), RedirectList(), + ui::PAGE_TRANSITION_TYPED, SOURCE_BROWSED, false); // This won't actually delete the URL, rather it'll empty out the visits. // This triggers blocking on the BookmarkModel. @@ -3809,6 +3840,15 @@ TEST_F(HistoryBackendTest, DeleteFTSIndexDatabases) { EXPECT_TRUE(base::PathExists(db2_actual)); // Symlinks shouldn't be followed. } +// Tests that calling DatabaseErrorCallback doesn't cause crash. (Regression +// test for https://crbug.com/796138) +TEST_F(HistoryBackendTest, DatabaseError) { + EXPECT_EQ(nullptr, backend_->GetTypedURLSyncBridge()); + backend_->DatabaseErrorCallback(SQLITE_CORRUPT, nullptr); + // Run loop to let any posted callbacks run before TearDown(). + base::RunLoop().RunUntilIdle(); +} + // Common implementation for the two tests below, given that the only difference // between them is the type of the notification sent out. void InMemoryHistoryBackendTest::TestAddingAndChangingURLRows( @@ -3881,8 +3921,8 @@ TEST_F(InMemoryHistoryBackendTest, OnURLsDeletedPiecewise) { // correctly removed, and the non-typed URL does not magically appear. URLRow cached_row1; EXPECT_NE(0, mem_backend_->db()->GetRowForURL(row1.url(), &cached_row1)); - EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row2.url(), NULL)); - EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row3.url(), NULL)); + EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row2.url(), nullptr)); + EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row3.url(), nullptr)); EXPECT_EQ(row1.id(), cached_row1.id()); } @@ -3898,9 +3938,9 @@ TEST_F(InMemoryHistoryBackendTest, OnURLsDeletedEnMasse) { std::set<GURL>()); // Expect that everything goes away. - EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row1.url(), NULL)); - EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row2.url(), NULL)); - EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row3.url(), NULL)); + EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row1.url(), nullptr)); + EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row2.url(), nullptr)); + EXPECT_EQ(0, mem_backend_->db()->GetRowForURL(row3.url(), nullptr)); } void InMemoryHistoryBackendTest::PopulateTestURLsAndSearchTerms( @@ -3943,8 +3983,8 @@ TEST_F(InMemoryHistoryBackendTest, SetKeywordSearchTerms) { // at the low level that the rows are there. EXPECT_EQ(1u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term1)); EXPECT_EQ(1u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term2)); - EXPECT_TRUE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), NULL)); - EXPECT_TRUE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), NULL)); + EXPECT_TRUE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), nullptr)); + EXPECT_TRUE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), nullptr)); } TEST_F(InMemoryHistoryBackendTest, DeleteKeywordSearchTerms) { @@ -3968,8 +4008,8 @@ TEST_F(InMemoryHistoryBackendTest, DeleteKeywordSearchTerms) { // check at the low level that they are gone for good. EXPECT_EQ(0u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term1)); EXPECT_EQ(0u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term2)); - EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), NULL)); - EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), NULL)); + EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), nullptr)); + EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), nullptr)); } TEST_F(InMemoryHistoryBackendTest, DeleteAllSearchTermsForKeyword) { @@ -3993,8 +4033,8 @@ TEST_F(InMemoryHistoryBackendTest, DeleteAllSearchTermsForKeyword) { // check at the low level that they are gone for good. EXPECT_EQ(0u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term1)); EXPECT_EQ(0u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term2)); - EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), NULL)); - EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), NULL)); + EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), nullptr)); + EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), nullptr)); } TEST_F(InMemoryHistoryBackendTest, OnURLsDeletedWithSearchTerms) { @@ -4012,8 +4052,8 @@ TEST_F(InMemoryHistoryBackendTest, OnURLsDeletedWithSearchTerms) { // first URLRow should not be affected. EXPECT_EQ(1u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term1)); EXPECT_EQ(0u, GetNumberOfMatchingSearchTerms(kTestKeywordId, term2)); - EXPECT_TRUE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), NULL)); - EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), NULL)); + EXPECT_TRUE(mem_backend_->db()->GetKeywordSearchTermRow(row1.id(), nullptr)); + EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), nullptr)); } TEST_F(HistoryBackendTest, QueryMostVisitedURLs) { @@ -4029,7 +4069,7 @@ TEST_F(HistoryBackendTest, QueryMostVisitedURLs) { for (size_t i = 0; i < pages.size(); ++i) { HistoryAddPageArgs args; - args.url = GURL("http://example" + base::SizeTToString(i + 1) + ".com"); + args.url = GURL("http://example" + base::NumberToString(i + 1) + ".com"); args.time = base::Time::Now() - base::TimeDelta::FromDays(i + 1); args.transition = pages[i].first; args.consider_for_ntp_most_visited = pages[i].second; diff --git a/chromium/components/history/core/browser/history_database.cc b/chromium/components/history/core/browser/history_database.cc index c3db915b635..1640c737302 100644 --- a/chromium/components/history/core/browser/history_database.cc +++ b/chromium/components/history/core/browser/history_database.cc @@ -37,7 +37,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 // our database without *too* many bad effects. -const int kCurrentVersionNumber = 37; +const int kCurrentVersionNumber = 38; const int kCompatibleVersionNumber = 16; const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold"; const int kMaxHostsInMemory = 10000; @@ -366,13 +366,9 @@ SegmentID HistoryDatabase::GetSegmentID(VisitID visit_id) { "SELECT segment_id FROM visits WHERE id = ?")); s.BindInt64(0, visit_id); - if (s.Step()) { - if (s.ColumnType(0) == sql::COLUMN_TYPE_NULL) - return 0; - else - return s.ColumnInt64(0); - } - return 0; + if (!s.Step() || s.ColumnType(0) == sql::COLUMN_TYPE_NULL) + return 0; + return s.ColumnInt64(0); } base::Time HistoryDatabase::GetEarlyExpirationThreshold() { @@ -590,6 +586,13 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion() { meta_table_.SetVersionNumber(cur_version); } + if (cur_version == 37) { + if (!MigrateVisitSegmentNames()) + return LogMigrationFailure(37); + cur_version++; + meta_table_.SetVersionNumber(cur_version); + } + // ========================= ^^ new migration code goes here ^^ // ADDING NEW MIGRATION CODE // ========================= diff --git a/chromium/components/history/core/browser/history_service.cc b/chromium/components/history/core/browser/history_service.cc index 200e440b47e..f613fe214ef 100644 --- a/chromium/components/history/core/browser/history_service.cc +++ b/chromium/components/history/core/browser/history_service.cc @@ -53,6 +53,7 @@ #include "components/sync/model/sync_error_factory.h" #include "components/variations/variations_associated_data.h" #include "third_party/skia/include/core/SkBitmap.h" +#include "ui/base/page_transition_types.h" #if defined(OS_IOS) #include "base/critical_closure.h" @@ -380,8 +381,9 @@ void HistoryService::AddPage(const GURL& url, bool did_replace_entry) { DCHECK(thread_checker_.CalledOnValidThread()); AddPage(HistoryAddPageArgs(url, time, context_id, nav_entry_id, referrer, - redirects, transition, visit_source, - did_replace_entry, true)); + redirects, transition, + !ui::PageTransitionIsMainFrame(transition), + visit_source, did_replace_entry, true)); } void HistoryService::AddPage(const GURL& url, @@ -389,8 +391,8 @@ void HistoryService::AddPage(const GURL& url, VisitSource visit_source) { DCHECK(thread_checker_.CalledOnValidThread()); AddPage(HistoryAddPageArgs(url, time, nullptr, 0, GURL(), RedirectList(), - ui::PAGE_TRANSITION_LINK, visit_source, false, - true)); + ui::PAGE_TRANSITION_LINK, false, visit_source, + false, true)); } void HistoryService::AddPage(const HistoryAddPageArgs& add_page_args) { @@ -522,7 +524,7 @@ base::CancelableTaskTracker::TaskId HistoryService::GetFavicon( base::CancelableTaskTracker::TaskId HistoryService::GetFaviconsForURL( const GURL& page_url, - int icon_types, + const favicon_base::IconTypeSet& icon_types, const std::vector<int>& desired_sizes, const favicon_base::FaviconResultsCallback& callback, base::CancelableTaskTracker* tracker) { @@ -540,7 +542,7 @@ base::CancelableTaskTracker::TaskId HistoryService::GetFaviconsForURL( base::CancelableTaskTracker::TaskId HistoryService::GetLargestFaviconForURL( const GURL& page_url, - const std::vector<int>& icon_types, + const std::vector<favicon_base::IconTypeSet>& icon_types, int minimum_size_in_pixels, const favicon_base::FaviconRawBitmapCallback& callback, base::CancelableTaskTracker* tracker) { @@ -593,6 +595,18 @@ HistoryService::UpdateFaviconMappingsAndFetch( base::Bind(&RunWithFaviconResults, callback, base::Owned(results))); } +void HistoryService::DeleteFaviconMappings( + const base::flat_set<GURL>& page_urls, + favicon_base::IconType icon_type) { + TRACE_EVENT0("browser", "HistoryService::DeleteFaviconMappings"); + DCHECK(backend_task_runner_) << "History service being called after cleanup"; + DCHECK(thread_checker_.CalledOnValidThread()); + + ScheduleTask(PRIORITY_NORMAL, + base::Bind(&HistoryBackend::DeleteFaviconMappings, + history_backend_, page_urls, icon_type)); +} + void HistoryService::MergeFavicon( const GURL& page_url, const GURL& icon_url, @@ -633,6 +647,19 @@ void HistoryService::SetFavicons(const base::flat_set<GURL>& page_urls, page_urls_to_save, icon_type, icon_url, bitmaps)); } +void HistoryService::CloneFaviconMappingsForPages( + const GURL& page_url_to_read, + const favicon_base::IconTypeSet& icon_types, + const base::flat_set<GURL>& page_urls_to_write) { + DCHECK(backend_task_runner_) << "History service being called after cleanup"; + DCHECK(thread_checker_.CalledOnValidThread()); + + ScheduleTask(PRIORITY_NORMAL, + base::Bind(&HistoryBackend::CloneFaviconMappingsForPages, + history_backend_, page_url_to_read, icon_types, + page_urls_to_write)); +} + void HistoryService::SetOnDemandFavicons(const GURL& page_url, favicon_base::IconType icon_type, const GURL& icon_url, diff --git a/chromium/components/history/core/browser/history_service.h b/chromium/components/history/core/browser/history_service.h index 9b79d259d4f..bf1702f2d0a 100644 --- a/chromium/components/history/core/browser/history_service.h +++ b/chromium/components/history/core/browser/history_service.h @@ -668,7 +668,7 @@ class HistoryService : public syncer::SyncableService, public KeyedService { // will be fewer results. base::CancelableTaskTracker::TaskId GetFaviconsForURL( const GURL& page_url, - int icon_types, + const favicon_base::IconTypeSet& icon_types, const std::vector<int>& desired_sizes, const favicon_base::FaviconResultsCallback& callback, base::CancelableTaskTracker* tracker); @@ -686,7 +686,7 @@ class HistoryService : public syncer::SyncableService, public KeyedService { // long as its size is larger than a specific value. base::CancelableTaskTracker::TaskId GetLargestFaviconForURL( const GURL& page_url, - const std::vector<int>& icon_types, + const std::vector<favicon_base::IconTypeSet>& icon_types, int minimum_size_in_pixels, const favicon_base::FaviconRawBitmapCallback& callback, base::CancelableTaskTracker* tracker); @@ -715,6 +715,10 @@ class HistoryService : public syncer::SyncableService, public KeyedService { const favicon_base::FaviconResultsCallback& callback, base::CancelableTaskTracker* tracker); + // Deletes favicon mappings for each URL in |page_urls| and their redirects. + void DeleteFaviconMappings(const base::flat_set<GURL>& page_urls, + favicon_base::IconType icon_type); + // Used by FaviconService to set a favicon for |page_url| and |icon_url| with // |pixel_size|. // Example: @@ -753,6 +757,14 @@ class HistoryService : public syncer::SyncableService, public KeyedService { const GURL& icon_url, const std::vector<SkBitmap>& bitmaps); + // Causes each page in |page_urls_to_write| to be associated to the same + // icon as the page |page_url_to_read| for icon types matching |icon_types|. + // No-op if |page_url_to_read| has no mappings for |icon_types|. + void CloneFaviconMappingsForPages( + const GURL& page_url_to_read, + const favicon_base::IconTypeSet& icon_types, + const base::flat_set<GURL>& page_urls_to_write); + // Same as SetFavicons with three differences: // 1) It will be a no-op if there is an existing cached favicon for *any* type // for |page_url|. diff --git a/chromium/components/history/core/browser/history_service_unittest.cc b/chromium/components/history/core/browser/history_service_unittest.cc index 6e05213879c..5aff0e5f7b5 100644 --- a/chromium/components/history/core/browser/history_service_unittest.cc +++ b/chromium/components/history/core/browser/history_service_unittest.cc @@ -94,7 +94,7 @@ class HistoryServiceTest : public testing::Test { void CleanupHistoryService() { DCHECK(history_service_); - history_service_->ClearCachedDataForContextID(0); + history_service_->ClearCachedDataForContextID(nullptr); history_service_->SetOnBackendDestroyTask( base::MessageLoop::QuitWhenIdleClosure()); history_service_->Cleanup(); @@ -187,19 +187,18 @@ TEST_F(HistoryServiceTest, AddPage) { ASSERT_TRUE(history_service_.get()); // Add the page once from a child frame. const GURL test_url("http://www.google.com/"); - history_service_->AddPage(test_url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_MANUAL_SUBFRAME, - history::SOURCE_BROWSED, false); + history_service_->AddPage( + test_url, base::Time::Now(), nullptr, 0, GURL(), history::RedirectList(), + ui::PAGE_TRANSITION_MANUAL_SUBFRAME, history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); EXPECT_TRUE(query_url_row_.hidden()); // Hidden because of child frame. // Add the page once from the main frame (should unhide it). - history_service_->AddPage(test_url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED, false); + history_service_->AddPage(test_url, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url)); EXPECT_EQ(2, query_url_row_.visit_count()); // Added twice. EXPECT_EQ(0, query_url_row_.typed_count()); // Never typed. @@ -284,10 +283,9 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { // Add a non-typed visit to an intranet URL on an unvisited host. This should // get promoted to a typed visit. const GURL test_url("http://intranet_host/path"); - history_service_->AddPage( - test_url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED, false); + history_service_->AddPage(test_url, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(1, query_url_row_.typed_count()); @@ -300,10 +298,9 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { // Different path. const GURL test_url2("http://intranet_host/different_path"); - history_service_->AddPage( - test_url2, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED, false); + history_service_->AddPage(test_url2, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url2)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); @@ -313,10 +310,9 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { // No path. const GURL test_url3("http://intranet_host/"); - history_service_->AddPage( - test_url3, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED, false); + history_service_->AddPage(test_url3, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url3)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); @@ -326,10 +322,9 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { // Different scheme. const GURL test_url4("https://intranet_host/"); - history_service_->AddPage( - test_url4, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED, false); + history_service_->AddPage(test_url4, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url4)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); @@ -340,10 +335,8 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { // Different transition. const GURL test_url5("http://intranet_host/another_path"); history_service_->AddPage( - test_url5, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_AUTO_BOOKMARK, - history::SOURCE_BROWSED, false); + test_url5, base::Time::Now(), nullptr, 0, GURL(), history::RedirectList(), + ui::PAGE_TRANSITION_AUTO_BOOKMARK, history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url5)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); @@ -352,10 +345,9 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { ui::PAGE_TRANSITION_AUTO_BOOKMARK)); // Original URL. - history_service_->AddPage( - test_url, base::Time::Now(), NULL, 0, GURL(), - history::RedirectList(), ui::PAGE_TRANSITION_LINK, - history::SOURCE_BROWSED, false); + history_service_->AddPage(test_url, base::Time::Now(), nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), test_url)); EXPECT_EQ(2, query_url_row_.visit_count()); EXPECT_EQ(1, query_url_row_.typed_count()); @@ -367,7 +359,7 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { history::RedirectList redirects1 = {GURL("http://intranet1/path"), GURL("http://second1.com/"), GURL("http://third1.com/")}; - history_service_->AddPage(redirects1.back(), base::Time::Now(), NULL, 0, + history_service_->AddPage(redirects1.back(), base::Time::Now(), nullptr, 0, GURL(), redirects1, ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), redirects1.front())); @@ -381,7 +373,7 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { history::RedirectList redirects2 = {GURL("http://first2.com/"), GURL("http://second2.com/"), GURL("http://intranet2/path")}; - history_service_->AddPage(redirects2.back(), base::Time::Now(), NULL, 0, + history_service_->AddPage(redirects2.back(), base::Time::Now(), nullptr, 0, GURL(), redirects2, ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), redirects2.back())); @@ -395,7 +387,7 @@ TEST_F(HistoryServiceTest, MakeIntranetURLsTyped) { history::RedirectList redirects3 = {GURL("http://first3.com/"), GURL("http://intranet3/path"), GURL("http://third3.com/")}; - history_service_->AddPage(redirects3.back(), base::Time::Now(), NULL, 0, + history_service_->AddPage(redirects3.back(), base::Time::Now(), nullptr, 0, GURL(), redirects3, ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history_service_.get(), redirects3[1])); @@ -678,9 +670,8 @@ TEST_F(HistoryServiceTest, ProcessLocalDeleteDirectiveSyncOnline) { for (int64_t i = 1; i <= 10; ++i) { base::Time t = base::Time::UnixEpoch() + base::TimeDelta::FromMicroseconds(i); - history_service_->AddPage(test_url, t, NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_LINK, + history_service_->AddPage(test_url, t, nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED, false); } @@ -742,9 +733,8 @@ TEST_F(HistoryServiceTest, ProcessGlobalIdDeleteDirective) { for (int64_t i = 1; i <= 20; i++) { base::Time t = base::Time::UnixEpoch() + base::TimeDelta::FromMicroseconds(i); - history_service_->AddPage(test_url, t, NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_LINK, + history_service_->AddPage(test_url, t, nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED, false); } @@ -832,9 +822,8 @@ TEST_F(HistoryServiceTest, ProcessTimeRangeDeleteDirective) { for (int64_t i = 1; i <= 10; ++i) { base::Time t = base::Time::UnixEpoch() + base::TimeDelta::FromMicroseconds(i); - history_service_->AddPage(test_url, t, NULL, 0, GURL(), - history::RedirectList(), - ui::PAGE_TRANSITION_LINK, + history_service_->AddPage(test_url, t, nullptr, 0, GURL(), + history::RedirectList(), ui::PAGE_TRANSITION_LINK, history::SOURCE_BROWSED, false); } diff --git a/chromium/components/history/core/browser/history_types.cc b/chromium/components/history/core/browser/history_types.cc index 7365619b0a8..50ce78be147 100644 --- a/chromium/components/history/core/browser/history_types.cc +++ b/chromium/components/history/core/browser/history_types.cc @@ -43,7 +43,7 @@ const size_t* QueryResults::MatchesForURL(const GURL& url, if (found == url_to_results_.end()) { if (num_matches) *num_matches = 0; - return NULL; + return nullptr; } // All entries in the map should have at least one index, otherwise it @@ -72,7 +72,7 @@ void QueryResults::SetURLResults(std::vector<URLResult>&& results) { void QueryResults::DeleteURL(const GURL& url) { // Delete all instances of this URL. We re-query each time since each // mutation will cause the indices to change. - while (const size_t* match_indices = MatchesForURL(url, NULL)) + while (const size_t* match_indices = MatchesForURL(url, nullptr)) DeleteRange(*match_indices, *match_indices); } @@ -181,6 +181,13 @@ MostVisitedURL::MostVisitedURL(const GURL& url, base::Time last_forced_time) : url(url), title(title), last_forced_time(last_forced_time) {} +MostVisitedURL::MostVisitedURL(const GURL& url, + const base::string16& title, + const RedirectList& preceding_redirects) + : url(url), title(title) { + InitRedirects(preceding_redirects); +} + MostVisitedURL::MostVisitedURL(const MostVisitedURL& other) = default; // TODO(bug 706963) this should be implemented as "= default" when Android @@ -193,6 +200,21 @@ MostVisitedURL::MostVisitedURL(MostVisitedURL&& other) noexcept MostVisitedURL::~MostVisitedURL() {} +void MostVisitedURL::InitRedirects(const RedirectList& redirects_from) { + redirects.clear(); + + if (redirects_from.empty()) { + // Redirects must contain at least the target URL. + redirects.push_back(url); + } else { + redirects = redirects_from; + if (redirects.back() != url) { + // The last url must be the target URL. + redirects.push_back(url); + } + } +} + MostVisitedURL& MostVisitedURL::operator=(const MostVisitedURL&) = default; // FilteredURL ----------------------------------------------------------------- @@ -239,10 +261,10 @@ HistoryAddPageArgs::HistoryAddPageArgs() GURL(), RedirectList(), ui::PAGE_TRANSITION_LINK, + false, SOURCE_BROWSED, false, - true) { -} + true) {} HistoryAddPageArgs::HistoryAddPageArgs(const GURL& url, base::Time time, @@ -251,6 +273,7 @@ HistoryAddPageArgs::HistoryAddPageArgs(const GURL& url, const GURL& referrer, const RedirectList& redirects, ui::PageTransition transition, + bool hidden, VisitSource source, bool did_replace_entry, bool consider_for_ntp_most_visited) @@ -261,10 +284,10 @@ HistoryAddPageArgs::HistoryAddPageArgs(const GURL& url, referrer(referrer), redirects(redirects), transition(transition), + hidden(hidden), visit_source(source), did_replace_entry(did_replace_entry), - consider_for_ntp_most_visited(consider_for_ntp_most_visited) { -} + consider_for_ntp_most_visited(consider_for_ntp_most_visited) {} HistoryAddPageArgs::HistoryAddPageArgs(const HistoryAddPageArgs& other) = default; diff --git a/chromium/components/history/core/browser/history_types.h b/chromium/components/history/core/browser/history_types.h index fd6b86231a7..4f9c2cd6c6c 100644 --- a/chromium/components/history/core/browser/history_types.h +++ b/chromium/components/history/core/browser/history_types.h @@ -297,10 +297,17 @@ struct MostVisitedURL { MostVisitedURL(const GURL& url, const base::string16& title, base::Time last_forced_time = base::Time()); + MostVisitedURL(const GURL& url, + const base::string16& title, + const RedirectList& preceding_redirects); MostVisitedURL(const MostVisitedURL& other); MostVisitedURL(MostVisitedURL&& other) noexcept; ~MostVisitedURL(); + // Initializes |redirects| from |preceding_redirects|, ensuring that |url| is + // always present as the last item. + void InitRedirects(const RedirectList& preceding_redirects); + GURL url; base::string16 title; @@ -354,7 +361,7 @@ struct HistoryAddPageArgs { // HistoryAddPageArgs( // GURL(), base::Time(), NULL, 0, GURL(), // RedirectList(), ui::PAGE_TRANSITION_LINK, - // SOURCE_BROWSED, false, true) + // false, SOURCE_BROWSED, false, true) HistoryAddPageArgs(); HistoryAddPageArgs(const GURL& url, base::Time time, @@ -363,6 +370,7 @@ struct HistoryAddPageArgs { const GURL& referrer, const RedirectList& redirects, ui::PageTransition transition, + bool hidden, VisitSource source, bool did_replace_entry, bool consider_for_ntp_most_visited); @@ -376,6 +384,7 @@ struct HistoryAddPageArgs { GURL referrer; RedirectList redirects; ui::PageTransition transition; + bool hidden; VisitSource visit_source; bool did_replace_entry; // Specifies whether a page visit should contribute to the Most Visited tiles @@ -490,7 +499,7 @@ struct IconMapping { GURL icon_url; // The type of icon. - favicon_base::IconType icon_type = favicon_base::INVALID_ICON; + favicon_base::IconType icon_type = favicon_base::IconType::kInvalid; }; // Defines a favicon bitmap and its associated pixel size. diff --git a/chromium/components/history/core/browser/history_types_unittest.cc b/chromium/components/history/core/browser/history_types_unittest.cc index 09dd46cefb3..27ecabb116a 100644 --- a/chromium/components/history/core/browser/history_types_unittest.cc +++ b/chromium/components/history/core/browser/history_types_unittest.cc @@ -88,8 +88,8 @@ TEST(HistoryQueryResult, DeleteRange) { // Now delete everything and make sure it's deleted. results.DeleteRange(0, 1); EXPECT_EQ(0U, results.size()); - EXPECT_FALSE(results.MatchesForURL(url1, NULL)); - EXPECT_FALSE(results.MatchesForURL(url2, NULL)); + EXPECT_FALSE(results.MatchesForURL(url1, nullptr)); + EXPECT_FALSE(results.MatchesForURL(url2, nullptr)); } // Tests insertion and deletion by URL. @@ -106,7 +106,7 @@ TEST(HistoryQueryResult, ResultDeleteURL) { // The first one should be gone, and the second one should be at [0]. size_t match_count; - EXPECT_FALSE(results.MatchesForURL(url1, NULL)); + EXPECT_FALSE(results.MatchesForURL(url1, nullptr)); const size_t* matches = results.MatchesForURL(url2, &match_count); ASSERT_EQ(1U, match_count); EXPECT_TRUE(matches[0] == 0); @@ -114,7 +114,7 @@ TEST(HistoryQueryResult, ResultDeleteURL) { // Delete the second URL, there should be nothing left. results.DeleteURL(url2); EXPECT_EQ(0U, results.size()); - EXPECT_FALSE(results.MatchesForURL(url2, NULL)); + EXPECT_FALSE(results.MatchesForURL(url2, nullptr)); } } // namespace history diff --git a/chromium/components/history/core/browser/in_memory_history_backend.cc b/chromium/components/history/core/browser/in_memory_history_backend.cc index ab540e61bf8..9e11553e1ea 100644 --- a/chromium/components/history/core/browser/in_memory_history_backend.cc +++ b/chromium/components/history/core/browser/in_memory_history_backend.cc @@ -102,7 +102,8 @@ void InMemoryHistoryBackend::OnKeywordSearchTermDeleted( void InMemoryHistoryBackend::OnURLVisitedOrModified(const URLRow& url_row) { DCHECK(db_); DCHECK(url_row.id()); - if (url_row.typed_count() || db_->GetKeywordSearchTermRow(url_row.id(), NULL)) + if (url_row.typed_count() || + db_->GetKeywordSearchTermRow(url_row.id(), nullptr)) db_->InsertOrUpdateURLRowByID(url_row); else db_->DeleteURLRow(url_row.id()); diff --git a/chromium/components/history/core/browser/thumbnail_database.cc b/chromium/components/history/core/browser/thumbnail_database.cc index 01933fc464c..9d1ffa14d0f 100644 --- a/chromium/components/history/core/browser/thumbnail_database.cc +++ b/chromium/components/history/core/browser/thumbnail_database.cc @@ -11,6 +11,7 @@ #include <utility> #include "base/bind.h" +#include "base/bits.h" #include "base/debug/alias.h" #include "base/files/file_util.h" #include "base/memory/ref_counted_memory.h" @@ -50,7 +51,7 @@ namespace history { // id Unique ID. // url The URL at which the favicon file is located. // icon_type The type of the favicon specified in the rel attribute of -// the link tag. The FAVICON type is used for the default +// the link tag. The kFavicon type is used for the default // favicon.ico favicon. // // favicon_bitmaps This table contains the PNG encoded bitmap data of the @@ -106,7 +107,7 @@ void FillIconMapping(const sql::Statement& statement, icon_mapping->mapping_id = statement.ColumnInt64(0); icon_mapping->icon_id = statement.ColumnInt64(1); icon_mapping->icon_type = - static_cast<favicon_base::IconType>(statement.ColumnInt(2)); + ThumbnailDatabase::FromPersistedIconType(statement.ColumnInt(2)); icon_mapping->icon_url = GURL(statement.ColumnString(3)); icon_mapping->page_url = page_url; } @@ -167,7 +168,7 @@ bool InitTables(sql::Connection* db) { "(" "id INTEGER PRIMARY KEY," "url LONGVARCHAR NOT NULL," - // default icon_type FAVICON to be consistent with past migration. + // default icon_type kFavicon to be consistent with past migration. "icon_type INTEGER DEFAULT 1" ")"; if (!db->Execute(kFaviconsSql)) @@ -357,8 +358,10 @@ void ThumbnailDatabase::ComputeDatabaseMetrics() { db_.GetCachedStatement( SQL_FROM_HERE, "SELECT COUNT(*) FROM favicons WHERE icon_type IN (?, ?)")); - touch_icon_count.BindInt64(0, favicon_base::TOUCH_ICON); - touch_icon_count.BindInt64(1, favicon_base::TOUCH_PRECOMPOSED_ICON); + touch_icon_count.BindInt64( + 0, ToPersistedIconType(favicon_base::IconType::kTouchIcon)); + touch_icon_count.BindInt64( + 1, ToPersistedIconType(favicon_base::IconType::kTouchPrecomposedIcon)); UMA_HISTOGRAM_COUNTS_10000( "History.NumTouchIconsInDB", touch_icon_count.Step() ? touch_icon_count.ColumnInt(0) : 0); @@ -663,7 +666,7 @@ favicon_base::FaviconID ThumbnailDatabase::GetFaviconIDForFaviconURL( sql::Statement statement(db_.GetCachedStatement( SQL_FROM_HERE, "SELECT id FROM favicons WHERE url=? AND icon_type=?")); statement.BindString(0, URLDatabase::GURLToDatabaseURL(icon_url)); - statement.BindInt(1, icon_type); + statement.BindInt(1, ToPersistedIconType(icon_type)); if (!statement.Step()) return 0; // not cached @@ -686,7 +689,7 @@ bool ThumbnailDatabase::GetFaviconHeader(favicon_base::FaviconID icon_id, if (icon_url) *icon_url = GURL(statement.ColumnString(0)); if (icon_type) - *icon_type = static_cast<favicon_base::IconType>(statement.ColumnInt(1)); + *icon_type = FromPersistedIconType(statement.ColumnInt(1)); return true; } @@ -698,7 +701,7 @@ favicon_base::FaviconID ThumbnailDatabase::AddFavicon( sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE, "INSERT INTO favicons (url, icon_type) VALUES (?, ?)")); statement.BindString(0, URLDatabase::GURLToDatabaseURL(icon_url)); - statement.BindInt(1, icon_type); + statement.BindInt(1, ToPersistedIconType(icon_type)); if (!statement.Run()) return 0; @@ -735,7 +738,7 @@ bool ThumbnailDatabase::DeleteFavicon(favicon_base::FaviconID id) { bool ThumbnailDatabase::GetIconMappingsForPageURL( const GURL& page_url, - int required_icon_types, + const favicon_base::IconTypeSet& required_icon_types, std::vector<IconMapping>* filtered_mapping_data) { std::vector<IconMapping> mapping_data; if (!GetIconMappingsForPageURL(page_url, &mapping_data)) @@ -744,7 +747,7 @@ bool ThumbnailDatabase::GetIconMappingsForPageURL( bool result = false; for (std::vector<IconMapping>::iterator m = mapping_data.begin(); m != mapping_data.end(); ++m) { - if (m->icon_type & required_icon_types) { + if (required_icon_types.count(m->icon_type) != 0) { result = true; if (!filtered_mapping_data) return result; @@ -842,7 +845,7 @@ bool ThumbnailDatabase::InitIconMappingEnumerator( "FROM icon_mapping JOIN favicons ON (" "icon_mapping.icon_id = favicons.id) " "WHERE favicons.icon_type = ?")); - enumerator->statement_.BindInt(0, type); + enumerator->statement_.BindInt(0, ToPersistedIconType(type)); return enumerator->statement_.is_valid(); } @@ -974,6 +977,26 @@ bool ThumbnailDatabase::RetainDataForPageUrls( return transaction.Commit(); } +// static +int ThumbnailDatabase::ToPersistedIconType(favicon_base::IconType icon_type) { + if (icon_type == favicon_base::IconType::kInvalid) + return 0; + + return 1 << (static_cast<int>(icon_type) - 1); +} + +// static +favicon_base::IconType ThumbnailDatabase::FromPersistedIconType(int icon_type) { + if (icon_type == 0) + return favicon_base::IconType::kInvalid; + + int val = 1 + base::bits::Log2Floor(icon_type); + if (val > static_cast<int>(favicon_base::IconType::kMax)) + return favicon_base::IconType::kInvalid; + + return static_cast<favicon_base::IconType>(val); +} + sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db, const base::FilePath& db_name) { db->set_histogram_tag("Thumbnail"); @@ -1114,14 +1137,16 @@ sql::InitStatus ThumbnailDatabase::CantUpgradeToVersion(int cur_version) { bool ThumbnailDatabase::UpgradeToVersion7() { // Sizes column was never used, remove it. bool success = - db_.Execute("CREATE TABLE temp_favicons (" - "id INTEGER PRIMARY KEY," - "url LONGVARCHAR NOT NULL," - // default icon_type FAVICON to be consistent with - // past migration. - "icon_type INTEGER DEFAULT 1)") && - db_.Execute("INSERT INTO temp_favicons (id, url, icon_type) " - "SELECT id, url, icon_type FROM favicons") && + db_.Execute( + "CREATE TABLE temp_favicons (" + "id INTEGER PRIMARY KEY," + "url LONGVARCHAR NOT NULL," + // default icon_type kFavicon to be consistent with + // past migration. + "icon_type INTEGER DEFAULT 1)") && + db_.Execute( + "INSERT INTO temp_favicons (id, url, icon_type) " + "SELECT id, url, icon_type FROM favicons") && db_.Execute("DROP TABLE favicons") && db_.Execute("ALTER TABLE temp_favicons RENAME TO favicons") && db_.Execute("CREATE INDEX IF NOT EXISTS favicons_url ON favicons(url)"); diff --git a/chromium/components/history/core/browser/thumbnail_database.h b/chromium/components/history/core/browser/thumbnail_database.h index bd29eb4938b..7b1a1eda3d6 100644 --- a/chromium/components/history/core/browser/thumbnail_database.h +++ b/chromium/components/history/core/browser/thumbnail_database.h @@ -186,9 +186,10 @@ class ThumbnailDatabase { // Returns true if there are icon mappings for the given page and icon types. // The matched icon mappings are returned in the |mapping_data| parameter if // it is not NULL. - bool GetIconMappingsForPageURL(const GURL& page_url, - int required_icon_types, - std::vector<IconMapping>* mapping_data); + bool GetIconMappingsForPageURL( + const GURL& page_url, + const favicon_base::IconTypeSet& required_icon_types, + std::vector<IconMapping>* mapping_data); // Returns true if there is any matched icon mapping for the given page. // All matched icon mappings are returned in descent order of IconType if @@ -245,6 +246,12 @@ class ThumbnailDatabase { // so failure causes any outer transaction to be rolled back. bool RetainDataForPageUrls(const std::vector<GURL>& urls_to_keep); + // For historical reasons, and for backward compatibility, the icon type + // values stored in the DB are powers of two. Conversion functions + // exposed publicly for testing. + static int ToPersistedIconType(favicon_base::IconType icon_type); + static favicon_base::IconType FromPersistedIconType(int icon_type); + private: FRIEND_TEST_ALL_PREFIXES(ThumbnailDatabaseTest, RetainDataForPageUrls); FRIEND_TEST_ALL_PREFIXES(ThumbnailDatabaseTest, diff --git a/chromium/components/history/core/browser/thumbnail_database_unittest.cc b/chromium/components/history/core/browser/thumbnail_database_unittest.cc index 3bf1afb842b..ff98ed1540e 100644 --- a/chromium/components/history/core/browser/thumbnail_database_unittest.cc +++ b/chromium/components/history/core/browser/thumbnail_database_unittest.cc @@ -193,7 +193,7 @@ class ThumbnailDatabaseTest : public testing::Test { return std::unique_ptr<ThumbnailDatabase>(); } - std::unique_ptr<ThumbnailDatabase> db(new ThumbnailDatabase(NULL)); + std::unique_ptr<ThumbnailDatabase> db(new ThumbnailDatabase(nullptr)); EXPECT_EQ(sql::INIT_OK, db->Init(file_name_)); db->BeginTransaction(); @@ -213,7 +213,7 @@ class ThumbnailDatabaseTest : public testing::Test { }; TEST_F(ThumbnailDatabaseTest, AddIconMapping) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); @@ -223,7 +223,7 @@ TEST_F(ThumbnailDatabaseTest, AddIconMapping) { GURL url("http://google.com"); base::Time time = base::Time::Now(); favicon_base::FaviconID id = - db.AddFavicon(url, favicon_base::TOUCH_ICON, favicon, + db.AddFavicon(url, favicon_base::IconType::kTouchIcon, favicon, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(0, id); @@ -248,7 +248,8 @@ TEST_F(ThumbnailDatabaseTest, scoped_refptr<base::RefCountedBytes> favicon(new base::RefCountedBytes(data)); GURL url("http://google.com"); - favicon_base::FaviconID icon = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID icon = + db.AddFavicon(url, favicon_base::IconType::kFavicon); ASSERT_NE(0, icon); FaviconBitmapID bitmap = db.AddFaviconBitmap( icon, favicon, FaviconBitmapType::ON_DEMAND, add_time, gfx::Size()); @@ -274,7 +275,8 @@ TEST_F(ThumbnailDatabaseTest, AddFaviconBitmapCreatesCorrectTimestamps) { scoped_refptr<base::RefCountedBytes> favicon(new base::RefCountedBytes(data)); GURL url("http://google.com"); - favicon_base::FaviconID icon = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID icon = + db.AddFavicon(url, favicon_base::IconType::kFavicon); ASSERT_NE(0, icon); FaviconBitmapID bitmap = db.AddFaviconBitmap( icon, favicon, FaviconBitmapType::ON_VISIT, add_time, gfx::Size()); @@ -300,7 +302,8 @@ TEST_F(ThumbnailDatabaseTest, TouchUpdatesOnDemandFavicons) { // Create an on-demand favicon. GURL url("http://google.com"); - favicon_base::FaviconID icon = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID icon = + db.AddFavicon(url, favicon_base::IconType::kFavicon); ASSERT_NE(0, icon); FaviconBitmapID bitmap = db.AddFaviconBitmap( icon, favicon, FaviconBitmapType::ON_DEMAND, start, gfx::Size()); @@ -331,7 +334,8 @@ TEST_F(ThumbnailDatabaseTest, TouchUpdatesOnlyInfrequently) { // Create an on-demand favicon. GURL url("http://google.com"); - favicon_base::FaviconID icon = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID icon = + db.AddFavicon(url, favicon_base::IconType::kFavicon); ASSERT_NE(0, icon); FaviconBitmapID bitmap = db.AddFaviconBitmap( icon, favicon, FaviconBitmapType::ON_DEMAND, start, gfx::Size()); @@ -358,7 +362,8 @@ TEST_F(ThumbnailDatabaseTest, TouchDoesNotUpdateStandardFavicons) { // Create a standard favicon. GURL url("http://google.com"); - favicon_base::FaviconID icon = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID icon = + db.AddFavicon(url, favicon_base::IconType::kFavicon); EXPECT_NE(0, icon); FaviconBitmapID bitmap = db.AddFaviconBitmap( icon, favicon, FaviconBitmapType::ON_VISIT, start, gfx::Size()); @@ -390,7 +395,7 @@ TEST_F(ThumbnailDatabaseTest, GetOldOnDemandFaviconsReturnsOld) { GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon = - db.AddFavicon(url, favicon_base::FAVICON, favicon, + db.AddFavicon(url, favicon_base::IconType::kFavicon, favicon, FaviconBitmapType::ON_DEMAND, start, gfx::Size()); ASSERT_NE(0, icon); // Associate two different URLs with the icon. @@ -425,7 +430,7 @@ TEST_F(ThumbnailDatabaseTest, GetOldOnDemandFaviconsDoesNotReturnExpired) { GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon = - db.AddFavicon(url, favicon_base::FAVICON, favicon, + db.AddFavicon(url, favicon_base::IconType::kFavicon, favicon, FaviconBitmapType::ON_VISIT, start, gfx::Size()); ASSERT_NE(0, icon); GURL page_url("http://google.com/"); @@ -453,7 +458,7 @@ TEST_F(ThumbnailDatabaseTest, GetOldOnDemandFaviconsDoesNotReturnFresh) { GURL url("http://google.com/favicon.ico"); favicon_base::FaviconID icon = - db.AddFavicon(url, favicon_base::FAVICON, favicon, + db.AddFavicon(url, favicon_base::IconType::kFavicon, favicon, FaviconBitmapType::ON_DEMAND, start, gfx::Size()); ASSERT_NE(0, icon); ASSERT_NE(0, db.AddIconMapping(GURL("http://google.com/"), icon)); @@ -482,8 +487,8 @@ TEST_F(ThumbnailDatabaseTest, GetOldOnDemandFaviconsDoesNotDeleteStandard) { scoped_refptr<base::RefCountedBytes> favicon(new base::RefCountedBytes(data)); favicon_base::FaviconID icon = db.AddFavicon( - GURL("http://google.com/favicon.ico"), favicon_base::FAVICON, favicon, - FaviconBitmapType::ON_VISIT, start, gfx::Size()); + GURL("http://google.com/favicon.ico"), favicon_base::IconType::kFavicon, + favicon, FaviconBitmapType::ON_VISIT, start, gfx::Size()); ASSERT_NE(0, icon); ASSERT_NE(0, db.AddIconMapping(GURL("http://google.com/"), icon)); @@ -495,7 +500,7 @@ TEST_F(ThumbnailDatabaseTest, GetOldOnDemandFaviconsDoesNotDeleteStandard) { } TEST_F(ThumbnailDatabaseTest, DeleteIconMappings) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); @@ -503,30 +508,34 @@ TEST_F(ThumbnailDatabaseTest, DeleteIconMappings) { scoped_refptr<base::RefCountedBytes> favicon(new base::RefCountedBytes(data)); GURL url("http://google.com"); - favicon_base::FaviconID id = db.AddFavicon(url, favicon_base::TOUCH_ICON); + favicon_base::FaviconID id = + db.AddFavicon(url, favicon_base::IconType::kTouchIcon); base::Time time = base::Time::Now(); db.AddFaviconBitmap(id, favicon, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_LT(0, db.AddIconMapping(url, id)); - favicon_base::FaviconID id2 = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID id2 = + db.AddFavicon(url, favicon_base::IconType::kFavicon); EXPECT_LT(0, db.AddIconMapping(url, id2)); ASSERT_NE(id, id2); std::vector<IconMapping> icon_mapping; EXPECT_TRUE(db.GetIconMappingsForPageURL(url, &icon_mapping)); ASSERT_EQ(2u, icon_mapping.size()); - EXPECT_EQ(icon_mapping.front().icon_type, favicon_base::TOUCH_ICON); - EXPECT_TRUE(db.GetIconMappingsForPageURL(url, favicon_base::FAVICON, NULL)); + EXPECT_EQ(icon_mapping.front().icon_type, favicon_base::IconType::kTouchIcon); + EXPECT_TRUE(db.GetIconMappingsForPageURL( + url, {favicon_base::IconType::kFavicon}, nullptr)); db.DeleteIconMappings(url); - EXPECT_FALSE(db.GetIconMappingsForPageURL(url, NULL)); - EXPECT_FALSE(db.GetIconMappingsForPageURL(url, favicon_base::FAVICON, NULL)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(url, nullptr)); + EXPECT_FALSE(db.GetIconMappingsForPageURL( + url, {favicon_base::IconType::kFavicon}, nullptr)); } TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURL) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); @@ -535,7 +544,8 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURL) { GURL url("http://google.com"); - favicon_base::FaviconID id1 = db.AddFavicon(url, favicon_base::TOUCH_ICON); + favicon_base::FaviconID id1 = + db.AddFavicon(url, favicon_base::IconType::kTouchIcon); base::Time time = base::Time::Now(); db.AddFaviconBitmap(id1, favicon, FaviconBitmapType::ON_VISIT, time, kSmallSize); @@ -543,7 +553,8 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURL) { kLargeSize); EXPECT_LT(0, db.AddIconMapping(url, id1)); - favicon_base::FaviconID id2 = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID id2 = + db.AddFavicon(url, favicon_base::IconType::kFavicon); EXPECT_NE(id1, id2); db.AddFaviconBitmap(id2, favicon, FaviconBitmapType::ON_VISIT, time, kSmallSize); @@ -557,7 +568,7 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURL) { } TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrls) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); @@ -581,7 +592,7 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrls) { new base::RefCountedStaticMemory(kBlob2, sizeof(kBlob2))); favicon_base::FaviconID kept_id1 = - db.AddFavicon(kIconUrl1, favicon_base::FAVICON); + db.AddFavicon(kIconUrl1, favicon_base::IconType::kFavicon); db.AddFaviconBitmap(kept_id1, favicon1, FaviconBitmapType::ON_VISIT, base::Time::Now(), kLargeSize); db.AddIconMapping(kPageUrl1, kept_id1); @@ -589,13 +600,13 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrls) { db.AddIconMapping(kPageUrl4, kept_id1); favicon_base::FaviconID unkept_id = - db.AddFavicon(kIconUrl2, favicon_base::FAVICON); + db.AddFavicon(kIconUrl2, favicon_base::IconType::kFavicon); db.AddFaviconBitmap(unkept_id, favicon1, FaviconBitmapType::ON_VISIT, base::Time::Now(), kLargeSize); db.AddIconMapping(kPageUrl2, unkept_id); favicon_base::FaviconID kept_id2 = - db.AddFavicon(kIconUrl5, favicon_base::FAVICON); + db.AddFavicon(kIconUrl5, favicon_base::IconType::kFavicon); db.AddFaviconBitmap(kept_id2, favicon2, FaviconBitmapType::ON_VISIT, base::Time::Now(), kLargeSize); db.AddIconMapping(kPageUrl5, kept_id2); @@ -611,27 +622,12 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrls) { EXPECT_TRUE(db.RetainDataForPageUrls(pages_to_keep)); // Mappings from the retained urls should be left. - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl3, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl5, - favicon_base::FAVICON, - kIconUrl5, - kLargeSize, - sizeof(kBlob2), - kBlob2)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, favicon_base::IconType::kFavicon, + kIconUrl1, kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl3, favicon_base::IconType::kFavicon, + kIconUrl1, kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl5, favicon_base::IconType::kFavicon, + kIconUrl5, kLargeSize, sizeof(kBlob2), kBlob2)); // The ones not retained should be missing. EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); @@ -643,21 +639,21 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrls) { // Test that RetainDataForPageUrls() expires retained favicons. TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrlsExpiresRetainedFavicons) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); scoped_refptr<base::RefCountedStaticMemory> favicon1( new base::RefCountedStaticMemory(kBlob1, sizeof(kBlob1))); favicon_base::FaviconID kept_id = db.AddFavicon( - kIconUrl1, favicon_base::FAVICON, favicon1, FaviconBitmapType::ON_VISIT, - base::Time::Now(), gfx::Size()); + kIconUrl1, favicon_base::IconType::kFavicon, favicon1, + FaviconBitmapType::ON_VISIT, base::Time::Now(), gfx::Size()); db.AddIconMapping(kPageUrl1, kept_id); EXPECT_TRUE(db.RetainDataForPageUrls(std::vector<GURL>(1u, kPageUrl1))); favicon_base::FaviconID new_favicon_id = - db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::FAVICON); + db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon); ASSERT_NE(0, new_favicon_id); std::vector<FaviconBitmap> new_favicon_bitmaps; db.GetFaviconBitmaps(new_favicon_id, &new_favicon_bitmaps); @@ -669,7 +665,7 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrlsExpiresRetainedFavicons) { // Tests that deleting a favicon deletes the favicon row and favicon bitmap // rows from the database. TEST_F(ThumbnailDatabaseTest, DeleteFavicon) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); @@ -681,21 +677,22 @@ TEST_F(ThumbnailDatabaseTest, DeleteFavicon) { new base::RefCountedBytes(data2)); GURL url("http://google.com"); - favicon_base::FaviconID id = db.AddFavicon(url, favicon_base::FAVICON); + favicon_base::FaviconID id = + db.AddFavicon(url, favicon_base::IconType::kFavicon); base::Time last_updated = base::Time::Now(); db.AddFaviconBitmap(id, favicon1, FaviconBitmapType::ON_VISIT, last_updated, kSmallSize); db.AddFaviconBitmap(id, favicon2, FaviconBitmapType::ON_VISIT, last_updated, kLargeSize); - EXPECT_TRUE(db.GetFaviconBitmaps(id, NULL)); + EXPECT_TRUE(db.GetFaviconBitmaps(id, nullptr)); EXPECT_TRUE(db.DeleteFavicon(id)); - EXPECT_FALSE(db.GetFaviconBitmaps(id, NULL)); + EXPECT_FALSE(db.GetFaviconBitmaps(id, nullptr)); } TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); @@ -708,7 +705,7 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { base::Time time = base::Time::Now(); favicon_base::FaviconID id = - db.AddFavicon(icon_url, favicon_base::FAVICON, favicon, + db.AddFavicon(icon_url, favicon_base::IconType::kFavicon, favicon, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(0, db.AddIconMapping(page_url, id)); std::vector<IconMapping> icon_mappings; @@ -716,7 +713,7 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { EXPECT_EQ(page_url, icon_mappings.front().page_url); EXPECT_EQ(id, icon_mappings.front().icon_id); - EXPECT_EQ(favicon_base::FAVICON, icon_mappings.front().icon_type); + EXPECT_EQ(favicon_base::IconType::kFavicon, icon_mappings.front().icon_type); EXPECT_EQ(icon_url, icon_mappings.front().icon_url); // Add a touch icon @@ -725,7 +722,7 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { new base::RefCountedBytes(data); favicon_base::FaviconID id2 = - db.AddFavicon(icon_url, favicon_base::TOUCH_ICON, favicon2, + db.AddFavicon(icon_url, favicon_base::IconType::kTouchIcon, favicon2, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(0, db.AddIconMapping(page_url, id2)); @@ -734,7 +731,8 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { EXPECT_EQ(page_url, icon_mappings.front().page_url); EXPECT_EQ(id2, icon_mappings.front().icon_id); - EXPECT_EQ(favicon_base::TOUCH_ICON, icon_mappings.front().icon_type); + EXPECT_EQ(favicon_base::IconType::kTouchIcon, + icon_mappings.front().icon_type); EXPECT_EQ(icon_url, icon_mappings.front().icon_url); // Add a touch precomposed icon @@ -742,8 +740,8 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { new base::RefCountedBytes(data2); favicon_base::FaviconID id3 = - db.AddFavicon(icon_url, favicon_base::TOUCH_PRECOMPOSED_ICON, favicon3, - FaviconBitmapType::ON_VISIT, time, gfx::Size()); + db.AddFavicon(icon_url, favicon_base::IconType::kTouchPrecomposedIcon, + favicon3, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(0, db.AddIconMapping(page_url, id3)); icon_mappings.clear(); @@ -751,7 +749,7 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { EXPECT_EQ(page_url, icon_mappings.front().page_url); EXPECT_EQ(id3, icon_mappings.front().icon_id); - EXPECT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON, + EXPECT_EQ(favicon_base::IconType::kTouchPrecomposedIcon, icon_mappings.front().icon_type); EXPECT_EQ(icon_url, icon_mappings.front().icon_url); } @@ -759,21 +757,25 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) { // Test that when multiple icon types are passed to GetIconMappingsForPageURL() // that the results are filtered according to the passed in types. TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLWithIconTypes) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); const GURL kPageUrl("http://www.google.com"); - AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl1, favicon_base::FAVICON); - AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl2, favicon_base::TOUCH_ICON); - AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl3, favicon_base::TOUCH_ICON); + AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl1, + favicon_base::IconType::kFavicon); + AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl2, + favicon_base::IconType::kTouchIcon); + AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl3, + favicon_base::IconType::kTouchIcon); AddAndMapFaviconSimple(&db, kPageUrl, kIconUrl5, - favicon_base::TOUCH_PRECOMPOSED_ICON); + favicon_base::IconType::kTouchPrecomposedIcon); - // Only the mappings for FAVICON and TOUCH_ICON should be returned. + // Only the mappings for kFavicon and kTouchIcon should be returned. std::vector<IconMapping> icon_mappings; EXPECT_TRUE(db.GetIconMappingsForPageURL( - kPageUrl, favicon_base::FAVICON | favicon_base::TOUCH_ICON, + kPageUrl, + {favicon_base::IconType::kFavicon, favicon_base::IconType::kTouchIcon}, &icon_mappings)); SortMappingsByIconUrl(&icon_mappings); @@ -784,7 +786,7 @@ TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLWithIconTypes) { } TEST_F(ThumbnailDatabaseTest, HasMappingFor) { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); db.BeginTransaction(); @@ -794,22 +796,22 @@ TEST_F(ThumbnailDatabaseTest, HasMappingFor) { // Add a favicon which will have icon_mappings base::Time time = base::Time::Now(); favicon_base::FaviconID id1 = - db.AddFavicon(GURL("http://google.com"), favicon_base::FAVICON, favicon, - FaviconBitmapType::ON_VISIT, time, gfx::Size()); + db.AddFavicon(GURL("http://google.com"), favicon_base::IconType::kFavicon, + favicon, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(id1, 0); // Add another type of favicon time = base::Time::Now(); favicon_base::FaviconID id2 = db.AddFavicon( - GURL("http://www.google.com/icon"), favicon_base::TOUCH_ICON, favicon, - FaviconBitmapType::ON_VISIT, time, gfx::Size()); + GURL("http://www.google.com/icon"), favicon_base::IconType::kTouchIcon, + favicon, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(id2, 0); // Add 3rd favicon time = base::Time::Now(); favicon_base::FaviconID id3 = db.AddFavicon( - GURL("http://www.google.com/icon"), favicon_base::TOUCH_ICON, favicon, - FaviconBitmapType::ON_VISIT, time, gfx::Size()); + GURL("http://www.google.com/icon"), favicon_base::IconType::kTouchIcon, + favicon, FaviconBitmapType::ON_VISIT, time, gfx::Size()); EXPECT_NE(id3, 0); // Add 2 icon mapping @@ -831,7 +833,7 @@ TEST_F(ThumbnailDatabaseTest, HasMappingFor) { // Test loading version 3 database. TEST_F(ThumbnailDatabaseTest, Version3) { std::unique_ptr<ThumbnailDatabase> db = LoadFromGolden("Favicons.v3.sql"); - ASSERT_TRUE(db.get() != NULL); + ASSERT_TRUE(db.get() != nullptr); VerifyTablesAndColumns(&db->db_); // Version 3 is deprecated, the data should all be gone. @@ -841,7 +843,7 @@ TEST_F(ThumbnailDatabaseTest, Version3) { // Test loading version 4 database. TEST_F(ThumbnailDatabaseTest, Version4) { std::unique_ptr<ThumbnailDatabase> db = LoadFromGolden("Favicons.v4.sql"); - ASSERT_TRUE(db.get() != NULL); + ASSERT_TRUE(db.get() != nullptr); VerifyTablesAndColumns(&db->db_); // Version 4 is deprecated, the data should all be gone. @@ -851,7 +853,7 @@ TEST_F(ThumbnailDatabaseTest, Version4) { // Test loading version 5 database. TEST_F(ThumbnailDatabaseTest, Version5) { std::unique_ptr<ThumbnailDatabase> db = LoadFromGolden("Favicons.v5.sql"); - ASSERT_TRUE(db.get() != NULL); + ASSERT_TRUE(db.get() != nullptr); VerifyTablesAndColumns(&db->db_); // Version 5 is deprecated, the data should all be gone. @@ -861,7 +863,7 @@ TEST_F(ThumbnailDatabaseTest, Version5) { // Test loading version 6 database. TEST_F(ThumbnailDatabaseTest, Version6) { std::unique_ptr<ThumbnailDatabase> db = LoadFromGolden("Favicons.v6.sql"); - ASSERT_TRUE(db.get() != NULL); + ASSERT_TRUE(db.get() != nullptr); VerifyTablesAndColumns(&db->db_); // Version 6 is deprecated, the data should all be gone. @@ -871,73 +873,41 @@ TEST_F(ThumbnailDatabaseTest, Version6) { // Test loading version 7 database. TEST_F(ThumbnailDatabaseTest, Version7) { std::unique_ptr<ThumbnailDatabase> db = LoadFromGolden("Favicons.v7.sql"); - ASSERT_TRUE(db.get() != NULL); + ASSERT_TRUE(db.get() != nullptr); VerifyTablesAndColumns(&db->db_); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl2, - favicon_base::FAVICON, - kIconUrl2, - kLargeSize, - sizeof(kBlob2), - kBlob2)); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl3, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl3, - favicon_base::TOUCH_ICON, - kIconUrl3, - kLargeSize, - sizeof(kBlob2), - kBlob2)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl2, + favicon_base::IconType::kFavicon, kIconUrl2, + kLargeSize, sizeof(kBlob2), kBlob2)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, + favicon_base::IconType::kTouchIcon, kIconUrl3, + kLargeSize, sizeof(kBlob2), kBlob2)); } // Test loading version 8 database. TEST_F(ThumbnailDatabaseTest, Version8) { std::unique_ptr<ThumbnailDatabase> db = LoadFromGolden("Favicons.v8.sql"); - ASSERT_TRUE(db.get() != NULL); + ASSERT_TRUE(db.get() != nullptr); VerifyTablesAndColumns(&db->db_); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl2, - favicon_base::FAVICON, - kIconUrl2, - kLargeSize, - sizeof(kBlob2), - kBlob2)); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl3, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(db.get(), - kPageUrl3, - favicon_base::TOUCH_ICON, - kIconUrl3, - kLargeSize, - sizeof(kBlob2), - kBlob2)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl2, + favicon_base::IconType::kFavicon, kIconUrl2, + kLargeSize, sizeof(kBlob2), kBlob2)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, + favicon_base::IconType::kTouchIcon, kIconUrl3, + kLargeSize, sizeof(kBlob2), kBlob2)); } TEST_F(ThumbnailDatabaseTest, Recovery) { @@ -961,23 +931,15 @@ TEST_F(ThumbnailDatabaseTest, Recovery) { // Test that the contents make sense after clean open. { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl2, - favicon_base::FAVICON, - kIconUrl2, - kLargeSize, - sizeof(kBlob2), - kBlob2)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl2, + favicon_base::IconType::kFavicon, kIconUrl2, + kLargeSize, sizeof(kBlob2), kBlob2)); } // Corrupt the |icon_mapping.page_url| index by deleting an element @@ -1004,14 +966,14 @@ TEST_F(ThumbnailDatabaseTest, Recovery) { { sql::test::ScopedErrorExpecter expecter; expecter.ExpectError(SQLITE_CORRUPT); - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); // Data for kPageUrl2 was deleted, but the index entry remains, // this will throw SQLITE_CORRUPT. The corruption handler will // recover the database and poison the handle, so the outer call // fails. - EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); ASSERT_TRUE(expecter.SawExpectedErrors()); } @@ -1028,20 +990,16 @@ TEST_F(ThumbnailDatabaseTest, Recovery) { // Database should also be recovered at higher levels. { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); // Now this fails because there is no mapping. - EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); // Other data was retained by recovery. - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); } // Corrupt the database again by adjusting the header. @@ -1061,17 +1019,13 @@ TEST_F(ThumbnailDatabaseTest, Recovery) { { sql::test::ScopedErrorExpecter expecter; expecter.ExpectError(SQLITE_CORRUPT); - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); - EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL)); - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); ASSERT_TRUE(expecter.SawExpectedErrors()); } @@ -1116,14 +1070,14 @@ TEST_F(ThumbnailDatabaseTest, Recovery7) { { sql::test::ScopedErrorExpecter expecter; expecter.ExpectError(SQLITE_CORRUPT); - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); // Data for kPageUrl2 was deleted, but the index entry remains, // this will throw SQLITE_CORRUPT. The corruption handler will // recover the database and poison the handle, so the outer call // fails. - EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); ASSERT_TRUE(expecter.SawExpectedErrors()); } @@ -1140,20 +1094,16 @@ TEST_F(ThumbnailDatabaseTest, Recovery7) { // Database should also be recovered at higher levels. { - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); // Now this fails because there is no mapping. - EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); // Other data was retained by recovery. - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); } // Corrupt the database again by adjusting the header. @@ -1173,17 +1123,13 @@ TEST_F(ThumbnailDatabaseTest, Recovery7) { { sql::test::ScopedErrorExpecter expecter; expecter.ExpectError(SQLITE_CORRUPT); - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); - EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL)); - EXPECT_TRUE(CheckPageHasIcon(&db, - kPageUrl1, - favicon_base::FAVICON, - kIconUrl1, - kLargeSize, - sizeof(kBlob1), - kBlob1)); + EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr)); + EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, + favicon_base::IconType::kFavicon, kIconUrl1, + kLargeSize, sizeof(kBlob1), kBlob1)); ASSERT_TRUE(expecter.SawExpectedErrors()); } @@ -1217,7 +1163,7 @@ TEST_F(ThumbnailDatabaseTest, Recovery6) { { sql::test::ScopedErrorExpecter expecter; expecter.ExpectError(SQLITE_CORRUPT); - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); ASSERT_TRUE(expecter.SawExpectedErrors()); } @@ -1265,7 +1211,7 @@ TEST_F(ThumbnailDatabaseTest, Recovery5) { { sql::test::ScopedErrorExpecter expecter; expecter.ExpectError(SQLITE_CORRUPT); - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); ASSERT_TRUE(expecter.SawExpectedErrors()); } @@ -1305,7 +1251,7 @@ TEST_F(ThumbnailDatabaseTest, WildSchema) { // All schema flaws should be cleaned up by Init(). // TODO(shess): Differentiate between databases which need Raze() // and those which can be salvaged. - ThumbnailDatabase db(NULL); + ThumbnailDatabase db(nullptr); ASSERT_EQ(sql::INIT_OK, db.Init(db_path)); // Verify that the resulting schema is correct, whether it @@ -1314,4 +1260,30 @@ TEST_F(ThumbnailDatabaseTest, WildSchema) { } } +TEST(ThumbnailDatabaseIconTypeTest, ShouldBeBackwardCompatible) { + EXPECT_EQ(0, ThumbnailDatabase::ToPersistedIconType( + favicon_base::IconType::kInvalid)); + EXPECT_EQ(1, ThumbnailDatabase::ToPersistedIconType( + favicon_base::IconType::kFavicon)); + EXPECT_EQ(2, ThumbnailDatabase::ToPersistedIconType( + favicon_base::IconType::kTouchIcon)); + EXPECT_EQ(4, ThumbnailDatabase::ToPersistedIconType( + favicon_base::IconType::kTouchPrecomposedIcon)); + EXPECT_EQ(8, ThumbnailDatabase::ToPersistedIconType( + favicon_base::IconType::kWebManifestIcon)); + + EXPECT_EQ(favicon_base::IconType::kInvalid, + ThumbnailDatabase::FromPersistedIconType(0)); + EXPECT_EQ(favicon_base::IconType::kFavicon, + ThumbnailDatabase::FromPersistedIconType(1)); + EXPECT_EQ(favicon_base::IconType::kTouchIcon, + ThumbnailDatabase::FromPersistedIconType(2)); + EXPECT_EQ(favicon_base::IconType::kTouchPrecomposedIcon, + ThumbnailDatabase::FromPersistedIconType(4)); + EXPECT_EQ(favicon_base::IconType::kWebManifestIcon, + ThumbnailDatabase::FromPersistedIconType(8)); + EXPECT_EQ(favicon_base::IconType::kInvalid, + ThumbnailDatabase::FromPersistedIconType(16)); +} + } // namespace history diff --git a/chromium/components/history/core/browser/top_sites_backend.cc b/chromium/components/history/core/browser/top_sites_backend.cc index a35248e43ff..b71945d2457 100644 --- a/chromium/components/history/core/browser/top_sites_backend.cc +++ b/chromium/components/history/core/browser/top_sites_backend.cc @@ -137,7 +137,7 @@ void TopSitesBackend::SetPageThumbnailOnDBThread(const MostVisitedURL& url, void TopSitesBackend::ResetDatabaseOnDBThread(const base::FilePath& file_path) { DCHECK(db_task_runner_->RunsTasksInCurrentSequence()); - db_.reset(NULL); + db_.reset(nullptr); sql::Connection::Delete(db_path_); db_.reset(new TopSitesDatabase()); InitDBOnDBThread(db_path_); diff --git a/chromium/components/history/core/browser/top_sites_cache.cc b/chromium/components/history/core/browser/top_sites_cache.cc index c436e68cb0e..b4465d3320f 100644 --- a/chromium/components/history/core/browser/top_sites_cache.cc +++ b/chromium/components/history/core/browser/top_sites_cache.cc @@ -39,6 +39,16 @@ void TopSitesCache::SetThumbnails(const URLToImagesMap& images) { images_ = images; } +void TopSitesCache::ClearUnreferencedThumbnails() { + URLToImagesMap images_to_keep; + for (const std::pair<GURL, Images>& entry : images_) { + if (IsKnownURL(entry.first)) { + images_to_keep.insert(entry); + } + } + images_ = std::move(images_to_keep); +} + Images* TopSitesCache::GetImage(const GURL& url) { return &images_[GetCanonicalURL(url)]; } diff --git a/chromium/components/history/core/browser/top_sites_cache.h b/chromium/components/history/core/browser/top_sites_cache.h index 37c94826cbb..65f861e2124 100644 --- a/chromium/components/history/core/browser/top_sites_cache.h +++ b/chromium/components/history/core/browser/top_sites_cache.h @@ -51,6 +51,8 @@ class TopSitesCache { void SetThumbnails(const URLToImagesMap& images); const URLToImagesMap& images() const { return images_; } + void ClearUnreferencedThumbnails(); + // Returns the thumbnail as an Image for the specified url. This adds an entry // for |url| if one has not yet been added. Images* GetImage(const GURL& url); diff --git a/chromium/components/history/core/browser/top_sites_cache_unittest.cc b/chromium/components/history/core/browser/top_sites_cache_unittest.cc index 5ca85700192..c317f3cab7d 100644 --- a/chromium/components/history/core/browser/top_sites_cache_unittest.cc +++ b/chromium/components/history/core/browser/top_sites_cache_unittest.cc @@ -7,9 +7,12 @@ #include <stddef.h> #include <set> +#include <string> +#include <vector> #include "base/logging.h" #include "base/macros.h" +#include "base/memory/ref_counted_memory.h" #include "base/strings/string16.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" @@ -35,6 +38,11 @@ class TopSitesCacheTest : public testing::Test { // Initializes |top_sites_| and |cache_| based on |spec|. void InitTopSiteCache(const char** spec, size_t size); + bool HasPageThumbnail(const GURL& url) { + scoped_refptr<base::RefCountedMemory> memory; + return cache_.GetPageThumbnail(url, &memory); + } + MostVisitedURLList top_sites_; TopSitesCache cache_; @@ -255,6 +263,44 @@ TEST_F(TopSitesCacheTest, CacheForcedURLs) { EXPECT_EQ(2u, cache_.GetNumNonForcedURLs()); } +TEST_F(TopSitesCacheTest, ClearUnreferencedThumbnails) { + InitTopSiteCache(kTopSitesSpecBasic, arraysize(kTopSitesSpecBasic)); + + // A "primary" URL. + const GURL url1("http://www.google.com"); + ASSERT_TRUE(cache_.IsKnownURL(url1)); + // A URL that's part of a redirect chain. + const GURL url2("https://www.gogle.com"); + ASSERT_TRUE(cache_.IsKnownURL(url2)); + + // Add thumbnails for these two URLs. + Images thumbnail1; + thumbnail1.thumbnail = + new base::RefCountedBytes(std::vector<unsigned char>()); + Images thumbnail2; + thumbnail2.thumbnail = + new base::RefCountedBytes(std::vector<unsigned char>()); + URLToImagesMap images; + images[cache_.GetCanonicalURL(url1)] = thumbnail1; + images[cache_.GetCanonicalURL(url2)] = thumbnail2; + cache_.SetThumbnails(images); + + ASSERT_TRUE(HasPageThumbnail(url1)); + ASSERT_TRUE(HasPageThumbnail(url2)); + + // Since both URLs are known, ClearUnreferencedThumbnails should do nothing. + cache_.ClearUnreferencedThumbnails(); + EXPECT_TRUE(HasPageThumbnail(url1)); + EXPECT_TRUE(HasPageThumbnail(url2)); + + // After the top sites themselves are cleared, ClearUnreferencedThumbnails + // should clear the corresponding thumbnails. + cache_.SetTopSites(MostVisitedURLList()); + cache_.ClearUnreferencedThumbnails(); + EXPECT_FALSE(HasPageThumbnail(url1)); + EXPECT_FALSE(HasPageThumbnail(url2)); +} + } // namespace } // namespace history diff --git a/chromium/components/history/core/browser/top_sites_database.cc b/chromium/components/history/core/browser/top_sites_database.cc index 520a0d32ae5..46acde473a9 100644 --- a/chromium/components/history/core/browser/top_sites_database.cc +++ b/chromium/components/history/core/browser/top_sites_database.cc @@ -690,7 +690,7 @@ sql::Connection* TopSitesDatabase::CreateDB(const base::FilePath& db_name) { db->set_cache_size(32); if (!db->Open(db_name)) - return NULL; + return nullptr; return db.release(); } diff --git a/chromium/components/history/core/browser/top_sites_impl.cc b/chromium/components/history/core/browser/top_sites_impl.cc index daf38913134..908fc22844a 100644 --- a/chromium/components/history/core/browser/top_sites_impl.cc +++ b/chromium/components/history/core/browser/top_sites_impl.cc @@ -31,6 +31,7 @@ #include "components/history/core/browser/page_usage_data.h" #include "components/history/core/browser/top_sites_cache.h" #include "components/history/core/browser/top_sites_observer.h" +#include "components/history/core/browser/top_sites_provider.h" #include "components/history/core/browser/url_utils.h" #include "components/history/core/common/thumbnail_score.h" #include "components/prefs/pref_registry_simple.h" @@ -84,8 +85,6 @@ bool DoTitlesDiffer(const MostVisitedURLList& old_list, // temp_images_ for details. const size_t kMaxTempTopImages = 8; -const int kDaysOfHistory = 90; - // The delay for the first HistoryService query at startup. constexpr base::TimeDelta kFirstDelayAtStartup = base::TimeDelta::FromSeconds(15); @@ -115,6 +114,7 @@ bool TopSitesImpl::histogram_recorded_ = false; TopSitesImpl::TopSitesImpl(PrefService* pref_service, HistoryService* history_service, + std::unique_ptr<TopSitesProvider> provider, const PrepopulatedPageList& prepopulated_pages, const CanAddURLToHistoryFn& can_add_url_to_history) : backend_(nullptr), @@ -123,11 +123,13 @@ TopSitesImpl::TopSitesImpl(PrefService* pref_service, prepopulated_pages_(prepopulated_pages), pref_service_(pref_service), history_service_(history_service), + provider_(std::move(provider)), can_add_url_to_history_(can_add_url_to_history), loaded_(false), history_service_observer_(this) { DCHECK(pref_service_); DCHECK(!can_add_url_to_history_.is_null()); + DCHECK(provider_); } void TopSitesImpl::Init(const base::FilePath& db_name) { @@ -413,8 +415,8 @@ void TopSitesImpl::StartQueryForMostVisited() { if (!history_service_) return; - history_service_->QueryMostVisitedURLs( - num_results_to_request_from_history(), kDaysOfHistory, + provider_->ProvideTopSites( + num_results_to_request_from_history(), base::Bind(&TopSitesImpl::OnTopSitesAvailableFromHistory, base::Unretained(this)), &cancelable_task_tracker_); @@ -754,6 +756,7 @@ void TopSitesImpl::SetTopSites(const MostVisitedURLList& new_top_sites, // thread safe cache ...) as this method is invoked during startup at which // point the caches haven't been updated yet. cache_->SetTopSites(top_sites); + cache_->ClearUnreferencedThumbnails(); // See if we have any temp thumbnails for the new sites, and promote them to // proper thumbnails. diff --git a/chromium/components/history/core/browser/top_sites_impl.h b/chromium/components/history/core/browser/top_sites_impl.h index 7e6cb3581c9..dc40668f6d1 100644 --- a/chromium/components/history/core/browser/top_sites_impl.h +++ b/chromium/components/history/core/browser/top_sites_impl.h @@ -8,6 +8,7 @@ #include <stddef.h> #include <list> +#include <memory> #include <set> #include <string> #include <utility> @@ -27,6 +28,7 @@ #include "components/history/core/browser/history_types.h" #include "components/history/core/browser/top_sites.h" #include "components/history/core/browser/top_sites_backend.h" +#include "components/history/core/browser/top_sites_provider.h" #include "components/history/core/common/thumbnail_score.h" #include "third_party/skia/include/core/SkColor.h" #include "ui/gfx/image/image.h" @@ -65,6 +67,7 @@ class TopSitesImpl : public TopSites, public HistoryServiceObserver { TopSitesImpl(PrefService* pref_service, HistoryService* history_service, + std::unique_ptr<TopSitesProvider> provider, const PrepopulatedPageList& prepopulated_pages, const CanAddURLToHistoryFn& can_add_url_to_history); @@ -315,6 +318,9 @@ class TopSitesImpl : public TopSites, public HistoryServiceObserver { // must outlive TopSitesImpl. HistoryService* history_service_; + // The provider to query for most visited URLs. + std::unique_ptr<TopSitesProvider> provider_; + // Can URL be added to the history? CanAddURLToHistoryFn can_add_url_to_history_; diff --git a/chromium/components/history/core/browser/top_sites_impl_unittest.cc b/chromium/components/history/core/browser/top_sites_impl_unittest.cc index 682116d8c2c..a47746b5cbb 100644 --- a/chromium/components/history/core/browser/top_sites_impl_unittest.cc +++ b/chromium/components/history/core/browser/top_sites_impl_unittest.cc @@ -16,6 +16,7 @@ #include "base/task/cancelable_task_tracker.h" #include "base/test/scoped_task_environment.h" #include "build/build_config.h" +#include "components/history/core/browser/default_top_sites_provider.h" #include "components/history/core/browser/history_client.h" #include "components/history/core/browser/history_constants.h" #include "components/history/core/browser/history_database_params.h" @@ -279,6 +280,7 @@ class TopSitesImplTest : public HistoryUnitTestBase { base::string16(), -1, -1, 0)); top_sites_impl_ = new TopSitesImpl( pref_service_.get(), history_service_.get(), + std::make_unique<DefaultTopSitesProvider>(history_service_.get()), prepopulated_pages, base::Bind(MockCanAddURLToHistory)); top_sites_impl_->Init(scoped_temp_dir_.GetPath().Append(kTopSitesFilename)); } @@ -627,6 +629,8 @@ TEST_F(TopSitesImplTest, ThumbnailRemoved) { // Reset the thumbnails and make sure we don't get it back. SetTopSites(MostVisitedURLList()); + EXPECT_FALSE(top_sites()->GetPageThumbnail(url, false, &result)); + // Recreating the TopSites object should also not bring it back. RefreshTopSitesAndRecreate(); EXPECT_FALSE(top_sites()->GetPageThumbnail(url, false, &result)); } diff --git a/chromium/components/history/core/browser/top_sites_provider.h b/chromium/components/history/core/browser/top_sites_provider.h new file mode 100644 index 00000000000..156e8665c2b --- /dev/null +++ b/chromium/components/history/core/browser/top_sites_provider.h @@ -0,0 +1,28 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_HISTORY_CORE_BROWSER_TOP_SITES_PROVIDER_H_ +#define COMPONENTS_HISTORY_CORE_BROWSER_TOP_SITES_PROVIDER_H_ + +#include "components/history/core/browser/history_service.h" + +namespace history { + +// Interface for querying the URLs that should be included in TopSites. +class TopSitesProvider { + public: + virtual ~TopSitesProvider() {} + + // Asynchronously queries for the list of URLs to include in TopSites, running + // |callback| when complete. |tracker| can be used to interrupt an + // in-progress call. + virtual void ProvideTopSites( + int result_count, + const HistoryService::QueryMostVisitedURLsCallback& callback, + base::CancelableTaskTracker* tracker) = 0; +}; + +} // namespace history + +#endif // COMPONENTS_HISTORY_CORE_BROWSER_TOP_SITES_PROVIDER_H_ diff --git a/chromium/components/history/core/browser/typed_url_model_type_controller.cc b/chromium/components/history/core/browser/typed_url_model_type_controller.cc index 38d45165f95..b1587826449 100644 --- a/chromium/components/history/core/browser/typed_url_model_type_controller.cc +++ b/chromium/components/history/core/browser/typed_url_model_type_controller.cc @@ -5,6 +5,7 @@ #include "components/history/core/browser/typed_url_model_type_controller.h" #include "base/bind.h" +#include "components/history/core/browser/history_backend.h" #include "components/history/core/browser/history_db_task.h" #include "components/history/core/browser/history_service.h" #include "components/prefs/pref_service.h" @@ -24,22 +25,25 @@ namespace { // the tasks we want to run. class RunTaskOnHistoryThread : public HistoryDBTask { public: - explicit RunTaskOnHistoryThread(const base::Closure& task) : task_(task) {} + explicit RunTaskOnHistoryThread(ModelTypeController::BridgeTask task) + : task_(std::move(task)) {} bool RunOnDBThread(HistoryBackend* backend, HistoryDatabase* db) override { // Invoke the task, then free it immediately so we don't keep a reference // around all the way until DoneRunOnMainThread() is invoked back on the // main thread - we want to release references as soon as possible to avoid // keeping them around too long during shutdown. - task_.Run(); - task_.Reset(); + TypedURLSyncBridge* bridge = backend->GetTypedURLSyncBridge(); + DCHECK(bridge); + + std::move(task_).Run(bridge); return true; } void DoneRunOnMainThread() override {} protected: - base::Closure task_; + ModelTypeController::BridgeTask task_; }; } // namespace @@ -65,7 +69,7 @@ bool TypedURLModelTypeController::ReadyForStart() const { } void TypedURLModelTypeController::PostBridgeTask(const base::Location& location, - const BridgeTask& task) { + BridgeTask task) { history::HistoryService* history = sync_client()->GetHistoryService(); if (!history) { // History must be disabled - don't start. @@ -73,8 +77,9 @@ void TypedURLModelTypeController::PostBridgeTask(const base::Location& location, return; } - TypedURLSyncBridge* bridge = history->GetTypedURLSyncBridge(); - PostTaskOnHistoryThread(base::Bind(task, bridge)); + history->ScheduleDBTask( + std::make_unique<RunTaskOnHistoryThread>(std::move(task)), + &task_tracker_); } void TypedURLModelTypeController::OnSavingBrowserHistoryDisabledChanged() { @@ -84,25 +89,10 @@ void TypedURLModelTypeController::OnSavingBrowserHistoryDisabledChanged() { // generate an unrecoverable error. This can be fixed by restarting // Chrome (on restart, typed urls will not be a registered type). if (state() != NOT_RUNNING && state() != STOPPING) { - PostTaskOnHistoryThread(base::Bind( - &TypedURLModelTypeController::ReportModelError, base::AsWeakPtr(this), - syncer::ModelError(FROM_HERE, - "History saving is now disabled by policy."))); + ReportModelError(syncer::ModelError( + FROM_HERE, "History saving is now disabled by policy.")); } } } -void TypedURLModelTypeController::PostTaskOnHistoryThread( - const base::Closure& task) { - history::HistoryService* history = sync_client()->GetHistoryService(); - if (!history) { - // History must be disabled - don't start. - LOG(WARNING) << "Cannot access history service."; - return; - } - - history->ScheduleDBTask(std::make_unique<RunTaskOnHistoryThread>(task), - &task_tracker_); -} - } // namespace history diff --git a/chromium/components/history/core/browser/typed_url_model_type_controller.h b/chromium/components/history/core/browser/typed_url_model_type_controller.h index 77e4f460c6e..d528d409fbd 100644 --- a/chromium/components/history/core/browser/typed_url_model_type_controller.h +++ b/chromium/components/history/core/browser/typed_url_model_type_controller.h @@ -23,13 +23,10 @@ class TypedURLModelTypeController : public syncer::ModelTypeController { private: // syncer::ModelTypeController implementation. - void PostBridgeTask(const base::Location& location, - const BridgeTask& task) override; + void PostBridgeTask(const base::Location& location, BridgeTask task) override; void OnSavingBrowserHistoryDisabledChanged(); - void PostTaskOnHistoryThread(const base::Closure& task); - // Name of the pref that indicates whether saving history is disabled. const char* history_disabled_pref_name_; diff --git a/chromium/components/history/core/browser/typed_url_sync_bridge.cc b/chromium/components/history/core/browser/typed_url_sync_bridge.cc index c81a656f92d..c9bef84f2e2 100644 --- a/chromium/components/history/core/browser/typed_url_sync_bridge.cc +++ b/chromium/components/history/core/browser/typed_url_sync_bridge.cc @@ -90,7 +90,6 @@ TypedURLSyncBridge::TypedURLSyncBridge( history_backend_observer_(this) { DCHECK(history_backend_); DCHECK(sequence_checker_.CalledOnValidSequence()); - DCHECK(sync_metadata_database_); } TypedURLSyncBridge::~TypedURLSyncBridge() { @@ -152,8 +151,9 @@ base::Optional<ModelError> TypedURLSyncBridge::MergeSyncData( &updated_synced_urls); } - base::Optional<ModelError> error = WriteToHistoryBackend( - &new_synced_urls, &updated_synced_urls, NULL, &new_synced_visits, NULL); + base::Optional<ModelError> error = + WriteToHistoryBackend(&new_synced_urls, &updated_synced_urls, nullptr, + &new_synced_visits, nullptr); if (error) return error; @@ -191,7 +191,6 @@ base::Optional<ModelError> TypedURLSyncBridge::ApplySyncChanges( std::unique_ptr<MetadataChangeList> metadata_change_list, EntityChangeList entity_changes) { DCHECK(sequence_checker_.CalledOnValidSequence()); - DCHECK(sync_metadata_database_); std::vector<GURL> pending_deleted_urls; TypedURLVisitVector new_synced_visits; @@ -202,7 +201,7 @@ base::Optional<ModelError> TypedURLSyncBridge::ApplySyncChanges( for (const EntityChange& entity_change : entity_changes) { if (entity_change.type() == EntityChange::ACTION_DELETE) { URLRow url_row; - int64_t url_id = sync_metadata_database_->StorageKeyToURLID( + int64_t url_id = TypedURLSyncMetadataDatabase::StorageKeyToURLID( entity_change.storage_key()); if (!history_backend_->GetURLByID(url_id, &url_row)) { // Ignoring the case that there is no matching URLRow with URLID @@ -262,12 +261,11 @@ base::Optional<ModelError> TypedURLSyncBridge::ApplySyncChanges( void TypedURLSyncBridge::GetData(StorageKeyList storage_keys, DataCallback callback) { DCHECK(sequence_checker_.CalledOnValidSequence()); - DCHECK(sync_metadata_database_); auto batch = base::MakeUnique<MutableDataBatch>(); for (const std::string& key : storage_keys) { URLRow url_row; - URLID url_id = sync_metadata_database_->StorageKeyToURLID(key); + URLID url_id = TypedURLSyncMetadataDatabase::StorageKeyToURLID(key); ++num_db_accesses_; if (!history_backend_->GetURLByID(url_id, &url_row)) { @@ -277,7 +275,8 @@ void TypedURLSyncBridge::GetData(StorageKeyList storage_keys, } VisitVector visits_vector; - FixupURLAndGetVisits(&url_row, &visits_vector); + if (!FixupURLAndGetVisits(&url_row, &visits_vector)) + continue; std::unique_ptr<syncer::EntityData> entity_data = CreateEntityData(url_row, visits_vector); if (!entity_data.get()) { @@ -306,7 +305,8 @@ void TypedURLSyncBridge::GetAllData(DataCallback callback) { auto batch = base::MakeUnique<MutableDataBatch>(); for (URLRow& url : typed_urls) { VisitVector visits_vector; - FixupURLAndGetVisits(&url, &visits_vector); + if (!FixupURLAndGetVisits(&url, &visits_vector)) + continue; std::unique_ptr<syncer::EntityData> entity_data = CreateEntityData(url, visits_vector); if (!entity_data.get()) { @@ -349,6 +349,7 @@ void TypedURLSyncBridge::OnURLVisited(HistoryBackend* history_backend, const RedirectList& redirects, base::Time visit_time) { DCHECK(sequence_checker_.CalledOnValidSequence()); + DCHECK(sync_metadata_database_); if (processing_syncer_changes_) return; // These are changes originating from us, ignore. @@ -367,6 +368,7 @@ void TypedURLSyncBridge::OnURLVisited(HistoryBackend* history_backend, void TypedURLSyncBridge::OnURLsModified(HistoryBackend* history_backend, const URLRows& changed_urls) { DCHECK(sequence_checker_.CalledOnValidSequence()); + DCHECK(sync_metadata_database_); if (processing_syncer_changes_) return; // These are changes originating from us, ignore. @@ -393,6 +395,7 @@ void TypedURLSyncBridge::OnURLsDeleted(HistoryBackend* history_backend, const URLRows& deleted_rows, const std::set<GURL>& favicon_urls) { DCHECK(sequence_checker_.CalledOnValidSequence()); + DCHECK(sync_metadata_database_); if (processing_syncer_changes_) return; // These are changes originating from us, ignore. @@ -440,6 +443,12 @@ void TypedURLSyncBridge::Init() { LoadMetadata(); } +void TypedURLSyncBridge::OnDatabaseError() { + sync_metadata_database_ = nullptr; + change_processor()->ReportError(FROM_HERE, + "HistoryDatabase encountered error"); +} + int TypedURLSyncBridge::GetErrorPercentage() const { return num_db_accesses_ ? (100 * num_db_errors_ / num_db_accesses_) : 0; } diff --git a/chromium/components/history/core/browser/typed_url_sync_bridge.h b/chromium/components/history/core/browser/typed_url_sync_bridge.h index cb92a96ff0f..3596e394706 100644 --- a/chromium/components/history/core/browser/typed_url_sync_bridge.h +++ b/chromium/components/history/core/browser/typed_url_sync_bridge.h @@ -56,6 +56,10 @@ class TypedURLSyncBridge : public syncer::ModelTypeSyncBridge, // Must be called after creation and before any operations. void Init(); + // Called by HistoryBackend when database error is reported through + // DatabaseErrorCallback. + void OnDatabaseError(); + // Returns the percentage of DB accesses that have resulted in an error. int GetErrorPercentage() const; @@ -223,7 +227,7 @@ class TypedURLSyncBridge : public syncer::ModelTypeSyncBridge, // A non-owning pointer to the database, which is for storing typed urls sync // metadata and state. - TypedURLSyncMetadataDatabase* const sync_metadata_database_; + TypedURLSyncMetadataDatabase* sync_metadata_database_; // Statistics for the purposes of tracking the percentage of DB accesses that // fail for each client via UMA. diff --git a/chromium/components/history/core/browser/typed_url_sync_bridge_unittest.cc b/chromium/components/history/core/browser/typed_url_sync_bridge_unittest.cc index e598864298f..2b61ceba28c 100644 --- a/chromium/components/history/core/browser/typed_url_sync_bridge_unittest.cc +++ b/chromium/components/history/core/browser/typed_url_sync_bridge_unittest.cc @@ -228,7 +228,7 @@ class TestHistoryBackend : public HistoryBackend { class TypedURLSyncBridgeTest : public testing::Test { public: - TypedURLSyncBridgeTest() : typed_url_sync_bridge_(NULL) {} + TypedURLSyncBridgeTest() : typed_url_sync_bridge_(nullptr) {} ~TypedURLSyncBridgeTest() override {} void SetUp() override { @@ -455,7 +455,7 @@ class TypedURLSyncBridgeTest : public testing::Test { return bridge()->sync_metadata_database_; } - const RecordingModelTypeChangeProcessor& processor() { return *processor_; } + RecordingModelTypeChangeProcessor& processor() { return *processor_; } protected: base::MessageLoop message_loop_; @@ -470,18 +470,21 @@ class TypedURLSyncBridgeTest : public testing::Test { // Add two typed urls locally and verify bridge can get them from GetAllData. TEST_F(TypedURLSyncBridgeTest, GetAllData) { // Add two urls to backend. - VisitVector visits1, visits2; + VisitVector visits1, visits2, visits3; URLRow row1 = MakeTypedUrlRow(kURL, kTitle, 1, 3, false, &visits1); URLRow row2 = MakeTypedUrlRow(kURL2, kTitle2, 2, 4, false, &visits2); + URLRow expired_row = MakeTypedUrlRow("http://expired.com/", kTitle, 1, + kExpiredVisit, false, &visits3); fake_history_backend_->SetVisitsForUrl(row1, visits1); fake_history_backend_->SetVisitsForUrl(row2, visits2); + fake_history_backend_->SetVisitsForUrl(expired_row, visits3); // Create the same data in sync. TypedUrlSpecifics typed_url1, typed_url2; WriteToTypedUrlSpecifics(row1, visits1, &typed_url1); WriteToTypedUrlSpecifics(row2, visits2, &typed_url2); - // Check that the local cache is still correct. + // Check that the local cache is still correct, expired row is filtered out. VerifyAllLocalHistoryData({typed_url1, typed_url2}); } @@ -1563,4 +1566,10 @@ TEST_F(TypedURLSyncBridgeTest, LocalExpiredTypedUrlDoNotSync) { url_specifics.visit_transitions(0)); } +// Tests that database error gets reported to processor as model type error. +TEST_F(TypedURLSyncBridgeTest, DatabaseError) { + processor().ExpectError(); + bridge()->OnDatabaseError(); +} + } // namespace history diff --git a/chromium/components/history/core/browser/typed_url_sync_metadata_database.cc b/chromium/components/history/core/browser/typed_url_sync_metadata_database.cc index f216e89f1e5..8f6c1f1c04a 100644 --- a/chromium/components/history/core/browser/typed_url_sync_metadata_database.cc +++ b/chromium/components/history/core/browser/typed_url_sync_metadata_database.cc @@ -38,12 +38,10 @@ bool TypedURLSyncMetadataDatabase::GetAllSyncMetadata( } sync_pb::ModelTypeState model_type_state; - if (GetModelTypeState(&model_type_state)) { - metadata_batch->SetModelTypeState(model_type_state); - } else { + if (!GetModelTypeState(&model_type_state)) return false; - } + metadata_batch->SetModelTypeState(model_type_state); return true; } diff --git a/chromium/components/history/core/browser/typed_url_syncable_service.cc b/chromium/components/history/core/browser/typed_url_syncable_service.cc index 98bf4b9ac3a..630be9ad98e 100644 --- a/chromium/components/history/core/browser/typed_url_syncable_service.cc +++ b/chromium/components/history/core/browser/typed_url_syncable_service.cc @@ -194,8 +194,8 @@ syncer::SyncMergeResult TypedUrlSyncableService::MergeDataAndStartSyncing( sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes)); if (!merge_result.error().IsSet()) { - WriteToHistoryBackend(&new_synced_urls, &updated_synced_urls, NULL, - &new_synced_visits, NULL); + WriteToHistoryBackend(&new_synced_urls, &updated_synced_urls, nullptr, + &new_synced_visits, nullptr); } history_backend_observer_.Add(history_backend_); @@ -1004,11 +1004,9 @@ bool TypedUrlSyncableService::FixupURLAndGetVisits(URLRow* url, size_t num_expired_visits = 0; for (auto& visit : *visits) { base::Time time = visit.visit_time; - if (history_backend_->IsExpiredVisitTime(time)) { - ++num_expired_visits; - } else { + if (!history_backend_->IsExpiredVisitTime(time)) break; - } + ++num_expired_visits; } if (num_expired_visits != 0) { if (num_expired_visits == visits->size()) { diff --git a/chromium/components/history/core/browser/typed_url_syncable_service_unittest.cc b/chromium/components/history/core/browser/typed_url_syncable_service_unittest.cc index 844b182be35..64641c22eea 100644 --- a/chromium/components/history/core/browser/typed_url_syncable_service_unittest.cc +++ b/chromium/components/history/core/browser/typed_url_syncable_service_unittest.cc @@ -213,7 +213,7 @@ class TestHistoryBackend : public HistoryBackend { class TypedUrlSyncableServiceTest : public testing::Test { public: - TypedUrlSyncableServiceTest() : typed_url_sync_service_(NULL) {} + TypedUrlSyncableServiceTest() : typed_url_sync_service_(nullptr) {} ~TypedUrlSyncableServiceTest() override {} void SetUp() override { diff --git a/chromium/components/history/core/browser/url_database.cc b/chromium/components/history/core/browser/url_database.cc index 5fb9762943c..1ef2f03f61c 100644 --- a/chromium/components/history/core/browser/url_database.cc +++ b/chromium/components/history/core/browser/url_database.cc @@ -313,7 +313,7 @@ bool URLDatabase::AutocompleteForPrefix(const std::string& prefix, return !results->empty(); } -bool URLDatabase::IsTypedHost(const std::string& host) { +bool URLDatabase::IsTypedHost(const std::string& host, std::string* scheme) { const char* schemes[] = { url::kHttpScheme, url::kHttpsScheme, @@ -324,8 +324,11 @@ bool URLDatabase::IsTypedHost(const std::string& host) { std::string scheme_and_host(schemes[i]); scheme_and_host += url::kStandardSchemeSeparator + host; if (AutocompleteForPrefix(scheme_and_host + '/', 1, true, &dummy) || - AutocompleteForPrefix(scheme_and_host + ':', 1, true, &dummy)) + AutocompleteForPrefix(scheme_and_host + ':', 1, true, &dummy)) { + if (scheme != nullptr) + *scheme = schemes[i]; return true; + } } return false; } diff --git a/chromium/components/history/core/browser/url_database.h b/chromium/components/history/core/browser/url_database.h index dd8f2feff7a..d3a96427621 100644 --- a/chromium/components/history/core/browser/url_database.h +++ b/chromium/components/history/core/browser/url_database.h @@ -167,8 +167,10 @@ class URLDatabase { URLRows* results); // Returns true if the database holds some past typed navigation to a URL on - // the provided hostname. - bool IsTypedHost(const std::string& host); + // the provided hostname. If the return value is true and |scheme| is not + // nullptr, |scheme| holds the scheme of one of the corresponding entries in + // the database. + bool IsTypedHost(const std::string& host, std::string* scheme); // Tries to find the shortest URL beginning with |base| that strictly // prefixes |url|, and has minimum visit_ and typed_counts as specified. diff --git a/chromium/components/history/core/browser/visitsegment_database.cc b/chromium/components/history/core/browser/visitsegment_database.cc index afe0712caad..b088f292abc 100644 --- a/chromium/components/history/core/browser/visitsegment_database.cc +++ b/chromium/components/history/core/browser/visitsegment_database.cc @@ -16,6 +16,7 @@ #include "base/logging.h" #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/strings/string_piece.h" #include "base/strings/string_util.h" #include "components/history/core/browser/page_usage_data.h" #include "sql/statement.h" @@ -104,17 +105,19 @@ std::string VisitSegmentDatabase::ComputeSegmentName(const GURL& url) { // TODO(brettw) this should probably use the registry controlled // domains service. GURL::Replacements r; - const char kWWWDot[] = "www."; - const int kWWWDotLen = arraysize(kWWWDot) - 1; - std::string host = url.host(); - // Remove www. to avoid some dups. - if (static_cast<int>(host.size()) > kWWWDotLen && - base::StartsWith(host, kWWWDot, base::CompareCase::INSENSITIVE_ASCII)) { - r.SetHost(host.c_str(), - url::Component(kWWWDotLen, - static_cast<int>(host.size()) - kWWWDotLen)); + + // Strip various common prefixes in order to group the resulting hostnames + // together and avoid duplicates. + for (base::StringPiece prefix : {"www.", "m.", "mobile.", "touch."}) { + if (host.size() > prefix.size() && + base::StartsWith(host, prefix, base::CompareCase::INSENSITIVE_ASCII)) { + r.SetHost(host.c_str(), + url::Component(prefix.size(), host.size() - prefix.size())); + break; + } } + // Remove other stuff we don't want. r.ClearUsername(); r.ClearPassword(); @@ -122,6 +125,10 @@ std::string VisitSegmentDatabase::ComputeSegmentName(const GURL& url) { r.ClearRef(); r.ClearPort(); + // Canonicalize https to http in order to avoid duplicates. + if (url.SchemeIs(url::kHttpsScheme)) + r.SetSchemeStr(url::kHttpScheme); + return url.ReplaceComponents(r).spec(); } @@ -324,4 +331,84 @@ bool VisitSegmentDatabase::MigratePresentationIndex() { transaction.Commit(); } +bool VisitSegmentDatabase::MigrateVisitSegmentNames() { + sql::Statement select( + GetDB().GetUniqueStatement("SELECT id, name FROM segments")); + if (!select.is_valid()) + return false; + + bool success = true; + while (select.Step()) { + SegmentID id = select.ColumnInt64(0); + std::string old_name = select.ColumnString(1); + std::string new_name = ComputeSegmentName(GURL(old_name)); + if (new_name.empty() || old_name == new_name) + continue; + + SegmentID to_segment_id = GetSegmentNamed(new_name); + if (to_segment_id) { + // |new_name| is already in use, so merge. + success = success && MergeSegments(/*from_segment_id=*/id, to_segment_id); + } else { + // Trivial rename of the segment. + success = success && RenameSegment(id, new_name); + } + } + return success; +} + +bool VisitSegmentDatabase::RenameSegment(SegmentID segment_id, + const std::string& new_name) { + sql::Statement statement(GetDB().GetCachedStatement( + SQL_FROM_HERE, "UPDATE segments SET name = ? WHERE id = ?")); + statement.BindString(0, new_name); + statement.BindInt64(1, segment_id); + return statement.Run(); +} + +bool VisitSegmentDatabase::MergeSegments(SegmentID from_segment_id, + SegmentID to_segment_id) { + sql::Transaction transaction(&GetDB()); + if (!transaction.Begin()) + return false; + + // For each time slot where there are visits for the absorbed segment + // (|from_segment_id|), add them to the absorbing/staying segment + // (|to_segment_id|). + sql::Statement select( + GetDB().GetCachedStatement(SQL_FROM_HERE, + "SELECT time_slot, visit_count FROM " + "segment_usage WHERE segment_id = ?")); + select.BindInt64(0, from_segment_id); + while (select.Step()) { + base::Time ts = base::Time::FromInternalValue(select.ColumnInt64(0)); + int64_t visit_count = select.ColumnInt64(1); + IncreaseSegmentVisitCount(to_segment_id, ts, visit_count); + } + + // Update all references in the visits database. + sql::Statement update(GetDB().GetCachedStatement( + SQL_FROM_HERE, "UPDATE visits SET segment_id = ? WHERE segment_id = ?")); + update.BindInt64(0, to_segment_id); + update.BindInt64(1, from_segment_id); + if (!update.Run()) + return false; + + // Delete old segment usage data. + sql::Statement deletion1(GetDB().GetCachedStatement( + SQL_FROM_HERE, "DELETE FROM segment_usage WHERE segment_id = ?")); + deletion1.BindInt64(0, from_segment_id); + if (!deletion1.Run()) + return false; + + // Delete old segment data. + sql::Statement deletion2(GetDB().GetCachedStatement( + SQL_FROM_HERE, "DELETE FROM segments WHERE id = ?")); + deletion2.BindInt64(0, from_segment_id); + if (!deletion2.Run()) + return false; + + return transaction.Commit(); +} + } // namespace history diff --git a/chromium/components/history/core/browser/visitsegment_database.h b/chromium/components/history/core/browser/visitsegment_database.h index 6f078678b11..e50685affa3 100644 --- a/chromium/components/history/core/browser/visitsegment_database.h +++ b/chromium/components/history/core/browser/visitsegment_database.h @@ -6,6 +6,7 @@ #define COMPONENTS_HISTORY_CORE_BROWSER_VISITSEGMENT_DATABASE_H_ #include <memory> +#include <string> #include "base/callback_forward.h" #include "base/macros.h" @@ -87,7 +88,21 @@ class VisitSegmentDatabase { // presentation table is removed entirely. bool MigratePresentationIndex(); + // Runs ComputeSegmentName() to recompute 'name'. If multiple segments have + // the same name, they are merged by: + // 1. Choosing one arbitrary |segment_id| and updating all references. + // 2. Merging duplicate |segment_usage| entries (add up visit counts). + // 3. Deleting old data for the absorbed segment. + bool MigrateVisitSegmentNames(); + private: + // Updates the |name| column for a single segment. Returns true on success. + bool RenameSegment(SegmentID segment_id, const std::string& new_name); + // Merges two segments such that data is aggregated, all former references to + // |from_segment_id| are updated to |to_segment_id| and |from_segment_id| is + // deleted. Returns true on success. + bool MergeSegments(SegmentID from_segment_id, SegmentID to_segment_id); + DISALLOW_COPY_AND_ASSIGN(VisitSegmentDatabase); }; diff --git a/chromium/components/history/core/browser/web_history_service.cc b/chromium/components/history/core/browser/web_history_service.cc index 199ed0e1621..b54c4f115b7 100644 --- a/chromium/components/history/core/browser/web_history_service.cc +++ b/chromium/components/history/core/browser/web_history_service.cc @@ -282,12 +282,9 @@ class RequestImpl : public WebHistoryService::Request, // Converts a time into a string for use as a parameter in a request to the // history server. std::string ServerTimeString(base::Time time) { - if (time < base::Time::UnixEpoch()) { + if (time < base::Time::UnixEpoch()) return base::Int64ToString(0); - } else { - return base::Int64ToString( - (time - base::Time::UnixEpoch()).InMicroseconds()); - } + return base::Int64ToString((time - base::Time::UnixEpoch()).InMicroseconds()); } // Returns a URL for querying the history server for a query specified by @@ -391,7 +388,7 @@ std::unique_ptr<base::DictionaryValue> WebHistoryService::ReadResponse( if (request->GetResponseCode() == net::HTTP_OK) { std::unique_ptr<base::Value> value = base::JSONReader::Read(request->GetResponseBody()); - if (value.get() && value.get()->IsType(base::Value::Type::DICTIONARY)) + if (value.get() && value.get()->is_dict()) result.reset(static_cast<base::DictionaryValue*>(value.release())); else DLOG(WARNING) << "Non-JSON response received from history server."; diff --git a/chromium/components/history/ios/browser/web_state_top_sites_observer.h b/chromium/components/history/ios/browser/web_state_top_sites_observer.h index 367fe1fcd2f..720af3cdf7b 100644 --- a/chromium/components/history/ios/browser/web_state_top_sites_observer.h +++ b/chromium/components/history/ios/browser/web_state_top_sites_observer.h @@ -30,7 +30,9 @@ class WebStateTopSitesObserver // web::WebStateObserver implementation. void NavigationItemCommitted( + web::WebState* web_state, const web::LoadCommittedDetails& load_details) override; + void WebStateDestroyed(web::WebState* web_state) override; // Underlying TopSites instance, may be null during testing. TopSites* top_sites_; diff --git a/chromium/components/history/ios/browser/web_state_top_sites_observer.mm b/chromium/components/history/ios/browser/web_state_top_sites_observer.mm index d414a52a8dc..7db3652948c 100644 --- a/chromium/components/history/ios/browser/web_state_top_sites_observer.mm +++ b/chromium/components/history/ios/browser/web_state_top_sites_observer.mm @@ -31,17 +31,23 @@ void WebStateTopSitesObserver::CreateForWebState(web::WebState* web_state, WebStateTopSitesObserver::WebStateTopSitesObserver(web::WebState* web_state, TopSites* top_sites) - : web::WebStateObserver(web_state), top_sites_(top_sites) { + : top_sites_(top_sites) { + web_state->AddObserver(this); } WebStateTopSitesObserver::~WebStateTopSitesObserver() { } void WebStateTopSitesObserver::NavigationItemCommitted( + web::WebState* web_state, const web::LoadCommittedDetails& load_details) { DCHECK(load_details.item); if (top_sites_) top_sites_->OnNavigationCommitted(load_details.item->GetURL()); } +void WebStateTopSitesObserver::WebStateDestroyed(web::WebState* web_state) { + web_state->RemoveObserver(this); +} + } // namespace history |