[LargeIconService] Allow decoding of images in the service

components/favicon/core/large_icon_service.cc

 // Copyright 2015 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/large_icon_service.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/stringprintf.h"
 #include "base/task_runner.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon_base/fallback_icon_style.h"
 #include "components/favicon_base/favicon_types.h"
 #include "components/favicon_base/favicon_util.h"
+#include "components/image_fetcher/core/image_decoder.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/size.h"
 #include "url/url_canon.h"
 
 namespace favicon {
 namespace {
 
 const char kGoogleServerV2RequestFormat[] =
     "https://t0.gstatic.com/"
@@ skipping 191 matching lines @@
 }
 
 }  // namespace
 
 LargeIconService::LargeIconService(
     FaviconService* favicon_service,
     const scoped_refptr<base::TaskRunner>& background_task_runner,
     std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher)
     : favicon_service_(favicon_service),
       background_task_runner_(background_task_runner),
-      image_fetcher_(std::move(image_fetcher)) {
+      image_fetcher_(std::move(image_fetcher)),
+      weak_ptr_factory_(this) {
   large_icon_types_.push_back(favicon_base::IconType::FAVICON);
   large_icon_types_.push_back(favicon_base::IconType::TOUCH_ICON);
   large_icon_types_.push_back(favicon_base::IconType::TOUCH_PRECOMPOSED_ICON);
 }
 
 LargeIconService::~LargeIconService() {
 }
 
 base::CancelableTaskTracker::TaskId
     LargeIconService::GetLargeIconOrFallbackStyle(
@@ skipping 12 matching lines @@
   // TODO(beaudoin): For now this is just a wrapper around
   //   GetLargestRawFaviconForPageURL. Add the logic required to select the best
   //   possible large icon. Also add logic to fetch-on-demand when the URL of
   //   a large icon is known but its bitmap is not available.
   return favicon_service_->GetLargestRawFaviconForPageURL(
       page_url, large_icon_types_, min_source_size_in_pixel,
       base::Bind(&LargeIconWorker::OnIconLookupComplete, worker),
       tracker);
 }
 
+base::CancelableTaskTracker::TaskId
+LargeIconService::GetLargeIconImageOrFallbackStyle(
+    const GURL& page_url,
+    int min_source_size_in_pixel,
+    int desired_size_in_pixel,
+    const favicon_base::LargeIconImageCallback& callback,
+    base::CancelableTaskTracker* tracker) {
+  return GetLargeIconOrFallbackStyle(
+      page_url, min_source_size_in_pixel, desired_size_in_pixel,
+      base::Bind(&LargeIconService::DecodeRawBitmap,
+                 weak_ptr_factory_.GetWeakPtr(), callback),
+      tracker);
+}
+
 void LargeIconService::
     GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache(
         const GURL& page_url,
         int min_source_size_in_pixel,
         const base::Callback<void(bool success)>& callback) {
   DCHECK_LE(0, min_source_size_in_pixel);
 
   const GURL trimmed_page_url = TrimPageUrlForGoogleServer(page_url);
   const GURL icon_url =
       GetIconUrlForGoogleServerV2(trimmed_page_url, min_source_size_in_pixel);
 
   // Do not download if the URL is invalid after trimming, or there is a
   // previous cache miss recorded for |icon_url|.
   if (!trimmed_page_url.is_valid() || !image_fetcher_ ||
       favicon_service_->WasUnableToDownloadFavicon(icon_url)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                   base::Bind(callback, false));
     return;
   }
 
   image_fetcher_->SetDataUseServiceName(
       data_use_measurement::DataUseUserData::LARGE_ICON_SERVICE);
   image_fetcher_->StartOrQueueNetworkRequest(
       icon_url.spec(), icon_url,
       base::Bind(&OnFetchIconFromGoogleServerComplete, favicon_service_,
                  page_url, callback));
 }
 
+void LargeIconService::DecodeRawBitmap(
+    const favicon_base::LargeIconImageCallback& callback,
+    const favicon_base::LargeIconResult& raw_result) {
+  if (!raw_result.bitmap.is_valid()) {
+    // Make a copy of the icon style as it cannot be moved out from a const ref.
+    favicon_base::LargeIconImageResult result(
+        new favicon_base::FallbackIconStyle(
+            *raw_result.fallback_icon_style.get()));
+    callback.Run(result);
+    return;
+  }
+  image_fetcher_->GetImageDecoder()->DecodeImage(
+      std::string(
+          reinterpret_cast<const char*>(raw_result.bitmap.bitmap_data->front()),
+          raw_result.bitmap.bitmap_data->size()),
+      gfx::Size(),
+      base::Bind(&LargeIconService::DecodingRawBitmapFinished,
+                 base::Unretained(this), callback));

[pkotwicz, 2017/03/30 19:14:23]

Can the decoding occur in the service worker?

Also can we avoid:
1) decoding the PNG to SkBitmap in ResizeLargeIconOnBackgroundThreadIfValid()
2) encoding the SkBitmap to PNG in ResizeLargeIconOnBackgroundThreadIfValid()
3) decoding the PNG to SkBitmap in DecodingRawBitmapFinished()

- You can pass to LargeIconWorker an enum to indicate which type of image you
want and use gfx::Image to hold either a gfx::ImageSkia or PNG data.

Please don't change LargeIconService::GetLargeIconOrFallbackStyle() to use a
LargeIconImageCallback (instead of having both GetLargeIconOrFallbackStyle() and
GetLargeIconImageOrFallbackStyle()). Large icons are large and take a while to
convert from PNG data to SkBitmap (I don't want to make it easier to overlook
the magical PNG image decoding which occurs in gfx::Image)

I would want to rename your new method to
LargeIconService::GetLargeNativeImageOrFallbackStyle() and have the callback
return a gfx::NativeImage. However, this change would likely be super
controversial so I won't ask you to make it

[jkrcal, 2017/03/31 12:31:11]

ImageDecoder is not happy about being in the service worker. On top of that,
this way of decoding caused weird lags in the UI.

Therefore, I've gotten rid of the ImageDecoder and now use the previously
implemented SkImage decoding. The difference is that I return early if resizing
or encoding is not needed.

I am not familiar with gfx::NativeImage so I suggest to post-pone this idea for
later.

[pkotwicz, 2017/04/02 20:46:52]

I was suggesting postponing switching to gfx::NativeImage.

+}
+
+void LargeIconService::DecodingRawBitmapFinished(
+    const favicon_base::LargeIconImageCallback& callback,
+    const gfx::Image& image) {
+  favicon_base::LargeIconImageResult result(image);
+  callback.Run(result);
+}
+
 }  // namespace favicon