Adds support for saving favicon size into history database.

chrome/browser/history/history_backend.h

Index: chrome/browser/history/history_backend.h
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index 1bea48930eee146b956f0d45b60b95d3b51c673b..08b16c7a78c022f048ff7a94457afcba3019871e 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -23,6 +23,7 @@
 #include "chrome/browser/history/visit_tracker.h"
 #include "chrome/browser/search_engines/template_url_id.h"
 #include "sql/init_status.h"
+#include "ui/base/layout.h"
 
 class BookmarkService;
 class TestingProfile;
@@ -248,29 +249,49 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
 
   // Favicon -------------------------------------------------------------------
 
-  void GetFavicon(scoped_refptr<GetFaviconRequest> request,
-                  const GURL& icon_url,
-                  int icon_types);
+  void 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);
 
-  void GetFaviconForURL(scoped_refptr<GetFaviconRequest> request,
-                        const GURL& page_url,
-                        int icon_types);
+  void GetFaviconsForURL(
+      scoped_refptr<GetFaviconRequest> request,
+      const GURL& page_url,
+      int icon_types,
+      int desired_size_in_dip,
+      const std::vector<ui::ScaleFactor>& desired_scale_factors);
 
-  void GetFaviconForID(scoped_refptr<GetFaviconRequest> request, FaviconID id);
+  void GetFaviconForID(
+      scoped_refptr<GetFaviconRequest> request,
+      FaviconID favicon_id,
+      int desired_size_in_dip,
+      ui::ScaleFactor desired_scale_factor);
+
+  void 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);
 
-  void SetFavicon(const GURL& page_url,
-                  const GURL& icon_url,
-                  scoped_refptr<base::RefCountedMemory> data,
-                  IconType icon_type);
+  void MergeFavicon(
+      const GURL& page_url,
+      const GURL& icon_url,
+      IconType icon_type,
+      scoped_refptr<base::RefCountedMemory> bitmap_data,
+      const gfx::Size& pixel_size);
 
-  void UpdateFaviconMappingAndFetch(scoped_refptr<GetFaviconRequest> request,
-                                    const GURL& page_url,
-                                    const GURL& icon_url,
-                                    IconType icon_type);
+  void SetFavicons(
+      const GURL& page_url,
+      IconType icon_type,
+      const std::vector<FaviconBitmapData>& favicon_bitmap_data,
+      const IconURLSizesMap& icon_url_sizes);
 
-  void SetFaviconOutOfDateForPage(const GURL& page_url);
+  void SetFaviconsOutOfDateForPage(const GURL& page_url);
 
-  void CloneFavicon(const GURL& old_page_url, const GURL& new_page_url);
+  void CloneFavicons(const GURL& old_page_url, const GURL& new_page_url);
 
   void SetImportedFavicons(
       const std::vector<ImportedFaviconUsage>& favicon_usage);
@@ -459,15 +480,31 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitSource);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageArgsSource);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddVisitsSource);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetMostRecentVisits);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, RemoveVisitsSource);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, RemoveVisitsTransitions);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, MigrationVisitSource);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, MigrationIconMapping);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFaviconMapping);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddOrUpdateIconMapping);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetSameFaviconURLForTwoPages);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetMostRecentVisits);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconForURL);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
+                           SetFaviconMappingsForPageAndRedirects);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
+                           SetFaviconMappingsForPageDuplicates);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFavicons);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
+                           SetFaviconsSameFaviconURLForTwoPages);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFaviconsDeleteBitmaps);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, SetFaviconsReplaceBitmapData);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, IsMergeValid);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, MergeFaviconPageURLNotInDB);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, MergeFaviconPageURLInDB);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
+                           UpdateFaviconMappingsAndFetchMultipleIconTypes);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconsFromDBEmpty);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
+                           GetFaviconsFromDBSelectClosestMatch);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconsFromDBSingleIconURL);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconsFromDBIconType);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconsFromDBExpired);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
                            CloneFaviconIsRestrictedToSameDomain);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, QueryFilteredURLs);
@@ -579,32 +616,106 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
 
   // Favicons ------------------------------------------------------------------
 
-  // Used by both UpdateFaviconMappingAndFetch and GetFavicon.
-  // If page_url is non-null and SetFavicon has previously been invoked for
-  // icon_url the favicon url for page_url (and all redirects) is set to
-  // icon_url.
-  // Only a single type can be given in icon_type when page_url is specified.
-  void UpdateFaviconMappingAndFetchImpl(
-      const GURL* page_url,
-      const GURL& icon_url,
+  // Used by both UpdateFaviconMappingsAndFetch and GetFavicons.
+  // If |page_url| is non-null, the icon urls for |page_url| (and all
+  // redirects) are set to the subset of |icon_urls| for which icons are
+  // already stored in the database.
+  // |icon_types| can be multiple icon types only if |icon_types| is
+  // TOUCH_ICON | TOUCH_PRECOMPOSED_ICON.
+  // If multiple icon types are specified, |page_url| will be mapped to the
+  // icon URLs of the largest type available in the database.
+  void UpdateFaviconMappingsAndFetchImpl(
       scoped_refptr<GetFaviconRequest> request,
-      int icon_type);
-
-  // Sets the favicon url id for page_url to id. This will also broadcast
-  // notifications as necessary.
-  void SetFaviconMapping(const GURL& page_url,
-                         FaviconID id,
-                         IconType icon_type);
-
-  // Updates the FaviconID associated with the url of a page. If there is an
-  // existing mapping between |page_url| and |id| this does nothing and returns
-  // false. If the mapping needs to be added or updated, true is returned. If
-  // there is an existing mapping but it does not map to |id|, then the |id| of
-  // the replaced FaviconID is set in |replaced_icon_id|.
-  bool AddOrUpdateIconMapping(const GURL& page_url,
-                              FaviconID id,
-                              IconType icon_type,
-                              FaviconID* replaced_icon_id);
+      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);
+
+  // Returns true if it is valid to map |page_url| to |icon_url| and set a
+  // favicon bitmap of |pixel_size| at |icon_url|. If the merge is valid,
+  // |merge_pixel_size| is set to the pixel size to use for adding / setting
+  // the favicon bitmap for the merge to be valid.

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

Document its set to empty if new.

+  bool IsMergeValid(const GURL& page_url,
+                    const GURL& icon_url,
+                    IconType icon_type,
+                    const gfx::Size& pixel_size,
+                    gfx::Size* merge_pixe_size);
+
+  // Set the favicon bitmaps for the favicon at |icon_id|.
+  void SetFaviconBitmaps(
+      FaviconID icon_id,
+      const std::vector<FaviconBitmapData>& favicon_bitmap_data);
+
+  // Returns true if |favicon_bitmap_data| and |icon_url_sizes| passed to
+  // SetFavicons are valid.

[michaelbai, 2012/09/05 19:08:18]

Could you add comment for what the valid params are?

+  bool ValidateSetFaviconsParams(
+      const std::vector<FaviconBitmapData>& favicon_bitmap_data,
+      const IconURLSizesMap& icon_url_sizes) const;
+
+  // Sets the sizes that the thumbnail database knows that the favicon at
+  // |icon_id| is available from the web. See history_types.h for a more
+  // detailed description of FaviconSizes.
+  void SetFaviconSizes(FaviconID icon_id, const FaviconSizes& sizes);
+
+  // Returns true if there are favicons for |page_url| and one of the types in
+  // |icon_types|.
+  // |favicon_bitmap_results| is set to the favicon bitmaps which most closely
+  // match |desired_dip_size| and |desired_scale_factors|.
+  // If |icon_types| contains multiple icon types and there are several matched
+  // icon types in the database, results will only returned for a single icon
+  // type in the priority of TOUCH_PRECOMPOSED_ICON, TOUCH_ICON, and FAVICON.
+  // See the comment for GetFaviconResultsForBestMatch() for more details on how
+  // |favicon_bitmap_results| is constructed.
+  // |icon_url_sizes| is set to a mapping of all the icon URLs which are mapped
+  // to |page_url| to the sizes of the favicon bitmaps available at each icon
+  // URL on the web.
+  bool GetFaviconsFromDB(
+      const GURL& page_url,
+      int icon_types,
+      const int desired_size_in_dip,
+      const std::vector<ui::ScaleFactor>& desired_scale_factors,
+      std::vector<FaviconBitmapResult>* favicon_bitmap_results,
+      IconURLSizesMap* icon_url_sizes);
+
+  // Returns the favicon bitmaps which most closly match |desired_dip_size| and
+  // |desired_scale_factors| in |favicon_bitmap_results|. Goodness is computed
+  // on a per favicon id basis, thus all of |favicon_bitmap_results| are
+  // guaranteed to be for the same FaviconID. |favicon_bitmap_results| may not
+  // contain all the favicon bitmaps for the selected FaviconID which are in
+  // the database.
+  // Returns true on success.

[michaelbai, 2012/09/05 19:08:18]

From this comment, it is not clear to me that, 
- how many favicons could be returned?
- what's the best match?

Why don't you return all favicons, and find out the best match in
favicon_service, just like what you did for resize.

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

Updated the comment to address your concerns.

The reason that I don't return all favicons is to avoid retreiving favicons
which we don't use. (I actually had a CL up previously where I returned all
favicons).
I don't mind going back to the previous state though.

+  bool GetFaviconBitmapResultsForBestMatch(
+      const std::vector<FaviconID>& candidate_favicon_ids,
+      int desired_size_in_dip,
+      const std::vector<ui::ScaleFactor>& desired_scale_factors,
+      std::vector<FaviconBitmapResult>* favicon_bitmap_results);
+
+  // Build mapping of the icon URLs for |favicon_ids| to the sizes of the
+  // favicon bitmaps available at each icon URL on the web. Favicon bitmaps
+  // may not be cached in the thumbnail database for any of the sizes in the
+  // returned map. See history_types.h for a more detailed description of
+  // IconURLSizesMap.
+  // Returns true if map was successfully built.
+  bool BuildIconURLSizesMap(const std::vector<FaviconID>& favicon_ids,

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

nit, when you wrap wrap all arguments (like you did in
SetFaviconMappingsForPageAndRedirects).

+       IconURLSizesMap* icon_url_sizes);
+
+  // Define type used solely in SetFaviconMappingsForPageAndRedirects.
+  typedef std::map<GURL, FaviconID> IconURLFaviconIDMap;
+
+  // Maps the favicon ids in |icon_url_id_map| to |page_url| (and all
+  // redirects) for |icon_type|. This will also broadcast notifications as
+  // necessary.
+  void SetFaviconMappingsForPageAndRedirects(
+      const GURL& page_url,
+      IconType icon_type,
+      const IconURLFaviconIDMap& icon_url_id_map);
+
+  // Maps the favicon ids in |icon_url_id_map| to |page_url| for |icon_type|.
+  // Returns true if the function changed some of the mappings to |page_url|.
+  bool SetFaviconMappingsForPage(const GURL& page_url,
+                                 IconType icon_type,
+                                 const IconURLFaviconIDMap& icon_url_id_map);
 
   // Generic stuff -------------------------------------------------------------
 
@@ -648,19 +759,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   // NULL during testing.
   BookmarkService* GetBookmarkService();
 
-  // If there is a favicon for |page_url| and one of the types in |icon_types|,
-  // |favicon| is set appropriately and true is returned.
-  bool GetFaviconFromDB(
-      const GURL& page_url,
-      int icon_types,
-      std::vector<FaviconBitmapResult>* favicon_bitmap_results);
-
-  // Get favicon by the given |favicon_id|, the |favicon| is set appropriately
-  // and true if returned on success, otherwise false is returned.
-  bool GetFaviconFromDB(
-      FaviconID favicon_id,
-      std::vector<FaviconBitmapResult>* favicon_bitmap_results);
-
   // Notify any observers of an addition to the visit database.
   void NotifyVisitObservers(const VisitRow& visit);