Make NotificationImageLoader::ScaleDownIfNeeded non-static private

NotificationImageLoader::ScaleDownIfNeeded is always called by the
NotificationImageLoader::DidFinishLoading() clients with the exact type stored
in NotificationImageLoader instance. We can make it private and unconditionally
call it in ::DidFinishLoading() before serving the SkBitmap to the clients.

This refactoring will ease the transition from CurrentTimeTicks() to
clock->TimeTicks() in https://crrev.com/c/1643869.

Bug: 919383
Change-Id: I8c4cb273914f976de19ea2d543c1afeac9569886
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652931
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#667976}

third_party/blink/renderer/modules/notifications/notification_image_loader.cc

@@ -53,51 +53,6 @@ NotificationImageLoader::NotificationImageLoader(Type type)
 
 NotificationImageLoader::~NotificationImageLoader() = default;
 
-// static
-SkBitmap NotificationImageLoader::ScaleDownIfNeeded(const SkBitmap& image,
-                                                    Type type) {
-  int max_width_px = 0, max_height_px = 0;
-  switch (type) {
-    case Type::kImage:
-      max_width_px = kWebNotificationMaxImageWidthPx;
-      max_height_px = kWebNotificationMaxImageHeightPx;
-      break;
-    case Type::kIcon:
-      max_width_px = kWebNotificationMaxIconSizePx;
-      max_height_px = kWebNotificationMaxIconSizePx;
-      break;
-    case Type::kBadge:
-      max_width_px = kWebNotificationMaxBadgeSizePx;
-      max_height_px = kWebNotificationMaxBadgeSizePx;
-      break;
-    case Type::kActionIcon:
-      max_width_px = kWebNotificationMaxActionIconSizePx;
-      max_height_px = kWebNotificationMaxActionIconSizePx;
-      break;
-  }
-  DCHECK_GT(max_width_px, 0);
-  DCHECK_GT(max_height_px, 0);
-  // TODO(peter): Explore doing the scaling on a background thread.
-  if (image.width() > max_width_px || image.height() > max_height_px) {
-    double scale =
-        std::min(static_cast<double>(max_width_px) / image.width(),
-                 static_cast<double>(max_height_px) / image.height());
-    TimeTicks start_time = CurrentTimeTicks();
-    // TODO(peter): Try using RESIZE_BETTER for large images.
-    SkBitmap scaled_image = skia::ImageOperations::Resize(
-        image, skia::ImageOperations::RESIZE_BEST,
-        static_cast<int>(std::lround(scale * image.width())),
-        static_cast<int>(std::lround(scale * image.height())));
-    NOTIFICATION_HISTOGRAM_COUNTS(
-        LoadScaleDownTime, type,
-        base::saturated_cast<base::HistogramBase::Sample>(
-            (CurrentTimeTicks() - start_time).InMilliseconds()),
-        1000 * 10 /* 10 seconds max */);
-    return scaled_image;
-  }
-  return image;
-}
-
 void NotificationImageLoader::Start(ExecutionContext* context,
                                     const KURL& url,
                                     ImageCallback image_callback) {
@@ -167,7 +122,8 @@ void NotificationImageLoader::DidFinishLoading(uint64_t resource_identifier) {
       // The |ImageFrame*| is owned by the decoder.
       ImageFrame* image_frame = decoder->DecodeFrameBufferAtIndex(0);
       if (image_frame) {
-        std::move(image_callback_).Run(image_frame->Bitmap());
+        std::move(image_callback_)
+            .Run(ScaleDownIfNeeded(image_frame->Bitmap()));
         return;
       }
     }
@@ -198,4 +154,47 @@ void NotificationImageLoader::RunCallbackWithEmptyBitmap() {
   std::move(image_callback_).Run(SkBitmap());
 }
 
+SkBitmap NotificationImageLoader::ScaleDownIfNeeded(const SkBitmap& image) {
+  int max_width_px = 0, max_height_px = 0;
+  switch (type_) {
+    case Type::kImage:
+      max_width_px = kWebNotificationMaxImageWidthPx;
+      max_height_px = kWebNotificationMaxImageHeightPx;
+      break;
+    case Type::kIcon:
+      max_width_px = kWebNotificationMaxIconSizePx;
+      max_height_px = kWebNotificationMaxIconSizePx;
+      break;
+    case Type::kBadge:
+      max_width_px = kWebNotificationMaxBadgeSizePx;
+      max_height_px = kWebNotificationMaxBadgeSizePx;
+      break;
+    case Type::kActionIcon:
+      max_width_px = kWebNotificationMaxActionIconSizePx;
+      max_height_px = kWebNotificationMaxActionIconSizePx;
+      break;
+  }
+  DCHECK_GT(max_width_px, 0);
+  DCHECK_GT(max_height_px, 0);
+  // TODO(peter): Explore doing the scaling on a background thread.
+  if (image.width() > max_width_px || image.height() > max_height_px) {
+    double scale =
+        std::min(static_cast<double>(max_width_px) / image.width(),
+                 static_cast<double>(max_height_px) / image.height());
+    TimeTicks start_time = CurrentTimeTicks();
+    // TODO(peter): Try using RESIZE_BETTER for large images.
+    SkBitmap scaled_image = skia::ImageOperations::Resize(
+        image, skia::ImageOperations::RESIZE_BEST,
+        static_cast<int>(std::lround(scale * image.width())),
+        static_cast<int>(std::lround(scale * image.height())));
+    NOTIFICATION_HISTOGRAM_COUNTS(
+        LoadScaleDownTime, type_,
+        base::saturated_cast<base::HistogramBase::Sample>(
+            (CurrentTimeTicks() - start_time).InMilliseconds()),
+        1000 * 10 /* 10 seconds max */);
+    return scaled_image;
+  }
+  return image;
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/notifications/notification_image_loader.h

@@ -40,10 +40,6 @@ class MODULES_EXPORT NotificationImageLoader final
   explicit NotificationImageLoader(Type type);
   ~NotificationImageLoader() override;
 
-  // Scales down |image| according to its type and returns result. If it is
-  // already small enough, |image| is returned unchanged.
-  static SkBitmap ScaleDownIfNeeded(const SkBitmap& image, Type type);
-
   // Asynchronously downloads an image from the given url, decodes the loaded
   // data, and passes the bitmap to the callback. Times out if the load takes
   // too long and ImageCallback is invoked with an empty bitmap.
@@ -67,6 +63,10 @@ class MODULES_EXPORT NotificationImageLoader final
   }
 
  private:
+  // Scales down |image| according to its |type_| and returns result. If it is
+  // already small enough, |image| is returned unchanged.
+  SkBitmap ScaleDownIfNeeded(const SkBitmap& image);
+
   void RunCallbackWithEmptyBitmap();
 
   Type type_;

third_party/blink/renderer/modules/notifications/notification_resources_loader.cc

@@ -93,29 +93,25 @@ void NotificationResourcesLoader::LoadImage(
 }
 
 void NotificationResourcesLoader::DidLoadImage(const SkBitmap& image) {
-  image_ = NotificationImageLoader::ScaleDownIfNeeded(
-      image, NotificationImageLoader::Type::kImage);
+  image_ = image;
   DidFinishRequest();
 }
 
 void NotificationResourcesLoader::DidLoadIcon(const SkBitmap& image) {
-  icon_ = NotificationImageLoader::ScaleDownIfNeeded(
-      image, NotificationImageLoader::Type::kIcon);
+  icon_ = image;
   DidFinishRequest();
 }
 
 void NotificationResourcesLoader::DidLoadBadge(const SkBitmap& image) {
-  badge_ = NotificationImageLoader::ScaleDownIfNeeded(
-      image, NotificationImageLoader::Type::kBadge);
+  badge_ = image;
   DidFinishRequest();
 }
 
 void NotificationResourcesLoader::DidLoadActionIcon(wtf_size_t action_index,
                                                     const SkBitmap& image) {
   DCHECK_LT(action_index, action_icons_.size());
 
-  action_icons_[action_index] = NotificationImageLoader::ScaleDownIfNeeded(
-      image, NotificationImageLoader::Type::kActionIcon);
+  action_icons_[action_index] = image;
   DidFinishRequest();
 }