Index: content/child/notifications/notification_manager.cc |
diff --git a/content/child/notifications/notification_manager.cc b/content/child/notifications/notification_manager.cc |
index 4cc51d7db4f6cf60d8d93831c82dde7d623241a8..436c92e2fa1d0a878d55ccbf61e3d0cff8112429 100644 |
--- a/content/child/notifications/notification_manager.cc |
+++ b/content/child/notifications/notification_manager.cc |
@@ -32,6 +32,18 @@ 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) { |
Peter Beverloo
2016/02/05 15:43:21
nit: HasResourcesToFetch (Chromium style)
Michael van Ouwerkerk
2016/02/08 14:38:52
Done.
|
+ if (!notification_data.icon.isEmpty()) |
+ return true; |
+ for (const auto& action : notification_data.actions) { |
+ if (!action.icon.isEmpty()) |
+ return true; |
+ } |
+ return false; |
+} |
+ |
} // namespace |
static base::LazyInstance<base::ThreadLocalPointer<NotificationManager>>::Leaky |
@@ -73,13 +85,13 @@ void NotificationManager::show( |
const blink::WebSecurityOrigin& origin, |
const blink::WebNotificationData& notification_data, |
blink::WebNotificationDelegate* delegate) { |
- if (notification_data.icon.isEmpty()) { |
+ if (!hasResourcesToFetch(notification_data)) { |
DisplayPageNotification(origin, notification_data, delegate, |
NotificationResources()); |
return; |
} |
- notifications_tracker_.FetchPageNotificationResources( |
+ notifications_tracker_.FetchNotificationResources( |
Peter Beverloo
2016/02/05 15:43:21
DCHECK_EQ(0u, notification_data.actions.size());
Michael van Ouwerkerk
2016/02/08 14:38:52
Sure. Let's put that at the start of the function
|
notification_data, delegate, |
base::Bind(&NotificationManager::DisplayPageNotification, |
base::Unretained(this), // this owns |notifications_tracker_| |
@@ -116,15 +128,20 @@ void NotificationManager::showPersistent( |
return; |
} |
- if (notification_data.icon.isEmpty()) { |
+ if (!hasResourcesToFetch(notification_data)) { |
+ NotificationResources notification_resources; |
+ if (!notification_data.actions.isEmpty()) { |
Peter Beverloo
2016/02/05 15:43:21
It would be great to document why this is signific
Michael van Ouwerkerk
2016/02/08 14:38:52
Yes, it's a subtle one.
|
+ notification_resources.action_icons.resize( |
+ notification_data.actions.size()); |
+ } |
DisplayPersistentNotification( |
origin, notification_data, service_worker_registration_id, |
- std::move(owned_callbacks), NotificationResources()); |
+ std::move(owned_callbacks), notification_resources); |
return; |
} |
- notifications_tracker_.FetchPersistentNotificationResources( |
- notification_data, |
+ notifications_tracker_.FetchNotificationResources( |
+ notification_data, nullptr /* delegate */, |
base::Bind(&NotificationManager::DisplayPersistentNotification, |
base::Unretained(this), // this owns |notifications_tracker_| |
origin, notification_data, service_worker_registration_id, |
@@ -159,7 +176,7 @@ void NotificationManager::getNotifications( |
} |
void NotificationManager::close(blink::WebNotificationDelegate* delegate) { |
- if (notifications_tracker_.CancelPageNotificationFetches(delegate)) |
+ if (notifications_tracker_.CancelResourceFetches(delegate)) |
return; |
for (auto& iter : active_page_notifications_) { |
@@ -189,7 +206,7 @@ void NotificationManager::closePersistent( |
void NotificationManager::notifyDelegateDestroyed( |
blink::WebNotificationDelegate* delegate) { |
- if (notifications_tracker_.CancelPageNotificationFetches(delegate)) |
+ if (notifications_tracker_.CancelResourceFetches(delegate)) |
return; |
for (auto& iter : active_page_notifications_) { |
@@ -306,6 +323,9 @@ void NotificationManager::DisplayPageNotification( |
const blink::WebNotificationData& notification_data, |
blink::WebNotificationDelegate* delegate, |
const NotificationResources& notification_resources) { |
+ DCHECK_EQ(notification_data.actions.size(), 0u); |
+ DCHECK_EQ(notification_resources.action_icons.size(), 0u); |
+ |
int notification_id = |
notification_dispatcher_->GenerateNotificationId(CurrentWorkerId()); |
@@ -324,6 +344,9 @@ void NotificationManager::DisplayPersistentNotification( |
int64_t service_worker_registration_id, |
scoped_ptr<blink::WebNotificationShowCallbacks> callbacks, |
const NotificationResources& notification_resources) { |
+ DCHECK_EQ(notification_data.actions.size(), |
+ notification_resources.action_icons.size()); |
+ |
// TODO(peter): GenerateNotificationId is more of a request id. Consider |
// renaming the method in the NotificationDispatcher if this makes sense. |
int request_id = |