Adds support for saving favicon size into history database.

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 <list>
 #include <map>
 #include <set>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/api/bookmarks/bookmark_service.h"
 #include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/cancelable_request.h"
+#include "chrome/browser/favicon/select_favicon_frames.h"
 #include "chrome/browser/history/history_notifications.h"
 #include "chrome/browser/history/history_publisher.h"
 #include "chrome/browser/history/in_memory_history_backend.h"
 #include "chrome/browser/history/page_usage_data.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/history/visit_filter.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/download_persistent_store_info.h"
@@ skipping 52 matching lines @@
 static const int kSessionCloseTimeWindowSecs = 10;
 
 // The maximum number of items we'll allow in the redirect list before
 // deleting some.
 static const int kMaxRedirectCount = 32;
 
 // The number of days old a history entry can be before it is considered "old"
 // and is archived.
 static const int kArchiveDaysThreshold = 90;
 
+// The maximum number of icons URLs per page which can be stored in the
+// thumbnail database.
+static const size_t kMaxFaviconsPerPage = 8;
+
+// The maximum number of bitmaps for a single icon URL which can be stored in
+// the thumbnail database.
+static const size_t kMaxFaviconBitmapsPerIconURL = 8;
+
 // Converts from PageUsageData to MostVisitedURL. |redirects| is a
 // list of redirects for this URL. Empty list means no redirects.
 MostVisitedURL MakeMostVisitedURL(const PageUsageData& page_data,
                                   const RedirectList& redirects) {
   MostVisitedURL mv;
   mv.url = page_data.GetURL();
   mv.title = page_data.GetTitle();
   if (redirects.empty()) {
     // Redirects must contain at least the target url.
     mv.redirects.push_back(mv.url);
@@ skipping 1655 matching lines @@
         URLID url_id;
         if ((url_id = db_->GetRowForURL(redirects.back(), NULL)))
           success = thumbnail_db_->GetPageThumbnail(url_id, data);
       }
     }
   }
 
   return success;
 }
 
-void HistoryBackend::GetFavicon(scoped_refptr<GetFaviconRequest> request,
-                                const GURL& icon_url,
-                                int icon_types) {
-  UpdateFaviconMappingAndFetchImpl(NULL, icon_url, request, icon_types);
+void HistoryBackend::GetFavicons(
+    scoped_refptr<GetFaviconRequest> request,
+    const std::vector<GURL>& icon_urls,
+    int icon_types,
+    int desired_size_in_dip,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  UpdateFaviconMappingsAndFetchImpl(request, NULL, icon_urls, icon_types,
+      desired_size_in_dip, desired_scale_factors);
 }
 
-void HistoryBackend::UpdateFaviconMappingAndFetch(
+void HistoryBackend::GetFaviconsForURL(
     scoped_refptr<GetFaviconRequest> request,
     const GURL& page_url,
-    const GURL& icon_url,
-    IconType icon_type) {
-  UpdateFaviconMappingAndFetchImpl(&page_url, icon_url, request, icon_type);
+    int icon_types,
+    int desired_size_in_dip,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  if (request->canceled())
+    return;
+
+  std::vector<FaviconBitmapResult> favicon_bitmap_results;
+  IconURLSizesMap icon_url_sizes;
+
+  // Get results from DB.
+  GetFaviconsFromDB(page_url, icon_types, desired_size_in_dip,
+      desired_scale_factors, &favicon_bitmap_results, &icon_url_sizes);
+
+  request->ForwardResult(request->handle(), favicon_bitmap_results,
+                         icon_url_sizes);
 }
 
-void HistoryBackend::SetFaviconOutOfDateForPage(const GURL& page_url) {
+void HistoryBackend::GetFaviconForID(scoped_refptr<GetFaviconRequest> request,
+                                     FaviconID id,
+                                     int desired_size_in_dip,
+                                     ui::ScaleFactor desired_scale_factor) {
+  if (request->canceled())
+    return;
+
+  std::vector<FaviconID> candidate_favicon_ids;
+  candidate_favicon_ids.push_back(id);
+  std::vector<ui::ScaleFactor> desired_scale_factors;
+  desired_scale_factors.push_back(desired_scale_factor);
+
+  // Get results from DB.
+  std::vector<FaviconBitmapResult> favicon_bitmap_results;
+  GetFaviconBitmapResultsForBestMatch(candidate_favicon_ids,
+      desired_size_in_dip, desired_scale_factors, &favicon_bitmap_results);
+
+  IconURLSizesMap icon_url_sizes;
+  BuildIconURLSizesMap(candidate_favicon_ids, &icon_url_sizes);
+
+  request->ForwardResult(request->handle(), favicon_bitmap_results,
+                         icon_url_sizes);
+}
+
+void HistoryBackend::UpdateFaviconMappingsAndFetch(
+    scoped_refptr<GetFaviconRequest> request,
+    const GURL& page_url,
+    const std::vector<GURL>& icon_urls,
+    int icon_types,
+    int desired_size_in_dip,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  UpdateFaviconMappingsAndFetchImpl(request, &page_url, icon_urls, icon_types,
+      desired_size_in_dip, desired_scale_factors);
+}
+
+void HistoryBackend::MergeFavicon(
+    const GURL& page_url,
+    const GURL& icon_url,
+    IconType icon_type,
+    scoped_refptr<base::RefCountedMemory> bitmap_data,
+    const gfx::Size& pixel_size) {
+  if (!thumbnail_db_.get() || !db_.get())
+    return;
+
+  gfx::Size merge_pixel_size;
+  if (!IsMergeValid(page_url, icon_url, icon_type, pixel_size,
+                    &merge_pixel_size)) {
+    return;
+  }
+
+  FaviconID favicon_id =
+      thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type, NULL);
+  if (favicon_id) {
+    FaviconBitmapData bitmap_data_element;
+    bitmap_data_element.bitmap_data = bitmap_data;
+    bitmap_data_element.pixel_size = merge_pixel_size;
+    bitmap_data_element.icon_url = icon_url;
+    std::vector<FaviconBitmapData> favicon_bitmap_data;
+    favicon_bitmap_data.push_back(bitmap_data_element);
+    SetFaviconBitmaps(favicon_id, favicon_bitmap_data);
+  } else {
+    FaviconSizes favicon_sizes;
+    favicon_sizes.push_back(merge_pixel_size);

[sky, 2012/09/04 20:52:29]

How does an empty size end up getting mapped correctly?

[pkotwicz, 2012/09/04 22:40:54]

I am unsure as to what you are asking

[pkotwicz, 2012/09/05 22:39:26]

I have updated MergeFavicon().
GetDefaultFaviconSizes() now returns an empty FaviconSizes vector.
However, GetDefaultFaviconSizes() is not valid input to SetFavicons().
Practically, any uses where we would want to call SetFavicons() with
GetDefaultFaviconSizes() should use MergeFavicon() instead.
However, this slightly inconvenient for unittests.

+    favicon_id = thumbnail_db_->AddFavicon(icon_url,
+                                           icon_type,
+                                           favicon_sizes,
+                                           bitmap_data,
+                                           base::Time::Now(),
+                                           merge_pixel_size);
+    IconURLFaviconIDMap icon_url_id_map;
+    icon_url_id_map[icon_url] = favicon_id;
+    SetFaviconMappingsForPageAndRedirects(page_url, icon_type,
+                                          icon_url_id_map);
+  }
+}
+
+void HistoryBackend::SetFavicons(
+    const GURL& page_url,
+    IconType icon_type,
+    const std::vector<FaviconBitmapData>& favicon_bitmap_data,
+    const IconURLSizesMap& icon_url_sizes) {
+  if (!thumbnail_db_.get() || !db_.get())
+    return;
+
+  DCHECK(ValidateSetFaviconsParams(favicon_bitmap_data, icon_url_sizes));
+
+  // Build map of FaviconBitmapResults for each icon url.
+  typedef std::map<GURL, std::vector<FaviconBitmapData> >
+      BitmapDataByIconURL;
+  BitmapDataByIconURL grouped_by_icon_url;
+  for (size_t i = 0; i < favicon_bitmap_data.size(); ++i) {
+    const GURL& icon_url = favicon_bitmap_data[i].icon_url;
+    grouped_by_icon_url[icon_url].push_back(favicon_bitmap_data[i]);
+  }
+
+  IconURLFaviconIDMap icon_url_id_map;
+  for (IconURLSizesMap::const_iterator it = icon_url_sizes.begin();
+       it != icon_url_sizes.end(); ++it) {
+    const GURL& icon_url = it->first;
+    FaviconID icon_id =
+        thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type, NULL);
+    if (icon_id)
+      SetFaviconSizes(icon_id, it->second);
+    else
+      icon_id = thumbnail_db_->AddFavicon(icon_url, icon_type, it->second);
+    icon_url_id_map[icon_url] = 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())
+      SetFaviconBitmaps(icon_id, grouped_by_icon_url_it->second);
+  }
+
+  SetFaviconMappingsForPageAndRedirects(page_url, icon_type, icon_url_id_map);
+}
+
+void HistoryBackend::SetFaviconsOutOfDateForPage(const GURL& page_url) {
   std::vector<IconMapping> icon_mappings;
 
   if (!thumbnail_db_.get() ||
       !thumbnail_db_->GetIconMappingsForPageURL(page_url,
                                                 &icon_mappings))
     return;
 
   for (std::vector<IconMapping>::iterator m = icon_mappings.begin();
        m != icon_mappings.end(); ++m) {
     thumbnail_db_->SetFaviconOutOfDate(m->icon_id);
   }
   ScheduleCommit();
 }
 
-void HistoryBackend::CloneFavicon(const GURL& old_page_url,
-                                  const GURL& new_page_url) {
+void HistoryBackend::CloneFavicons(const GURL& old_page_url,
+                                   const GURL& new_page_url) {
   if (!thumbnail_db_.get())
     return;
 
   // Prevent cross-domain cloning.
   if (old_page_url.GetOrigin() != new_page_url.GetOrigin())
     return;
 
   thumbnail_db_->CloneIconMappings(old_page_url, new_page_url);
   ScheduleCommit();
 }
@@ skipping 57 matching lines @@
 
   if (!favicons_changed.empty()) {
     // Send the notification about the changed favicon URLs.
     FaviconChangeDetails* changed_details = new FaviconChangeDetails;
     changed_details->urls.swap(favicons_changed);
     BroadcastNotifications(chrome::NOTIFICATION_FAVICON_CHANGED,
                            changed_details);
   }
 }
 
-void HistoryBackend::UpdateFaviconMappingAndFetchImpl(
+void HistoryBackend::UpdateFaviconMappingsAndFetchImpl(
+    scoped_refptr<GetFaviconRequest> request,
     const GURL* page_url,
-    const GURL& icon_url,
-    scoped_refptr<GetFaviconRequest> request,
-    int icon_types) {
-  // Check only a single type was given when the page_url was specified.
-  DCHECK(!page_url || (page_url && (icon_types == FAVICON ||
-      icon_types == TOUCH_ICON || icon_types == TOUCH_PRECOMPOSED_ICON)));
-
-  if (request->canceled())
+    const std::vector<GURL>& icon_urls,
+    int icon_types,
+    int desired_size_in_dip,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  // |icon_types| must be either a single icon type or icon types which are
+  // equivalent.
+  DCHECK(icon_types == FAVICON ||
+         icon_types == TOUCH_ICON ||
+         icon_types == TOUCH_PRECOMPOSED_ICON ||
+         icon_types == (TOUCH_ICON | TOUCH_PRECOMPOSED_ICON));
+
+  if (request->canceled() || !thumbnail_db_.get())
     return;
 
-  std::vector<history::FaviconBitmapResult> favicon_bitmap_results;
-
-  if (thumbnail_db_.get()) {
-    IconType icon_type;
-    const FaviconID favicon_id =
-        thumbnail_db_->GetFaviconIDForFaviconURL(
-            icon_url, icon_types, &icon_type);
+  std::vector<FaviconID> favicon_ids;
+  IconURLFaviconIDMap icon_url_id_map;
+
+  // The icon type for which the mappings will the updated and data will be
+  // returned.
+  IconType selected_icon_type = INVALID_ICON;
+
+  for (size_t i = 0; i < icon_urls.size(); ++i) {
+    const GURL& icon_url = icon_urls[i];
+    IconType icon_type_out;
+    const FaviconID favicon_id = thumbnail_db_->GetFaviconIDForFaviconURL(
+        icon_url, icon_types, &icon_type_out);
+
     if (favicon_id) {
-      GetFaviconFromDB(favicon_id, &favicon_bitmap_results);
-
-      if (page_url)
-        SetFaviconMapping(*page_url, favicon_id, icon_type);
+      if (icon_type_out > selected_icon_type) {

[sky, 2012/09/05 00:15:51]

This is subtle enough that you should add a comment.

+        selected_icon_type = icon_type_out;
+        favicon_ids.clear();
+        icon_url_id_map.clear();
+      }
+      if (icon_type_out == selected_icon_type) {
+        favicon_ids.push_back(favicon_id);

[sky, 2012/09/04 20:52:29]

It feels cumbersome to have to index into icon_url_id_map. Is it worth using a
pair <id, url>?

[pkotwicz, 2012/09/05 22:39:26]

Replaced this with a std::vector<FaviconID>

+        icon_url_id_map[icon_url] = favicon_id;
+      }
     }
     // else case, haven't cached entry yet. Caller is responsible for

[sky, 2012/09/04 20:52:29]

This comment doesn't really match anymore. Nuke it.

-    // downloading the favicon and invoking SetFavicon.
-  }
-  // TODO(pkotwicz): Pass map of |icon_url| to the sizes available from the web
-  // at |icon_url| instead of an empty map.
+    // downloading the favicon and invoking SetFavicons.
+  }
+
+  if (page_url) {
+    SetFaviconMappingsForPageAndRedirects(*page_url, selected_icon_type,
+                                          icon_url_id_map);
+  }
+
+  std::vector<FaviconBitmapResult> favicon_bitmap_results;
+  IconURLSizesMap icon_url_sizes;
+  GetFaviconBitmapResultsForBestMatch(favicon_ids, desired_size_in_dip,
+      desired_scale_factors, &favicon_bitmap_results);
+  BuildIconURLSizesMap(favicon_ids, &icon_url_sizes);
+
   request->ForwardResult(request->handle(), favicon_bitmap_results,
-                         IconURLSizesMap());
-}
-
-void HistoryBackend::GetFaviconForURL(
-    scoped_refptr<GetFaviconRequest> request,
+                         icon_url_sizes);
+}
+
+bool HistoryBackend::IsMergeValid(const GURL& page_url,
+                                  const GURL& icon_url,
+                                  IconType icon_type,
+                                  const gfx::Size& pixel_size,
+                                  gfx::Size* merge_pixel_size) {
+  // Get FaviconIDs for |page_url| and |icon_type|.
+  std::vector<IconMapping> icon_mappings;
+  thumbnail_db_->GetIconMappingsForPageURL(page_url, icon_type,
+                                           &icon_mappings);
+
+  if (!icon_mappings.empty()) {
+    // There are already mappings for |page_url|. The merge is invalid if
+    // |icon_url| is not part of the mappings.
+    bool found_icon_url = false;
+    for (size_t i = 0; i < icon_mappings.size(); ++i) {
+      if (icon_mappings[i].icon_url == icon_url) {
+        found_icon_url = true;
+        break;
+      }
+    }
+    if (!found_icon_url)
+      return false;
+  }
+
+  FaviconID favicon_id =
+      thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type, NULL);
+
+  if (favicon_id) {
+    FaviconSizes favicon_sizes;
+    thumbnail_db_->GetFaviconHeader(favicon_id, NULL, NULL, &favicon_sizes);
+
+    // The merge is valid if the favicon sizes for |favicon_id| are unkown.

[sky, 2012/09/04 20:52:29]

unknown

+    if (favicon_sizes == GetDefaultFaviconSizes()) {
+      *merge_pixel_size = gfx::Size();
+      return true;
+    }
+
+    // The merge is invalid if |pixel_size| is not a subset of the favicon
+    // sizes of |favicon_id|.
+    FaviconSizes::iterator favicon_sizes_it = std::find(favicon_sizes.begin(),
+        favicon_sizes.end(), pixel_size);
+    if (favicon_sizes_it == favicon_sizes.end())
+      return false;
+
+    *merge_pixel_size = *favicon_sizes_it;
+    return true;
+  }
+
+  // The merge is valid if the database knows neither |page_url| or |icon_url|.
+  // Use GetDefaultFaviconSizes() as the complete set of favicon sizes the
+  // favicon is available from the web are unknown.
+  *merge_pixel_size = gfx::Size();
+  return true;
+}
+
+void HistoryBackend::SetFaviconBitmaps(
+    FaviconID icon_id,
+    const std::vector<FaviconBitmapData>& favicon_bitmap_data) {
+  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;
+        break;
+      }
+    }
+    if (bitmap_id) {
+      thumbnail_db_->SetFaviconBitmap(bitmap_id,
+          bitmap_data_element.bitmap_data, base::Time::Now());
+    } else {
+      thumbnail_db_->AddFaviconBitmap(icon_id, bitmap_data_element.bitmap_data,
+          base::Time::Now(), bitmap_data_element.pixel_size);
+    }
+  }
+}
+
+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 (IconURLSizesMap::const_iterator it = icon_url_sizes.begin();
+       it != icon_url_sizes.end(); ++it) {
+    if (it->second.size() > kMaxFaviconBitmapsPerIconURL)
+      return false;
+  }
+
+  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;
+  }
+  return true;
+}
+
+void HistoryBackend::SetFaviconSizes(
+    FaviconID icon_id,
+    const FaviconSizes& favicon_sizes) {
+  std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
+  thumbnail_db_->GetFaviconBitmapIDSizes(icon_id, &bitmap_id_sizes);
+
+  // 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);
+  }
+
+  thumbnail_db_->SetFaviconSizes(icon_id, favicon_sizes);
+}
+
+bool HistoryBackend::GetFaviconsFromDB(
     const GURL& page_url,
-    int icon_types) {
-  if (request->canceled())
-    return;
-
-  std::vector<FaviconBitmapResult> favicon_bitmap_results;
-
-  // Get the favicon from DB.
-  GetFaviconFromDB(page_url, icon_types, &favicon_bitmap_results);
-
-  // TODO(pkotwicz): Pass map of matched icon URLs for |icon_types| to the
-  // sizes that each icon URL is available at from the web instead of an
-  // empty map.
-  request->ForwardResult(request->handle(), favicon_bitmap_results,
-                         IconURLSizesMap());
-}
-
-void HistoryBackend::GetFaviconForID(scoped_refptr<GetFaviconRequest> request,
-                                     FaviconID id) {
-  if (request->canceled())
-    return;
-
- std::vector<FaviconBitmapResult> favicon_bitmap_results;
-
-  // Get the favicon from DB.
-  GetFaviconFromDB(id, &favicon_bitmap_results);
-
-  // TODO(pkotwicz): Pass map of the matched icon URL to the sizes that it is
-  // available from the web instead of an empty map.
-  request->ForwardResult(request->handle(), favicon_bitmap_results,
-                         IconURLSizesMap());
-}
-
-void HistoryBackend::SetFavicon(
+    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) {
+  DCHECK(favicon_bitmap_results);
+  DCHECK(icon_url_sizes);
+
+  if (!db_.get() || !thumbnail_db_.get())
+    return false;
+
+  // Time the query.
+  TimeTicks beginning_time = TimeTicks::Now();
+
+  // Get FaviconIDs for |page_url| and one of |icon_types|.
+  std::vector<IconMapping> icon_mappings;
+  thumbnail_db_->GetIconMappingsForPageURL(page_url, icon_types,
+                                           &icon_mappings);
+  std::vector<FaviconID> favicon_ids;
+  for (size_t i = 0; i < icon_mappings.size(); ++i)
+    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);
+  UMA_HISTOGRAM_TIMES("History.GetFavIconFromDB",  // historical name
+                      TimeTicks::Now() - beginning_time);
+  return success && !icon_url_sizes->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) {
+  if (candidate_favicon_ids.empty())
+    return false;
+
+  // Find the FaviconID and the FaviconBitmapIDs which best match
+  // |desired_size_in_dip| and |desired_scale_factors|.
+  // TODO(pkotwicz): Select bitmap results from multiple favicons once
+  // content::FaviconStatus supports multiple icon URLs.
+  FaviconID best_favicon_id = 0;
+  std::vector<FaviconBitmapID> best_bitmap_ids;
+  // Set |highest_score| less than min valid score of 0.
+  float highest_score = -1.0f;

[sky, 2012/09/04 20:52:29]

nit: add a kInvalidScore some where and use it here.

+  for (size_t i = 0; i < candidate_favicon_ids.size(); ++i) {
+    std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
+    thumbnail_db_->GetFaviconBitmapIDSizes(candidate_favicon_ids[i],
+                                           &bitmap_id_sizes);
+
+    std::vector<FaviconBitmapID> candidate_bitmap_ids;
+    float score;
+    SelectFaviconBitmapIDs(bitmap_id_sizes,
+                           desired_scale_factors,
+                           desired_size_in_dip,
+                           &candidate_bitmap_ids,
+                           &score);
+    if (score > highest_score) {
+      best_favicon_id = candidate_favicon_ids[i],

[sky, 2012/09/04 20:52:29]

does this compile?

[pkotwicz, 2012/09/05 22:39:26]

Done.

+      best_bitmap_ids.swap(candidate_bitmap_ids);
+      highest_score = score;
+    }
+  }
+
+  // Construct FaviconBitmapResults from |best_favicon_id| and
+  // |best_bitmap_ids|.
+  GURL icon_url;
+  IconType icon_type;
+  if (!thumbnail_db_->GetFaviconHeader(best_favicon_id, &icon_url,
+                                       &icon_type, NULL)) {
+    return false;
+  }
+
+  favicon_bitmap_results->clear();

[sky, 2012/09/04 20:52:29]

Shouldn't this be early on in the function?

[pkotwicz, 2012/09/05 22:39:26]

Done.

+  for (size_t i = 0; i < best_bitmap_ids.size(); ++i) {
+    base::Time last_updated;
+    FaviconBitmapResult bitmap_result;
+    bitmap_result.icon_url = icon_url;
+    bitmap_result.icon_type = icon_type;
+    if (!thumbnail_db_->GetFaviconBitmap(best_bitmap_ids[i],
+                                         &last_updated,
+                                         &bitmap_result.bitmap_data,
+                                         &bitmap_result.pixel_size)) {
+      return false;
+    }
+
+    bitmap_result.expired = (Time::Now() - last_updated) >
+        TimeDelta::FromDays(kFaviconRefetchDays);
+    if (bitmap_result.is_valid())
+      favicon_bitmap_results->push_back(bitmap_result);
+  }
+  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;
+}
+
+void HistoryBackend::SetFaviconMappingsForPageAndRedirects(
     const GURL& page_url,
-    const GURL& icon_url,
-    scoped_refptr<base::RefCountedMemory> data,
-    IconType icon_type) {
-  DCHECK(data.get());
-  if (!thumbnail_db_.get() || !db_.get())
-    return;
-
-  FaviconID id = thumbnail_db_->GetFaviconIDForFaviconURL(
-      icon_url, icon_type, NULL);
-  if (!id) {
-    id = thumbnail_db_->AddFavicon(icon_url,
-                                   icon_type,
-                                   GetDefaultFaviconSizes(),
-                                   data,
-                                   Time::Now(),
-                                   gfx::Size());
-  } else {
-    thumbnail_db_->DeleteFaviconBitmapsForFavicon(id);
-    thumbnail_db_->AddFaviconBitmap(id, data, Time::Now(), gfx::Size());
-  }
-
-  SetFaviconMapping(page_url, id, icon_type);
-}
-
-void HistoryBackend::SetFaviconMapping(const GURL& page_url,
-                                       FaviconID id,
-                                       IconType icon_type) {
+    IconType icon_type,
+    const IconURLFaviconIDMap& icon_url_id_map) {
   if (!thumbnail_db_.get())
     return;
 
   // Find all the pages whose favicons we should set, we want to set it for
   // all the pages in the redirect chain if it redirected.
   history::RedirectList dummy_list;
   history::RedirectList* redirects;
   RedirectCache::iterator iter = recent_redirects_.Get(page_url);
   if (iter != recent_redirects_.end()) {
     redirects = &iter->second;
 
     // This redirect chain should have the destination URL as the last item.
     DCHECK(!redirects->empty());
     DCHECK(redirects->back() == page_url);
   } else {
     // No redirect chain stored, make up one containing the URL we want to we
     // can use the same logic below.
     dummy_list.push_back(page_url);
     redirects = &dummy_list;
   }
 
   std::set<GURL> favicons_changed;
 
-  // Save page <-> favicon association.
+  // Save page <-> favicon associations.
   for (history::RedirectList::const_iterator i(redirects->begin());
        i != redirects->end(); ++i) {
-    FaviconID replaced_id;
-    if (AddOrUpdateIconMapping(*i, id, icon_type, &replaced_id)) {
-      // The page's favicon ID changed. This means that the one we just
-      // changed from could have been orphaned, and we need to re-check it.
-      // This is not super fast, but this case will get triggered rarely,
-      // since normally a page will always map to the same favicon ID. It
-      // will mostly happen for favicons we import.
-      if (replaced_id && !thumbnail_db_->HasMappingFor(replaced_id))
-        thumbnail_db_->DeleteFavicon(replaced_id);
-
+    if (SetFaviconMappingsForPage(*i, icon_type, icon_url_id_map)) {
       favicons_changed.insert(*i);
     }
   }
 
   // Send the notification about the changed favicons.
   FaviconChangeDetails* changed_details = new FaviconChangeDetails;
   changed_details->urls.swap(favicons_changed);
   BroadcastNotifications(chrome::NOTIFICATION_FAVICON_CHANGED,
                          changed_details);
 
   ScheduleCommit();
 }
 
-bool HistoryBackend::AddOrUpdateIconMapping(const GURL& page_url,
-                                            FaviconID id,
-                                            IconType icon_type,
-                                            FaviconID* replaced_icon) {
-  *replaced_icon = 0;
+bool HistoryBackend::SetFaviconMappingsForPage(
+    const GURL& page_url,
+    IconType icon_type,
+    const IconURLFaviconIDMap& icon_url_id_map) {
+  DCHECK_LE(icon_url_id_map.size(), kMaxFaviconsPerPage);
+  bool mappings_changed = false;
+
+  // Two icon types are considered 'equivalent' if one of the icon types is
+  // ICON_TOUCH and the other is ICON_PRECOMPOSED_TOUCH.
+  //
+  // Sets the icon mappings of |page_url| for |icon_type| to the icon URLs
+  // and favicon ids in |icon_url_id_map|. The mappings from |page_url| for
+  // any icon URLs not in |icon_url_id_map| are removed.
+  // The icon mappings of |page_url| for equivalent icon types are removed.
+  // Remove any favicons which are orphaned as a result of the removal of the
+  // icon mappings.
+
+  IconURLFaviconIDMap icon_mappings_to_add = icon_url_id_map;
+
   std::vector<IconMapping> icon_mappings;
-  if (!thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings)) {
-    // There is no mapping add it directly.
-    thumbnail_db_->AddIconMapping(page_url, id);
-    return true;
-  }
-  // Iterate all matched icon mappings,
-  // a. If the given icon id and matched icon id are same, return.
-  // b. If the given icon type and matched icon type are same, but icon id
-  //    are not, update the IconMapping.
-  // c. If the given icon_type and matched icon type are not same, but
-  //    either of them is ICON_TOUCH or ICON_PRECOMPOSED_TOUCH, update the
-  //    IconMapping.
-  // d. Otherwise add a icon mapping.
+  thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings);
   for (std::vector<IconMapping>::iterator m = icon_mappings.begin();
        m != icon_mappings.end(); ++m) {
-    if (m->icon_id == id)
-      // The mapping is already there.
-      return false;
+    if ((icon_type == TOUCH_ICON && m->icon_type == TOUCH_PRECOMPOSED_ICON) ||
+       (icon_type == TOUCH_PRECOMPOSED_ICON && m->icon_type == TOUCH_ICON) ||
+       icon_type == m->icon_type) {
+      IconURLFaviconIDMap::iterator it =
+          icon_mappings_to_add.find(m->icon_url);
+      if (it == icon_mappings_to_add.end()) {
+        thumbnail_db_->DeleteIconMapping(m->mapping_id);
+      } else {
+        FaviconID icon_id = it->second;
+        icon_mappings_to_add.erase(it);
+        if (icon_id == m->icon_id) {
+          // Keep icon mapping.
+          continue;
+        }
 
-    if ((icon_type == TOUCH_ICON && m->icon_type == TOUCH_PRECOMPOSED_ICON) ||
-        (icon_type == TOUCH_PRECOMPOSED_ICON && m->icon_type == TOUCH_ICON) ||
-        (icon_type == m->icon_type)) {
-      thumbnail_db_->UpdateIconMapping(m->mapping_id, id);
-      *replaced_icon = m->icon_id;
-      return true;
+        thumbnail_db_->UpdateIconMapping(m->mapping_id, icon_id);
+      }
+      // Removing / updating the icon mapping may have orphaned the associated
+      // favicon so we must recheck it. This is not super fast, but this case
+      // will get triggered rarely, since normally a page will always map to
+      // the same favicon IDs. It will mostly happen for favicons we import.
+      if (!thumbnail_db_->HasMappingFor(m->icon_id))
+        thumbnail_db_->DeleteFavicon(m->icon_id);
+      mappings_changed = true;
     }
   }
-  thumbnail_db_->AddIconMapping(page_url, id);
-  return true;
+
+  for (IconURLFaviconIDMap::iterator it = icon_mappings_to_add.begin();
+       it != icon_mappings_to_add.end(); ++it) {
+    thumbnail_db_->AddIconMapping(page_url, it->second);
+    mappings_changed = true;
+  }
+  return mappings_changed;
 }
 
 void HistoryBackend::Commit() {
   if (!db_.get())
     return;
 
   // Note that a commit may not actually have been scheduled if a caller
   // explicitly calls this instead of using ScheduleCommit. Likewise, we
   // may reset the flag written by a pending commit. But this is OK! It
   // will merely cause extra commits (which is kind of the idea). We
@@ skipping 363 matching lines @@
 
   return true;
 }
 
 BookmarkService* HistoryBackend::GetBookmarkService() {
   if (bookmark_service_)
     bookmark_service_->BlockTillLoaded();
   return bookmark_service_;
 }
 
-bool HistoryBackend::GetFaviconFromDB(
-    const GURL& page_url,
-    int icon_types,
-    std::vector<FaviconBitmapResult>* favicon_bitmap_results) {
-  DCHECK(favicon_bitmap_results);
-
-  if (!db_.get() || !thumbnail_db_.get())
-    return false;
-
-  bool success = false;
-  // Time the query.
-  TimeTicks beginning_time = TimeTicks::Now();
-
-  std::vector<IconMapping> icon_mappings;
-  // Iterate over the known icons looking for one that includes one of the
-  // requested types.
-  if (thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings)) {
-    for (std::vector<IconMapping>::iterator i = icon_mappings.begin();
-         i != icon_mappings.end(); ++i) {
-      if ((i->icon_type & icon_types) &&
-          GetFaviconFromDB(i->icon_id, favicon_bitmap_results)) {
-        success = true;
-        break;
-      }
-    }
-  }
-  UMA_HISTOGRAM_TIMES("History.GetFavIconFromDB",  // historical name
-                      TimeTicks::Now() - beginning_time);
-  return success;
-}
-
-bool HistoryBackend::GetFaviconFromDB(
-    FaviconID favicon_id,
-    std::vector<history::FaviconBitmapResult>* favicon_bitmap_results) {
-  FaviconBitmapResult bitmap_result;
-  Time last_updated;
-  if (!thumbnail_db_->GetFaviconHeader(favicon_id,
-                                       &bitmap_result.icon_url,
-                                       &bitmap_result.icon_type,
-                                       NULL)) {
-    return false;
-  }
-
-  std::vector<FaviconBitmap> favicon_bitmaps;
-  thumbnail_db_->GetFaviconBitmaps(favicon_id, &favicon_bitmaps);
-  if (favicon_bitmaps.empty())
-    return false;
-
-  bitmap_result.bitmap_data = favicon_bitmaps[0].bitmap_data;
-  bitmap_result.expired = (Time::Now() - favicon_bitmaps[0].last_updated) >
-      TimeDelta::FromDays(kFaviconRefetchDays);
-
-  favicon_bitmap_results->clear();
-  if (bitmap_result.is_valid())
-    favicon_bitmap_results->push_back(bitmap_result);
-  return true;
-}
-
 void HistoryBackend::NotifyVisitObservers(const VisitRow& visit) {
   BriefVisitInfo info;
   info.url_id = visit.url_id;
   info.time = visit.visit_time;
   info.transition = visit.transition;
   // If we don't have a delegate yet during setup or shutdown, we will drop
   // these notifications.
   if (delegate_.get())
     delegate_->NotifyVisitDBObserversOnAddVisit(info);
 }
 
 }  // namespace history