[Refactor] Change FaviconService API to take in a desired pixel size

components/bookmarks/browser/bookmark_client.h

Index: components/bookmarks/browser/bookmark_client.h
diff --git a/components/bookmarks/browser/bookmark_client.h b/components/bookmarks/browser/bookmark_client.h
index 192d75ad91a5441b86135aea846b436df78b7670..2a81fcc93b46522f94bf10a4f838f0f60e19dd1a 100644
--- a/components/bookmarks/browser/bookmark_client.h
+++ b/components/bookmarks/browser/bookmark_client.h
@@ -39,18 +39,24 @@ class BookmarkClient : public KeyedService {
   // Returns true if the embedder favors touch icons over favicons.
   virtual bool PreferTouchIcon();
 
-  // Requests the favicon of any of |icon_types| whose pixel sizes most
-  // closely match |desired_size_in_dip| (if value is 0, the largest favicon
-  // is returned) and desired scale factor for |page_url|. |callback| is run
-  // when the bits have been fetched. |icon_types| can be any combination of
-  // IconType value, but only one icon will be returned.
-  virtual base::CancelableTaskTracker::TaskId GetFaviconImageForURL(
+  // Requests a multi-resolution favicon from the history cache of type
+  // favicon_base::FAVICON for the web page at |page_url|. The retrieved
+  // data is resized such that the image passed to |callback| has
+  // gfx::kFaviconSize width and height.
+  // |callback| is run when the favicon has been fetched.
+  virtual base::CancelableTaskTracker::TaskId GetFaviconImageForPageURL(
       const GURL& page_url,
-      int icon_types,
-      int desired_size_in_dip,
       const favicon_base::FaviconImageCallback& callback,
       base::CancelableTaskTracker* tracker);
 
+  // Requests the largest favicon of type favicon_base::TOUCH_ICON in the

[sky, 2014/07/10 00:03:39]

Can we pass in favicon_base::IconType so that we only have one method?

[pkotwicz, 2014/07/11 01:23:08]

The main reason that I split the method into two is because it was confusing to
explain that for a request of
favicon_base::FAVICON the returned gfx::Image is a multi-resolution 16x16 DIP
favicon
favicon_base::TOUCH_ICON the returned gfx::Image is a single resolution image
which is of the largest size available in the database.

[sky, 2014/07/11 16:26:51]

This interface is very narrowly used, so I'm fine with cryptic wording.

[pkotwicz, 2014/07/11 18:47:44]

Done!

+  // history cache for the web page at |page_url|.
+  // |callback| is run when the favicon has been fetched.
+  virtual base::CancelableTaskTracker::TaskId GetTouchFaviconImageForPageURL(
+     const GURL& page_url,
+     const favicon_base::FaviconImageCallback& callback,
+     base::CancelableTaskTracker* tracker);
+
   // Returns true if the embedder supports typed count for URL.
   virtual bool SupportsTypedCountForNodes();