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 <list>
 #include <map>
 #include <set>
 #include <vector>
@@ skipping 18 matching lines @@
 #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"
 #include "googleurl/src/gurl.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "ui/gfx/favicon_size.h"
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/history/android/android_provider_backend.h"
 #endif
 
 using base::Time;
 using base::TimeDelta;
 using base::TimeTicks;
 
 /* The HistoryBackend consists of a number of components:
@@ skipping 38 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 favicon bitmaps stored for a single page.
+const size_t kMaxFaviconsPerPage = 30;
+
 // 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 1659 matching lines @@
           success = thumbnail_db_->GetPageThumbnail(url_id, data);
       }
     }
   }
 
   return success;
 }
 
 void HistoryBackend::GetFavicon(scoped_refptr<GetFaviconRequest> request,
                                 const GURL& icon_url,
+                                const gfx::Size& pixel_size,
                                 int icon_types) {
-  UpdateFaviconMappingAndFetchImpl(NULL, icon_url, request, icon_types);
+  UpdateFaviconMappingAndFetchImpl(NULL, icon_url, request, pixel_size,
+                                   icon_types);
 }
 
 void HistoryBackend::UpdateFaviconMappingAndFetch(
     scoped_refptr<GetFaviconRequest> request,
     const GURL& page_url,
     const GURL& icon_url,
+    const gfx::Size& pixel_size,
     IconType icon_type) {
-  UpdateFaviconMappingAndFetchImpl(&page_url, icon_url, request, icon_type);
+  UpdateFaviconMappingAndFetchImpl(&page_url, icon_url, request, pixel_size,
+                                   icon_type);
 }
 
 void HistoryBackend::SetFaviconOutOfDateForPage(const GURL& page_url) {
   std::vector<IconMapping> icon_mappings;
 
   if (!thumbnail_db_.get() ||
       !thumbnail_db_->GetIconMappingsForPageURL(page_url,
                                                 &icon_mappings))
     return;
 
@@ skipping 21 matching lines @@
     const std::vector<ImportedFaviconUsage>& favicon_usage) {
   if (!db_.get() || !thumbnail_db_.get())
     return;
 
   Time now = Time::Now();
 
   // Track all URLs that had their favicons set or updated.
   std::set<GURL> favicons_changed;
 
   for (size_t i = 0; i < favicon_usage.size(); i++) {
-    FaviconID favicon_id = thumbnail_db_->GetFaviconIDForFaviconURL(
-        favicon_usage[i].favicon_url, history::FAVICON, NULL);
-    if (!favicon_id) {
+    std::vector<FaviconIDAndSize> favicon_id_size_listing;
+    thumbnail_db_->GetFaviconIDsForFaviconURL(favicon_usage[i].favicon_url,
+        history::FAVICON, &favicon_id_size_listing);
+    if (favicon_id_size_listing.empty()) {
       // This favicon doesn't exist yet, so we create it using the given data.
-      favicon_id = thumbnail_db_->AddFavicon(favicon_usage[i].favicon_url,
-                                             history::FAVICON);
+      gfx::Size default_size =
+          gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize);
+      FaviconID favicon_id = thumbnail_db_->AddFavicon(
+          favicon_usage[i].favicon_url, default_size, history::FAVICON);
       if (!favicon_id)
         continue;  // Unable to add the favicon.
       thumbnail_db_->SetFavicon(favicon_id,
           new base::RefCountedBytes(favicon_usage[i].png_data), now);
+
+      FaviconIDAndSize favicon_id_size;
+      favicon_id_size.icon_id = favicon_id;
+      favicon_id_size.icon_size = default_size;
+      favicon_id_size_listing.push_back(favicon_id_size);
     }
 
     // Save the mapping from all the URLs to the favicon.
     BookmarkService* bookmark_service = GetBookmarkService();
     for (std::set<GURL>::const_iterator url = favicon_usage[i].urls.begin();
          url != favicon_usage[i].urls.end(); ++url) {
       URLRow url_row;
       if (!db_->GetRowForURL(*url, &url_row)) {
         // If the URL is present as a bookmark, add the url in history to
         // save the favicon mapping. This will match with what history db does
         // for regular bookmarked URLs with favicons - when history db is
         // cleaned, we keep an entry in the db with 0 visits as long as that
         // url is bookmarked.
         if (bookmark_service && bookmark_service_->IsBookmarked(*url)) {
           URLRow url_info(*url);
           url_info.set_visit_count(0);
           url_info.set_typed_count(0);
           url_info.set_last_visit(base::Time());
           url_info.set_hidden(false);
           db_->AddURL(url_info);
-          thumbnail_db_->AddIconMapping(*url, favicon_id);
+
+          for (std::vector<FaviconIDAndSize>::iterator favicon_id_size =
+                   favicon_id_size_listing.begin();
+               favicon_id_size != favicon_id_size_listing.end();
+               ++favicon_id_size) {
+            thumbnail_db_->AddIconMapping(*url, favicon_id_size->icon_id);
+          }
           favicons_changed.insert(*url);
         }
       } else {
-        if (!thumbnail_db_->GetIconMappingForPageURL(*url, FAVICON, NULL)) {
+        if (!thumbnail_db_->GetIconMappingsForPageURL(*url, FAVICON, NULL)) {
           // URL is present in history, update the favicon *only* if it is not
           // set already.
-          thumbnail_db_->AddIconMapping(*url, favicon_id);
+          for (std::vector<FaviconIDAndSize>::iterator favicon_id_size =
+                   favicon_id_size_listing.begin();
+               favicon_id_size != favicon_id_size_listing.end();
+               ++favicon_id_size) {
+            thumbnail_db_->AddIconMapping(*url, favicon_id_size->icon_id);
+          }
           favicons_changed.insert(*url);
         }
       }
     }
   }
 
   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(
     const GURL* page_url,
     const GURL& icon_url,
     scoped_refptr<GetFaviconRequest> request,
+    const gfx::Size& pixel_size,
     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())
     return;
 
   FaviconData favicon;
 
   if (thumbnail_db_.get()) {
-    const FaviconID favicon_id =
-        thumbnail_db_->GetFaviconIDForFaviconURL(
-            icon_url, icon_types, &favicon.icon_type);
+    std::vector<FaviconIDAndSize> favicon_id_size_listing;
+    thumbnail_db_->GetFaviconIDsForFaviconURL(icon_url, icon_types,
+        &favicon_id_size_listing);
+    FaviconID favicon_id = GetFaviconID(favicon_id_size_listing, pixel_size);
     if (favicon_id) {
-      scoped_refptr<base::RefCountedBytes> data = new base::RefCountedBytes();
-      favicon.known_icon = true;
-      Time last_updated;
-      if (thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data->data(),
-                                    NULL, NULL)) {
-        favicon.expired = (Time::Now() - last_updated) >
-            TimeDelta::FromDays(kFaviconRefetchDays);
-        favicon.image_data = data;
+      GetFaviconFromDB(favicon_id, &favicon);
+      if (page_url) {
+        SetFaviconMapping(*page_url, favicon_id, pixel_size, favicon.icon_type);
       }
-
-      if (page_url)
-        SetFaviconMapping(*page_url, favicon_id, favicon.icon_type);
     }
     // else case, haven't cached entry yet. Caller is responsible for
     // downloading the favicon and invoking SetFavicon.
   }
   request->ForwardResult(request->handle(), favicon);
 }
 
+FaviconID HistoryBackend::GetFaviconID(
+    const std::vector<FaviconIDAndSize>& favicon_id_size_listing,
+    const gfx::Size& desired_pixel_size) const {
+  // Give preference to favicon sizes which will result in downscaling rather
+  // than upscaling.
+  FaviconID closest_favicon_id = 0;
+  gfx::Size closest_pixel_size;
+  for (size_t i = 0; i < favicon_id_size_listing.size(); ++i) {
+    gfx::Size size = favicon_id_size_listing[i].icon_size;
+    if (closest_pixel_size.GetArea() < desired_pixel_size.GetArea() &&

[stevenjb, 2012/08/01 00:00:31]

Area may not be the best method for comparison. For example area would give
16x64 the same priority as 32x32. 
Exact matches should always be given highest priority. 
After that I thin we are better off just comparing first height, then width (if
heights are equal).

[pkotwicz, 2012/08/02 19:47:52]

Agree

+        size.GetArea() > closest_pixel_size.GetArea()) {
+      closest_favicon_id = favicon_id_size_listing[i].icon_id;
+      closest_pixel_size = size;
+    } else if (closest_pixel_size.GetArea() > desired_pixel_size.GetArea() &&
+               size.GetArea() < closest_pixel_size.GetArea() &&
+               size.GetArea() >= desired_pixel_size.GetArea()) {
+      closest_favicon_id = favicon_id_size_listing[i].icon_id;
+      closest_pixel_size = size;
+    }
+
+    if (closest_pixel_size == desired_pixel_size)
+      break;
+  }
+  return closest_favicon_id;
+}
+
 void HistoryBackend::GetFaviconForURL(
     scoped_refptr<GetFaviconRequest> request,
     const GURL& page_url,
+    const gfx::Size& pixel_size,
     int icon_types) {
   if (request->canceled())
     return;
 
   FaviconData favicon;
 
   // Get the favicon from DB.
-  GetFaviconFromDB(page_url, icon_types, &favicon);
+  GetFaviconFromDB(page_url, pixel_size, icon_types, &favicon);
 
   request->ForwardResult(request->handle(), favicon);
 }
 
 void HistoryBackend::GetFaviconForID(scoped_refptr<GetFaviconRequest> request,
                                      FaviconID id) {
   if (request->canceled())
     return;
 
   FaviconData favicon;
   GetFaviconFromDB(id, &favicon);
   request->ForwardResult(request->handle(), favicon);
 }
 
 void HistoryBackend::SetFavicon(
     const GURL& page_url,
     const GURL& icon_url,
     scoped_refptr<base::RefCountedMemory> data,
+    const gfx::Size& requested_size,
     IconType icon_type) {
+  // For M22, store |requested_size| as the favicon's pixel size into the
+  // favicon database table.
+  // TODO(pkotwicz): Fix this.
   DCHECK(data.get());
   if (!thumbnail_db_.get() || !db_.get())
     return;
 
-  FaviconID id = thumbnail_db_->GetFaviconIDForFaviconURL(
-      icon_url, icon_type, NULL);
+  std::vector<FaviconIDAndSize> favicon_id_size_listing;
+  thumbnail_db_->GetFaviconIDsForFaviconURL(icon_url, icon_type,
+                                            &favicon_id_size_listing);
+
+  FaviconID id = 0;
+  for (size_t i = 0; i < favicon_id_size_listing.size(); ++i) {
+    if (favicon_id_size_listing[i].icon_size == requested_size) {
+      id = favicon_id_size_listing[i].icon_id;
+      break;
+    }
+  }
+
   if (!id)
-    id = thumbnail_db_->AddFavicon(icon_url, icon_type);
-
+    id = thumbnail_db_->AddFavicon(icon_url, requested_size, icon_type);
   // Set the image data.
   thumbnail_db_->SetFavicon(id, data, Time::Now());
 
-  SetFaviconMapping(page_url, id, icon_type);
+  SetFaviconMapping(page_url, id, requested_size, icon_type);
 }
 
 void HistoryBackend::SetFaviconMapping(const GURL& page_url,
                                        FaviconID id,
+                                       const gfx::Size& pixel_size,
                                        IconType icon_type) {
   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.
   for (history::RedirectList::const_iterator i(redirects->begin());
        i != redirects->end(); ++i) {
     FaviconID replaced_id;
-    if (AddOrUpdateIconMapping(*i, id, icon_type, &replaced_id)) {
+    if (AddOrUpdateIconMapping(*i, id, pixel_size, 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);
 
       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,
+                                            const gfx::Size& pixel_size,
                                             IconType icon_type,
                                             FaviconID* replaced_icon) {
   *replaced_icon = 0;
   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.
+  // b. If the given icon size and the matched icon size are the same and
+  //    either:
+  //      i.  the given icon type and the matched icon type are the same.
+  //      ii. the given icon type and matched icon type are not the same
+  //          but either of them is ICON_TOUCH or ICON_PRECOMPOSED_TOUCH.

[stevenjb, 2012/08/01 00:00:31]

Do we care about differentiating between touch and touch_precomposed for Chrome?
i.e. could we just treat those as the same type?

[pkotwicz, 2012/08/02 19:47:52]

I'll look into this.

+  //    update the IconMapping.
+  // c. If there are no matches, but the number of mappings for page url
+  //    has exceeded kMaxFaviconsPerPage, update an arbitrary mapping. This
+  //    is a last line of defense and this property should be enforced
+  //    elsewhere as well.

[stevenjb, 2012/08/01 00:00:31]

This seems a bit arbitrary. Have we considered defining a fixed set of icon
sizes (which might vary by platform) and constraining pixel_size to that list?

[pkotwicz, 2012/08/02 19:47:52]

We are actually going to be doing this too.
It still feels like a good idea to constrain the maximum amount of bitmaps we
store for a single page.

+  // d. Otherwise add an icon mapping.
+
   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;
+    if (pixel_size == m->icon_pixel_size) {
+      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;
+      }
     }
   }
+
+  if (icon_mappings.size() >= kMaxFaviconsPerPage) {
+    thumbnail_db_->UpdateIconMapping(icon_mappings[0].mapping_id, id);
+    *replaced_icon = icon_mappings[0].icon_id;

[stevenjb, 2012/08/01 00:00:31]

Shouldn't this logic be in AddIconMapping? It's not clear here what |mapping_id|
refers to?

[pkotwicz, 2012/08/02 19:47:52]

I would rather not add extra SQL queries to AddIconMapping which is why I put
this logic here.
It is true that the picking of icon_mappings[0].mapping_id is completely
arbitrary. (Though all |icon_mappings| are mappings for the same page) However,
I don't think there are any negative consequences of this being the case.

+    return true;
+  }
+
   thumbnail_db_->AddIconMapping(page_url, id);
   return true;
 }
 
 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
@@ skipping 369 matching lines @@
 }
 
 BookmarkService* HistoryBackend::GetBookmarkService() {
   if (bookmark_service_)
     bookmark_service_->BlockTillLoaded();
   return bookmark_service_;
 }
 
 bool HistoryBackend::GetFaviconFromDB(
     const GURL& page_url,
+    const gfx::Size& pixel_size,
     int icon_types,
     FaviconData* favicon) {
   DCHECK(favicon);
 
   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)) {
+  if (thumbnail_db_->GetIconMappingsForPageURL(page_url, icon_types,
+                                               &icon_mappings)) {
+    std::vector<FaviconIDAndSize> favicon_id_size_listing;
     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)) {
-        success = true;
-        break;
-      }
+      FaviconIDAndSize favicon_id_size;
+      favicon_id_size.icon_id = i->icon_id;
+      favicon_id_size.icon_size = i->icon_pixel_size;
+      favicon_id_size_listing.push_back(favicon_id_size);
     }
+    FaviconID favicon_id = GetFaviconID(favicon_id_size_listing, pixel_size);
+
+    if (favicon_id && GetFaviconFromDB(favicon_id, favicon))
+      success = true;
   }
   UMA_HISTOGRAM_TIMES("History.GetFavIconFromDB",  // historical name
                       TimeTicks::Now() - beginning_time);
   return success;
 }
 
 bool HistoryBackend::GetFaviconFromDB(FaviconID favicon_id,
                                       FaviconData* favicon) {
   Time last_updated;
   scoped_refptr<base::RefCountedBytes> data = new base::RefCountedBytes();
 
+  favicon->known_icon = true;
   if (!thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data->data(),
-                                 &favicon->icon_url, &favicon->icon_type))
+      &favicon->icon_url, &favicon->requested_size, &favicon->icon_type))
     return false;
 
   favicon->expired = (Time::Now() - last_updated) >
       TimeDelta::FromDays(kFaviconRefetchDays);
-  favicon->known_icon = true;
-  favicon->image_data = data;
+  favicon->bitmap_data = data;
   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