Adds support for saving favicon size into history database.

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 0e3f1b391f6a1061adec4f0a9bc0fdaf5268a6e6..d548c4ce09bf28bebd5b407e173aac9c3ffdfe3a 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/bookmarks/bookmark_service.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"
@@ -94,6 +95,14 @@ static const int kMaxRedirectCount = 32;
 // and is archived.
 static const int kArchiveDaysThreshold = 90;
 
+// The maximum number of icons URLs per page which can be stored in the
+// thumbnail database.
+const size_t kMaxFaviconsPerPage = 8;
+
+// The maximum number of bitmaps for a single icon URL which can be stored in
+// the thumbnail database.
+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,
@@ -1771,21 +1780,71 @@ bool HistoryBackend::GetThumbnailFromOlderRedirect(
   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::GetFavicon(
+    scoped_refptr<GetFaviconRequest> request,
+    const std::vector<GURL>& icon_urls,
+    int icon_types,
+    const gfx::Size& desired_dip_size,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  UpdateFaviconMappingsAndFetchImpl(NULL, icon_urls, request, icon_types,
+                                    desired_dip_size, desired_scale_factors);
 }
 
-void HistoryBackend::UpdateFaviconMappingAndFetch(
+void HistoryBackend::GetFaviconForURL(
     scoped_refptr<GetFaviconRequest> request,
     const GURL& page_url,
+    int icon_types,
+    const gfx::Size& desired_dip_size,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  if (request->canceled())
+    return;
+
+  FaviconData favicon_data;
+  std::vector<GURL> mapped_icon_urls;
+
+  // Get the favicons from DB.
+  GetFaviconFromDB(page_url, icon_types, desired_dip_size,
+      desired_scale_factors, &favicon_data, &mapped_icon_urls);
+
+  request->ForwardResult(request->handle(), favicon_data, mapped_icon_urls);
+}
+
+void HistoryBackend::UpdateFaviconMappingsAndFetch(
+    scoped_refptr<GetFaviconRequest> request,
+    const GURL& page_url,
+    const std::vector<GURL>& icon_urls,
+    IconType icon_type,
+    const gfx::Size& desired_dip_size,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors) {
+  UpdateFaviconMappingsAndFetchImpl(&page_url, icon_urls, request, icon_type,
+      desired_dip_size, desired_scale_factors);
+}
+
+void HistoryBackend::AddFavicons(
+    const GURL& page_url,
     const GURL& icon_url,
-    IconType icon_type) {
-  UpdateFaviconMappingAndFetchImpl(&page_url, icon_url, request, icon_type);
+    IconType icon_type,
+    const std::vector<FaviconBitmapData>& favicon_bitmaps) {
+  /*FaviconData favicon_data;
+  GetFaviconsFromDB(page_url, icon_type, &favicon_data);
+
+  IconURLSizesMap icon_url_sizes;
+  if (favicon_data.known_icon) {
+    icon_url_sizes = favicon_data.icon_url_sizes;
+  } else {
+    // The thumbnail database doesn't know about |page_url|. Guess at what
+    // |icon_url_sizes| should be.
+    for (size_t i = 0; i < elements.size(); ++i) {
+      const GURL& icon_url = elements[i].icon_url;
+      icon_url_sizes[icon_url].InsertSize(elements[i].pixel_size);
+    }
+  }
+
+  SetFavicons(page_url, icon_type, elements, icon_url_sizes);
+  */
 }
 
-void HistoryBackend::SetFaviconOutOfDateForPage(const GURL& page_url) {
+void HistoryBackend::SetFaviconsOutOfDateForPage(const GURL& page_url) {
   std::vector<IconMapping> icon_mappings;
 
   if (!thumbnail_db_.get() ||
@@ -1800,8 +1859,8 @@ void HistoryBackend::SetFaviconOutOfDateForPage(const GURL& page_url) {
   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;
 
@@ -1809,7 +1868,7 @@ void HistoryBackend::CloneFavicon(const GURL& old_page_url,
   if (old_page_url.GetOrigin() != new_page_url.GetOrigin())
     return;
 
-  thumbnail_db_->CloneIconMapping(old_page_url, new_page_url);
+  thumbnail_db_->CloneIconMappings(old_page_url, new_page_url);
   ScheduleCommit();
 }
 
@@ -1825,14 +1884,14 @@ void HistoryBackend::SetImportedFavicons(
 
   for (size_t i = 0; i < favicon_usage.size(); i++) {
     FaviconID favicon_id = thumbnail_db_->GetFaviconIDForFaviconURL(
-        favicon_usage[i].favicon_url, history::FAVICON, NULL);
+        favicon_usage[i].favicon_url, history::FAVICON);
     if (!favicon_id) {
       // This favicon doesn't exist yet, so we create it using the given data.
       // TODO(pkotwicz): Pass in real pixel size.
       favicon_id = thumbnail_db_->AddFavicon(
           favicon_usage[i].favicon_url,
           history::FAVICON,
-          std::string("0 0"),
+          kDefaultFaviconSizes,
           new base::RefCountedBytes(favicon_usage[i].png_data),
           now,
           gfx::Size());
@@ -1860,7 +1919,9 @@ void HistoryBackend::SetImportedFavicons(
           favicons_changed.insert(*url);
         }
       } else {
-        if (!thumbnail_db_->GetIconMappingForPageURL(*url, FAVICON, NULL)) {
+        std::vector<IconMapping> icon_mappings_unused;
+        if (!thumbnail_db_->GetIconMappingsForPageURL(*url, FAVICON,
+                                                      &icon_mappings_unused)) {
           // URL is present in history, update the favicon *only* if it is not
           // set already.
           thumbnail_db_->AddIconMapping(*url, favicon_id);
@@ -1879,11 +1940,13 @@ void HistoryBackend::SetImportedFavicons(
   }
 }
 
-void HistoryBackend::UpdateFaviconMappingAndFetchImpl(
+void HistoryBackend::UpdateFaviconMappingsAndFetchImpl(
     const GURL* page_url,
-    const GURL& icon_url,
+    const std::vector<GURL>& icon_urls,
     scoped_refptr<GetFaviconRequest> request,
-    int icon_types) {
+    int icon_types,
+    const gfx::Size& desired_dip_size,
+    const std::vector<ui::ScaleFactor> desired_scale_factors) {
   // 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)));
@@ -1892,70 +1955,268 @@ void HistoryBackend::UpdateFaviconMappingAndFetchImpl(
     return;
 
   FaviconData favicon_data;
-
+  std::vector<GURL> icon_urls_in_db;
+  IconURLFaviconIDMap icon_url_id_map;
   if (thumbnail_db_.get()) {
-    IconType icon_type;
-    const FaviconID favicon_id =
-        thumbnail_db_->GetFaviconIDForFaviconURL(
-            icon_url, icon_types, &icon_type);
-    if (favicon_id) {
-      GetFaviconFromDB(favicon_id, &favicon_data);
-
-      if (page_url)
-        SetFaviconMapping(*page_url, favicon_id, icon_type);
+    std::vector<FaviconID> favicon_ids;
+    for (size_t i = 0; i < icon_urls.size(); ++i) {
+      const GURL& icon_url = icon_urls[i];
+      const FaviconID favicon_id =
+          thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_types);
+      if (favicon_id) {
+        favicon_ids.push_back(favicon_id);
+        icon_urls_in_db.push_back(icon_url);
+        icon_url_id_map[icon_url] = favicon_id;
+      }
+      // else case, haven't cached entry yet. Caller is responsible for
+      // downloading the favicon and invoking SetFavicons.
     }
-    // else case, haven't cached entry yet. Caller is responsible for
-    // downloading the favicon and invoking SetFavicon.
+    GetFaviconDataForBestMatch(favicon_ids, desired_dip_size,
+        desired_scale_factors, &favicon_data);
+  }
+
+  if (page_url) {
+    SetFaviconMappingsForPageAndRedirects(
+        *page_url, favicon_data.icon_type, icon_url_id_map);
   }
-  // TODO(pkotwicz): Pass in icon_url if it exists in database instead of empty
-  // vector.
-  request->ForwardResult(request->handle(), favicon_data, std::vector<GURL>());
+  request->ForwardResult(request->handle(), favicon_data, icon_urls_in_db);
 }
 
-void HistoryBackend::GetFaviconForURL(
-    scoped_refptr<GetFaviconRequest> request,
+void HistoryBackend::SetFavicons(
     const GURL& page_url,
-    int icon_types) {
-  if (request->canceled())
+    const GURL& icon_url,
+    IconType icon_type,
+    const std::vector<FaviconBitmapData>& favicon_bitmaps,
+    const IconURLSizesMap& icon_url_sizes) {
+  /*if (!thumbnail_db_.get() || !db_.get())
     return;
 
-  FaviconData favicon_data;
+  DCHECK(ValidateSetFaviconsParams(elements, icon_url_sizes));
+
+  // Build map of |elements| for icon url.
+  typedef std::map<GURL, std::vector<FaviconBitmapData> > BitmapDataByIconURL;
+  BitmapDataByIconURL grouped_by_icon_url;
+  for (size_t i = 0; i < elements.size(); ++i) {
+    const GURL& icon_url = elements[i].icon_url;
+    grouped_by_icon_url[icon_url].push_back(elements[i]);
+  }
+
+  IconURLFaviconIDMap icon_url_id_map;
+  for (ElementsByIconURL::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);
+    if (!icon_id)
+      icon_id = thumbnail_db_->AddFavicon(icon_url, icon_type);
+    icon_url_id_map[icon_url] = icon_id;
+
+    // Update sizes first because it may result in deleting favicon bitmaps.
+    SetFaviconSizes(icon_id, icon_url_sizes.find(icon_url)->second);
+    SetFaviconBitmaps(icon_id, it->second);
+  }
+
+  // Update sizes for favicons which have entries in |icon_url_sizes| but not
+  // in |elements|.
+  for (IconURLSizesMap::const_iterator it = icon_url_sizes.begin();
+       it != icon_url_sizes.end(); ++it) {
+    const GURL& icon_url = it->first;
+    if (grouped_by_icon_url.find(icon_url) == grouped_by_icon_url.end()) {
+      FaviconID icon_id =
+          thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type);
+      if (!icon_id)
+        icon_id = thumbnail_db_->AddFavicon(icon_url, icon_type);
+      icon_url_id_map[icon_url] = icon_id;
+      SetFaviconSizes(icon_id, it->second);
+    }
+  }
+
+  SetFaviconMappingsForPageAndRedirects(page_url, icon_type, icon_url_id_map);
+  */
+}
 
-  // Get the favicon from DB.
-  GetFaviconFromDB(page_url, icon_types, &favicon_data);
+void HistoryBackend::SetFaviconBitmaps(
+    FaviconID icon_id,
+    const std::vector<FaviconBitmapData>& favicon_bitmaps) {
+  /*
+  std::vector<FaviconBitmapIDSize> bitmap_id_size_listing;
+  thumbnail_db_->GetFaviconBitmapIDSizeListing(icon_id,
+                                               &bitmap_id_size_listing);
 
-  // TODO(pkotwicz): Pass in matched icon URLs for icon_types instead of empty
-  // vector.
-  request->ForwardResult(request->handle(), favicon_data, std::vector<GURL>());
+  // A nested loop is ok because in practice neither |elements| nor
+  // |bitmap_id_size_listing| will have many elements.
+  for (size_t i = 0; i < elements.size(); ++i) {
+    const FaviconDataElement& element = elements[i];
+    FaviconBitmapID bitmap_id = 0;
+    for (size_t j = 0; j < bitmap_id_size_listing.size(); ++j) {
+      if (bitmap_id_size_listing[j].pixel_size == element.pixel_size) {
+        bitmap_id = bitmap_id_size_listing[j].bitmap_id;
+        break;
+      }
+    }
+    if (bitmap_id) {
+      thumbnail_db_->SetFaviconBitmap(bitmap_id, element.bitmap_data,
+                                      base::Time::Now());
+    } else {
+      thumbnail_db_->AddFaviconBitmap(icon_id, element.bitmap_data,
+                                      base::Time::Now(), element.pixel_size);
+    }
+  }
+  */
 }
 
-void HistoryBackend::SetFavicon(
+bool HistoryBackend::ValidateSetFaviconsParams(
+    const std::vector<FaviconBitmapData>& favicon_bitmaps,
+    const IconURLSizesMap& icon_url_sizes) const {
+  /*if (single_favicon_bitmap_per_icon_type_) {
+    if (icon_url_sizes.size() != favicon_bitmaps.size())
+      return false;
+
+    if (icon_url_sizes.size() > 1)
+      return false;
+
+    if (icon_url_sizes.size() == 1) {
+      const FaviconSizes& sizes = icon_url_sizes.begin()->second;
+      if (!sizes.num_sizes() != 1 || !sizes.has_size(gfx::Size()))
+        return false;
+    }
+  } else {
+    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.num_sizes() > kMaxFaviconBitmapsPerIconURL)
+        return false;
+    }
+  }
+
+  for (size_t i = 0; i < favicon_bitmaps.size(); ++i) {
+    IconURLSizesMap::const_iterator it =
+        icon_url_sizes.find(favicon_bitmaps[i].icon_url);
+    if (it == favicon_bitmaps.end() ||
+        !it->second.has_size(favicon_bitmaps[i].pixel_size) ||
+        !favicon_bitmaps[i].bitmap_data.get())
+      return false;
+  }
+  */
+  return true;
+}
+
+void HistoryBackend::SetFaviconSizes(
+    FaviconID icon_id,
+    const FaviconSizes& sizes) {
+  /*std::vector<FaviconBitmapIDSize> bitmap_id_size_listing;
+  thumbnail_db_->GetFaviconBitmapIDSizeListing(icon_id,
+                                               &bitmap_id_size_listing);
+
+  // Remove bitmaps whose pixel size is not contained in |sizes|.
+  for (size_t i = 0; i < bitmap_id_size_listing.size(); ++i) {
+    const gfx::Size& pixel_size = bitmap_id_size_listing[i].pixel_size;
+    if (!sizes.has_size(pixel_size))
+      thumbnail_db_->DeleteFaviconBitmap(bitmap_id_size_listing[i].bitmap_id);
+  }
+
+  thumbnail_db_->SetFaviconSizes(icon_id, sizes.ToString());*/
+}
+
+bool HistoryBackend::GetFaviconFromDB(
     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;
+    int icon_types,
+    const gfx::Size& desired_dip_size,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors,
+    FaviconData* favicon_data,
+    std::vector<GURL>* mapped_icon_urls) {
+  DCHECK(favicon_data);
+
+  if (!db_.get() || !thumbnail_db_.get())
+    return false;
+
+  // Time the query.
+  TimeTicks beginning_time = TimeTicks::Now();
+
+  std::vector<IconMapping> icon_mappings;
+  thumbnail_db_->GetIconMappingsForPageURL(page_url, icon_types,
+                                           &icon_mappings);
+
+  std::vector<FaviconID> favicon_ids;

[sky, 2012/08/24 16:35:42]

Add a description as to what this does.

+  std::vector<GURL> icon_urls;
+  for (size_t i = 0; i < icon_mappings.size(); ++i) {
+    favicon_ids.push_back(icon_mappings[i].icon_id);
+    mapped_icon_urls->push_back(icon_mappings[i].icon_url);
+  }
+
+  bool no_errors = GetFaviconDataForBestMatch(favicon_ids, desired_dip_size,
+      desired_scale_factors, favicon_data);
+
+  UMA_HISTOGRAM_TIMES("History.GetFavIconFromDB",  // historical name
+                      TimeTicks::Now() - beginning_time);
+  return no_errors && !mapped_icon_urls->empty();
+}
+
+bool HistoryBackend::GetFaviconDataForBestMatch(
+    const std::vector<FaviconID>& candidate_favicon_ids,
+    const gfx::Size& desired_dip_size,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors,
+    FaviconData* favicon_data) {
+  if (candidate_favicon_ids.empty())
+    return false;
+
+  // Find best match given |desired_dip_size| and |desired_scale_factors|.
+  FaviconID best_favicon_id = 0;
+  scoped_ptr<std::vector<FaviconBitmapID> > best_bitmap_ids(NULL);

[sky, 2012/08/24 16:35:42]

Why do you need a scoped_ptr here and below?

+  float highest_score = -1.0f;
+  for (size_t i = 0; i < candidate_favicon_ids.size(); ++i) {
+    scoped_ptr<std::vector<FaviconBitmapID> > candidate_bitmap_ids(
+        new std::vector<FaviconBitmapID>());
+    float score;
+    std::vector<FaviconBitmapIDSize> bitmap_id_size_list;
+    thumbnail_db_->GetFaviconBitmapIDSizeList(candidate_favicon_ids[i],
+                                              &bitmap_id_size_list);
+    // Currently only support requesting square favicons.
+    SelectFaviconBitmapIDs(bitmap_id_size_list,
+                           desired_scale_factors,
+                           desired_dip_size.width(),
+                           candidate_bitmap_ids.get(),
+                           &score);
+    if (score > highest_score) {
+      best_favicon_id = candidate_favicon_ids[i],
+      best_bitmap_ids.swap(candidate_bitmap_ids);

[sky, 2012/08/24 16:35:42]

This seems like it only picks bitmaps for a unique faviconid.

Don't you need to track the best score for each size you need? By size I mean
the combination of size and scale factors.

[pkotwicz, 2012/09/04 16:18:36]

Yes this is true. I added a TODO to make this different but I think it is best
to keep it this way for now till we update FaviconStatus.

To reply to your comment above, I use the scoped_ptr to avoid the vector copy
from assigning |candidate_bitmap_ids| to |best_bitmap_ids|.

[sky, 2012/09/04 17:18:07]

Huh? Use swap if you're really worried.

+      highest_score = score;
+    }
+  }
+
+  // Construct FaviconData from |best_favicon_id| and |best_bitmap_ids|.
+  if (!thumbnail_db_->GetFaviconHeader(best_favicon_id,
+                                       &favicon_data->icon_url,
+                                       &favicon_data->icon_type,
+                                       &favicon_data->sizes)) {
+    return false;
+  }
 
-  FaviconID id = thumbnail_db_->GetFaviconIDForFaviconURL(
-      icon_url, icon_type, NULL);
-  if (id)
-    thumbnail_db_->DeleteFaviconBitmapsForFavicon(id);
+  favicon_data->bitmaps.clear();
+  for (size_t i = 0; i < best_bitmap_ids->size(); ++i) {
+    base::Time last_updated;
+    FaviconBitmapData favicon_bitmap_data;
+    if (!thumbnail_db_->GetFaviconBitmap((*best_bitmap_ids)[i],
+                                         &last_updated,
+                                         &favicon_bitmap_data.bitmap_data,
+                                         &favicon_bitmap_data.pixel_size)) {
+      return false;
+    }
 
-  id = thumbnail_db_->AddFavicon(icon_url,
-                                 icon_type,
-                                 "0 0",
-                                 data,
-                                 Time::Now(),
-                                 gfx::Size());
+    favicon_bitmap_data.expired = (Time::Now() - last_updated) >
+        TimeDelta::FromDays(kFaviconRefetchDays);
+    favicon_data->bitmaps.push_back(favicon_bitmap_data);
+  }
 
-  SetFaviconMapping(page_url, id, icon_type);
+  favicon_data->known_icon = true;
+  return true;
 }
 
-void HistoryBackend::SetFaviconMapping(const GURL& page_url,
-                                       FaviconID id,
-                                       IconType icon_type) {
+void HistoryBackend::SetFaviconMappingsForPageAndRedirects(
+    const GURL& page_url,
+    IconType icon_type,
+    const IconURLFaviconIDMap& icon_url_id_map) {
   if (!thumbnail_db_.get())
     return;
 
@@ -1979,19 +2240,10 @@ void HistoryBackend::SetFaviconMapping(const GURL& page_url,
 
   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);
     }
   }
@@ -2005,41 +2257,68 @@ void HistoryBackend::SetFaviconMapping(const GURL& page_url,
   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;
+
+  IconURLFaviconIDMap icon_mappings_to_add = icon_url_id_map;
+
+  // Two icon types are considered 'equivalent' if
+  // a. The two icon types are equal.
+  // b. If one of the icon types is ICON_TOUCH and the other is
+  //    ICON_PRECOMPOSED_TOUCH.
+  //
+  // For each of the urls in |icon_url_id_map|.
+  // a. If one of the icon mappings has the same icon url as the url from
+  //    |icon_url_id_map|.
+  //    i.  If the matched icon mapping has the same type as the given
+  //        |icon_type|, the icon mapping was previously created.
+  //    ii. If the matched icon mapping has an 'equivalent' icon type to the
+  //        given |icon_type|, update the icon mapping.
+  // b. Otherwise, add the icon mapping represented by the url.
+  // c. If there are any icon mappings with an icon_url not in
+  //    |icon_url_id_map| and the mapping type is the same or equivalent to the
+  //    given |icon_type|, remove the mapping.
+
   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)) {
-      thumbnail_db_->UpdateIconMapping(m->mapping_id, id);
-      *replaced_icon = m->icon_id;
-      return true;
+      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 {
+        icon_mappings_to_add.erase(it);
+        if (icon_type == m->icon_type) {
+          // The mapping is already there.
+          continue;
+        }
+        thumbnail_db_->UpdateIconMapping(m->mapping_id, it->second);
+      }
+      // 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() {
@@ -2423,57 +2702,6 @@ BookmarkService* HistoryBackend::GetBookmarkService() {
   return bookmark_service_;
 }
 
-bool HistoryBackend::GetFaviconFromDB(
-    const GURL& page_url,
-    int icon_types,
-    FaviconData* favicon_data) {
-  DCHECK(favicon_data);
-
-  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_data)) {
-        success = true;
-        break;
-      }
-    }
-  }
-  UMA_HISTOGRAM_TIMES("History.GetFavIconFromDB",  // historical name
-                      TimeTicks::Now() - beginning_time);
-  return success;
-}
-
-bool HistoryBackend::GetFaviconFromDB(FaviconID favicon_id,
-                                      FaviconData* favicon_data) {
-  Time last_updated;
-  scoped_refptr<base::RefCountedMemory> data;
-
-  favicon_data->known_icon = true;
-  if (!thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data,
-      &favicon_data->icon_url, &favicon_data->icon_type))
-    return false;
-
-  FaviconBitmapData favicon_bitmap_data;
-  favicon_bitmap_data.expired = (Time::Now() - last_updated) >
-      TimeDelta::FromDays(kFaviconRefetchDays);
-  favicon_bitmap_data.bitmap_data = data;
-
-  favicon_data->bitmaps.clear();
-  favicon_data->bitmaps.push_back(favicon_bitmap_data);
-  return true;
-}
-
 void HistoryBackend::NotifyVisitObservers(const VisitRow& visit) {
   BriefVisitInfo info;
   info.url_id = visit.url_id;