Find Favicon in priority of icon_type.

chrome/browser/history/history_backend.cc

 // Copyright (c) 2012 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 "chrome/browser/history/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
@@ skipping 1681 matching lines @@
     int desired_size_in_dip,
     const std::vector<ui::ScaleFactor>& desired_scale_factors,
     std::vector<chrome::FaviconBitmapResult>* bitmap_results) {
   UpdateFaviconMappingsAndFetchImpl(NULL, icon_urls, icon_types,
                                     desired_size_in_dip, desired_scale_factors,
                                     bitmap_results);
 }
 
 void HistoryBackend::GetFaviconsForURL(
     const GURL& page_url,
-    int icon_types,
+    const std::vector<int>& icon_types_priority,
     int desired_size_in_dip,
     const std::vector<ui::ScaleFactor>& desired_scale_factors,
+    int threshold_for_next_icon_types,
     std::vector<chrome::FaviconBitmapResult>* bitmap_results) {
-  DCHECK(bitmap_results);
-  GetFaviconsFromDB(page_url, icon_types, desired_size_in_dip,
-                    desired_scale_factors, bitmap_results);
+  chrome::FaviconBitmapResult bitmap_result;
+  int bitmap_size_in_pixel = 0;
+  for (std::vector<int>::const_iterator i = icon_types_priority.begin();
+      i != icon_types_priority.end(); i++) {
+    GetFaviconsFromDB(page_url, *i, desired_size_in_dip, desired_scale_factors,

[sky, 2013/10/08 23:47:52]

Can we do one request with a bitmask of the types in icon_types_priority and
then do the ordering?

[michaelbai, 2013/10/09 18:35:09]

I see your point, as the icon_types could be a composition of icon_type, it
might be a little complicated to sort the result and find the best one here.

On 2013/10/08 23:47:52, sky wrote:

[sky, 2013/10/09 20:33:24]

Why? It doesn't seems like it's going to be much more complicated than what you
have here. Also, you have to assume going to the DB is a LOT slower than some
sorting here.

[michaelbai, 2013/10/10 05:51:43]

I tried, it seemed it is complicated, GetFaviconsFromDB will only return one
best fit icon, not all the icon which met the criteria.  To make the
GetFaviconsFromDB returns all icons requires change a lot of code including
API's definition,  like GetIconMappingsForPageURL needs to return all icon
mappings instead of one and how to calculate best match icon in
GetFaviconBitmapResultsForBestMatch.


On 2013/10/09 20:33:24, sky wrote:

[michaelbai, 2013/10/10 18:23:03]

To be more specific, AFAIK

- GetIconMappingsForPageURL needs to change to return all matched icons of given
icon_types.

- GetFaviconBitmapResultsForBestMatch needs return best matched icon for each
requested icon_type

I don't know how others depend on the output of above 2 functions.

- GetFaviconsForURL needs to have exact same output as before when
icon_types_priority.size() == 0.

The change seems big, let me know what's your opinion.

On 2013/10/10 05:51:43, michaelbai wrote:

+                      bitmap_results);
+    if (icon_types_priority.size() == 1)
+      return;
+
+    for (std::vector<chrome::FaviconBitmapResult>::const_iterator j =
+             bitmap_results->begin(); j != bitmap_results->end(); j++) {
+      if (bitmap_size_in_pixel < j->pixel_size.width() &&
+          bitmap_size_in_pixel < j->pixel_size.height()) {
+        bitmap_size_in_pixel = std::min(j->pixel_size.width(),
+                                        j->pixel_size.height());
+        bitmap_result = *j;
+      }
+    }
+    bitmap_results->clear();
+    if (bitmap_size_in_pixel > threshold_for_next_icon_types)
+      break;
+  }
+  if (bitmap_size_in_pixel != 0)
+    bitmap_results->push_back(bitmap_result);
 }
 
 void HistoryBackend::GetFaviconForID(
     chrome::FaviconID favicon_id,
     int desired_size_in_dip,
     ui::ScaleFactor desired_scale_factor,
     std::vector<chrome::FaviconBitmapResult>* bitmap_results) {
   std::vector<chrome::FaviconID> favicon_ids;
   favicon_ids.push_back(favicon_id);
   std::vector<ui::ScaleFactor> desired_scale_factors;
@@ skipping 1164 matching lines @@
   int rank = kPageVisitStatsMaxTopSites;
   std::map<GURL, int>::const_iterator it = most_visited_urls_map_.find(url);
   if (it != most_visited_urls_map_.end())
     rank = (*it).second;
   UMA_HISTOGRAM_ENUMERATION("History.TopSitesVisitsByRank",
                             rank, kPageVisitStatsMaxTopSites + 1);
 }
 #endif
 
 }  // namespace history