 Chromium Code Reviews
 Chromium Code Reviews Issue 2906883003:
  Deprecate per notification type delegates.  (Closed)
    
  
    Issue 2906883003:
  Deprecate per notification type delegates.  (Closed) 
  | Index: content/test/mock_platform_notification_service.cc | 
| diff --git a/content/test/mock_platform_notification_service.cc b/content/test/mock_platform_notification_service.cc | 
| index 47990a26fa89ce4a54846224f3b6c6161b350c74..7e2052fb0c22df30c98988e42b06e0eaec02254f 100644 | 
| --- a/content/test/mock_platform_notification_service.cc | 
| +++ b/content/test/mock_platform_notification_service.cc | 
| @@ -8,7 +8,6 @@ | 
| #include "base/strings/nullable_string16.h" | 
| #include "base/strings/utf_string_conversions.h" | 
| #include "content/public/browser/browser_thread.h" | 
| -#include "content/public/browser/desktop_notification_delegate.h" | 
| #include "content/public/browser/notification_event_dispatcher.h" | 
| #include "content/public/browser/permission_type.h" | 
| #include "content/public/common/persistent_notification_status.h" | 
| @@ -34,7 +33,6 @@ void MockPlatformNotificationService::DisplayNotification( | 
| const GURL& origin, | 
| const PlatformNotificationData& notification_data, | 
| const NotificationResources& notification_resources, | 
| - std::unique_ptr<DesktopNotificationDelegate> delegate, | 
| base::Closure* cancel_callback) { | 
| DCHECK_CURRENTLY_ON(BrowserThread::UI); | 
| DCHECK(cancel_callback); | 
| @@ -43,10 +41,10 @@ void MockPlatformNotificationService::DisplayNotification( | 
| weak_factory_.GetWeakPtr(), notification_id); | 
| ReplaceNotificationIfNeeded(notification_id); | 
| + non_persistent_notifications_.insert(notification_id); | 
| - non_persistent_notifications_[notification_id] = std::move(delegate); | 
| - non_persistent_notifications_[notification_id]->NotificationDisplayed(); | 
| - | 
| + content::NotificationEventDispatcher::GetInstance() | 
| 
Peter Beverloo
2017/06/08 06:48:11
nit: drop content::, elsewhere too (I see that the
 
Miguel Garcia
2017/06/09 12:24:52
Done.
 | 
| + ->DispatchNonPersistentShowEvent(notification_id); | 
| notification_id_map_[base::UTF16ToUTF8(notification_data.title)] = | 
| notification_id; | 
| } | 
| @@ -100,7 +98,6 @@ void MockPlatformNotificationService::SimulateClick( | 
| int action_index, | 
| const base::NullableString16& reply) { | 
| DCHECK_CURRENTLY_ON(BrowserThread::UI); | 
| - | 
| const auto notification_id_iter = notification_id_map_.find(title); | 
| if (notification_id_iter == notification_id_map_.end()) | 
| return; | 
| @@ -122,8 +119,8 @@ void MockPlatformNotificationService::SimulateClick( | 
| } else if (non_persistent_iter != non_persistent_notifications_.end()) { | 
| DCHECK_EQ(action_index, -1) << "Action buttons are only supported for " | 
| "persistent notifications"; | 
| - | 
| - non_persistent_iter->second->NotificationClick(); | 
| + content::NotificationEventDispatcher::GetInstance() | 
| + ->DispatchNonPersistentClickEvent(notification_id); | 
| } | 
| } | 
| @@ -169,11 +166,11 @@ MockPlatformNotificationService::CheckPermissionOnIOThread( | 
| void MockPlatformNotificationService::Close( | 
| const std::string& notification_id) { | 
| DCHECK_CURRENTLY_ON(BrowserThread::UI); | 
| - auto iterator = non_persistent_notifications_.find(notification_id); | 
| - if (iterator == non_persistent_notifications_.end()) | 
| - return; | 
| - | 
| - iterator->second->NotificationClosed(); | 
| + const auto non_persistent_iter = | 
| + non_persistent_notifications_.find(notification_id); | 
| + content::NotificationEventDispatcher::GetInstance() | 
| + ->DispatchNonPersistentCloseEvent(notification_id); | 
| 
Peter Beverloo
2017/06/08 06:48:11
Should we check for having a valid |non_persistent
 
Miguel Garcia
2017/06/09 12:24:52
Done.
 | 
| + non_persistent_notifications_.erase(non_persistent_iter); | 
| } | 
| void MockPlatformNotificationService::ReplaceNotificationIfNeeded( | 
| @@ -186,7 +183,6 @@ void MockPlatformNotificationService::ReplaceNotificationIfNeeded( | 
| DCHECK(non_persistent_iter == non_persistent_notifications_.end()); | 
| persistent_notifications_.erase(persistent_iter); | 
| } else if (non_persistent_iter != non_persistent_notifications_.end()) { | 
| - non_persistent_iter->second->NotificationClosed(); | 
| non_persistent_notifications_.erase(non_persistent_iter); | 
| } | 
| 
Peter Beverloo
2017/06/08 06:48:11
Hmm. Since calling erase() with a key that's in in
 
Miguel Garcia
2017/06/09 12:24:52
Good point
 | 
| } |