Extend FaviconService to support fetching favicons from a Google server

components/favicon/core/favicon_service.cc

 // 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.
 
 #include "components/favicon/core/favicon_service.h"
 
 #include <stddef.h>
 #include <cmath>
 #include <utility>
 
 #include "base/hash.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "components/favicon/core/favicon_client.h"
 #include "components/favicon_base/favicon_util.h"
 #include "components/favicon_base/select_favicon_frames.h"
 #include "components/history/core/browser/history_service.h"
+#include "components/image_fetcher/image_fetcher.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
 namespace favicon {
 namespace {
 
+const char kGoogleFaviconServiceRequestFormat[] =
+    "https://s2.googleusercontent.com/s2/"
+    "favicons?domain=%s&src=%s&sz=%d&alt=404";
+
+const int kGoogleFaviconServiceSupportedSizes[] = {16, 24, 32, 48, 64};
+
+int GetGoogleFaviconServiceSupportedSize(int arbitrary_size) {
+  // Take the smallest size larger than arbitrary_size.
+  for (int size : kGoogleFaviconServiceSupportedSizes) {
+    if (size >= arbitrary_size)
+      return size;
+  }
+  // Or at least the largest available size.
+  return kGoogleFaviconServiceSupportedSizes
+      [arraysize(kGoogleFaviconServiceSupportedSizes) - 1];
+}
+
+GURL GetGoogleFaviconServiceURL(GURL page_url,
+                                const std::string& client_id,
+                                int supported_size) {
+  return GURL(base::StringPrintf(kGoogleFaviconServiceRequestFormat,
+                                 page_url.spec().c_str(),
+                                 client_id.c_str(),
+                                 supported_size));
+}
+
 // Helper to run callback with empty results if we cannot get the history
 // service.
 base::CancelableTaskTracker::TaskId RunWithEmptyResultAsync(
     const favicon_base::FaviconResultsCallback& callback,
     base::CancelableTaskTracker* tracker) {
   scoped_refptr<base::SingleThreadTaskRunner> thread_runner(
       base::ThreadTaskRunnerHandle::Get());
   return tracker->PostTask(
       thread_runner.get(), FROM_HERE,
       base::Bind(callback,
                  std::vector<favicon_base::FaviconRawBitmapResult>()));
 }
 
 // Returns a vector of pixel edge sizes from |size_in_dip| and
 // favicon_base::GetFaviconScales().
 std::vector<int> GetPixelSizesForFaviconScales(int size_in_dip) {
   std::vector<float> scales = favicon_base::GetFaviconScales();
   std::vector<int> sizes_in_pixel;
   for (size_t i = 0; i < scales.size(); ++i) {
     sizes_in_pixel.push_back(std::ceil(size_in_dip * scales[i]));
   }
   return sizes_in_pixel;
 }
 
 }  // namespace
 
-FaviconService::FaviconService(std::unique_ptr<FaviconClient> favicon_client,
-                               history::HistoryService* history_service)
+FaviconService::FaviconService(
+    std::unique_ptr<FaviconClient> favicon_client,
+    history::HistoryService* history_service,
+    std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher)
     : favicon_client_(std::move(favicon_client)),
-      history_service_(history_service) {}
+      history_service_(history_service),
+      image_fetcher_(std::move(image_fetcher)) {}
 
 FaviconService::~FaviconService() {
 }
 
 // static
 void FaviconService::FaviconResultsCallbackRunner(
     const favicon_base::FaviconResultsCallback& callback,
     const std::vector<favicon_base::FaviconRawBitmapResult>* results) {
   callback.Run(*results);
 }
@@ skipping 123 matching lines @@
     base::CancelableTaskTracker* tracker) {
   TRACE_EVENT0("browser", "FaviconService::GetFaviconForPageURL");
   return GetFaviconForPageURLImpl(
       page_url,
       icon_types,
       GetPixelSizesForFaviconScales(desired_size_in_dip),
       callback,
       tracker);
 }
 
+void FaviconService::DownloadFromGoogleServerFaviconImageForPageURL(
+    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) {
+  if (!image_fetcher_.get()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(callback, favicon_base::FaviconImageResult()));
+    return;
+  }
+
+  int supported_size =
+      GetGoogleFaviconServiceSupportedSize(desired_size_in_pixel);
+  GURL image_url = GetGoogleFaviconServiceURL(page_url, client_id,
+                                              supported_size);
+
+  image_fetcher_->SetDataUseServiceName(data_use_service_name);
+  image_fetcher_->StartOrQueueNetworkRequest(

[pkotwicz, 2016/09/21 20:55:53]

I think that ImageFetcherImpl::StartOrQueueNetworkRequest() does an HTTP GET
request.

I assume that if someone malicious:
- used one of Chrome's client IDs (The client IDs will not be in the internal
portion of Chrome?)
- does millions of HTTP GET requests
That this would not affect Chrome's legitimate requests?

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

This would probably affect Chrome's legitimate requests. None of the both
favicon services teams I am in contact with seem to be too concerned about
having such a public API. The client ID is just for basic understanding of the
traffic.

Do you see any other reasonably simple mechanism that 
 - would work also for non-signed-in users and
 - would not be prone to such simple attacks?

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

I don't know of a mechanism which would not be prone to simple attacks. (I am no
security expert though)

I don't understand why we need a client ID at all (other than for one Google app
to affect another Google app's usage of the service).

In the feature that I work on (WebAPKs) we use an API key as well. I don't
understand why we are using the API key in the case of WebAPKs either

The API key is probably OK because it is "the Google Way"

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

Acknowledged.

+      image_url.spec(), image_url,
+      base::Bind(&FaviconService::
+                     StoreFaviconFromGoogleServiceAndRunFaviconImageCallback,
+                 base::Unretained(this), callback, page_url));
+}
+
 base::CancelableTaskTracker::TaskId
 FaviconService::UpdateFaviconMappingsAndFetch(
     const GURL& page_url,
     const std::vector<GURL>& icon_urls,
     int icon_types,
     int desired_size_in_dip,
     const favicon_base::FaviconResultsCallback& callback,
     base::CancelableTaskTracker* tracker) {
   if (history_service_) {
     return history_service_->UpdateFaviconMappingsAndFetch(
@@ skipping 130 matching lines @@
     const favicon_base::FaviconRawBitmapCallback& callback,
     int desired_size_in_pixel,
     const std::vector<favicon_base::FaviconRawBitmapResult>&
         favicon_bitmap_results) {
   TRACE_EVENT0("browser",
                "FaviconService::RunFaviconRawBitmapCallbackWithBitmapResults");
   callback.Run(
       ResizeFaviconBitmapResult(favicon_bitmap_results, desired_size_in_pixel));
 }
 
+void FaviconService::StoreFaviconFromGoogleServiceAndRunFaviconImageCallback(
+      const favicon_base::FaviconImageCallback& callback,
+      const GURL& page_url,
+      const std::string& icon_url,
+      const gfx::Image& image) {
+  favicon_base::FaviconImageResult image_result;
+  image_result.icon_url = GURL(icon_url);
+  image_result.image = image;
+  if (image_result.image.IsEmpty()) {

[pkotwicz, 2016/12/07 04:31:22]

Can you move this if to line 412

I am suggesting:
if (image.IsEmpty()) {
  callback.Run(base::FaviconImageResult());
}
favicon base::FaviconImageResult image_result;
...

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

Sure. It's much better this way!
I was not sure what is the contract (whether the icon_url can be empty).

+    callback.Run(image_result);
+    return;
+  }
+
+  favicon_base::SetFaviconColorSpace(&image_result.image);
+  // Store the favicon in the cache.
+  SetFavicons(page_url, image_result.icon_url,
+              favicon_base::IconType::FAVICON, image_result.image);
+  // Mark them as out-of-date so that they are refetched when we visit the
+  // original page any time in the future.
+  SetFaviconOutOfDateForPage(page_url);
+
+  callback.Run(image_result);
+}
+
 }  // namespace favicon