Extend FaviconService to support fetching favicons from a Google server

components/favicon/core/favicon_service.h

Index: components/favicon/core/favicon_service.h
diff --git a/components/favicon/core/favicon_service.h b/components/favicon/core/favicon_service.h
index 9585934169f21c7407aff49adb9fa1718a3a30af..dff48cd5a13a28ecfa1c0e330174009f5edd18f4 100644
--- a/components/favicon/core/favicon_service.h
+++ b/components/favicon/core/favicon_service.h
@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/callback.h"
@@ -15,6 +16,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/favicon_base/favicon_callback.h"
 #include "components/favicon_base/favicon_types.h"
 #include "components/favicon_base/favicon_usage_data.h"
@@ -26,6 +28,10 @@ namespace history {
 class HistoryService;
 }
 
+namespace image_fetcher {
+class ImageFetcher;
+}
+
 namespace favicon {
 
 class FaviconClient;
@@ -37,7 +43,8 @@ class FaviconService : public KeyedService {
  public:
   // The FaviconClient must outlive the constructed FaviconService.
   FaviconService(std::unique_ptr<FaviconClient> favicon_client,
-                 history::HistoryService* history_service);
+                 history::HistoryService* history_service,
+                 std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher);
 
   ~FaviconService() override;
 
@@ -135,6 +142,36 @@ class FaviconService : public KeyedService {
       const favicon_base::FaviconResultsCallback& callback,
       base::CancelableTaskTracker* tracker);
 
+  // Downloads the favicon for the page at |page_url| with size
+  // |desired_size_in_pixel| from a Google server. Currently, only sizes 16, 24,
+  // 32, 48, and 64 are supported and only FAVICON IconType is supported. If you

[stkhapugin, 2016/09/28 16:12:18]

I know this may not be the best place to ask, but on iOS we use up to 144x144
icons. Can we request higher-resolution favicons somehow? 

[jkrcal, 2016/12/09 13:57:44]

Having another function for fetching larger APPLE_TOUCH_ICONS will be part of
follow-up work (when we move to the new favicon service mentioned above).

+  // specify any different size < 64, the smallest larger size is fetched. If
+  // you set |desired_size_in_pixel| > 64, the 64px size is fetched. If the
+  // fetched size is not available, the largest available size is upscaled to
+  // the fetched size (by the Google server, at indexing time). If no icon is
+  // available for the given url and size, the callback is called with a result
+  // with an empty image.
+  //
+  // This function does not check the local cache, it always directly calls the
+  // server. However, if the request is successful, it stores the resulting
+  // favicon in the cache for future use (possibly overwriting previous image in
+  // the cache for the given url). The fetched favicon is immediately marked as
+  // expired so that when the user visits |page_url| for the next time, the
+  // favicon is re-downloaded.
+  //
+  // The Google favicon server requires to specify a string |client_id|
+  // used to identify your use case. Please negotiate a quota for your
+  // |client_id| before making use of this function in production (see
+  // g3doc/social/favicon/); use an arbitrary string in development builds. Use
+  // |data_use_service_name| to count the fetches to your data use (or
+  // NOT_TAGGED if not interested).
+  void DownloadFromGoogleServerFaviconImageForPageURL(

[stkhapugin, 2016/09/28 16:12:18]

It looks like every other async call here returns a TaskId to cancel unnecessary
async operations. Can you make this cancellable as well? 

[jkrcal, 2016/12/09 13:57:44]

Done.

+      const GURL& page_url,
+      int desired_size_in_pixel,
+      std::string client_id,
+      data_use_measurement::DataUseUserData::ServiceName data_use_service_name,
+      const favicon_base::FaviconImageCallback& callback);
+
   // Set the favicon mappings to |page_url| for |icon_types| in the history
   // database.
   // Sample |icon_urls|:
@@ -244,9 +281,18 @@ class FaviconService : public KeyedService {
       const std::vector<favicon_base::FaviconRawBitmapResult>&
           favicon_bitmap_results);
 
+  // Intermediate callback for DownloadFromGoogleServerFaviconImageForPageURL()
+  // so that the client code can deal solely with FaviconImageCallback.
+  void StoreFaviconFromGoogleServiceAndRunFaviconImageCallback(
+      const favicon_base::FaviconImageCallback& callback,
+      const GURL& domain_url,
+      const std::string& icon_url,
+      const gfx::Image& image);
+
   base::hash_set<MissingFaviconURLHash> missing_favicon_urls_;
   std::unique_ptr<FaviconClient> favicon_client_;
   history::HistoryService* history_service_;
+  std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(FaviconService);
 };