Fetch notification action icons and pass them through in resources.

content/child/notifications/pending_notifications_tracker.h

Index: content/child/notifications/pending_notifications_tracker.h
diff --git a/content/child/notifications/pending_notifications_tracker.h b/content/child/notifications/pending_notifications_tracker.h
index 1dbeafef380a2d507507a2bbb37d33eef9d549c6..375c28979182243295b661039ef93e4b6dde15cb 100644
--- a/content/child/notifications/pending_notifications_tracker.h
+++ b/content/child/notifications/pending_notifications_tracker.h
@@ -7,13 +7,11 @@
 
 #include <map>
 
+#include "base/callback_forward.h"
 #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"
-
-class SkBitmap;
+#include "content/common/content_export.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -26,77 +24,60 @@ class WebNotificationDelegate;
 
 namespace content {
 
-class NotificationImageLoader;
+class PendingNotification;
+class PendingNotificationsTrackerTest;
 struct NotificationResources;
 
 // Type definition for the callback signature which is to be invoked when the
 // resources associated with a notification have been fetched.
-using NotificationResourcesFetchedCallback =
+using NotificationResourcesCallback =

[Peter Beverloo, 2016/02/05 15:43:21]

This can live in the PendingNotificationsTracker class since it's only used
there. (Which'll allow you to drop the Notification prefix, too.)

[Michael van Ouwerkerk, 2016/02/08 14:38:53]

Done.

     base::Callback<void(const NotificationResources&)>;
 
-// 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.
-//
+// Tracks all pending Web Notifications as PendingNotification instances. The
+// pending notifications (and their associated data) are stored in this class.
 // The pending notification tracker is owned by the NotificationManager, and
 // lives on the thread that manager has been associated with.
-class PendingNotificationsTracker {
+class CONTENT_EXPORT PendingNotificationsTracker {
  public:
   explicit PendingNotificationsTracker(
-      scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner);
+      const 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(
+  // Adds a pending notification for which to fetch resources. The |delegate|
+  // may be null, but if it is not it can be used to cancel the fetches.

[Peter Beverloo, 2016/02/05 15:43:21]

nit: s/but if it is not it/but non-null values/

[Michael van Ouwerkerk, 2016/02/08 14:38:53]

Done.

+  // Resource fetches will be started asynchronously on the main thread.
+  void FetchNotificationResources(

[Peter Beverloo, 2016/02/05 15:43:21]

Up to you, but for consistency reasons with FetchesFinished you may want to
consider calling this FetchResources(). The fact that it's for notifications is
implied by the class name.

[Michael van Ouwerkerk, 2016/02/08 14:38:53]

Done.

       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);
+      const NotificationResourcesCallback& resources_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);
+  // Cancels all pending and in-flight fetches for the notification
+  // identified by |delegate|. Returns whether the notification was cancelled.
+  bool CancelResourceFetches(blink::WebNotificationDelegate* delegate);

[Peter Beverloo, 2016/02/05 15:43:21]

Should this method explicitly disallow non-NULL values (+DCHECK)? It would catch
cases where we for whatever reason end up trying to cancel persistent
notifications.

[Michael van Ouwerkerk, 2016/02/08 14:38:53]

Sure, we can explicitly disallow null values (and document that).

 
  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);
+  friend class PendingNotificationsTrackerTest;
 
-  scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
+  // To be called on the worker thread when the pending notification
+  // identified by |notification_id| has finished fetching the resources. The
+  // |delegate| may be null.
+  void FetchesFinished(blink::WebNotificationDelegate* delegate,
+                       int notification_id,
+                       const NotificationResourcesCallback& resources_callback);
+
+  // Used to generate ids for tracking pending notifications.
+  int32_t next_notification_id_;
 
-  struct PendingNotification;
+  scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
 
-  // List of the notifications whose resources are still being fetched.
+  // 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);
 };