[LargeIconService] Allow decoding of images in the service

components/favicon/core/large_icon_service.cc

Index: components/favicon/core/large_icon_service.cc
diff --git a/components/favicon/core/large_icon_service.cc b/components/favicon/core/large_icon_service.cc
index 7d519677c51780055520e90d6e9a6f3a0217b1ff..1bac3214b5bf9079cf6f601a8a9570e9c56abd3e 100644
--- a/components/favicon/core/large_icon_service.cc
+++ b/components/favicon/core/large_icon_service.cc
@@ -21,6 +21,7 @@
 #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"
@@ -232,7 +233,8 @@ LargeIconService::LargeIconService(
     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);
@@ -265,6 +267,20 @@ base::CancelableTaskTracker::TaskId
       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,
@@ -293,4 +309,31 @@ void LargeIconService::
                  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