Move encoding SkBitmaps to PNG out of FaviconService::SetFavicons() and onto the history thread

chrome/browser/history/history_service.h

Index: chrome/browser/history/history_service.h
diff --git a/chrome/browser/history/history_service.h b/chrome/browser/history/history_service.h
index 02cf82b368bc990404e5ab40929d7a6f5414b3c2..8ac73dfc40029366d679970d8a9f21ca2451aab9 100644
--- a/chrome/browser/history/history_service.h
+++ b/chrome/browser/history/history_service.h
@@ -45,6 +45,7 @@ class PageUsageData;
 class PageUsageRequest;
 class Profile;
 struct ImportedFaviconUsage;
+class SkBitmap;
 
 namespace base {
 class FilePath;
@@ -696,21 +697,16 @@ class HistoryService : public content::NotificationObserver,
                     scoped_refptr<base::RefCountedMemory> bitmap_data,
                     const gfx::Size& pixel_size);
 
-  // Used by the FaviconService to set the favicons for a page on the history
-  // backend.
-  // |favicon_bitmap_data| replaces all the favicon bitmaps mapped to
-  // |page_url|.
-  // |expired| and |icon_type| fields in FaviconBitmapData are ignored.
-  // Use MergeFavicon() if |favicon_bitmap_data| is incomplete, and favicon
-  // bitmaps in the database should be preserved if possible. For instance,
-  // favicon bitmaps from sync are 1x only. MergeFavicon() is used to avoid
-  // deleting the 2x favicon bitmap if it is present in the history backend.
-  // See HistoryBackend::ValidateSetFaviconsParams() for more details on the
-  // criteria for |favicon_bitmap_data| to be valid.
-  void SetFavicons(const GURL& page_url,
-                   favicon_base::IconType icon_type,
-                   const std::vector<favicon_base::FaviconRawBitmapData>&
-                       favicon_bitmap_data);
+  // Used by the FaviconService to replace all of the favicon bitmaps mapped to
+  // |page_url| for |icon_type|.
+  // Use MergeFavicon() if |bitmaps| is incomplete, and favicon bitmaps in the
+  // database should be preserved if possible. For instance, favicon bitmaps
+  // from sync are 1x only. MergeFavicon() is used to avoid deleting the 2x
+  // favicon bitmap if it is present in the history backend.
+  void SetFavicon(const GURL& page_url,

[sky, 2014/09/02 17:57:42]

I think the plural is more typical of what we use, eg SetFaviconsOutOfDate.
CloneFavicons...

[pkotwicz, 2014/09/03 04:15:08]

SetFavicon() makes more sense to me. (It matches with the names of the history
database tables)
There is one favicon per icon URL.
There are multiple favicons (each with its own icon URL) per page URL. This is
the case for SetFaviconsOutOfDateForPage(), CloneFavicons() etc.
There are multiple favicon bitmaps per icon URL

[sky, 2014/09/03 15:34:08]

I get what you're saying, but I still think plural is more consistent with with
the rest of this class.

[pkotwicz, 2014/09/03 20:18:56]

Done.

+                  favicon_base::IconType icon_type,
+                  const GURL& icon_url,
+                  const std::vector<SkBitmap>& bitmaps);
 
   // Used by the FaviconService to mark the favicon for the page as being out
   // of date.