Extend FaviconService to support fetching favicons from a Google server

components/favicon/core/favicon_service.h

 // 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.
 
 #ifndef COMPONENTS_FAVICON_CORE_FAVICON_SERVICE_H_
 #define COMPONENTS_FAVICON_CORE_FAVICON_SERVICE_H_
 
 #include <stdint.h>
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/containers/hash_tables.h"
 #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"
 #include "components/keyed_service/core/keyed_service.h"
 
 class GURL;
 
 namespace history {
 class HistoryService;
 }
 
+namespace image_fetcher {
+class ImageFetcher;
+}
+
 namespace favicon {
 
 class FaviconClient;
 
 // The favicon service provides methods to access favicons. It calls the history
 // backend behind the scenes. The callbacks are run asynchronously, even in the
 // case of an error.
 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;
 
   // We usually pass parameters with pointer to avoid copy. This function is a
   // helper to run FaviconResultsCallback with pointer parameters.
   static void FaviconResultsCallbackRunner(
       const favicon_base::FaviconResultsCallback& callback,
       const std::vector<favicon_base::FaviconRawBitmapResult>* results);
 
   //////////////////////////////////////////////////////////////////////////////
@@ skipping 77 matching lines @@
       const favicon_base::FaviconRawBitmapCallback& callback,
       base::CancelableTaskTracker* tracker);
 
   base::CancelableTaskTracker::TaskId GetFaviconForPageURL(
       const GURL& page_url,
       int icon_types,
       int desired_size_in_dip,
       const favicon_base::FaviconResultsCallback& callback,
       base::CancelableTaskTracker* tracker);
 
+  // If there are any FAVICONs for the page at |page_url| in the local cache, it
+  // uses only the local cache and never requests the icon from any Google
+  // server. If there is no icon in the cache larger than
+  // |minimum_size_in_pixels|, the result is an empty image. Otherwise, the
+  // result is a favicon rescaled to |desired_size_in_pixel|. If there are
+  // multiple icons to choose from, the one with size closest to the desired one
+  // is used (always preferring larger than |desired_size_in_pixel| over smaller
+  // ones).
+  //
+  // If there are no FAVICONs for the page at |page_url|, the function downloads
+  // the favicon for the page at |page_url| with size |desired_size_in_pixel|
+  // from a Google server. Currently, only icons of FAVICON IconType and of
+  // sizes 16, 24, 32, 48, and 64 are supported. If you 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.
+  //
+  // 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).
+  base::CancelableTaskTracker::TaskId
+  GetRawFaviconForPageURLDownloadFromGoogleServerIfNeeded(

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

Peter, in an offline discussion, you've suggested the name
GetRawFaviconForPageURLDownloadFromGoogleServerIfNeeded. Why "Raw" when it
returns an gfx::Image?  (and not RawData)

[pkotwicz, 2016/12/09 19:46:30]

The goal of this function is to get a single resolution bitmap as a opposed to a
multi resolution gfx::Image. We currently return single resolution bitmaps as
base::FaviconRawBitmapResult

I need to think some more about what the best function name would be. I
understand why you are currently using FaviconImageResult

[jkrcal, 2016/12/14 15:18:50]

Okay, thanks. Will keep it as it is, rename it in later revisions.

+      const GURL& page_url,
+      int minimum_size_in_pixels,
+      int desired_size_in_pixels,
+      std::string client_id,
+      data_use_measurement::DataUseUserData::ServiceName data_use_service_name,
+      const favicon_base::FaviconImageCallback& callback,
+      base::CancelableTaskTracker* tracker);
+
   // Set the favicon mappings to |page_url| for |icon_types| in the history
   // database.
   // Sample |icon_urls|:
   //  { ICON_URL1 -> TOUCH_ICON, known to the database,
   //    ICON_URL2 -> TOUCH_ICON, not known to the database,
   //    ICON_URL3 -> TOUCH_PRECOMPOSED_ICON, known to the database }
   // The new mappings are computed from |icon_urls| with these rules:
   // 1) Any urls in |icon_urls| which are not already known to the database are
   //    rejected.
   //    Sample new mappings to |page_url|: { ICON_URL1, ICON_URL3 }
@@ skipping 89 matching lines @@
   // Intermediate callback for GetRawFavicon() and GetRawFaviconForPageURL()
   // so that history service can deal solely with FaviconResultsCallback.
   // Resizes favicon_base::FaviconRawBitmapResult if necessary and runs
   // |callback|.
   void RunFaviconRawBitmapCallbackWithBitmapResults(
       const favicon_base::FaviconRawBitmapCallback& callback,
       int desired_size_in_pixel,
       const std::vector<favicon_base::FaviconRawBitmapResult>&
           favicon_bitmap_results);
 
+  // Intermediate callback for
+  // GetRawFaviconForPageURLDownloadFromGoogleServerIfNeeded() so that the
+  // client code can deal solely with FaviconImageCallback.
+  void RunFaviconImageCallbackOrDownloadFromGoogleFaviconServer(
+      const GURL& page_url,
+      int minimum_size_in_pixels,
+      int desired_size_in_pixels,
+      std::string client_id,
+      data_use_measurement::DataUseUserData::ServiceName data_use_service_name,
+      const favicon_base::FaviconImageCallback& callback,
+      const std::vector<favicon_base::FaviconRawBitmapResult>&
+          favicon_bitmap_results);
+
+  // Intermediate callback for
+  // RunFaviconImageCallbackOrDownloadFromGoogleFaviconServer() 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_;

[pkotwicz, 2016/12/09 19:46:30]

Is the security team OK with decoding images from the web in the browser
process? The security team has been uncomfortable with this in the past. (Hence
the existance of WebContents::DownloadImage())

If they are ok with this, that's great but I would love to know the reasoning
for this change in policy

[jkrcal, 2016/12/14 15:18:50]

componentes/image_fetcher uses an isolated process for decoding the images
(applying https://cs.chromium.org/chromium/src/chrome/browser/image_decoder.h).

[pkotwicz, 2016/12/15 01:22:27]

You are right. I see now :)

 
   DISALLOW_COPY_AND_ASSIGN(FaviconService);
 };
 
 }  // namespace favicon
 
 #endif  // COMPONENTS_FAVICON_CORE_FAVICON_SERVICE_H_