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

content/renderer/notification_provider.cc

Index: content/renderer/notification_provider.cc
diff --git a/content/renderer/notification_provider.cc b/content/renderer/notification_provider.cc
index c4d869c2b1caea82e31f46e5997d5e64675c4808..16227b7db0d8239ba60b7152ad5f0d6069036e4c 100644
--- a/content/renderer/notification_provider.cc
+++ b/content/renderer/notification_provider.cc
@@ -4,14 +4,21 @@
 
 #include "content/renderer/notification_provider.h"
 
+#include <vector>
+
 #include "base/strings/string_util.h"
+#include "content/child/image_decoder.h"
 #include "content/common/desktop_notification_messages.h"
 #include "content/common/frame_messages.h"
 #include "content/renderer/render_frame_impl.h"
+#include "third_party/WebKit/public/platform/Platform.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
+#include "third_party/WebKit/public/platform/WebURLLoader.h"
+#include "third_party/WebKit/public/platform/WebURLLoaderClient.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 using blink::WebDocument;
 using blink::WebNotification;
@@ -19,37 +26,190 @@ using blink::WebNotificationPresenter;
 using blink::WebSecurityOrigin;
 using blink::WebString;
 using blink::WebURL;
+using blink::WebURLError;
+using blink::WebURLLoader;
+using blink::WebURLRequest;
 using blink::WebUserGestureIndicator;
 
 namespace content {
 
-NotificationProvider::NotificationProvider(RenderFrame* render_frame)
-    : RenderFrameObserver(render_frame) {
+// NotificationProvider::IconDownloader ----------------------------------------
+
+// Downloads the icon associated with a notification and decodes the received
+// image. This must be completed before notifications are shown to the user.
+// Icon downloaders must not be re-used for multiple notifications or icons.
+class NotificationProvider::IconDownloader : public blink::WebURLLoaderClient {

[jamesr, 2014/10/06 22:56:24]

put this in its own .h/.cc file.  It's too much code to slam into another .cc
and hiding it makes it impossible to unit test (which would probably be a good
idea, if not now then further down the line)

[Peter Beverloo, 2014/10/13 19:09:30]

Moved to its own file. I've filed http://crbug.com/423039 to keep track of this.

+  typedef base::Callback<void(int, const SkBitmap&)> DownloadCompletedCallback;
+
+ public:
+  IconDownloader(int notification_id,
+                 const DownloadCompletedCallback& callback);
+
+  virtual ~IconDownloader();
+
+  // Downloads the notification's image and invokes the callback with the
+  // decoded SkBitmap when the download has succeeded, or with an empty SkBitmap
+  // in case the download has failed.
+  void Start(const WebURL& icon_url);
+
+  // Cancels the current image download. The callback will not be invoked.
+  void Cancel();
+
+  // blink::WebURLLoaderClient implementation.
+  virtual void didReceiveData(WebURLLoader* loader,
+                              const char* data,
+                              int data_length,
+                              int encoded_data_length);
+  virtual void didFinishLoading(WebURLLoader* loader,
+                                double finish_time,
+                                int64_t total_encoded_data_length);
+  virtual void didFail(WebURLLoader* loader, const WebURLError& error);
+
+  int notification_id() const {
+    return notification_id_;
+  }
+
+ private:
+  int notification_id_;
+  DownloadCompletedCallback callback_;
+
+  scoped_ptr<WebURLLoader> loader_;
+  bool completed_;
+
+  std::vector<uint8_t> buffer_;
+
+  DISALLOW_COPY_AND_ASSIGN(IconDownloader);
+};
+
+NotificationProvider::IconDownloader::IconDownloader(
+    int notification_id,
+    const DownloadCompletedCallback& callback)
+    : notification_id_(notification_id),
+      callback_(callback),
+      completed_(false) {}
+
+NotificationProvider::IconDownloader::~IconDownloader() {}
+
+void NotificationProvider::IconDownloader::Start(const WebURL& icon_url) {
+  DCHECK(!loader_);
+
+  WebURLRequest request(icon_url);
+  request.setRequestContext(WebURLRequest::RequestContextImage);
+
+  loader_.reset(blink::Platform::current()->createURLLoader());
+  loader_->loadAsynchronously(request, this);
+}
+
+void NotificationProvider::IconDownloader::Cancel() {
+  DCHECK(loader_);
+
+  completed_ = true;
+  loader_->cancel();
+}
+
+void NotificationProvider::IconDownloader::didReceiveData(
+    WebURLLoader* loader,
+    const char* data,
+    int data_length,
+    int encoded_data_length) {
+  DCHECK(!completed_);
+  DCHECK_GT(data_length, 0);
+
+  buffer_.insert(buffer_.end(), data, data + data_length);
+}
+
+void NotificationProvider::IconDownloader::didFinishLoading(
+    WebURLLoader* loader,
+    double finish_time,
+    int64_t total_encoded_data_length) {
+  DCHECK(!completed_);
+
+  SkBitmap icon;
+  if (!buffer_.empty()) {
+    ImageDecoder decoder;
+    icon = decoder.Decode(&buffer_[0], buffer_.size());
+  }
+
+  completed_ = true;
+  callback_.Run(notification_id_, icon);
 }
 
-NotificationProvider::~NotificationProvider() {
+void NotificationProvider::IconDownloader::didFail(
+    WebURLLoader* loader, const WebURLError& error) {
+  if (completed_)
+    return;
+
+  completed_ = true;
+  callback_.Run(notification_id_, SkBitmap());
 }
 
+// NotificationProvider --------------------------------------------------------
+
+NotificationProvider::NotificationProvider(RenderFrame* render_frame)
+    : RenderFrameObserver(render_frame) {}
+
+NotificationProvider::~NotificationProvider() {}
+
 bool NotificationProvider::show(const WebNotification& notification) {
-  WebDocument document = render_frame()->GetWebFrame()->document();
   int notification_id = manager_.RegisterNotification(notification);
+  if (notification.iconURL().isEmpty()) {
+    DisplayNotification(notification_id, SkBitmap());
+    return true;
+  }
+
+  scoped_ptr<IconDownloader> downloader(
+      new IconDownloader(notification_id,
+          base::Bind(&NotificationProvider::DisplayNotification,
+                     base::Unretained(this))));
+
+  downloader->Start(notification.iconURL());
+
+  pending_notifications_.push_back(downloader.release());
+  return true;
+}
+
+void NotificationProvider::DisplayNotification(int notification_id,
+                                               const SkBitmap& icon) {
+  WebDocument document = render_frame()->GetWebFrame()->document();
+  WebNotification notification;
+
+  bool found = manager_.GetNotification(notification_id, &notification);
+  CHECK(found);

[jamesr, 2014/10/06 22:56:24]

what does failing this CHECK mean? is there a security property being tested
here that we need to fail in production over?

if this is just documenting an invariant that we expect to be true here, DCHECK
it. if this is needed for security it deserves a comment explaining what's going
on

[Peter Beverloo, 2014/10/13 19:09:30]

It means that display is being requested for a non-existing notification. The
Notification data provided by this getter will be send as part of an IPC, but
it's not possible to get here through any of the normal code-paths. Changed to
if+NOTREACHED.

+
+  RemovePendingNotification(notification_id);
 
   ShowDesktopNotificationHostMsgParams params;
   params.origin = GURL(document.securityOrigin().toString());
-  params.icon_url = notification.iconURL();
+  params.icon = icon;
   params.title = notification.title();
   params.body = notification.body();
   params.direction = notification.direction();
   params.replace_id = notification.replaceId();
-  return Send(new DesktopNotificationHostMsg_Show(
-      routing_id(), notification_id, params));
+
+  Send(new DesktopNotificationHostMsg_Show(routing_id(),
+                                           notification_id,
+                                           params));
+}
+
+bool NotificationProvider::RemovePendingNotification(int notification_id) {
+  PendingNotifications::iterator iter = pending_notifications_.begin();
+  for (; iter != pending_notifications_.end(); ++iter) {

[jamesr, 2014/10/06 22:56:24]

should probably be a const_iterator since you aren't going to modify it. or just
use auto.

[Peter Beverloo, 2014/10/13 19:09:30]

pending_notifications_ is a ScopedVector, so erase() will actually modify the
values (delete them). erase() also only accepts an iterator. Using auto would
give us a NotificationIconLoader* instance, not an iterator.

+    if ((*iter)->notification_id() != notification_id)
+      continue;
+
+    pending_notifications_.erase(iter);
+    return true;
+  }
+
+  return false;
 }
 
 void NotificationProvider::cancel(const WebNotification& notification) {
   int id;
   bool id_found = manager_.GetId(notification, id);
-  // Won't be found if the notification has already been closed by the user.
-  if (id_found)
+  // Won't be found if the notification has already been closed by the user,
+  // or if the notification's icon is still being requested.
+  if (id_found && !RemovePendingNotification(id))
     Send(new DesktopNotificationHostMsg_Cancel(routing_id(), id));
 }
 
@@ -58,8 +218,10 @@ void NotificationProvider::objectDestroyed(
   int id;
   bool id_found = manager_.GetId(notification, id);
   // Won't be found if the notification has already been closed by the user.
-  if (id_found)
+  if (id_found) {
+    RemovePendingNotification(id);
     manager_.UnregisterNotification(id);
+  }
 }
 
 WebNotificationPresenter::Permission NotificationProvider::checkPermission(