Add Notification images (Blink + desktop implementation)

third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp

Index: third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp
diff --git a/third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp b/third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp
index fcf1e910a021523870f1bac101ab4c9813de34b2..5380a3ed7f09c2296f83b78cd1a663678a5900d0 100644
--- a/third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp
+++ b/third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp
@@ -13,21 +13,25 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "wtf/CurrentTime.h"
 #include "wtf/Threading.h"
+#include <cmath>
 
 namespace blink {
 
 namespace {
 
-// Scales down |image| to fit within |maxSizePx| if its width or height is
-// larger than |maxSizePx| and returns the result. Otherwise does nothing and
-// returns |image| unchanged.
+// Scales down |image| to fit within |maxWidthPx|x|maxHeightPx| if it is larger
+// and returns the result. Otherwise does nothing and returns |image| unchanged.
 // TODO(mvanouwerkerk): Explore doing the scaling on a background thread.
-SkBitmap scaleDownIfNeeded(const SkBitmap& image, int maxSizePx)
+SkBitmap scaleDownIfNeeded(const SkBitmap& image, int maxWidthPx, int maxHeightPx)
 {
-    if (image.width() > maxSizePx || image.height() > maxSizePx) {
+    if (image.width() > maxWidthPx || image.height() > maxHeightPx) {
+        double scale = std::min(
+            maxWidthPx / (float) image.width(),
+            maxHeightPx / (float) image.height());

[Peter Beverloo, 2016/08/22 20:11:07]

nit: we don't do C-style casts in Chromium

[johnme, 2016/08/23 14:59:49]

Done.

         DEFINE_THREAD_SAFE_STATIC_LOCAL(CustomCountHistogram, scaleTimeHistogram, new CustomCountHistogram("Notifications.Icon.ScaleDownTime", 1, 1000 * 10 /* 10 seconds max */, 50 /* buckets */));
         double startTime = monotonicallyIncreasingTimeMS();
-        SkBitmap scaledImage = skia::ImageOperations::Resize(image, skia::ImageOperations::RESIZE_BEST, std::min(image.width(), maxSizePx), std::min(image.height(), maxSizePx));
+        skia::ImageOperations::ResizeMethod quality = std::max(image.width(), maxWidthPx) > 640 ? skia::ImageOperations::RESIZE_GOOD : skia::ImageOperations::RESIZE_BEST;

[Peter Beverloo, 2016/08/22 20:11:07]

I have two concerns about this:

  (1) It's unclear why you are doing this. Did you do performance measurements
that indicate that we should do GOOD resizing rather than BEST for larger
images? What are the results?
  (2) The `Notifications.Icon.ScaleDownTime` histogram will now measure
inconsistent things— it's impossible to know what kind of resizing was used.
Instead, these should be two histograms if we need different qualities.

[johnme, 2016/08/23 14:59:49]

It seems we should use RESIZE_BETTER in this case based on the comment:
  // This is intended, for example, for generation of large thumbnails
  // (hundreds of pixels in each dimension) from large sources, where
  // a linear filter would produce too many artifacts but where
  // a RESIZE_HIGH might be too costly time-wise.

Indeed https://goto.google.com/wegnw shows that each resize already takes 750ms+
at the 99th percentile for Svelte users, which seems pretty damning. Based on
that and b/5292691 I'd support using RESIZE_BETTER for icons as well.

But based on our offline discussion, I've punted this change and left you a
TODO, as you seemed to be aware of easy ways of benchmarking different scaling
algorithms.

+        SkBitmap scaledImage = skia::ImageOperations::Resize(image, quality, std::lround(scale * image.width()), std::lround(scale * image.height()));
         scaleTimeHistogram.count(monotonicallyIncreasingTimeMS() - startTime);
         return scaledImage;
     }
@@ -53,8 +57,9 @@ void NotificationResourcesLoader::start(ExecutionContext* executionContext, cons
     m_started = true;
 
     size_t numActions = notificationData.actions.size();
-    m_pendingRequestCount = 2 /* icon and badge */ + numActions;
+    m_pendingRequestCount = 3 /* image, icon, badge */ + numActions;
 
+    loadImage(executionContext, notificationData.image, WTF::bind(&NotificationResourcesLoader::didLoadImage, wrapWeakPersistent(this)));
     loadImage(executionContext, notificationData.icon, WTF::bind(&NotificationResourcesLoader::didLoadIcon, wrapWeakPersistent(this)));
     loadImage(executionContext, notificationData.badge, WTF::bind(&NotificationResourcesLoader::didLoadBadge, wrapWeakPersistent(this)));
 
@@ -66,6 +71,7 @@ void NotificationResourcesLoader::start(ExecutionContext* executionContext, cons
 std::unique_ptr<WebNotificationResources> NotificationResourcesLoader::getResources() const
 {
     std::unique_ptr<WebNotificationResources> resources(new WebNotificationResources());
+    resources->image = m_image;
     resources->icon = m_icon;
     resources->badge = m_badge;
     resources->actionIcons = m_actionIcons;
@@ -95,15 +101,21 @@ void NotificationResourcesLoader::loadImage(ExecutionContext* executionContext,
     imageLoader->start(executionContext, url, std::move(imageCallback));
 }
 
+void NotificationResourcesLoader::didLoadImage(const SkBitmap& image)
+{
+    m_image = scaleDownIfNeeded(image, kWebNotificationMaxImageWidthPx, kWebNotificationMaxImageHeightPx);
+    didFinishRequest();
+}
+
 void NotificationResourcesLoader::didLoadIcon(const SkBitmap& image)
 {
-    m_icon = scaleDownIfNeeded(image, kWebNotificationMaxIconSizePx);
+    m_icon = scaleDownIfNeeded(image, kWebNotificationMaxIconSizePx, kWebNotificationMaxIconSizePx);
     didFinishRequest();
 }
 
 void NotificationResourcesLoader::didLoadBadge(const SkBitmap& image)
 {
-    m_badge = scaleDownIfNeeded(image, kWebNotificationMaxBadgeSizePx);
+    m_badge = scaleDownIfNeeded(image, kWebNotificationMaxBadgeSizePx, kWebNotificationMaxBadgeSizePx);
     didFinishRequest();
 }
 
@@ -111,7 +123,7 @@ void NotificationResourcesLoader::didLoadActionIcon(size_t actionIndex, const Sk
 {
     DCHECK_LT(actionIndex, m_actionIcons.size());
 
-    m_actionIcons[actionIndex] = scaleDownIfNeeded(image, kWebNotificationMaxActionIconSizePx);
+    m_actionIcons[actionIndex] = scaleDownIfNeeded(image, kWebNotificationMaxActionIconSizePx, kWebNotificationMaxActionIconSizePx);
     didFinishRequest();
 }