diff options
author | pkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-23 23:47:27 +0000 |
---|---|---|
committer | pkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-23 23:47:27 +0000 |
commit | 2303da8ec4999ec703c8b958d49a56e07dbfb477 (patch) | |
tree | cecc11d9c6c79c96319b33a21053bb2d422fd68b /chrome/browser | |
parent | 3af83cfa1ea38d4b4f7c1084c24de17b62ee856e (diff) | |
download | chromium_src-2303da8ec4999ec703c8b958d49a56e07dbfb477.zip chromium_src-2303da8ec4999ec703c8b958d49a56e07dbfb477.tar.gz chromium_src-2303da8ec4999ec703c8b958d49a56e07dbfb477.tar.bz2 |
Cleanup history favicon code
This removes the currently unused IconURLSizesMap.
It also uses the 'expired' parameter in the 'favicon_bitmaps' table instead of the 'sizes' attribute in the 'favicons' table.
Given that we are unlikely going to move to storing unresized bitmaps to the database due to perf problems when resizing the favicon each time it is requested, using the IconURLSizesMap is probably not the right solution in the future anyways. This CL should make the code in HistoryBackend less scary.
BUG=None
Test=No functionality change
HistoryBackendTest.*
Review URL: https://chromiumcodereview.appspot.com/11746010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@178432 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/extensions/extension_web_ui.cc | 15 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_handler.cc | 80 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_handler.h | 6 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_handler_unittest.cc | 13 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_service.cc | 34 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_service.h | 25 | ||||
-rw-r--r-- | chrome/browser/history/android/android_provider_backend_unittest.cc | 30 | ||||
-rw-r--r-- | chrome/browser/history/history.cc | 25 | ||||
-rw-r--r-- | chrome/browser/history/history.h | 23 | ||||
-rw-r--r-- | chrome/browser/history/history_backend.cc | 227 | ||||
-rw-r--r-- | chrome/browser/history/history_backend.h | 69 | ||||
-rw-r--r-- | chrome/browser/history/history_backend_unittest.cc | 497 | ||||
-rw-r--r-- | chrome/browser/history/history_types.h | 3 | ||||
-rw-r--r-- | chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc | 22 |
14 files changed, 310 insertions, 759 deletions
diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc index 019e3db..35b679a 100644 --- a/chrome/browser/extensions/extension_web_ui.cc +++ b/chrome/browser/extensions/extension_web_ui.cc @@ -87,7 +87,6 @@ void RunFaviconCallbackAsync( const gfx::Image& image) { std::vector<history::FaviconBitmapResult>* favicon_bitmap_results = new std::vector<history::FaviconBitmapResult>(); - history::IconURLSizesMap* icon_url_sizes = new history::IconURLSizesMap(); const std::vector<gfx::ImageSkiaRep>& image_reps = image.AsImageSkia().image_reps(); @@ -111,23 +110,11 @@ void RunFaviconCallbackAsync( } } - // Populate IconURLSizesMap such that all the icon URLs in - // |favicon_bitmap_results| are present in |icon_url_sizes|. - // Populate the favicon sizes with the relevant pixel sizes in the - // extension's icon set. - for (size_t i = 0; i < favicon_bitmap_results->size(); ++i) { - const history::FaviconBitmapResult& bitmap_result = - (*favicon_bitmap_results)[i]; - const GURL& icon_url = bitmap_result.icon_url; - (*icon_url_sizes)[icon_url].push_back(bitmap_result.pixel_size); - } - base::MessageLoopProxy::current()->PostTask( FROM_HERE, base::Bind(&FaviconService::FaviconResultsCallbackRunner, callback, - base::Owned(favicon_bitmap_results), - base::Owned(icon_url_sizes))); + base::Owned(favicon_bitmap_results))); } } // namespace diff --git a/chrome/browser/favicon/favicon_handler.cc b/chrome/browser/favicon/favicon_handler.cc index 539ae64..a25f536 100644 --- a/chrome/browser/favicon/favicon_handler.cc +++ b/chrome/browser/favicon/favicon_handler.cc @@ -94,59 +94,41 @@ bool IsValid(const history::FaviconBitmapResult& bitmap_result) { return bitmap_result.is_valid(); } -// Return list with the current value and historical values of -// history::GetDefaultFaviconSizes(). -const std::vector<history::FaviconSizes>& GetHistoricalDefaultFaviconSizes() { - CR_DEFINE_STATIC_LOCAL( - std::vector<history::FaviconSizes>, kHistoricalFaviconSizes, ()); - if (kHistoricalFaviconSizes.empty()) { - kHistoricalFaviconSizes.push_back(history::GetDefaultFaviconSizes()); - // The default favicon sizes used to be a single empty size. - history::FaviconSizes old_favicon_sizes; - old_favicon_sizes.push_back(gfx::Size()); - kHistoricalFaviconSizes.push_back(old_favicon_sizes); - } - return kHistoricalFaviconSizes; -} - // Returns true if at least one of the bitmaps in |bitmap_results| is expired or -// if |bitmap_results| is known to be incomplete. +// if |bitmap_results| is missing favicons for |desired_size_in_dip| and one of +// the scale factors in FaviconUtil::GetFaviconScaleFactors(). bool HasExpiredOrIncompleteResult( - const std::vector<history::FaviconBitmapResult>& bitmap_results, - const history::IconURLSizesMap& icon_url_sizes) { + int desired_size_in_dip, + const std::vector<history::FaviconBitmapResult>& bitmap_results) { // Check if at least one of the bitmaps is expired. std::vector<history::FaviconBitmapResult>::const_iterator it = std::find_if(bitmap_results.begin(), bitmap_results.end(), IsExpired); if (it != bitmap_results.end()) return true; - // |bitmap_results| is known to be incomplete if the favicon sizes in - // |icon_url_sizes| for any of the icon URLs in |bitmap_results| are unknown. - // The favicon sizes for an icon URL are unknown if MergeFavicon() has set - // them to the default favicon sizes. - - std::set<GURL> icon_urls; + // Any favicon size is good if the desired size is 0. + if (desired_size_in_dip == 0) + return false; + + // Check if the favicon for at least one of the scale factors is missing. + // |bitmap_results| should always be complete for data inserted by + // FaviconHandler as the FaviconHandler stores favicons resized to all + // of FaviconUtil::GetFaviconScaleFactors() into the history backend. + // Examples of when |bitmap_results| can be incomplete: + // - Favicons inserted into the history backend by sync. + // - Favicons for imported bookmarks. + std::vector<gfx::Size> favicon_sizes; for (size_t i = 0; i < bitmap_results.size(); ++i) - icon_urls.insert(bitmap_results[i].icon_url); - - for (std::set<GURL>::iterator it = icon_urls.begin(); it != icon_urls.end(); - ++it) { - const GURL& icon_url = *it; - history::IconURLSizesMap::const_iterator icon_url_sizes_it = - icon_url_sizes.find(icon_url); - if (icon_url_sizes_it == icon_url_sizes.end()) { - // |icon_url_sizes| should have an entry for each icon URL in - // |bitmap_results|. - NOTREACHED(); - return true; - } - - const history::FaviconSizes& sizes = icon_url_sizes_it->second; - const std::vector<history::FaviconSizes>& historical_sizes = - GetHistoricalDefaultFaviconSizes(); - std::vector<history::FaviconSizes>::const_iterator historical_it = - std::find(historical_sizes.begin(), historical_sizes.end(), sizes); - if (historical_it != historical_sizes.end()) + favicon_sizes.push_back(bitmap_results[i].pixel_size); + + std::vector<ui::ScaleFactor> scale_factors = + FaviconUtil::GetFaviconScaleFactors(); + for (size_t i = 0; i < scale_factors.size(); ++i) { + int edge_size_in_pixel = floor( + desired_size_in_dip * ui::GetScaleFactorScale(scale_factors[i])); + std::vector<gfx::Size>::iterator it = std::find(favicon_sizes.begin(), + favicon_sizes.end(), gfx::Size(edge_size_in_pixel, edge_size_in_pixel)); + if (it == favicon_sizes.end()) return true; } return false; @@ -483,8 +465,7 @@ bool FaviconHandler::ShouldSaveFavicon(const GURL& url) { } void FaviconHandler::OnFaviconDataForInitialURL( - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes) { + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results) { NavigationEntry* entry = GetEntry(); if (!entry) return; @@ -494,7 +475,7 @@ void FaviconHandler::OnFaviconDataForInitialURL( bool has_results = !favicon_bitmap_results.empty(); favicon_expired_or_incomplete_ = has_results && HasExpiredOrIncompleteResult( - favicon_bitmap_results, icon_url_sizes); + preferred_icon_size(), favicon_bitmap_results); if (has_results && icon_types_ == history::FAVICON && !entry->GetFavicon().valid && @@ -566,15 +547,14 @@ void FaviconHandler::DownloadFaviconOrAskHistory( } void FaviconHandler::OnFaviconData( - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes) { + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results) { NavigationEntry* entry = GetEntry(); if (!entry) return; bool has_results = !favicon_bitmap_results.empty(); bool has_expired_or_incomplete_result = HasExpiredOrIncompleteResult( - favicon_bitmap_results, icon_url_sizes); + preferred_icon_size(), favicon_bitmap_results); // No need to update the favicon url. By the time we get here // UpdateFaviconURL will have set the favicon url. diff --git a/chrome/browser/favicon/favicon_handler.h b/chrome/browser/favicon/favicon_handler.h index 135858f..1c39431 100644 --- a/chrome/browser/favicon/favicon_handler.h +++ b/chrome/browser/favicon/favicon_handler.h @@ -190,8 +190,7 @@ class FaviconHandler { // See description above class for details. void OnFaviconDataForInitialURL( - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes); + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results); // If the favicon has expired, asks the renderer to download the favicon. // Otherwise asks history to update the mapping between page url and icon @@ -202,8 +201,7 @@ class FaviconHandler { // See description above class for details. void OnFaviconData( - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes); + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results); // Schedules a download for the specified entry. This adds the request to // download_requests_. diff --git a/chrome/browser/favicon/favicon_handler_unittest.cc b/chrome/browser/favicon/favicon_handler_unittest.cc index 3f824a7..445c168 100644 --- a/chrome/browser/favicon/favicon_handler_unittest.cc +++ b/chrome/browser/favicon/favicon_handler_unittest.cc @@ -314,18 +314,7 @@ namespace { void HistoryRequestHandler::InvokeCallback() { if (!callback_.is_null()) { - history::IconURLSizesMap icon_url_sizes; - // Build IconURLSizesMap such that the requirement that all the icon URLs - // in |history_results_| be present in |icon_url_sizes| holds. - // Add the pixel size for each of |history_results_| to |icon_url_sizes| - // as empty favicon sizes has a special meaning. - for (size_t i = 0; i < history_results_.size(); ++i) { - const history::FaviconBitmapResult& bitmap_result = history_results_[i]; - const GURL& icon_url = bitmap_result.icon_url; - icon_url_sizes[icon_url].push_back(bitmap_result.pixel_size); - } - - callback_.Run(history_results_, icon_url_sizes); + callback_.Run(history_results_); } } diff --git a/chrome/browser/favicon/favicon_service.cc b/chrome/browser/favicon/favicon_service.cc index e4d6350..2f8f31c 100644 --- a/chrome/browser/favicon/favicon_service.cc +++ b/chrome/browser/favicon/favicon_service.cc @@ -25,11 +25,10 @@ namespace { void CancelOrRunFaviconResultsCallback( const CancelableTaskTracker::IsCanceledCallback& is_canceled, const FaviconService::FaviconResultsCallback& callback, - const std::vector<history::FaviconBitmapResult>& results, - const history::IconURLSizesMap& size_map) { + const std::vector<history::FaviconBitmapResult>& results) { if (is_canceled.Run()) return; - callback.Run(results, size_map); + callback.Run(results); } // Helper to run callback with empty results if we cannot get the history @@ -40,9 +39,7 @@ CancelableTaskTracker::TaskId RunWithEmptyResultAsync( return tracker->PostTask( base::MessageLoopProxy::current(), FROM_HERE, - Bind(callback, - std::vector<history::FaviconBitmapResult>(), - history::IconURLSizesMap())); + Bind(callback, std::vector<history::FaviconBitmapResult>())); } } // namespace @@ -54,9 +51,8 @@ FaviconService::FaviconService(HistoryService* history_service) // static void FaviconService::FaviconResultsCallbackRunner( const FaviconResultsCallback& callback, - const std::vector<history::FaviconBitmapResult>* results, - const history::IconURLSizesMap* size_map) { - callback.Run(*results, *size_map); + const std::vector<history::FaviconBitmapResult>* results) { + callback.Run(*results); } CancelableTaskTracker::TaskId FaviconService::GetFaviconImage( @@ -243,7 +239,6 @@ void FaviconService::SetFavicons( image_skia.EnsureRepsForSupportedScaleFactors(); const std::vector<gfx::ImageSkiaRep>& image_reps = image_skia.image_reps(); std::vector<history::FaviconBitmapData> favicon_bitmap_data; - history::FaviconSizes favicon_sizes; for (size_t i = 0; i < image_reps.size(); ++i) { scoped_refptr<base::RefCountedBytes> bitmap_data( new base::RefCountedBytes()); @@ -258,21 +253,10 @@ void FaviconService::SetFavicons( bitmap_data_element.icon_url = icon_url; favicon_bitmap_data.push_back(bitmap_data_element); - - // Construct favicon sizes from a guess at what the HTML 5 'sizes' - // attribute in the link tag is. - // TODO(pkotwicz): Plumb the HTML 5 sizes attribute to FaviconHandler. - favicon_sizes.push_back(pixel_size); } } - // TODO(pkotwicz): Tell the database about all the icon URLs associated - // with |page_url|. - history::IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = favicon_sizes; - - history_service_->SetFavicons(page_url, icon_type, favicon_bitmap_data, - icon_url_sizes); + history_service_->SetFavicons(page_url, icon_type, favicon_bitmap_data); } FaviconService::~FaviconService() {} @@ -308,8 +292,7 @@ CancelableTaskTracker::TaskId FaviconService::GetFaviconForURLImpl( void FaviconService::RunFaviconImageCallbackWithBitmapResults( const FaviconImageCallback& callback, int desired_size_in_dip, - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes_map) { + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results) { history::FaviconImageResult image_result; image_result.image = FaviconUtil::SelectFaviconFramesFromPNGs( favicon_bitmap_results, @@ -324,8 +307,7 @@ void FaviconService::RunFaviconRawCallbackWithBitmapResults( const FaviconRawCallback& callback, int desired_size_in_dip, ui::ScaleFactor desired_scale_factor, - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes_map) { + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results) { if (favicon_bitmap_results.empty() || !favicon_bitmap_results[0].is_valid()) { callback.Run(history::FaviconBitmapResult()); return; diff --git a/chrome/browser/favicon/favicon_service.h b/chrome/browser/favicon/favicon_service.h index d68bf74..12ad688 100644 --- a/chrome/browser/favicon/favicon_service.h +++ b/chrome/browser/favicon/favicon_service.h @@ -77,29 +77,14 @@ class FaviconService : public CancelableRequestProvider, // platform (eg MacOS) in addition to 1x. The vector has at most one result // for each of the scale factors. There are less entries if a single result // is the best bitmap to use for several scale factors. - // - // Second argument: - // a) If the callback is called as a result of GetFaviconForURL(): - // The second argument is a map of the icon URLs mapped to |page_url| to - // the sizes at which the favicon is available from the web. - // b) If the callback is called as a result of GetFavicon() or - // UpdateFaviconMappingsAndFetch(): - // The second argument is a map of the subset of |icon_urls| known to the - // history backend to a vector of sizes of the favicon bitmaps at each - // URL. If none of |icon_urls| are known to the history backend, an empty - // map is returned. - // See history_types.h for more information about IconURLSizesMap. - typedef base::Callback< - void(const std::vector<history::FaviconBitmapResult>&, - const history::IconURLSizesMap&)> + typedef base::Callback<void(const std::vector<history::FaviconBitmapResult>&)> FaviconResultsCallback; // We usually pass parameters with pointer to avoid copy. This function is a // helper to run FaviconResultsCallback with pointer parameters. static void FaviconResultsCallbackRunner( const FaviconResultsCallback& callback, - const std::vector<history::FaviconBitmapResult>* results, - const history::IconURLSizesMap* size_map); + const std::vector<history::FaviconBitmapResult>* results); // Requests the favicon at |icon_url| of |icon_type| whose size most closely // matches |desired_size_in_dip|. If |desired_size_in_dip| is 0, the largest @@ -256,8 +241,7 @@ class FaviconService : public CancelableRequestProvider, void RunFaviconImageCallbackWithBitmapResults( const FaviconImageCallback& callback, int desired_size_in_dip, - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes_map); + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results); // Intermediate callback for GetRawFavicon() and GetRawFaviconForURL() // so that history service can deal solely with FaviconResultsCallback. @@ -266,8 +250,7 @@ class FaviconService : public CancelableRequestProvider, const FaviconRawCallback& callback, int desired_size_in_dip, ui::ScaleFactor desired_scale_factor, - const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results, - const history::IconURLSizesMap& icon_url_sizes_map); + const std::vector<history::FaviconBitmapResult>& favicon_bitmap_results); DISALLOW_COPY_AND_ASSIGN(FaviconService); }; diff --git a/chrome/browser/history/android/android_provider_backend_unittest.cc b/chrome/browser/history/android/android_provider_backend_unittest.cc index fe034eb..e383100 100644 --- a/chrome/browser/history/android/android_provider_backend_unittest.cc +++ b/chrome/browser/history/android/android_provider_backend_unittest.cc @@ -255,15 +255,7 @@ TEST_F(AndroidProviderBackendTest, UpdateTables) { std::vector<history::FaviconBitmapData> favicon_bitmap_data; favicon_bitmap_data.push_back(bitmap_data_element); - FaviconSizes favicon_sizes; - favicon_sizes.push_back(gfx::Size()); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[GURL()] = favicon_sizes; - - history_backend->SetFavicons(url2, - FAVICON, - favicon_bitmap_data, - icon_url_sizes); + history_backend->SetFavicons(url2, FAVICON, favicon_bitmap_data); history_backend->Closing(); } @@ -413,15 +405,7 @@ TEST_F(AndroidProviderBackendTest, QueryHistoryAndBookmarks) { std::vector<history::FaviconBitmapData> favicon_bitmap_data; favicon_bitmap_data.push_back(bitmap_data_element); - FaviconSizes favicon_sizes; - favicon_sizes.push_back(gfx::Size()); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[GURL()] = favicon_sizes; - - history_backend->SetFavicons(url2, - FAVICON, - favicon_bitmap_data, - icon_url_sizes); + history_backend->SetFavicons(url2, FAVICON, favicon_bitmap_data); history_backend->Closing(); } @@ -1835,15 +1819,7 @@ TEST_F(AndroidProviderBackendTest, QueryWithoutThumbnailDB) { std::vector<history::FaviconBitmapData> favicon_bitmap_data; favicon_bitmap_data.push_back(bitmap_data_element); - FaviconSizes favicon_sizes; - favicon_sizes.push_back(gfx::Size()); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[GURL()] = favicon_sizes; - - history_backend->SetFavicons(url2, - FAVICON, - favicon_bitmap_data, - icon_url_sizes); + history_backend->SetFavicons(url2, FAVICON, favicon_bitmap_data); history_backend->Closing(); } diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc index c23d28e..ff36321 100644 --- a/chrome/browser/history/history.cc +++ b/chrome/browser/history/history.cc @@ -107,8 +107,8 @@ void DerefDownloadId( void RunWithFaviconResults( const FaviconService::FaviconResultsCallback& callback, - const HistoryBackend::FaviconResults* results) { - callback.Run(results->bitmap_results, results->size_map); + std::vector<history::FaviconBitmapResult>* bitmap_results) { + callback.Run(*bitmap_results); } // Extract history::URLRows into GURLs for VisitedLinkMaster. @@ -618,8 +618,8 @@ CancelableTaskTracker::TaskId HistoryService::GetFavicons( DCHECK(thread_checker_.CalledOnValidThread()); LoadBackendIfNecessary(); - HistoryBackend::FaviconResults* results = - new HistoryBackend::FaviconResults(); + std::vector<history::FaviconBitmapResult>* results = + new std::vector<history::FaviconBitmapResult>(); return tracker->PostTaskAndReply( thread_->message_loop_proxy(), FROM_HERE, @@ -639,8 +639,8 @@ CancelableTaskTracker::TaskId HistoryService::GetFaviconsForURL( DCHECK(thread_checker_.CalledOnValidThread()); LoadBackendIfNecessary(); - HistoryBackend::FaviconResults* results = - new HistoryBackend::FaviconResults(); + std::vector<history::FaviconBitmapResult>* results = + new std::vector<history::FaviconBitmapResult>(); return tracker->PostTaskAndReply( thread_->message_loop_proxy(), FROM_HERE, @@ -659,8 +659,8 @@ CancelableTaskTracker::TaskId HistoryService::GetFaviconForID( DCHECK(thread_checker_.CalledOnValidThread()); LoadBackendIfNecessary(); - HistoryBackend::FaviconResults* results = - new HistoryBackend::FaviconResults(); + std::vector<history::FaviconBitmapResult>* results = + new std::vector<history::FaviconBitmapResult>(); return tracker->PostTaskAndReply( thread_->message_loop_proxy(), FROM_HERE, @@ -681,8 +681,8 @@ CancelableTaskTracker::TaskId HistoryService::UpdateFaviconMappingsAndFetch( DCHECK(thread_checker_.CalledOnValidThread()); LoadBackendIfNecessary(); - HistoryBackend::FaviconResults* results = - new HistoryBackend::FaviconResults(); + std::vector<history::FaviconBitmapResult>* results = + new std::vector<history::FaviconBitmapResult>(); return tracker->PostTaskAndReply( thread_->message_loop_proxy(), FROM_HERE, @@ -709,14 +709,13 @@ void HistoryService::MergeFavicon( void HistoryService::SetFavicons( const GURL& page_url, history::IconType icon_type, - const std::vector<history::FaviconBitmapData>& favicon_bitmap_data, - const history::IconURLSizesMap& icon_url_sizes) { + const std::vector<history::FaviconBitmapData>& favicon_bitmap_data) { DCHECK(thread_checker_.CalledOnValidThread()); if (!CanAddURL(page_url)) return; ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::SetFavicons, page_url, - icon_type, favicon_bitmap_data, icon_url_sizes); + icon_type, favicon_bitmap_data); } void HistoryService::SetFaviconsOutOfDateForPage(const GURL& page_url) { diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h index 99f512f..a716bcc 100644 --- a/chrome/browser/history/history.h +++ b/chrome/browser/history/history.h @@ -804,26 +804,19 @@ class HistoryService : public CancelableRequestProvider, // Used by the FaviconService to set the favicons for a page on the history // backend. - // |favicon_bitmap_data| is a listing of additional favicon bitmaps to store - // for |page_url|. + // |favicon_bitmap_data| replaces all the favicon bitmaps mapped to + // |page_url|. // |expired| and |icon_type| fields in FaviconBitmapData are ignored. - // |icon_url_sizes| is a mapping of all the icon urls of the favicons - // available for |page_url| to the sizes that those favicons are available - // from the web. - // |favicon_bitmap_data| does not need to have entries for all the icon urls - // or sizes listed in |icon_url_sizes|. However, the icon urls and pixel - // sizes in |favicon_bitmap_data| must be a subset of |icon_url_sizes|. It is - // important that |icon_url_sizes| be complete as mappings to favicons whose - // icon url or pixel size is not in |icon_url_sizes| will be deleted. - // Use MergeFavicon() if any of the icon URLs for |page_url| or any of the - // favicon sizes of the icon URLs are not known. + // Use MergeFavicon() if |favicon_bitmap_data| is incomplete, and favicon + // bitmaps in the database should be preserved if possible. For instance, + // favicon bitmaps from sync are 1x only. MergeFavicon() is used to avoid + // deleting the 2x favicon bitmap if it is present in the history backend. // See HistoryBackend::ValidateSetFaviconsParams() for more details on the - // criteria for |favicon_bitmap_data| and |icon_url_sizes| to be valid. + // criteria for |favicon_bitmap_data| to be valid. void SetFavicons( const GURL& page_url, history::IconType icon_type, - const std::vector<history::FaviconBitmapData>& favicon_bitmap_data, - const history::IconURLSizesMap& icon_url_sizes); + const std::vector<history::FaviconBitmapData>& favicon_bitmap_data); // Used by the FaviconService to mark the favicon for the page as being out // of date. diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc index 50c2d62..63b8b5b 100644 --- a/chrome/browser/history/history_backend.cc +++ b/chrome/browser/history/history_backend.cc @@ -1844,23 +1844,15 @@ bool HistoryBackend::GetThumbnailFromOlderRedirect( return success; } -HistoryBackend::FaviconResults::FaviconResults() {} -HistoryBackend::FaviconResults::~FaviconResults() {} - -void HistoryBackend::FaviconResults::Clear() { - bitmap_results.clear(); - size_map.clear(); -} - void HistoryBackend::GetFavicons( const std::vector<GURL>& icon_urls, int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results) { + std::vector<FaviconBitmapResult>* bitmap_results) { UpdateFaviconMappingsAndFetchImpl(NULL, icon_urls, icon_types, - desired_size_in_dip, desired_scale_factors, - results); + desired_size_in_dip, desired_scale_factors, + bitmap_results); } void HistoryBackend::GetFaviconsForURL( @@ -1868,18 +1860,17 @@ void HistoryBackend::GetFaviconsForURL( int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results) { - DCHECK(results); + std::vector<FaviconBitmapResult>* bitmap_results) { + DCHECK(bitmap_results); GetFaviconsFromDB(page_url, icon_types, desired_size_in_dip, - desired_scale_factors, &results->bitmap_results, - &results->size_map); + desired_scale_factors, bitmap_results); } void HistoryBackend::GetFaviconForID( FaviconID favicon_id, int desired_size_in_dip, ui::ScaleFactor desired_scale_factor, - FaviconResults* results) { + std::vector<FaviconBitmapResult>* bitmap_results) { std::vector<FaviconID> favicon_ids; favicon_ids.push_back(favicon_id); std::vector<ui::ScaleFactor> desired_scale_factors; @@ -1889,8 +1880,7 @@ void HistoryBackend::GetFaviconForID( GetFaviconBitmapResultsForBestMatch(favicon_ids, desired_size_in_dip, desired_scale_factors, - &results->bitmap_results); - BuildIconURLSizesMap(favicon_ids, &results->size_map); + bitmap_results); } void HistoryBackend::UpdateFaviconMappingsAndFetch( @@ -1899,10 +1889,10 @@ void HistoryBackend::UpdateFaviconMappingsAndFetch( int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results) { + std::vector<FaviconBitmapResult>* bitmap_results) { UpdateFaviconMappingsAndFetchImpl(&page_url, icon_urls, icon_types, - desired_size_in_dip, desired_scale_factors, - results); + desired_size_in_dip, desired_scale_factors, + bitmap_results); } void HistoryBackend::MergeFavicon( @@ -1952,6 +1942,10 @@ void HistoryBackend::MergeFavicon( favicon_sizes.push_back(bitmap_id_sizes[i].pixel_size); if (!replaced_bitmap) { + // Set the preexisting favicon bitmaps as expired as the preexisting favicon + // bitmaps are not consistent with the merged in data. + thumbnail_db_->SetFaviconOutOfDate(favicon_id); + // Delete an arbitrary favicon bitmap to avoid going over the limit of // |kMaxFaviconBitmapsPerIconURL|. if (bitmap_id_sizes.size() >= kMaxFaviconBitmapsPerIconURL) { @@ -2018,8 +2012,11 @@ void HistoryBackend::MergeFavicon( continue; migrated_bitmaps = true; + + // Add the favicon bitmap as expired as it is not consistent with the + // merged in data. thumbnail_db_->AddFaviconBitmap(favicon_id, - bitmaps_to_copy[j].bitmap_data, base::Time::Now(), + bitmaps_to_copy[j].bitmap_data, base::Time(), bitmaps_to_copy[j].pixel_size); favicon_sizes.push_back(bitmaps_to_copy[j].pixel_size); @@ -2028,12 +2025,6 @@ void HistoryBackend::MergeFavicon( } } - if (migrated_bitmaps || !replaced_bitmap) { - // Set the favicon sizes to default to indicate that at least some of the - // favicon bitmaps for the favicon at |icon_url| are missing or stale. - thumbnail_db_->SetFaviconSizes(favicon_id, GetDefaultFaviconSizes()); - } - // 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) { @@ -2055,12 +2046,11 @@ void HistoryBackend::MergeFavicon( void HistoryBackend::SetFavicons( const GURL& page_url, IconType icon_type, - const std::vector<FaviconBitmapData>& favicon_bitmap_data, - const IconURLSizesMap& icon_url_sizes) { + const std::vector<FaviconBitmapData>& favicon_bitmap_data) { if (!thumbnail_db_.get() || !db_.get()) return; - DCHECK(ValidateSetFaviconsParams(favicon_bitmap_data, icon_url_sizes)); + DCHECK(ValidateSetFaviconsParams(favicon_bitmap_data)); // Build map of FaviconBitmapData for each icon url. typedef std::map<GURL, std::vector<FaviconBitmapData> > @@ -2076,36 +2066,25 @@ void HistoryBackend::SetFavicons( bool data_modified = false; std::vector<FaviconID> icon_ids; - for (IconURLSizesMap::const_iterator it = icon_url_sizes.begin(); - it != icon_url_sizes.end(); ++it) { + for (BitmapDataByIconURL::const_iterator it = grouped_by_icon_url.begin(); + it != grouped_by_icon_url.end(); ++it) { const GURL& icon_url = it->first; FaviconID icon_id = thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type, NULL); - if (icon_id) { - bool favicon_bitmaps_deleted = false; - SetFaviconSizes(icon_id, it->second, &favicon_bitmaps_deleted); - data_modified |= favicon_bitmaps_deleted; - } else { - icon_id = thumbnail_db_->AddFavicon(icon_url, icon_type, it->second); + + if (!icon_id) { + // TODO(pkotwicz): Remove the favicon sizes attribute from + // ThumbnailDatabase::AddFavicon(). + icon_id = thumbnail_db_->AddFavicon(icon_url, icon_type, + GetDefaultFaviconSizes()); data_modified = true; } icon_ids.push_back(icon_id); - BitmapDataByIconURL::iterator grouped_by_icon_url_it = - grouped_by_icon_url.find(icon_url); - if (grouped_by_icon_url_it != grouped_by_icon_url.end()) { - if (!data_modified) { - bool favicon_bitmaps_changed = false; - SetFaviconBitmaps(icon_id, - grouped_by_icon_url_it->second, - &favicon_bitmaps_changed); - data_modified |= favicon_bitmaps_changed; - } else { - SetFaviconBitmaps(icon_id, - grouped_by_icon_url_it->second, - NULL); - } - } + if (!data_modified) + SetFaviconBitmaps(icon_id, it->second, &data_modified); + else + SetFaviconBitmaps(icon_id, it->second, NULL); } data_modified |= @@ -2219,7 +2198,7 @@ void HistoryBackend::UpdateFaviconMappingsAndFetchImpl( int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results) { + std::vector<FaviconBitmapResult>* bitmap_results) { // If |page_url| is specified, |icon_types| must be either a single icon // type or icon types which are equivalent. DCHECK(!page_url || @@ -2227,7 +2206,7 @@ void HistoryBackend::UpdateFaviconMappingsAndFetchImpl( icon_types == TOUCH_ICON || icon_types == TOUCH_PRECOMPOSED_ICON || icon_types == (TOUCH_ICON | TOUCH_PRECOMPOSED_ICON)); - results->Clear(); + bitmap_results->clear(); if (!thumbnail_db_.get()) { return; @@ -2269,8 +2248,7 @@ void HistoryBackend::UpdateFaviconMappingsAndFetchImpl( } GetFaviconBitmapResultsForBestMatch(favicon_ids, desired_size_in_dip, - desired_scale_factors, &results->bitmap_results); - BuildIconURLSizesMap(favicon_ids, &results->size_map); + desired_scale_factors, bitmap_results); } void HistoryBackend::SetFaviconBitmaps( @@ -2283,89 +2261,75 @@ void HistoryBackend::SetFaviconBitmaps( std::vector<FaviconBitmapIDSize> bitmap_id_sizes; thumbnail_db_->GetFaviconBitmapIDSizes(icon_id, &bitmap_id_sizes); - // A nested loop is ok because in practice neither |favicon_bitmap_data| nor - // |bitmap_id_sizes| will have many elements. - for (size_t i = 0; i < favicon_bitmap_data.size(); ++i) { - const FaviconBitmapData& bitmap_data_element = favicon_bitmap_data[i]; - FaviconBitmapID bitmap_id = 0; - for (size_t j = 0; j < bitmap_id_sizes.size(); ++j) { - if (bitmap_id_sizes[j].pixel_size == bitmap_data_element.pixel_size) { - bitmap_id = bitmap_id_sizes[j].bitmap_id; + std::vector<FaviconBitmapData> to_add = favicon_bitmap_data; + + for (size_t i = 0; i < bitmap_id_sizes.size(); ++i) { + const gfx::Size& pixel_size = bitmap_id_sizes[i].pixel_size; + std::vector<FaviconBitmapData>::iterator match_it = to_add.end(); + for (std::vector<FaviconBitmapData>::iterator it = to_add.begin(); + it != to_add.end(); ++it) { + if (it->pixel_size == pixel_size) { + match_it = it; break; } } - if (bitmap_id) { + + FaviconBitmapID bitmap_id = bitmap_id_sizes[i].bitmap_id; + if (match_it == to_add.end()) { + thumbnail_db_->DeleteFaviconBitmap(bitmap_id); + + if (favicon_bitmaps_changed) + *favicon_bitmaps_changed = true; + } else { if (favicon_bitmaps_changed && !*favicon_bitmaps_changed && - IsFaviconBitmapDataEqual(bitmap_id, - bitmap_data_element.bitmap_data)) { + IsFaviconBitmapDataEqual(bitmap_id, match_it->bitmap_data)) { thumbnail_db_->SetFaviconBitmapLastUpdateTime( bitmap_id, base::Time::Now()); } else { - thumbnail_db_->SetFaviconBitmap(bitmap_id, - bitmap_data_element.bitmap_data, base::Time::Now()); + thumbnail_db_->SetFaviconBitmap(bitmap_id, match_it->bitmap_data, + base::Time::Now()); + if (favicon_bitmaps_changed) *favicon_bitmaps_changed = true; } - } else { - thumbnail_db_->AddFaviconBitmap(icon_id, bitmap_data_element.bitmap_data, - base::Time::Now(), bitmap_data_element.pixel_size); - if (favicon_bitmaps_changed) - *favicon_bitmaps_changed = true; + to_add.erase(match_it); } } -} -bool HistoryBackend::ValidateSetFaviconsParams( - const std::vector<FaviconBitmapData>& favicon_bitmap_data, - const IconURLSizesMap& icon_url_sizes) const { - if (icon_url_sizes.size() > kMaxFaviconsPerPage) - return false; + for (size_t i = 0; i < to_add.size(); ++i) { + thumbnail_db_->AddFaviconBitmap(icon_id, to_add[i].bitmap_data, + base::Time::Now(), to_add[i].pixel_size); - for (IconURLSizesMap::const_iterator it = icon_url_sizes.begin(); - it != icon_url_sizes.end(); ++it) { - if (it->second.size() > kMaxFaviconBitmapsPerIconURL) - return false; + if (favicon_bitmaps_changed) + *favicon_bitmaps_changed = true; } +} +bool HistoryBackend::ValidateSetFaviconsParams( + const std::vector<FaviconBitmapData>& favicon_bitmap_data) const { + typedef std::map<GURL, size_t> BitmapsPerIconURL; + BitmapsPerIconURL num_bitmaps_per_icon_url; for (size_t i = 0; i < favicon_bitmap_data.size(); ++i) { if (!favicon_bitmap_data[i].bitmap_data.get()) return false; - IconURLSizesMap::const_iterator it = - icon_url_sizes.find(favicon_bitmap_data[i].icon_url); - if (it == icon_url_sizes.end()) - return false; - - const FaviconSizes& favicon_sizes = it->second; - FaviconSizes::const_iterator it2 = std::find(favicon_sizes.begin(), - favicon_sizes.end(), favicon_bitmap_data[i].pixel_size); - if (it2 == favicon_sizes.end()) - return false; + const GURL& icon_url = favicon_bitmap_data[i].icon_url; + if (!num_bitmaps_per_icon_url.count(icon_url)) + num_bitmaps_per_icon_url[icon_url] = 1u; + else + ++num_bitmaps_per_icon_url[icon_url]; } - return true; -} -void HistoryBackend::SetFaviconSizes(FaviconID icon_id, - const FaviconSizes& favicon_sizes, - bool* favicon_bitmaps_deleted) { - *favicon_bitmaps_deleted = false; - - std::vector<FaviconBitmapIDSize> bitmap_id_sizes; - thumbnail_db_->GetFaviconBitmapIDSizes(icon_id, &bitmap_id_sizes); + if (num_bitmaps_per_icon_url.size() > kMaxFaviconsPerPage) + return false; - // Remove bitmaps whose pixel size is not contained in |favicon_sizes|. - for (size_t i = 0; i < bitmap_id_sizes.size(); ++i) { - const gfx::Size& pixel_size = bitmap_id_sizes[i].pixel_size; - FaviconSizes::const_iterator sizes_it = std::find(favicon_sizes.begin(), - favicon_sizes.end(), pixel_size); - if (sizes_it == favicon_sizes.end()) { - thumbnail_db_->DeleteFaviconBitmap(bitmap_id_sizes[i].bitmap_id); - *favicon_bitmaps_deleted = true; - } + for (BitmapsPerIconURL::const_iterator it = num_bitmaps_per_icon_url.begin(); + it != num_bitmaps_per_icon_url.end(); ++it) { + if (it->second > kMaxFaviconBitmapsPerIconURL) + return false; } - - thumbnail_db_->SetFaviconSizes(icon_id, favicon_sizes); + return true; } bool HistoryBackend::IsFaviconBitmapDataEqual( @@ -2387,12 +2351,9 @@ bool HistoryBackend::GetFaviconsFromDB( int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - std::vector<FaviconBitmapResult>* favicon_bitmap_results, - IconURLSizesMap* icon_url_sizes) { + std::vector<FaviconBitmapResult>* favicon_bitmap_results) { DCHECK(favicon_bitmap_results); - DCHECK(icon_url_sizes); favicon_bitmap_results->clear(); - icon_url_sizes->clear(); if (!db_.get() || !thumbnail_db_.get()) return false; @@ -2409,20 +2370,18 @@ bool HistoryBackend::GetFaviconsFromDB( favicon_ids.push_back(icon_mappings[i].icon_id); // Populate |favicon_bitmap_results| and |icon_url_sizes|. - bool success = - GetFaviconBitmapResultsForBestMatch(favicon_ids, - desired_size_in_dip, desired_scale_factors, favicon_bitmap_results) && - BuildIconURLSizesMap(favicon_ids, icon_url_sizes); + bool success = GetFaviconBitmapResultsForBestMatch(favicon_ids, + desired_size_in_dip, desired_scale_factors, favicon_bitmap_results); UMA_HISTOGRAM_TIMES("History.GetFavIconFromDB", // historical name TimeTicks::Now() - beginning_time); - return success && !icon_url_sizes->empty(); + return success && !favicon_bitmap_results->empty(); } bool HistoryBackend::GetFaviconBitmapResultsForBestMatch( const std::vector<FaviconID>& candidate_favicon_ids, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - std::vector<history::FaviconBitmapResult>* favicon_bitmap_results) { + std::vector<FaviconBitmapResult>* favicon_bitmap_results) { favicon_bitmap_results->clear(); if (candidate_favicon_ids.empty()) @@ -2493,22 +2452,6 @@ bool HistoryBackend::GetFaviconBitmapResultsForBestMatch( return true; } -bool HistoryBackend::BuildIconURLSizesMap( - const std::vector<FaviconID>& favicon_ids, - IconURLSizesMap* icon_url_sizes) { - icon_url_sizes->clear(); - for (size_t i = 0; i < favicon_ids.size(); ++i) { - GURL icon_url; - FaviconSizes favicon_sizes; - if (!thumbnail_db_->GetFaviconHeader(favicon_ids[i], &icon_url, NULL, - &favicon_sizes)) { - return false; - } - (*icon_url_sizes)[icon_url] = favicon_sizes; - } - return true; -} - bool HistoryBackend::SetFaviconMappingsForPageAndRedirects( const GURL& page_url, IconType icon_type, diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h index 254db76..58ee994 100644 --- a/chrome/browser/history/history_backend.h +++ b/chrome/browser/history/history_backend.h @@ -259,33 +259,24 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // Favicon ------------------------------------------------------------------- - struct FaviconResults { - FaviconResults(); - ~FaviconResults(); - void Clear(); - - std::vector<history::FaviconBitmapResult> bitmap_results; - IconURLSizesMap size_map; - }; - void GetFavicons(const std::vector<GURL>& icon_urls, int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results); + std::vector<FaviconBitmapResult>* bitmap_results); void GetFaviconsForURL( const GURL& page_url, int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results); + std::vector<FaviconBitmapResult>* bitmap_results); void GetFaviconForID( FaviconID favicon_id, int desired_size_in_dip, ui::ScaleFactor desired_scale_factor, - FaviconResults* results); + std::vector<FaviconBitmapResult>* bitmap_results); void UpdateFaviconMappingsAndFetch( const GURL& page_url, @@ -293,7 +284,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results); + std::vector<FaviconBitmapResult>* bitmap_results); void MergeFavicon(const GURL& page_url, const GURL& icon_url, @@ -304,8 +295,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, void SetFavicons( const GURL& page_url, IconType icon_type, - const std::vector<FaviconBitmapData>& favicon_bitmap_data, - const IconURLSizesMap& icon_url_sizes); + const std::vector<FaviconBitmapData>& favicon_bitmap_data); void SetFaviconsOutOfDateForPage(const GURL& page_url); @@ -517,7 +507,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, SetFaviconMappingsForPageAndRedirects); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFaviconMappingsForPageDuplicates); - FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFavicons); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFaviconsDeleteBitmaps); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFaviconsReplaceBitmapData); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, @@ -672,12 +661,14 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, int icon_types, int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - FaviconResults* results); + std::vector<FaviconBitmapResult>* results); // Set the favicon bitmaps for |icon_id|. // For each entry in |favicon_bitmap_data|, if a favicon bitmap already // exists at the entry's pixel size, replace the favicon bitmap's data with // the entry's bitmap data. Otherwise add a new favicon bitmap. + // Any favicon bitmaps already mapped to |icon_id| whose pixel sizes are not + // in |favicon_bitmap_data| are deleted. // If not NULL, |favicon_bitmaps_changed| is set to whether any of the bitmap // data at |icon_id| is changed as a result of calling this method. // Computing |favicon_bitmaps_changed| requires additional database queries @@ -687,31 +678,14 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, const std::vector<FaviconBitmapData>& favicon_bitmap_data, bool* favicon_bitmaps_changed); - // Returns true if |favicon_bitmap_data| and |icon_url_sizes| passed to - // SetFavicons() are valid. + // Returns true if |favicon_bitmap_data| passed to SetFavicons() is valid. // Criteria: - // 1) |icon_url_sizes| contains no more than - // kMaxFaviconsPerPage icon URLs. - // kMaxFaviconBitmapsPerIconURL favicon sizes for each icon URL. - // 2) The icon URLs and favicon sizes of |favicon_bitmap_data| are a subset - // of |icon_url_sizes|. - // 3) The favicon sizes for entries in |icon_url_sizes| which have associated - // data in |favicon_bitmap_data| is not history::GetDefaultFaviconSizes(). - // 4) FaviconBitmapData::bitmap_data contains non NULL bitmap data. + // 1) |favicon_bitmap_data| contains no more than + // kMaxFaviconsPerPage unique icon URLs. + // kMaxFaviconBitmapsPerIconURL favicon bitmaps for each icon URL. + // 2) FaviconBitmapData::bitmap_data contains non NULL bitmap data. bool ValidateSetFaviconsParams( - const std::vector<FaviconBitmapData>& favicon_bitmap_data, - const IconURLSizesMap& icon_url_sizes) const; - - // Sets the sizes that the thumbnail database knows that the favicon at - // |icon_id| is available from the web. See history_types.h for a more - // detailed description of FaviconSizes. - // Deletes any favicon bitmaps currently mapped to |icon_id| whose pixel - // sizes are not contained in |favicon_sizes|. - // |favicon_bitmaps_deleted| is set to true if at least one favicon bitmap - // is deleted. - void SetFaviconSizes(FaviconID icon_id, - const FaviconSizes& favicon_sizes, - bool* favicon_bitmaps_deleted); + const std::vector<FaviconBitmapData>& favicon_bitmap_data) const; // Returns true if the bitmap data at |bitmap_id| equals |new_bitmap_data|. bool IsFaviconBitmapDataEqual( @@ -729,16 +703,12 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // TOUCH_PRECOMPOSED_ICON, TOUCH_ICON, and FAVICON. See the comment for // GetFaviconResultsForBestMatch() for more details on how // |favicon_bitmap_results| is constructed. - // |icon_url_sizes| is set to a mapping of all the icon URLs which are mapped - // to |page_url| to the sizes of the favicon bitmaps available at each icon - // URL on the web. bool GetFaviconsFromDB( const GURL& page_url, int icon_types, const int desired_size_in_dip, const std::vector<ui::ScaleFactor>& desired_scale_factors, - std::vector<FaviconBitmapResult>* favicon_bitmap_results, - IconURLSizesMap* icon_url_sizes); + std::vector<FaviconBitmapResult>* favicon_bitmap_results); // Returns the favicon bitmaps which most closely match |desired_size_in_dip| // and |desired_scale_factors| in |favicon_bitmap_results|. If @@ -755,15 +725,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, const std::vector<ui::ScaleFactor>& desired_scale_factors, std::vector<FaviconBitmapResult>* favicon_bitmap_results); - // Build mapping of the icon URLs for |favicon_ids| to the sizes of the - // favicon bitmaps available at each icon URL on the web. Favicon bitmaps - // might not be cached in the thumbnail database for any of the sizes in the - // returned map. See history_types.h for a more detailed description of - // IconURLSizesMap. - // Returns true if map was successfully built. - bool BuildIconURLSizesMap(const std::vector<FaviconID>& favicon_ids, - IconURLSizesMap* icon_url_sizes); - // Maps the favicon ids in |icon_ids| to |page_url| (and all redirects) // for |icon_type|. // Returns true if the mappings for the page or any of its redirects were diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc index 9e55400..42d5e87 100644 --- a/chrome/browser/history/history_backend_unittest.cc +++ b/chrome/browser/history/history_backend_unittest.cc @@ -1287,33 +1287,29 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageAndRedirects) { const GURL icon_url1("http://www.google.com/icon"); const GURL icon_url2("http://www.google.com/icon2"); - // Create mapping for a page with two favicons. - IconURLSizesMap two_icon_url_sizes; - two_icon_url_sizes[icon_url1] = GetSizesSmallAndLarge(); - two_icon_url_sizes[icon_url2] = GetSizesSmallAndLarge(); + // Generate bitmap data for a page with two favicons. + std::vector<FaviconBitmapData> two_favicon_bitmap_data; + GenerateFaviconBitmapData(icon_url1, GetSizesSmallAndLarge(), + icon_url2, GetSizesSmallAndLarge(), &two_favicon_bitmap_data); - // Create a mapping for a page with a single favicon. - IconURLSizesMap one_icon_url_sizes; - one_icon_url_sizes[icon_url1] = GetSizesSmallAndLarge(); - - std::vector<FaviconBitmapData> favicon_bitmap_data; + // Generate bitmap data for a page with a single favicon. + std::vector<FaviconBitmapData> one_favicon_bitmap_data; + GenerateFaviconBitmapData(icon_url1, GetSizesSmallAndLarge(), + &one_favicon_bitmap_data); // Add two favicons - backend_->SetFavicons(url1, FAVICON, favicon_bitmap_data, - two_icon_url_sizes); + backend_->SetFavicons(url1, FAVICON, two_favicon_bitmap_data); EXPECT_EQ(2u, NumIconMappingsForPageURL(url1, FAVICON)); EXPECT_EQ(2u, NumIconMappingsForPageURL(url2, FAVICON)); // Add one touch_icon - backend_->SetFavicons(url1, TOUCH_ICON, favicon_bitmap_data, - one_icon_url_sizes); + backend_->SetFavicons(url1, TOUCH_ICON, one_favicon_bitmap_data); EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, TOUCH_ICON)); EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, TOUCH_ICON)); EXPECT_EQ(2u, NumIconMappingsForPageURL(url1, FAVICON)); // Add one TOUCH_PRECOMPOSED_ICON - backend_->SetFavicons(url1, TOUCH_PRECOMPOSED_ICON, favicon_bitmap_data, - one_icon_url_sizes); + backend_->SetFavicons(url1, TOUCH_PRECOMPOSED_ICON, one_favicon_bitmap_data); // The touch_icon was replaced. EXPECT_EQ(0u, NumIconMappingsForPageURL(url1, TOUCH_ICON)); EXPECT_EQ(2u, NumIconMappingsForPageURL(url1, FAVICON)); @@ -1321,38 +1317,35 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageAndRedirects) { EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, TOUCH_PRECOMPOSED_ICON)); // Add a touch_icon. - backend_->SetFavicons(url1, TOUCH_ICON, favicon_bitmap_data, - one_icon_url_sizes); + backend_->SetFavicons(url1, TOUCH_ICON, one_favicon_bitmap_data); EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, TOUCH_ICON)); EXPECT_EQ(2u, NumIconMappingsForPageURL(url1, FAVICON)); // The TOUCH_PRECOMPOSED_ICON was replaced. EXPECT_EQ(0u, NumIconMappingsForPageURL(url1, TOUCH_PRECOMPOSED_ICON)); // Add a single favicon. - backend_->SetFavicons(url1, FAVICON, favicon_bitmap_data, - one_icon_url_sizes); + backend_->SetFavicons(url1, FAVICON, one_favicon_bitmap_data); EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, TOUCH_ICON)); EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, FAVICON)); EXPECT_EQ(1u, NumIconMappingsForPageURL(url2, FAVICON)); // Add two favicons. - backend_->SetFavicons(url1, FAVICON, favicon_bitmap_data, - two_icon_url_sizes); + backend_->SetFavicons(url1, FAVICON, two_favicon_bitmap_data); EXPECT_EQ(1u, NumIconMappingsForPageURL(url1, TOUCH_ICON)); EXPECT_EQ(2u, NumIconMappingsForPageURL(url1, FAVICON)); } // Test that there is no churn in icon mappings from calling -// SetFavicons() twice with the same |icon_url_sizes| parameter. +// SetFavicons() twice with the same |favicon_bitmap_data| parameter. TEST_F(HistoryBackendTest, SetFaviconMappingsForPageDuplicates) { const GURL url("http://www.google.com/"); const GURL icon_url("http://www.google.com/icon"); - std::vector<FaviconBitmapData> favicon_bitmap_data; - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmallAndLarge(); + std::vector<FaviconBitmapData> favicon_bitmap_data; + GenerateFaviconBitmapData(icon_url, GetSizesSmallAndLarge(), + &favicon_bitmap_data); - backend_->SetFavicons(url, FAVICON, favicon_bitmap_data, icon_url_sizes); + backend_->SetFavicons(url, FAVICON, favicon_bitmap_data); std::vector<IconMapping> icon_mappings; EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( @@ -1360,7 +1353,7 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageDuplicates) { EXPECT_EQ(1u, icon_mappings.size()); IconMappingID mapping_id = icon_mappings[0].mapping_id; - backend_->SetFavicons(url, FAVICON, favicon_bitmap_data, icon_url_sizes); + backend_->SetFavicons(url, FAVICON, favicon_bitmap_data); icon_mappings.clear(); EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( @@ -1372,183 +1365,77 @@ TEST_F(HistoryBackendTest, SetFaviconMappingsForPageDuplicates) { EXPECT_EQ(mapping_id, icon_mappings[0].mapping_id); } -// Test that setting favicons for a page which already has data does the -// right thing. -TEST_F(HistoryBackendTest, SetFavicons) { +// Test that calling SetFavicons() with FaviconBitmapData of different pixel +// sizes than the initially passed in FaviconBitmapData deletes the no longer +// used favicon bitmaps. +TEST_F(HistoryBackendTest, SetFaviconsDeleteBitmaps) { const GURL page_url("http://www.google.com/"); - std::vector<FaviconBitmapData> favicon_bitmap_data; - IconURLSizesMap icon_url_sizes; - - // Set |page_url| as having two favicons each available from the web at two - // sizes. - const GURL icon_url1("http://www.google.com/icon1"); - const GURL icon_url2("http://www.google.com/icon2"); - - icon_url_sizes[icon_url1] = GetSizesSmallAndLarge(); - icon_url_sizes[icon_url2] = GetSizesSmallAndLarge(); - - // Set only sizes info for the favicons. - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); - - std::vector<IconMapping> icon_mappings; - EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( - page_url, &icon_mappings)); - EXPECT_EQ(2u, icon_mappings.size()); - for (size_t i = 0; i < icon_mappings.size(); ++i) { - EXPECT_FALSE(backend_->thumbnail_db_->GetFaviconBitmaps( - icon_mappings[i].icon_id, NULL)); - } + const GURL icon_url("http://www.google.com/icon"); - // Add bitmap data to the favicons. - GenerateFaviconBitmapData(icon_url1, - GetSizesSmall(), - icon_url2, - GetSizesSmallAndLarge(), + std::vector<FaviconBitmapData> favicon_bitmap_data; + GenerateFaviconBitmapData(icon_url, GetSizesSmallAndLarge(), &favicon_bitmap_data); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); - - icon_mappings.clear(); + // Test initial state. + std::vector<IconMapping> icon_mappings; EXPECT_TRUE(GetSortedIconMappingsForPageURL(page_url, &icon_mappings)); - EXPECT_EQ(2u, icon_mappings.size()); - - GURL icon_url; - IconType icon_type; - FaviconSizes favicon_sizes; - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, &icon_url, &icon_type, &favicon_sizes)); - EXPECT_EQ(icon_url1, icon_url); - EXPECT_EQ(FAVICON, icon_type); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); + EXPECT_EQ(1u, icon_mappings.size()); + EXPECT_EQ(icon_url, icon_mappings[0].icon_url); + EXPECT_EQ(FAVICON, icon_mappings[0].icon_type); + FaviconID favicon_id = icon_mappings[0].icon_id; std::vector<FaviconBitmap> favicon_bitmaps; - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmaps( - icon_mappings[0].icon_id, &favicon_bitmaps)); - EXPECT_EQ(1u, favicon_bitmaps.size()); - EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmaps[0].bitmap_data)); - EXPECT_EQ(kSmallSize, favicon_bitmaps[0].pixel_size); - - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[1].icon_id, &icon_url, &icon_type, &favicon_sizes)); - EXPECT_EQ(icon_url2, icon_url); - EXPECT_EQ(FAVICON, icon_type); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); - - favicon_bitmaps.clear(); - EXPECT_TRUE(GetSortedFaviconBitmaps(icon_mappings[1].icon_id, - &favicon_bitmaps)); - + EXPECT_TRUE(GetSortedFaviconBitmaps(favicon_id, &favicon_bitmaps)); EXPECT_EQ(2u, favicon_bitmaps.size()); - EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmaps[0].bitmap_data)); + FaviconBitmapID small_bitmap_id = favicon_bitmaps[0].bitmap_id; + EXPECT_NE(0, small_bitmap_id); + EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmaps[0].bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmaps[0].pixel_size); - EXPECT_TRUE(BitmapDataEqual('c', favicon_bitmaps[1].bitmap_data)); + FaviconBitmapID large_bitmap_id = favicon_bitmaps[1].bitmap_id; + EXPECT_NE(0, large_bitmap_id); + EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmaps[1].bitmap_data)); EXPECT_EQ(kLargeSize, favicon_bitmaps[1].pixel_size); - // Notifications should have been broadcast for each call to SetFavicons(). - EXPECT_EQ(2, num_broadcasted_notifications()); + // Call SetFavicons() with bitmap data for only the large bitmap. Check that + // the small bitmap is in fact deleted. + GenerateFaviconBitmapData(icon_url, GetSizesLarge(), &favicon_bitmap_data); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); - // Change the sizes for which the favicon at icon_url1 is available at from - // the web. Verify that all the data remains valid. - icon_url_sizes[icon_url1] = GetSizesTinySmallAndLarge(); - favicon_bitmap_data.clear(); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + scoped_refptr<base::RefCountedMemory> bitmap_data_out; + gfx::Size pixel_size_out; + EXPECT_FALSE(backend_->thumbnail_db_->GetFaviconBitmap(small_bitmap_id, + NULL, &bitmap_data_out, &pixel_size_out)); + EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmap(large_bitmap_id, + NULL, &bitmap_data_out, &pixel_size_out)); + EXPECT_TRUE(BitmapDataEqual('a', bitmap_data_out)); + EXPECT_EQ(kLargeSize, pixel_size_out); icon_mappings.clear(); - EXPECT_TRUE(GetSortedIconMappingsForPageURL(page_url, &icon_mappings)); - EXPECT_EQ(2u, icon_mappings.size()); - - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, &icon_url, &icon_type, &favicon_sizes)); - EXPECT_EQ(icon_url1, icon_url); - EXPECT_EQ(FAVICON, icon_type); - EXPECT_EQ(GetSizesTinySmallAndLarge(), favicon_sizes); - - favicon_bitmaps.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmaps( - icon_mappings[0].icon_id, &favicon_bitmaps)); - EXPECT_EQ(1u, favicon_bitmaps.size()); - - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[1].icon_id, &icon_url, &icon_type, &favicon_sizes)); - EXPECT_EQ(icon_url2, icon_url); - EXPECT_EQ(FAVICON, icon_type); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); - - favicon_bitmaps.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmaps( - icon_mappings[1].icon_id, &favicon_bitmaps)); - EXPECT_EQ(2u, favicon_bitmaps.size()); - - // No notifications should have been sent because changing the favicon sizes - // did not result in deleting any favicon bitmaps. - EXPECT_EQ(2, num_broadcasted_notifications()); -} - -// Test that changing the sizes that a favicon is available at from the web -// deletes stale favicons and favicon bitmaps. -TEST_F(HistoryBackendTest, SetFaviconsDeleteBitmaps) { - const GURL page_url("http://www.google.com/"); - const GURL icon_url("http://www.google.com/icon"); - - // Set |page_url| as having one favicon with two different sizes. - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmallAndLarge(); - - std::vector<FaviconBitmapData> favicon_bitmap_data; - GenerateFaviconBitmapData(icon_url, GetSizesSmallAndLarge(), - &favicon_bitmap_data); - - // Add bitmap data and sizes information to the database. - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); - - FaviconID favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url, FAVICON, NULL); - EXPECT_NE(0, favicon_id); - - std::vector<FaviconBitmap> favicon_bitmaps; - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmaps(favicon_id, - &favicon_bitmaps)); - EXPECT_EQ(2u, favicon_bitmaps.size()); + EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, + &icon_mappings)); + EXPECT_EQ(1u, icon_mappings.size()); + EXPECT_EQ(favicon_id, icon_mappings[0].icon_id); - // Change the bitmap sizes available from the web only to the small size only. - icon_url_sizes[icon_url] = GetSizesSmall(); + // Call SetFavicons() with no bitmap data. Check that the bitmaps and icon + // mappings are deleted. favicon_bitmap_data.clear(); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); - - favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url, FAVICON, NULL); - EXPECT_NE(0, favicon_id); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); - favicon_bitmaps.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmaps(favicon_id, - &favicon_bitmaps)); - EXPECT_EQ(1u, favicon_bitmaps.size()); - EXPECT_EQ(kSmallSize, favicon_bitmaps[0].pixel_size); + EXPECT_FALSE(backend_->thumbnail_db_->GetFaviconBitmap(large_bitmap_id, NULL, + NULL, NULL)); + icon_mappings.clear(); + EXPECT_FALSE(backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, + &icon_mappings)); - // Clear |icon_url_sizes|. SetFavicons() should delete the remaining favicon - // and its favicon bitmap. - icon_url_sizes.clear(); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, icon_url_sizes); - EXPECT_EQ(0, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( - icon_url, FAVICON, NULL)); - EXPECT_FALSE(backend_->thumbnail_db_->GetFaviconBitmaps(favicon_id, NULL)); + // Notifications should have been broadcast for each call to SetFavicons(). + EXPECT_EQ(3, num_broadcasted_notifications()); } // Test updating a single favicon bitmap's data via SetFavicons. TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { - const GURL page_url("http://www.google.com/"); const GURL icon_url("http://www.google.com/icon"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmall(); std::vector<unsigned char> data_initial; data_initial.push_back('a'); @@ -1562,8 +1449,7 @@ TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { favicon_bitmap_data.push_back(bitmap_data_element); // Add bitmap to the database. - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); FaviconID original_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, FAVICON, @@ -1580,8 +1466,7 @@ TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { std::vector<unsigned char> updated_data; updated_data.push_back('a'); favicon_bitmap_data[0].bitmap_data = new base::RefCountedBytes(updated_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); FaviconID updated_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, FAVICON, @@ -1599,8 +1484,7 @@ TEST_F(HistoryBackendTest, SetFaviconsReplaceBitmapData) { // Call SetFavicons() with identical data but a different bitmap. updated_data[0] = 'b'; favicon_bitmap_data[0].bitmap_data = new base::RefCountedBytes(updated_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); updated_favicon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, FAVICON, @@ -1629,23 +1513,19 @@ TEST_F(HistoryBackendTest, SetFaviconsSameFaviconURLForTwoPages) { GURL page_url1("http://www.google.com"); GURL page_url2("http://www.google.ca"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmallAndLarge(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url, GetSizesSmallAndLarge(), &favicon_bitmap_data); - backend_->SetFavicons(page_url1, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url1, FAVICON, favicon_bitmap_data); std::vector<GURL> icon_urls; icon_urls.push_back(icon_url); - HistoryBackend::FaviconResults results; + std::vector<FaviconBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( page_url2, icon_urls, FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &results); + GetScaleFactors1x2x(), &bitmap_results); // Check that the same FaviconID is mapped to both page URLs. std::vector<IconMapping> icon_mappings; @@ -1662,12 +1542,9 @@ TEST_F(HistoryBackendTest, SetFaviconsSameFaviconURLForTwoPages) { EXPECT_EQ(favicon_id, icon_mappings[0].icon_id); // Change the icon URL that |page_url1| is mapped to. - icon_url_sizes.clear(); - icon_url_sizes[icon_url_new] = GetSizesSmall(); GenerateFaviconBitmapData(icon_url_new, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url1, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url1, FAVICON, favicon_bitmap_data); // |page_url1| should map to a new FaviconID and have valid bitmap data. icon_mappings.clear(); @@ -1700,18 +1577,16 @@ TEST_F(HistoryBackendTest, SetFaviconsSameFaviconURLForTwoPages) { EXPECT_EQ(3, num_broadcasted_notifications()); } -// Test that there is no churn from calling UpdateFaviconMappingsAndFetch() -// for an icon URL which is already mapped to the passed in page URL. +// Test that no notifications are broadcast as a result of calling +// UpdateFaviconMappingsAndFetch() for an icon URL which is already +// mapped to the passed in page URL. TEST_F(HistoryBackendTest, UpdateFaviconMappingsAndFetchNoChange) { GURL page_url("http://www.google.com"); GURL icon_url("http://www.google.com/favicon.ico"); std::vector<FaviconBitmapData> favicon_bitmap_data; + GenerateFaviconBitmapData(icon_url, GetSizesSmall(), &favicon_bitmap_data); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmall(); - - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); FaviconID icon_id = backend_->thumbnail_db_->GetFaviconIDForFaviconURL( icon_url, FAVICON, NULL); @@ -1721,10 +1596,10 @@ TEST_F(HistoryBackendTest, UpdateFaviconMappingsAndFetchNoChange) { std::vector<GURL> icon_urls; icon_urls.push_back(icon_url); - HistoryBackend::FaviconResults results; + std::vector<FaviconBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( page_url, icon_urls, FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &results); + GetScaleFactors1x2x(), &bitmap_results); EXPECT_EQ(icon_id, backend_->thumbnail_db_->GetFaviconIDForFaviconURL( icon_url, FAVICON, NULL)); @@ -1747,21 +1622,17 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLNotInDB) { backend_->MergeFavicon(page_url, icon_url, FAVICON, bitmap_data, kSmallSize); - // |page_url| should now be mapped to |icon_url| and sizes should be set - // to GetDefaultFaviconSizes(). + // |page_url| should now be mapped to |icon_url| and the favicon bitmap should + // not be expired. std::vector<IconMapping> icon_mappings; EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings)); EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url, icon_mappings[0].icon_url); - FaviconSizes favicon_sizes; - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetDefaultFaviconSizes(), favicon_sizes); - FaviconBitmap favicon_bitmap; EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); @@ -1777,12 +1648,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLNotInDB) { EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url, icon_mappings[0].icon_url); - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetDefaultFaviconSizes(), favicon_sizes); - EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); } @@ -1793,15 +1660,11 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { GURL icon_url1("http:/www.google.com/favicon.ico"); GURL icon_url2("http://www.google.com/favicon2.ico"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url1] = GetSizesSmallAndLarge(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url1, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); // Test initial state. std::vector<IconMapping> icon_mappings; @@ -1810,13 +1673,9 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url1, icon_mappings[0].icon_url); - FaviconSizes favicon_sizes; - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); - FaviconBitmap favicon_bitmap; EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); @@ -1837,12 +1696,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url1, icon_mappings[0].icon_url); - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); - EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); @@ -1861,12 +1716,8 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url1, icon_mappings[0].icon_url); - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); - EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); @@ -1878,24 +1729,21 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { bitmap_data = new base::RefCountedBytes(data); backend_->MergeFavicon(page_url, icon_url1, FAVICON, bitmap_data, kTinySize); - // A new favicon bitmap should be created and favicon sizes should be set to - // the default. + // A new favicon bitmap should be created and the preexisting favicon bitmap + // ('b') should be expired. icon_mappings.clear(); EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings)); EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url1, icon_mappings[0].icon_url); - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetDefaultFaviconSizes(), favicon_sizes); - std::vector<FaviconBitmap> favicon_bitmaps; EXPECT_TRUE(GetSortedFaviconBitmaps(icon_mappings[0].icon_id, &favicon_bitmaps)); + EXPECT_NE(base::Time(), favicon_bitmaps[0].last_updated); EXPECT_TRUE(BitmapDataEqual('c', favicon_bitmaps[0].bitmap_data)); EXPECT_EQ(kTinySize, favicon_bitmaps[0].pixel_size); + EXPECT_EQ(base::Time(), favicon_bitmaps[1].last_updated); EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmaps[1].bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmaps[1].pixel_size); @@ -1913,18 +1761,15 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) { EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url2, icon_mappings[0].icon_url); - favicon_sizes.clear(); - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetDefaultFaviconSizes(), favicon_sizes); - favicon_bitmaps.clear(); EXPECT_TRUE(GetSortedFaviconBitmaps(icon_mappings[0].icon_id, &favicon_bitmaps)); + EXPECT_EQ(base::Time(), favicon_bitmaps[0].last_updated); EXPECT_TRUE(BitmapDataEqual('c', favicon_bitmaps[0].bitmap_data)); EXPECT_EQ(kTinySize, favicon_bitmaps[0].pixel_size); // The favicon being merged should take precedence over the preexisting // favicon bitmaps. + EXPECT_NE(base::Time(), favicon_bitmaps[1].last_updated); EXPECT_TRUE(BitmapDataEqual('d', favicon_bitmaps[1].bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmaps[1].pixel_size); @@ -1940,15 +1785,11 @@ TEST_F(HistoryBackendTest, MergeFaviconIconURLMappedToDifferentPageURL) { GURL page_url2("http://news.google.com"); GURL icon_url("http:/www.google.com/favicon.ico"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmallAndLarge(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url1, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url1, FAVICON, favicon_bitmap_data); // Test initial state. std::vector<IconMapping> icon_mappings; @@ -1957,13 +1798,9 @@ TEST_F(HistoryBackendTest, MergeFaviconIconURLMappedToDifferentPageURL) { EXPECT_EQ(1u, icon_mappings.size()); EXPECT_EQ(icon_url, icon_mappings[0].icon_url); - FaviconSizes favicon_sizes; - EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader( - icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes)); - EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes); - FaviconBitmap favicon_bitmap; EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); @@ -1994,6 +1831,7 @@ TEST_F(HistoryBackendTest, MergeFaviconIconURLMappedToDifferentPageURL) { EXPECT_EQ(icon_url, icon_url_in_db); EXPECT_TRUE(GetOnlyFaviconBitmap(favicon_id, &favicon_bitmap)); + EXPECT_NE(base::Time(), favicon_bitmap.last_updated); EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmap.bitmap_data)); EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size); } @@ -2039,16 +1877,12 @@ TEST_F(HistoryBackendTest, MergeFaviconShowsUpInGetFaviconsForURLResult) { GURL icon_url("http://www.google.com/favicon.ico"); GURL merged_icon_url("http://wwww.google.com/favicon2.ico"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmallAndLarge(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url, GetSizesSmallAndLarge(), &favicon_bitmap_data); // Set some preexisting favicons for |page_url|. - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); // Merge small favicon. std::vector<unsigned char> data; @@ -2060,15 +1894,15 @@ TEST_F(HistoryBackendTest, MergeFaviconShowsUpInGetFaviconsForURLResult) { // Request favicon bitmaps for both 1x and 2x to simulate request done by // BookmarkModel::GetFavicon(). - HistoryBackend::FaviconResults results; + std::vector<FaviconBitmapResult> bitmap_results; backend_->GetFaviconsForURL(page_url, FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &results); + GetScaleFactors1x2x(), &bitmap_results); - EXPECT_EQ(2u, results.bitmap_results.size()); - const FaviconBitmapResult& first_result = results.bitmap_results[0]; + EXPECT_EQ(2u, bitmap_results.size()); + const FaviconBitmapResult& first_result = bitmap_results[0]; const FaviconBitmapResult& result = (first_result.pixel_size == kSmallSize) ? first_result - : results.bitmap_results[1]; + : bitmap_results[1]; EXPECT_TRUE(BitmapDataEqual('c', result.bitmap_data)); } @@ -2082,33 +1916,25 @@ TEST_F(HistoryBackendTest, UpdateFaviconMappingsAndFetchMultipleIconTypes) { GURL icon_urlc("http://www.google.com/favicon3.ico"); // |page_url1| is mapped to |icon_urla| which if of type TOUCH_ICON. - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_urla] = GetSizesSmall(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_urla, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url1, TOUCH_ICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url1, TOUCH_ICON, favicon_bitmap_data); // |page_url2| is mapped to |icon_urlb| and |icon_urlc| which are of type // TOUCH_PRECOMPOSED_ICON. - icon_url_sizes.clear(); - icon_url_sizes[icon_urlb] = GetSizesSmall(); - icon_url_sizes[icon_urlc] = GetSizesSmall(); GenerateFaviconBitmapData(icon_urlb, GetSizesSmall(), icon_urlc, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url2, TOUCH_PRECOMPOSED_ICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url2, TOUCH_PRECOMPOSED_ICON, favicon_bitmap_data); std::vector<GURL> icon_urls; icon_urls.push_back(icon_urla); icon_urls.push_back(icon_urlb); icon_urls.push_back(icon_urlc); - HistoryBackend::FaviconResults results; + std::vector<FaviconBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( page_url3, icon_urls, (TOUCH_ICON | TOUCH_PRECOMPOSED_ICON), - kSmallSize.width(), GetScaleFactors1x2x(), &results); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results); // |page_url1| and |page_url2| should still be mapped to the same icon URLs. std::vector<IconMapping> icon_mappings; @@ -2143,12 +1969,9 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBEmpty) { const GURL page_url("http://www.google.com/"); std::vector<FaviconBitmapResult> bitmap_results; - IconURLSizesMap icon_url_sizes; EXPECT_FALSE(backend_->GetFaviconsFromDB(page_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results, - &icon_url_sizes)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results)); EXPECT_TRUE(bitmap_results.empty()); - EXPECT_TRUE(icon_url_sizes.empty()); } // Test the results of GetFaviconsFromDB() when there are matching favicons @@ -2157,47 +1980,35 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBNoFaviconBitmaps) { const GURL page_url("http://www.google.com/"); const GURL icon_url("http://www.google.com/icon1"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmallAndLarge(); - - // No favicon bitmaps. - std::vector<FaviconBitmapData> favicon_bitmap_data; - - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + FaviconID icon_id = backend_->thumbnail_db_->AddFavicon(icon_url, FAVICON, + GetSizesSmallAndLarge()); + EXPECT_NE(0, icon_id); + EXPECT_NE(0, backend_->thumbnail_db_->AddIconMapping(page_url, icon_id)); std::vector<FaviconBitmapResult> bitmap_results_out; - IconURLSizesMap icon_url_sizes_out; - EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &bitmap_results_out, &icon_url_sizes_out)); + EXPECT_FALSE(backend_->GetFaviconsFromDB(page_url, FAVICON, + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); EXPECT_TRUE(bitmap_results_out.empty()); - EXPECT_EQ(icon_url_sizes, icon_url_sizes_out); } // Test that GetFaviconsFromDB() returns results for the bitmaps which most // closely match the passed in desired size and scale factors. TEST_F(HistoryBackendTest, GetFaviconsFromDBSelectClosestMatch) { const GURL page_url("http://www.google.com/"); - const GURL icon_url("http://www.google.com/icon1"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesTinySmallAndLarge(); std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url, GetSizesTinySmallAndLarge(), &favicon_bitmap_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); std::vector<FaviconBitmapResult> bitmap_results_out; - IconURLSizesMap icon_url_sizes_out; EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &bitmap_results_out, &icon_url_sizes_out)); - - // The bitmap data for the 1x and 2x bitmaps should be returned as their sizes - // match exactly. + GetScaleFactors1x2x(), &bitmap_results_out)); + // The bitmap data for the small and large bitmaps should be returned as their + // sizes match exactly. EXPECT_EQ(2u, bitmap_results_out.size()); // No required order for results. if (bitmap_results_out[0].pixel_size == kLargeSize) { @@ -2217,8 +2028,6 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBSelectClosestMatch) { EXPECT_EQ(kLargeSize, bitmap_results_out[1].pixel_size); EXPECT_EQ(icon_url, bitmap_results_out[1].icon_url); EXPECT_EQ(FAVICON, bitmap_results_out[1].icon_type); - - EXPECT_EQ(icon_url_sizes, icon_url_sizes_out); } // Test that GetFaviconsFromDB() returns results from the icon URL whose @@ -2228,29 +2037,22 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBSingleIconURL) { const GURL icon_url1("http://www.google.com/icon1"); const GURL icon_url2("http://www.google.com/icon2"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url1] = GetSizesSmall(); - icon_url_sizes[icon_url2] = GetSizesSmallAndLarge(); std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url1, GetSizesSmall(), icon_url2, GetSizesLarge(), &favicon_bitmap_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); std::vector<FaviconBitmapResult> bitmap_results_out; - IconURLSizesMap icon_url_sizes_out; EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &bitmap_results_out, &icon_url_sizes_out)); + GetScaleFactors1x2x(), &bitmap_results_out)); // The results should have results for the icon URL with the large bitmap as // downscaling is preferred to upscaling. EXPECT_EQ(1u, bitmap_results_out.size()); EXPECT_EQ(kLargeSize, bitmap_results_out[0].pixel_size); EXPECT_EQ(icon_url2, bitmap_results_out[0].icon_url); - - EXPECT_EQ(icon_url_sizes, icon_url_sizes_out); } // Test the results of GetFaviconsFromDB() when called with different @@ -2260,39 +2062,28 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBIconType) { const GURL icon_url1("http://www.google.com/icon1.png"); const GURL icon_url2("http://www.google.com/icon2.png"); - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url1] = GetSizesSmall(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url1, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data, - icon_url_sizes); + backend_->SetFavicons(page_url, FAVICON, favicon_bitmap_data); - IconURLSizesMap touch_icon_url_sizes; - touch_icon_url_sizes[icon_url2] = GetSizesSmall(); GenerateFaviconBitmapData(icon_url2, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(page_url, TOUCH_ICON, favicon_bitmap_data, - touch_icon_url_sizes); + backend_->SetFavicons(page_url, TOUCH_ICON, favicon_bitmap_data); std::vector<FaviconBitmapResult> bitmap_results_out; - IconURLSizesMap icon_url_sizes_out; EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); EXPECT_EQ(1u, bitmap_results_out.size()); EXPECT_EQ(FAVICON, bitmap_results_out[0].icon_type); - EXPECT_EQ(icon_url_sizes, icon_url_sizes_out); + EXPECT_EQ(icon_url1, bitmap_results_out[0].icon_url); bitmap_results_out.clear(); - icon_url_sizes_out.clear(); EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, TOUCH_ICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); EXPECT_EQ(1u, bitmap_results_out.size()); EXPECT_EQ(TOUCH_ICON, bitmap_results_out[0].icon_type); - EXPECT_EQ(touch_icon_url_sizes, icon_url_sizes_out); + EXPECT_EQ(icon_url2, bitmap_results_out[0].icon_url); } // Test that GetFaviconsFromDB() correctly sets the expired flag for bitmap @@ -2312,10 +2103,8 @@ TEST_F(HistoryBackendTest, GetFaviconsFromDBExpired) { EXPECT_NE(0, backend_->thumbnail_db_->AddIconMapping(page_url, icon_id)); std::vector<FaviconBitmapResult> bitmap_results_out; - IconURLSizesMap icon_url_sizes_out; EXPECT_TRUE(backend_->GetFaviconsFromDB(page_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); EXPECT_EQ(1u, bitmap_results_out.size()); EXPECT_TRUE(bitmap_results_out[0].expired); @@ -2327,16 +2116,13 @@ TEST_F(HistoryBackendTest, UpdateFaviconMappingsAndFetchNoDB) { // Make the thumbnail database invalid. backend_->thumbnail_db_.reset(); - HistoryBackend::FaviconResults results; - results.bitmap_results.push_back(FaviconBitmapResult()); - results.size_map[GURL()] = FaviconSizes(); + std::vector<FaviconBitmapResult> bitmap_results; backend_->UpdateFaviconMappingsAndFetch( GURL(), std::vector<GURL>(), FAVICON, kSmallSize.width(), - GetScaleFactors1x2x(), &results); + GetScaleFactors1x2x(), &bitmap_results); - EXPECT_TRUE(results.bitmap_results.empty()); - EXPECT_TRUE(results.size_map.empty()); + EXPECT_TRUE(bitmap_results.empty()); } TEST_F(HistoryBackendTest, CloneFaviconIsRestrictedToSameDomain) { @@ -2346,39 +2132,30 @@ TEST_F(HistoryBackendTest, CloneFaviconIsRestrictedToSameDomain) { const GURL icon_url("http://www.google.com/icon.png"); // Add a favicon - IconURLSizesMap icon_url_sizes; - icon_url_sizes[icon_url] = GetSizesSmall(); - std::vector<FaviconBitmapData> favicon_bitmap_data; GenerateFaviconBitmapData(icon_url, GetSizesSmall(), &favicon_bitmap_data); - backend_->SetFavicons(url, FAVICON, favicon_bitmap_data, icon_url_sizes); + backend_->SetFavicons(url, FAVICON, favicon_bitmap_data); EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL( url, FAVICON, NULL)); // Validate starting state. std::vector<FaviconBitmapResult> bitmap_results_out; - IconURLSizesMap icon_url_sizes_out; EXPECT_TRUE(backend_->GetFaviconsFromDB(url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); EXPECT_FALSE(backend_->GetFaviconsFromDB(same_domain_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); EXPECT_FALSE(backend_->GetFaviconsFromDB(foreign_domain_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); // Same-domain cloning should work. backend_->CloneFavicons(url, same_domain_url); EXPECT_TRUE(backend_->GetFaviconsFromDB(same_domain_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); // Foreign-domain cloning is forbidden. backend_->CloneFavicons(url, foreign_domain_url); EXPECT_FALSE(backend_->GetFaviconsFromDB(foreign_domain_url, FAVICON, - kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out, - &icon_url_sizes_out)); + kSmallSize.width(), GetScaleFactors1x2x(), &bitmap_results_out)); } TEST_F(HistoryBackendTest, QueryFilteredURLs) { diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h index 8e382be..fdb435b 100644 --- a/chrome/browser/history/history_types.h +++ b/chrome/browser/history/history_types.h @@ -824,9 +824,6 @@ typedef std::vector<gfx::Size> FaviconSizes; // are unknown. const FaviconSizes& GetDefaultFaviconSizes(); -// A map from an icon URL to the FaviconSizes for that URL. -typedef std::map<GURL, FaviconSizes> IconURLSizesMap; - // Defines a favicon bitmap and its associated pixel size. struct FaviconBitmapIDSize { FaviconBitmapIDSize(); diff --git a/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc b/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc index 84ecc64..57b8d52 100644 --- a/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc +++ b/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc @@ -420,12 +420,11 @@ struct PossibleTestSingletonTraits : public DefaultSingletonTraits<Type> { void RunFaviconCallbackAsync( const FaviconService::FaviconResultsCallback& callback, - const std::vector<history::FaviconBitmapResult>* results, - const history::IconURLSizesMap* size_map) { + const std::vector<history::FaviconBitmapResult>* results) { base::MessageLoopProxy::current()->PostTask( FROM_HERE, base::Bind(&FaviconService::FaviconResultsCallbackRunner, - callback, base::Owned(results), base::Owned(size_map))); + callback, base::Owned(results))); } } // namespace @@ -505,15 +504,13 @@ void ChromeWebUIControllerFactory::GetFaviconForURL( ExtensionWebUI::GetFaviconForURL(profile, url, callback); #else RunFaviconCallbackAsync(callback, - new std::vector<history::FaviconBitmapResult>(), - new history::IconURLSizesMap()); + new std::vector<history::FaviconBitmapResult>()); #endif return; } std::vector<history::FaviconBitmapResult>* favicon_bitmap_results = new std::vector<history::FaviconBitmapResult>(); - history::IconURLSizesMap* icon_url_sizes = new history::IconURLSizesMap(); for (size_t i = 0; i < scale_factors.size(); ++i) { scoped_refptr<base::RefCountedMemory> bitmap(GetFaviconResourceBytes( @@ -534,18 +531,7 @@ void ChromeWebUIControllerFactory::GetFaviconForURL( } } - // Populate IconURLSizesMap such that the requirement that all the icon URLs - // in |favicon_bitmap_results| be present in |icon_url_sizes| holds. - // Populate the favicon sizes with the pixel sizes of the bitmaps available - // for |url|. - for (size_t i = 0; i < favicon_bitmap_results->size(); ++i) { - const history::FaviconBitmapResult& bitmap_result = - (*favicon_bitmap_results)[i]; - const GURL& icon_url = bitmap_result.icon_url; - (*icon_url_sizes)[icon_url].push_back(bitmap_result.pixel_size); - } - - RunFaviconCallbackAsync(callback, favicon_bitmap_results, icon_url_sizes); + RunFaviconCallbackAsync(callback, favicon_bitmap_results); } // static |