Find Favicon in priority of icon_type.

chrome/browser/history/history_backend.cc

Index: chrome/browser/history/history_backend.cc
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index feb1664dabcf0792729a04d66c0bd65cccccdcb6..5ed1026c105b3e1badd3d9f29d47c871e05c9d0d 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -1696,6 +1696,96 @@ void HistoryBackend::GetFavicons(
                                     bitmap_results);
 }
 
+void HistoryBackend::GetLargestFaviconForURL(
+      const GURL& page_url,
+      const std::vector<int>& icon_types,
+      int minimal_size_in_pixel,
+      std::vector<chrome::FaviconBitmapResult>* favicon_bitmap_results) {
+  DCHECK(favicon_bitmap_results);
+  favicon_bitmap_results->clear();
+
+  if (!db_ || !thumbnail_db_)
+    return;
+
+  // Time the query.

[sky, 2013/10/15 15:03:47]

nit: remove comment, just describes what the code is doing.

[michaelbai, 2013/10/15 19:36:31]

Done.

+  TimeTicks beginning_time = TimeTicks::Now();
+
+  int required_icon_types = 0;
+  for (std::vector<int>::const_iterator i = icon_types.begin();
+       i != icon_types.end(); ++i) {
+    required_icon_types |= *i;
+  }
+
+  std::vector<IconMapping> icon_mappings;
+  if (!thumbnail_db_->GetAllIconMappingsForPageURL(page_url,
+       required_icon_types, &icon_mappings)) {
+    return;
+  }
+  if (icon_mappings.empty())

[sky, 2013/10/15 15:03:47]

nit: merge with if on 1721.

[michaelbai, 2013/10/15 19:36:31]

Done.

+    return;
+
+  std::map<chrome::IconType, FaviconBitmap> largest_favicon_bitmaps;

[sky, 2013/10/15 15:03:47]

nit: add comments describing what you're doing here and 1745.

[michaelbai, 2013/10/15 19:36:31]

Done.

+  for (std::vector<IconMapping>::const_iterator i = icon_mappings.begin();
+       i != icon_mappings.end(); ++i) {
+    std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
+    thumbnail_db_->GetFaviconBitmapIDSizes(i->icon_id, &bitmap_id_sizes);
+    FaviconBitmap largest = largest_favicon_bitmaps[i->icon_type];

[sky, 2013/10/15 15:03:47]

This should be a ref. That way you don't need 1742.

[michaelbai, 2013/10/15 19:36:31]

Done.

+    for (std::vector<FaviconBitmapIDSize>::const_iterator j =

[pkotwicz, 2013/10/15 16:44:33]

largest will not be populated if all of the pixel sizes are empty? For instance,
FaviconSQLHandler::Insert() inserts favicons with an empty pixel size.

[michaelbai, 2013/10/15 19:36:31]

added 
if (largest_icon.pixel_size.width() == 0 ...

+             bitmap_id_sizes.begin(); j != bitmap_id_sizes.end(); ++j) {
+      if (largest.pixel_size.width() < j->pixel_size.width() &&
+          largest.pixel_size.height() < j->pixel_size.height()) {
+        largest.icon_id = i->icon_id;
+        largest.bitmap_id = j->bitmap_id;
+        largest.pixel_size = j->pixel_size;
+      }
+    }
+    largest_favicon_bitmaps[i->icon_type] = largest;
+  }
+
+  FaviconBitmap largest_icon;
+  for (std::vector<int>::const_iterator t = icon_types.begin();
+       t != icon_types.end(); ++t) {
+    for (std::map<chrome::IconType, FaviconBitmap>::const_iterator f =
+            largest_favicon_bitmaps.begin(); f != largest_favicon_bitmaps.end();
+        ++f) {
+      if (f->first & *t &&
+          largest_icon.pixel_size.width() < f->second.pixel_size.width() &&
+          largest_icon.pixel_size.height() < f->second.pixel_size.height())
+        largest_icon = f->second;
+    }
+    if (largest_icon.pixel_size.width() > minimal_size_in_pixel &&
+        largest_icon.pixel_size.height() > minimal_size_in_pixel)
+      break;
+  }
+
+  // Construct FaviconBitmapResults.

[sky, 2013/10/15 15:03:47]

nit: this comment isn't helpful, especially when bitmapresult isn't build until
a ways down.

[michaelbai, 2013/10/15 19:36:31]

Done.

+  GURL icon_url;
+  chrome::IconType icon_type;
+  if (!thumbnail_db_->GetFaviconHeader(largest_icon.icon_id, &icon_url,

[sky, 2013/10/15 15:03:47]

Might it be possible that you've never set largest_icon by the time you get
here?

[michaelbai, 2013/10/15 19:36:31]

added

 if (largest_icon.pixel_size.width() == 0 ...

+                                       &icon_type)) {
+    return;
+  }
+
+  base::Time last_updated;
+  chrome::FaviconBitmapResult bitmap_result;
+  bitmap_result.icon_url = icon_url;
+  bitmap_result.icon_type = icon_type;
+  if (!thumbnail_db_->GetFaviconBitmap(largest_icon.bitmap_id,
+                                       &last_updated,
+                                       &bitmap_result.bitmap_data,
+                                       &bitmap_result.pixel_size)) {
+    return;
+  }
+
+  bitmap_result.expired = (Time::Now() - last_updated) >
+      TimeDelta::FromDays(kFaviconRefetchDays);
+  if (bitmap_result.is_valid())
+    favicon_bitmap_results->push_back(bitmap_result);
+
+  HISTOGRAM_TIMES("History.GetLargestFaviconForURL",
+                  TimeTicks::Now() - beginning_time);
+}
+
 void HistoryBackend::GetFaviconsForURL(
     const GURL& page_url,
     int icon_types,