Factor out a PendingNotificationTracker from the NotificationManager.

The existing code ends up moving values between threads, whether by
value or hidden in a callback, causing instability and races in edge-
cases, for example when abruptly cancelling the load.

This patch introduces a PendingNotificationTracker, which completely
lives on the same thread as NotificationManager and will store all
information associated with the notification, ensuring that it never
inadvertently leaves the thread.

The NotificationImageLoader lives on the main thread and is in charge
of doing the actual fetch. Rather than storing either a callback with
the Notification's data, it knows a pending notification id given to
it by the PendingNotificationTracker. This allows us to re-associate
the data when the fetched resource(s) are available.

I'm adding some layout tests in the following patch to exercise the
image loader in a number of additional cases. Unit tests will be added
in a follow-up patch.

https://codereview.chromium.org/933153003/

BUG=458640

Review URL: https://codereview.chromium.org/939513002

Cr-Commit-Position: refs/heads/master@{#316921}

content/child/notifications/notification_image_loader.cc

@@ -11,6 +11,7 @@
 #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/WebURLRequest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 using blink::WebURL;
@@ -21,42 +22,34 @@ using blink::WebURLRequest;
 namespace content {
 
 NotificationImageLoader::NotificationImageLoader(
-    const NotificationImageLoadedCallback& callback)
+    const ImageLoadCompletedCallback& callback,
+    const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner)
     : callback_(callback),
+      worker_task_runner_(worker_task_runner),
+      notification_id_(0),
       completed_(false) {}
 
 NotificationImageLoader::~NotificationImageLoader() {
-  // The WebURLLoader instance must be destroyed on the same thread it was
-  // created on, being the main thread.
-  if (!main_thread_task_runner_->RunsTasksOnCurrentThread())
-    main_thread_task_runner_->DeleteSoon(FROM_HERE, url_loader_.release());
+  if (main_thread_task_runner_)
+    DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
 }
 
-void NotificationImageLoader::StartOnMainThread(
-    const WebURL& image_url,
-    const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner) {
+void NotificationImageLoader::StartOnMainThread(int notification_id,
+                                                const GURL& image_url) {
   DCHECK(ChildThreadImpl::current());
   DCHECK(!url_loader_);
-  DCHECK(worker_task_runner);
 
-  worker_task_runner_ = worker_task_runner;
   main_thread_task_runner_ = base::ThreadTaskRunnerHandle::Get();
+  notification_id_ = notification_id;
 
-  WebURLRequest request(image_url);
+  WebURL image_web_url(image_url);
+  WebURLRequest request(image_web_url);
   request.setRequestContext(WebURLRequest::RequestContextImage);
 
   url_loader_.reset(blink::Platform::current()->createURLLoader());
   url_loader_->loadAsynchronously(request, this);
 }
 
-SkBitmap NotificationImageLoader::GetDecodedImage() const {
-  if (buffer_.empty())
-    return SkBitmap();
-
-  ImageDecoder decoder;
-  return decoder.Decode(&buffer_[0], buffer_.size());
-}
-
 void NotificationImageLoader::didReceiveData(
     WebURLLoader* loader,
     const char* data,
@@ -86,11 +79,35 @@ void NotificationImageLoader::didFail(WebURLLoader* loader,
 }
 
 void NotificationImageLoader::RunCallbackOnWorkerThread() {
-  scoped_refptr<NotificationImageLoader> loader = make_scoped_refptr(this);
-  if (worker_task_runner_->BelongsToCurrentThread())
-    callback_.Run(loader);
-  else
-    worker_task_runner_->PostTask(FROM_HERE, base::Bind(callback_, loader));
+  url_loader_.reset();
+
+  completed_ = true;
+  SkBitmap icon = GetDecodedImage();
+
+  if (worker_task_runner_->BelongsToCurrentThread()) {
+    callback_.Run(notification_id_, icon);
+  } else {
+    worker_task_runner_->PostTask(
+        FROM_HERE, base::Bind(callback_, notification_id_, icon));
+  }
+}
+
+SkBitmap NotificationImageLoader::GetDecodedImage() const {
+  DCHECK(completed_);
+  if (buffer_.empty())
+    return SkBitmap();
+
+  ImageDecoder decoder;
+  return decoder.Decode(&buffer_[0], buffer_.size());
+}
+
+void NotificationImageLoader::DeleteOnCorrectThread() const {
+  if (!ChildThreadImpl::current()) {
+    main_thread_task_runner_->DeleteSoon(FROM_HERE, this);
+    return;
+  }
+
+  delete this;
 }
 
 }  // namespace content

content/child/notifications/notification_image_loader.h

@@ -10,8 +10,10 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/sequenced_task_runner_helpers.h"
 #include "third_party/WebKit/public/platform/WebURLLoaderClient.h"
 
+class GURL;
 class SkBitmap;
 
 namespace base {
@@ -26,33 +28,28 @@ class WebURLLoader;
 
 namespace content {
 
-class NotificationImageLoader;
-
-// Callback to be invoked when an image load has been completed.
-using NotificationImageLoadedCallback =
-    base::Callback<void(scoped_refptr<NotificationImageLoader>)>;
+struct NotificationImageLoaderDeleter;
 
 // Downloads the image associated with a notification and decodes the received
 // image. This must be completed before notifications are shown to the user.
-// Image downloaders must not be re-used for multiple notifications. The image
-// loader must be started from the main thread, but will invoke the callback on
-// the thread identified in the StartOnMainThread call.
+// Image downloaders must not be re-used for multiple notifications.
+//
+// All methods, except for the constructor, are expected to be used on the
+// renderer main thread.
 class NotificationImageLoader
     : public blink::WebURLLoaderClient,
-      public base::RefCountedThreadSafe<NotificationImageLoader> {
+      public base::RefCountedThreadSafe<NotificationImageLoader,
+                                        NotificationImageLoaderDeleter> {
+  using ImageLoadCompletedCallback = base::Callback<void(int, const SkBitmap&)>;
+
  public:
-  explicit NotificationImageLoader(
-      const NotificationImageLoadedCallback& callback);
-
-  // Asynchronously starts loading |image_url|.
-  // Must be called on the main thread. The callback should be executed by
-  // |worker_task_runner|.
-  void StartOnMainThread(
-      const blink::WebURL& image_url,
+  NotificationImageLoader(
+      const ImageLoadCompletedCallback& callback,
       const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner);
 
-  // Returns the SkBitmap resulting from decoding the loaded buffer.
-  SkBitmap GetDecodedImage() const;
+  // Asynchronously starts loading |image_url| using a Blink WebURLLoader. Must
+  // only be called on the main thread.
+  void StartOnMainThread(int notification_id, const GURL& image_url);
 
   // blink::WebURLLoaderClient implementation.
   virtual void didReceiveData(blink::WebURLLoader* loader,
@@ -66,7 +63,11 @@ class NotificationImageLoader
                        const blink::WebURLError& error);
 
  private:
-  friend class base::RefCountedThreadSafe<NotificationImageLoader>;
+  friend class base::DeleteHelper<NotificationImageLoader>;
+  friend class base::RefCountedThreadSafe<NotificationImageLoader,
+                                          NotificationImageLoaderDeleter>;
+  friend struct NotificationImageLoaderDeleter;
+
   virtual ~NotificationImageLoader();
 
   // Invokes the callback on the thread this image loader was started for. When
@@ -74,18 +75,34 @@ class NotificationImageLoader
   // For all other threads a task will be posted to the appropriate task runner.
   void RunCallbackOnWorkerThread();
 
-  NotificationImageLoadedCallback callback_;
+  // Returns a Skia bitmap, empty if buffer_ was empty or could not be decoded
+  // as an image, or a valid bitmap otherwise.
+  SkBitmap GetDecodedImage() const;
+
+  // Ensures that we delete the image loader on the main thread.
+  void DeleteOnCorrectThread() const;
+
+  ImageLoadCompletedCallback callback_;
 
-  scoped_ptr<blink::WebURLLoader> url_loader_;
   scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
+
+  int notification_id_;
   bool completed_;
 
+  scoped_ptr<blink::WebURLLoader> url_loader_;
+
   std::vector<uint8_t> buffer_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationImageLoader);
 };
 
+struct NotificationImageLoaderDeleter {
+  static void Destruct(const NotificationImageLoader* context) {
+    context->DeleteOnCorrectThread();
+  }
+};
+
 }  // namespace content
 
 #endif  // CONTENT_CHILD_NOTIFICATIONS_NOTIFICATION_IMAGE_LOADER_H_

content/child/notifications/notification_manager.cc

@@ -10,14 +10,12 @@
 #include "base/threading/thread_local.h"
 #include "content/child/notifications/notification_data_conversions.h"
 #include "content/child/notifications/notification_dispatcher.h"
-#include "content/child/notifications/notification_image_loader.h"
 #include "content/child/service_worker/web_service_worker_registration_impl.h"
 #include "content/child/thread_safe_sender.h"
 #include "content/child/worker_task_runner.h"
 #include "content/common/platform_notification_messages.h"
 #include "content/public/common/platform_notification_data.h"
 #include "third_party/WebKit/public/platform/WebSerializedOrigin.h"
-#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationData.h"
 #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationDelegate.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -40,9 +38,8 @@ NotificationManager::NotificationManager(
     base::SingleThreadTaskRunner* main_thread_task_runner,
     NotificationDispatcher* notification_dispatcher)
     : thread_safe_sender_(thread_safe_sender),
-      main_thread_task_runner_(main_thread_task_runner),
       notification_dispatcher_(notification_dispatcher),
-      weak_factory_(this) {
+      pending_notifications_(main_thread_task_runner) {
   g_notification_manager_tls.Pointer()->Set(this);
 }
 
@@ -73,15 +70,15 @@ void NotificationManager::show(
     const blink::WebNotificationData& notification_data,
     blink::WebNotificationDelegate* delegate) {
   if (notification_data.icon.isEmpty()) {
-    DisplayNotification(origin, notification_data, delegate,
-                        nullptr /* image_loader */);
+    DisplayPageNotification(origin, notification_data, delegate, SkBitmap());
     return;
   }
 
-  pending_page_notifications_[delegate] = CreateImageLoader(
-      notification_data.icon,
-      base::Bind(&NotificationManager::DisplayNotification,
-                 weak_factory_.GetWeakPtr(),
+  pending_notifications_.FetchPageNotificationResources(
+      notification_data,
+      delegate,
+      base::Bind(&NotificationManager::DisplayPageNotification,
+                 base::Unretained(this),  // this owns |pending_notifications_|
                  origin,
                  notification_data,
                  delegate));
@@ -93,48 +90,46 @@ void NotificationManager::showPersistent(
     blink::WebServiceWorkerRegistration* service_worker_registration,
     blink::WebNotificationShowCallbacks* callbacks) {
   DCHECK(service_worker_registration);
-
   int64 service_worker_registration_id =
       static_cast<WebServiceWorkerRegistrationImpl*>(
           service_worker_registration)->registration_id();
 
-  int request_id = persistent_notification_requests_.Add(callbacks);
+  scoped_ptr<blink::WebNotificationShowCallbacks> owned_callbacks(callbacks);
   if (notification_data.icon.isEmpty()) {
     DisplayPersistentNotification(origin,
                                   notification_data,
                                   service_worker_registration_id,
-                                  request_id,
-                                  nullptr /* image_loader */);
+                                  owned_callbacks.Pass(),
+                                  SkBitmap());
     return;
   }
 
-  pending_persistent_notifications_.insert(CreateImageLoader(
-      notification_data.icon,
+  pending_notifications_.FetchPersistentNotificationResources(
+      notification_data,
       base::Bind(&NotificationManager::DisplayPersistentNotification,
-                 weak_factory_.GetWeakPtr(),
+                 base::Unretained(this),  // this owns |pending_notifications_|
                  origin,
                  notification_data,
                  service_worker_registration_id,
-                 request_id)));
+                 base::Passed(&owned_callbacks)));
 }
 
 void NotificationManager::close(blink::WebNotificationDelegate* delegate) {
-  if (RemovePendingPageNotification(delegate))
+  if (pending_notifications_.CancelPageNotificationFetches(delegate))
     return;
 
-  auto iter = active_notifications_.begin();
-  for (; iter != active_notifications_.end(); ++iter) {
-    if (iter->second != delegate)
+  for (auto& iter : active_page_notifications_) {
+    if (iter.second != delegate)
       continue;
 
     thread_safe_sender_->Send(
-        new PlatformNotificationHostMsg_Close(iter->first));
-    active_notifications_.erase(iter);
+        new PlatformNotificationHostMsg_Close(iter.first));
+    active_page_notifications_.erase(iter.first);
     return;
   }
 
   // It should not be possible for Blink to call close() on a Notification which
-  // does not exist anymore in the manager.
+  // does not exist in either the pending or active notification lists.
   NOTREACHED();
 }
 
@@ -146,15 +141,14 @@ void NotificationManager::closePersistent(
 
 void NotificationManager::notifyDelegateDestroyed(
     blink::WebNotificationDelegate* delegate) {
-  if (RemovePendingPageNotification(delegate))
+  if (pending_notifications_.CancelPageNotificationFetches(delegate))
     return;
 
-  auto iter = active_notifications_.begin();
-  for (; iter != active_notifications_.end(); ++iter) {
-    if (iter->second != delegate)
+  for (auto& iter : active_page_notifications_) {
+    if (iter.second != delegate)
       continue;
 
-    active_notifications_.erase(iter);
+    active_page_notifications_.erase(iter.first);
     return;
   }
 }
@@ -182,85 +176,53 @@ bool NotificationManager::OnMessageReceived(const IPC::Message& message) {
 }
 
 void NotificationManager::OnDidShow(int notification_id) {
-  const auto& iter = active_notifications_.find(notification_id);
-  if (iter == active_notifications_.end())
+  const auto& iter = active_page_notifications_.find(notification_id);
+  if (iter == active_page_notifications_.end())
     return;
 
   iter->second->dispatchShowEvent();
 }
 
 void NotificationManager::OnDidClose(int notification_id) {
-  const auto& iter = active_notifications_.find(notification_id);
-  if (iter == active_notifications_.end())
+  const auto& iter = active_page_notifications_.find(notification_id);
+  if (iter == active_page_notifications_.end())
     return;
 
   iter->second->dispatchCloseEvent();
-  active_notifications_.erase(iter);
+  active_page_notifications_.erase(iter);
 }
 
 void NotificationManager::OnDidClick(int notification_id) {
-  const auto& iter = active_notifications_.find(notification_id);
-  if (iter == active_notifications_.end())
+  const auto& iter = active_page_notifications_.find(notification_id);
+  if (iter == active_page_notifications_.end())
     return;
 
   iter->second->dispatchClickEvent();
 }
 
-scoped_refptr<NotificationImageLoader> NotificationManager::CreateImageLoader(
-    const blink::WebURL& image_url,
-    const NotificationImageLoadedCallback& callback) const {
-  scoped_refptr<NotificationImageLoader> pending_notification(
-      new NotificationImageLoader(callback));
-
-  main_thread_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&NotificationImageLoader::StartOnMainThread,
-                            pending_notification, image_url,
-                            base::ThreadTaskRunnerHandle::Get()));
-
-  return pending_notification;
-}
-
-void NotificationManager::DisplayNotification(
+void NotificationManager::DisplayPageNotification(
     const blink::WebSerializedOrigin& origin,
     const blink::WebNotificationData& notification_data,
     blink::WebNotificationDelegate* delegate,
-    scoped_refptr<NotificationImageLoader> image_loader) {
+    const SkBitmap& icon) {
   int notification_id =
       notification_dispatcher_->GenerateNotificationId(CurrentWorkerId());
 
-  active_notifications_[notification_id] = delegate;
-
-  SkBitmap icon;
-  if (image_loader)
-    icon = image_loader->GetDecodedImage();
-
+  active_page_notifications_[notification_id] = delegate;
   thread_safe_sender_->Send(
       new PlatformNotificationHostMsg_Show(
           notification_id,
           GURL(origin.string()),
           icon,
           ToPlatformNotificationData(notification_data)));
-
-  // If this Notification contained an icon, it can be safely deleted now.
-  RemovePendingPageNotification(delegate);
 }
 
 void NotificationManager::DisplayPersistentNotification(
     const blink::WebSerializedOrigin& origin,
     const blink::WebNotificationData& notification_data,
     int64 service_worker_registration_id,
-    int request_id,
-    scoped_refptr<NotificationImageLoader> image_loader) {
-  blink::WebNotificationShowCallbacks* callbacks =
-      persistent_notification_requests_.Lookup(request_id);
-  DCHECK(callbacks);
-
-  SkBitmap icon;
-  if (image_loader) {
-    pending_persistent_notifications_.erase(image_loader);
-    icon = image_loader->GetDecodedImage();
-  }
-
+    scoped_ptr<blink::WebNotificationShowCallbacks> callbacks,
+    const SkBitmap& icon) {
   thread_safe_sender_->Send(
       new PlatformNotificationHostMsg_ShowPersistent(
           service_worker_registration_id,
@@ -271,18 +233,6 @@ void NotificationManager::DisplayPersistentNotification(
   // There currently isn't a case in which the promise would be rejected per
   // our implementation, so always resolve it here.
   callbacks->onSuccess();
-
-  persistent_notification_requests_.Remove(request_id);
-}
-
-bool NotificationManager::RemovePendingPageNotification(
-    blink::WebNotificationDelegate* delegate) {
-  const auto& iter = pending_page_notifications_.find(delegate);
-  if (iter == pending_page_notifications_.end())
-    return false;
-
-  pending_page_notifications_.erase(iter);
-  return true;
 }
 
 }  // namespace content

content/child/notifications/notification_manager.h

@@ -8,21 +8,15 @@
 #include <map>
 #include <set>
 
-#include "base/id_map.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "content/child/notifications/notification_dispatcher.h"
-#include "content/child/notifications/notification_image_loader.h"
+#include "content/child/notifications/pending_notifications_tracker.h"
 #include "content/child/worker_task_runner.h"
 #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationManager.h"
 
 class SkBitmap;
 
-namespace blink {
-class WebURL;
-}
-
 namespace content {
 
 class ThreadSafeSender;
@@ -73,56 +67,36 @@ class NotificationManager : public blink::WebNotificationManager,
   void OnDidClose(int notification_id);
   void OnDidClick(int notification_id);
 
-  // Asynchronously starts loading |image_url| on the main thread and returns a
-  // reference to the notification image loaded responsible for this. |callback|
-  // will be invoked on the calling thread when the load is complete.
-  scoped_refptr<NotificationImageLoader> CreateImageLoader(
-      const blink::WebURL& image_url,
-      const NotificationImageLoadedCallback& callback) const;
-
-  // Sends an IPC to the browser process to display the notification,
-  // accompanied by the downloaded icon.
-  void DisplayNotification(const blink::WebSerializedOrigin& origin,
-                           const blink::WebNotificationData& notification_data,
-                           blink::WebNotificationDelegate* delegate,
-                           scoped_refptr<NotificationImageLoader> image_loader);
-
-  // Sends an IPC to the browser process to display the persistent notification,
-  // accompanied by the downloaded icon.
+  // To be called when a page notification is ready to be displayed. Will
+  // inform the browser process about all available data. The |delegate|,
+  // owned by Blink, will be used to feed back events associated with the
+  // notification to the JavaScript object.
+  void DisplayPageNotification(
+      const blink::WebSerializedOrigin& origin,
+      const blink::WebNotificationData& notification_data,
+      blink::WebNotificationDelegate* delegate,
+      const SkBitmap& icon);
+
+  // To be called when a persistent notification is ready to be displayed. Will
+  // inform the browser process about all available data. The |callbacks| will
+  // be used to inform the Promise pending in Blink that the notification has
+  // been send to the browser process to be displayed.
   void DisplayPersistentNotification(
       const blink::WebSerializedOrigin& origin,
       const blink::WebNotificationData& notification_data,
       int64 service_worker_registration_id,
-      int request_id,
-      scoped_refptr<NotificationImageLoader> image_loader);
-
-  // Removes the notification identified by |delegate| from the set of
-  // pending notifications, and returns whether it could be found.
-  bool RemovePendingPageNotification(blink::WebNotificationDelegate* delegate);
+      scoped_ptr<blink::WebNotificationShowCallbacks> callbacks,
+      const SkBitmap& icon);
 
   scoped_refptr<ThreadSafeSender> thread_safe_sender_;
-  scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
   scoped_refptr<NotificationDispatcher> notification_dispatcher_;
 
-  // Tracking display requests for persistent notifications.
-  IDMap<blink::WebNotificationShowCallbacks, IDMapOwnPointer>
-      persistent_notification_requests_;
-
-  // A map tracking Page-bound notifications whose icon is still being
-  // downloaded. These downloads can be cancelled by the developer.
-  std::map<blink::WebNotificationDelegate*,
-           scoped_refptr<NotificationImageLoader>>
-      pending_page_notifications_;
-
-  // A set tracking Persistent notifications whose icon is still being
-  // downloaded. These downloads cannot be cancelled by the developer.
-  std::set<scoped_refptr<NotificationImageLoader>>
-      pending_persistent_notifications_;
+  // Tracker which stores all pending Notifications, both page and persistent
+  // ones, until all their associated resources have been fetched.
+  PendingNotificationsTracker pending_notifications_;
 
   // Map to store the delegate associated with a notification request Id.
-  std::map<int, blink::WebNotificationDelegate*> active_notifications_;
-
-  base::WeakPtrFactory<NotificationManager> weak_factory_;
+  std::map<int, blink::WebNotificationDelegate*> active_page_notifications_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationManager);
 };

content/child/notifications/pending_notifications_tracker.cc

+// Copyright 2015 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 "content/child/notifications/pending_notifications_tracker.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/thread_task_runner_handle.h"
+#include "content/child/notifications/notification_image_loader.h"
+#include "content/child/notifications/notification_manager.h"
+#include "third_party/WebKit/public/platform/WebSerializedOrigin.h"
+#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationData.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+namespace content {
+
+// Stores the information associated with a pending notification.
+struct PendingNotificationsTracker::PendingNotification {
+  PendingNotification(
+      const scoped_refptr<NotificationImageLoader>& image_loader,
+      const NotificationResourcesFetchedCallback& callback)
+      : image_loader(image_loader),
+        callback(callback) {}
+
+  scoped_refptr<NotificationImageLoader> image_loader;
+  NotificationResourcesFetchedCallback callback;
+};
+
+PendingNotificationsTracker::PendingNotificationsTracker(
+    scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner)
+    : main_thread_task_runner_(main_thread_task_runner),
+      weak_factory_(this) {}
+
+PendingNotificationsTracker::~PendingNotificationsTracker() {}
+
+void PendingNotificationsTracker::FetchPageNotificationResources(
+    const blink::WebNotificationData& notification_data,
+    blink::WebNotificationDelegate* delegate,
+    const NotificationResourcesFetchedCallback& callback) {
+  delegate_to_pending_id_map_[delegate] = FetchNotificationResources(
+      notification_data,
+      callback,
+      new NotificationImageLoader(
+          base::Bind(
+              &PendingNotificationsTracker::DidFetchPageNotification,
+              weak_factory_.GetWeakPtr(), delegate),
+          base::ThreadTaskRunnerHandle::Get()));
+}
+
+void PendingNotificationsTracker::FetchPersistentNotificationResources(
+    const blink::WebNotificationData& notification_data,
+    const NotificationResourcesFetchedCallback& callback) {
+  FetchNotificationResources(
+      notification_data,
+      callback,
+      new NotificationImageLoader(
+          base::Bind(
+              &PendingNotificationsTracker::DidFetchPersistentNotification,
+              weak_factory_.GetWeakPtr()),
+          base::ThreadTaskRunnerHandle::Get()));
+}
+
+bool PendingNotificationsTracker::CancelPageNotificationFetches(
+    blink::WebNotificationDelegate* delegate) {
+  auto iter = delegate_to_pending_id_map_.find(delegate);
+  if (iter == delegate_to_pending_id_map_.end())
+    return false;
+
+  pending_notifications_.Remove(iter->second);
+  delegate_to_pending_id_map_.erase(iter);
+
+  return true;
+}
+
+void PendingNotificationsTracker::DidFetchPageNotification(
+    blink::WebNotificationDelegate* delegate,
+    int notification_id,
+    const SkBitmap& icon) {
+  PendingNotification* pending_notification =
+      pending_notifications_.Lookup(notification_id);
+  DCHECK(pending_notification);
+
+  pending_notification->callback.Run(icon);
+
+  delegate_to_pending_id_map_.erase(delegate);
+  pending_notifications_.Remove(notification_id);
+}
+
+void PendingNotificationsTracker::DidFetchPersistentNotification(
+    int notification_id, const SkBitmap& icon) {
+  PendingNotification* pending_notification =
+      pending_notifications_.Lookup(notification_id);
+  DCHECK(pending_notification);
+
+  pending_notification->callback.Run(icon);
+
+  pending_notifications_.Remove(notification_id);
+}
+
+int PendingNotificationsTracker::FetchNotificationResources(
+    const blink::WebNotificationData& notification_data,
+    const NotificationResourcesFetchedCallback& callback,
+    const scoped_refptr<NotificationImageLoader>& image_loader) {
+  int notification_id = pending_notifications_.Add(
+      new PendingNotification(image_loader, callback));
+
+  main_thread_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&NotificationImageLoader::StartOnMainThread,
+                            image_loader, notification_id,
+                            GURL(notification_data.icon.spec())));
+
+  return notification_id;
+}
+
+}  // namespace content

content/child/notifications/pending_notifications_tracker.h

+// Copyright 2015 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.
+
+#ifndef CONTENT_CHILD_NOTIFICATIONS_PENDING_NOTIFICATIONS_TRACKER_H_
+#define CONTENT_CHILD_NOTIFICATIONS_PENDING_NOTIFICATIONS_TRACKER_H_
+
+#include <map>
+
+#include "base/id_map.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationManager.h"
+
+class SkBitmap;
+
+namespace base {
+class SingleThreadTaskRunner;
+}
+
+namespace blink {
+struct WebNotificationData;
+class WebNotificationDelegate;
+class WebSerializedOrigin;
+}
+
+namespace content {
+
+class NotificationImageLoader;
+class NotificationManager;
+
+// Type definition for the callback signature which is to be invoked when the
+// resources associated with a notification have been fetched.
+using NotificationResourcesFetchedCallback =
+    base::Callback<void(const SkBitmap&)>;
+
+// Tracks all aspects of all pending Web Notifications. Most notably, it's in
+// charge of ensuring that all resource fetches associated with the notification
+// are completed as expected. The data associated with the notifications is
+// stored in this class, to maintain thread integrity of their members.
+//
+// The pending notification tracker is owned by the NotificationManager, and
+// lives on the thread that manager has been associated with.
+class PendingNotificationsTracker {
+ public:
+  explicit PendingNotificationsTracker(
+      scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner);
+  ~PendingNotificationsTracker();
+
+  // Adds a page notification to the tracker. Resource fetches for the
+  // notification will be started on asynchronously the main thread.
+  void FetchPageNotificationResources(
+      const blink::WebNotificationData& notification_data,
+      blink::WebNotificationDelegate* delegate,
+      const NotificationResourcesFetchedCallback& callback);
+
+  // Adds a persistent notification to the tracker. Resource fetches for the
+  // notification will be started asynchronously on the main thread.
+  void FetchPersistentNotificationResources(
+      const blink::WebNotificationData& notification_data,
+      const NotificationResourcesFetchedCallback& callback);
+
+  // Cancels all pending and in-fligth fetches for the page notification
+  // identified by |delegate|. Returns if the notification was cancelled.
+  bool CancelPageNotificationFetches(blink::WebNotificationDelegate* delegate);
+
+ private:
+  // To be called on the worker thread when the pending page notification
+  // identified by |notification_id| has finished fetching the icon.
+  void DidFetchPageNotification(blink::WebNotificationDelegate* delegate,
+                                int notification_id,
+                                const SkBitmap& icon);
+
+  // To be called on the worker thread when the pending persistent notification
+  // identified by |notification_id| has finished fetching the icon.
+  void DidFetchPersistentNotification(int notification_id,
+                                      const SkBitmap& icon);
+
+  // Common code for starting to fetch resources associated with any kind of
+  // notification. Will return the id of the pending notification as allocated
+  // in the |pending_notifications_| map.
+  int FetchNotificationResources(
+      const blink::WebNotificationData& notification_data,
+      const NotificationResourcesFetchedCallback& callback,
+      const scoped_refptr<NotificationImageLoader>& image_loader);
+
+  scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
+
+  struct PendingNotification;
+
+  // List of the notifications whose resources are still being fetched.
+  IDMap<PendingNotification, IDMapOwnPointer> pending_notifications_;
+
+  // In order to be able to cancel pending page notifications by delegate, store
+  // a mapping of the delegate to the pending notification id as well.
+  std::map<blink::WebNotificationDelegate*, int> delegate_to_pending_id_map_;
+
+  base::WeakPtrFactory<PendingNotificationsTracker> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(PendingNotificationsTracker);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_CHILD_NOTIFICATIONS_PENDING_NOTIFICATIONS_TRACKER_H_

content/content_child.gypi

@@ -119,6 +119,8 @@
       'child/notifications/notification_image_loader.h',
       'child/notifications/notification_manager.cc',
       'child/notifications/notification_manager.h',
+      'child/notifications/pending_notifications_tracker.cc',
+      'child/notifications/pending_notifications_tracker.h',
       'child/npapi/np_channel_base.cc',
       'child/npapi/np_channel_base.h',
       'child/npapi/npobject_base.h',