Request the icon of a Web Notification in the renderer process.

chrome/browser/notifications/desktop_notification_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 "chrome/browser/notifications/desktop_notification_service.h"
 
 #include "base/bind.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 145 matching lines @@
     content::RenderFrameHost* render_frame_host,
     scoped_ptr<content::DesktopNotificationDelegate> delegate,
     base::Closure* cancel_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const GURL& origin = params.origin;
   NotificationObjectProxy* proxy =
       new NotificationObjectProxy(render_frame_host, delegate.Pass());
 
   base::string16 display_source = DisplayNameForOriginInProcessId(
       origin, render_frame_host->GetProcess()->GetID());
-  Notification notification(origin, params.icon_url, params.title,
-      params.body, params.direction, display_source, params.replace_id,
-      proxy);
+
+  // TODO(peter): gfx::Image only provides an API to convert a SkBitmap to an
+  // image using 1x scale. While the Views system will resize is for the
+  // appropriate resolution, doing so immediately should result in icons of

[dewittj, 2014/09/18 17:46:49]

What do you mean immediately?

Also gfx::Image does provide a way to set multiple scale factors, it's:

gfx::ImageSkia skia(gfx::ImageSkiaRep(icon_bitmap, 1.0f));
skia.AddRepresentation(gfx::ImageSkiaRep(icon_2x_bitmap, 2.0f));
...
gfx::Image result(skia)

[Peter Beverloo, 2014/09/19 14:15:48]

Ah, that's good to know, however, I don't think that's how we should implement
this when we support multiple image resolutions for Web Notifications, as it
would imply requesting all the images. Instead, we should download an SkBitmap
of the right format from the renderer process, and pass along the used scale
factor so that we can use that ImageSkiaRep constructor.

I've updated the comment.

[dewittj, 2014/09/24 18:28:44]

I don't think the "right format" is so simple, since it can change over time;
users can change the pixel density on their Chromebooks with a key combo!  But
still fine for now.

Also, I've been looking into this for the extension API, see
https://codereview.chromium.org/354733002/

+  // higher quality.
+  Notification notification(message_center::NOTIFICATION_TYPE_SIMPLE,
+                            origin,
+                            params.title,
+                            params.body,

[Mike West, 2014/09/18 19:48:42]

This is a huge signature.

It looks like you're passing in a number of `params` attributes. Perhaps a
future refactoring could just pass in a const ref to the params object itself?

[Peter Beverloo, 2014/09/19 14:15:48]

We're trying to trim down the number of Notification constructors to make
upcoming refactorings a bit easier. There aren't many call-sites, so the noise
is fine for now.

In the longer run we could consider doing this, but the struct is going to
include other data which does not relate to the Notification class as well.

+                            gfx::Image::CreateFrom1xBitmap(params.icon),
+                            blink::WebTextDirectionDefault,
+                            message_center::NotifierId(origin),
+                            display_source,
+                            params.replace_id,
+                            message_center::RichNotificationData(),
+                            proxy);
 
   // The webkit notification doesn't timeout.
   notification.set_never_timeout(true);
 
   g_browser_process->notification_ui_manager()->Add(notification, profile_);
   if (cancel_callback)
     *cancel_callback = base::Bind(&CancelNotification, proxy->id());
 
   DesktopNotificationProfileUtil::UsePermission(profile_, origin);
 }
@@ skipping 162 matching lines @@
   // Tell the IO thread that this extension's permission for notifications
   // has changed.
   extensions::InfoMap* extension_info_map =
       extensions::ExtensionSystem::Get(profile_)->info_map();
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&extensions::InfoMap::SetNotificationsDisabled,
                  extension_info_map, notifier_id.id, !enabled));
 #endif
 }