Move notification resource loading from content/child to blink

content/child/notifications/notification_manager.cc

Index: content/child/notifications/notification_manager.cc
diff --git a/content/child/notifications/notification_manager.cc b/content/child/notifications/notification_manager.cc
index 8961f7a656d21ae12186e8cf2b696d16de11d1d4..64bbf18adbe87bfa23367ab346b97517758ba49f 100644
--- a/content/child/notifications/notification_manager.cc
+++ b/content/child/notifications/notification_manager.cc
@@ -9,7 +9,6 @@
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/thread_task_runner_handle.h"
 #include "base/threading/thread_local.h"
 #include "content/child/notifications/notification_data_conversions.h"
 #include "content/child/notifications/notification_dispatcher.h"
@@ -31,18 +30,14 @@ int CurrentWorkerId() {
   return WorkerThread::GetCurrentId();
 }
 
-// Checks whether |notification_data| specifies any non-empty resources that
-// need to be fetched.
-bool HasResourcesToFetch(const blink::WebNotificationData& notification_data) {
-  if (!notification_data.icon.isEmpty())
-    return true;
-  if (!notification_data.badge.isEmpty())
-    return true;
-  for (const auto& action : notification_data.actions) {
-    if (!action.icon.isEmpty())
-      return true;
-  }
-  return false;
+NotificationResources ToNotificationResources(
+    std::unique_ptr<blink::WebNotificationResources> web_resources) {
+  NotificationResources resources;
+  resources.notification_icon = web_resources->icon;
+  resources.badge = web_resources->badge;
+  for (const auto& action_icon : web_resources->actionIcons)
+    resources.action_icons.push_back(action_icon);
+  return resources;
 }
 
 }  // namespace
@@ -52,11 +47,9 @@ static base::LazyInstance<base::ThreadLocalPointer<NotificationManager>>::Leaky
 
 NotificationManager::NotificationManager(
     ThreadSafeSender* thread_safe_sender,
-    base::SingleThreadTaskRunner* main_thread_task_runner,
     NotificationDispatcher* notification_dispatcher)
     : thread_safe_sender_(thread_safe_sender),
-      notification_dispatcher_(notification_dispatcher),
-      notifications_tracker_(main_thread_task_runner) {
+      notification_dispatcher_(notification_dispatcher) {
   g_notification_manager_tls.Pointer()->Set(this);
 }
 
@@ -66,13 +59,12 @@ NotificationManager::~NotificationManager() {
 
 NotificationManager* NotificationManager::ThreadSpecificInstance(
     ThreadSafeSender* thread_safe_sender,
-    base::SingleThreadTaskRunner* main_thread_task_runner,
     NotificationDispatcher* notification_dispatcher) {
   if (g_notification_manager_tls.Pointer()->Get())
     return g_notification_manager_tls.Pointer()->Get();
 
-  NotificationManager* manager = new NotificationManager(
-      thread_safe_sender, main_thread_task_runner, notification_dispatcher);
+  NotificationManager* manager =
+      new NotificationManager(thread_safe_sender, notification_dispatcher);
   if (CurrentWorkerId())
     WorkerThread::AddObserver(manager);
   return manager;
@@ -85,25 +77,19 @@ void NotificationManager::WillStopCurrentWorkerThread() {
 void NotificationManager::show(
     const blink::WebSecurityOrigin& origin,
     const blink::WebNotificationData& notification_data,
+    std::unique_ptr<blink::WebNotificationResources> notification_resources,
     blink::WebNotificationDelegate* delegate) {
   DCHECK_EQ(0u, notification_data.actions.size());
 
-  if (!HasResourcesToFetch(notification_data)) {
-    DisplayPageNotification(origin, notification_data, delegate,
-                            NotificationResources());
-    return;
-  }
-
-  notifications_tracker_.FetchResources(
-      notification_data, delegate,
-      base::Bind(&NotificationManager::DisplayPageNotification,
-                 base::Unretained(this),  // this owns |notifications_tracker_|
-                 origin, notification_data, delegate));
+  DisplayPageNotification(

[Peter Beverloo, 2016/04/13 18:32:27]

Can we coalesce that method into this one now? Dito re: showPersistent and
DisplayPersistentNotification?

[Michael van Ouwerkerk, 2016/04/14 13:42:11]

Done.

+      origin, notification_data, delegate,
+      ToNotificationResources(std::move(notification_resources)));
 }
 
 void NotificationManager::showPersistent(
     const blink::WebSecurityOrigin& origin,
     const blink::WebNotificationData& notification_data,
+    std::unique_ptr<blink::WebNotificationResources> notification_resources,
     blink::WebServiceWorkerRegistration* service_worker_registration,
     blink::WebNotificationShowCallbacks* callbacks) {
   DCHECK(service_worker_registration);
@@ -133,28 +119,10 @@ void NotificationManager::showPersistent(
     return;
   }
 
-  if (!HasResourcesToFetch(notification_data)) {
-    NotificationResources notification_resources;
-
-    // Action indices are expected to have a corresponding icon bitmap, which
-    // may be empty when the developer provided no (or an invalid) icon.
-    if (!notification_data.actions.isEmpty()) {
-      notification_resources.action_icons.resize(
-          notification_data.actions.size());
-    }
-
-    DisplayPersistentNotification(
-        origin, notification_data, service_worker_registration_id,
-        std::move(owned_callbacks), notification_resources);
-    return;
-  }
-
-  notifications_tracker_.FetchResources(
-      notification_data, nullptr /* delegate */,
-      base::Bind(&NotificationManager::DisplayPersistentNotification,
-                 base::Unretained(this),  // this owns |notifications_tracker_|
-                 origin, notification_data, service_worker_registration_id,
-                 base::Passed(&owned_callbacks)));
+  DisplayPersistentNotification(
+      origin, notification_data, service_worker_registration_id,
+      std::move(owned_callbacks),
+      ToNotificationResources(std::move(notification_resources)));
 }
 
 void NotificationManager::getNotifications(
@@ -185,9 +153,6 @@ void NotificationManager::getNotifications(
 }
 
 void NotificationManager::close(blink::WebNotificationDelegate* delegate) {
-  if (notifications_tracker_.CancelResourceFetches(delegate))
-    return;
-
   for (auto& iter : active_page_notifications_) {
     if (iter.second != delegate)
       continue;
@@ -215,9 +180,6 @@ void NotificationManager::closePersistent(
 
 void NotificationManager::notifyDelegateDestroyed(
     blink::WebNotificationDelegate* delegate) {
-  if (notifications_tracker_.CancelResourceFetches(delegate))
-    return;
-
   for (auto& iter : active_page_notifications_) {
     if (iter.second != delegate)
       continue;