Make GetDisplayedNotifications asynchronous.

content/shell/browser/layout_test/layout_test_notification_manager.cc

Index: content/shell/browser/layout_test/layout_test_notification_manager.cc
diff --git a/content/shell/browser/layout_test/layout_test_notification_manager.cc b/content/shell/browser/layout_test/layout_test_notification_manager.cc
index cfac368b75d264ebd9f7f0608687652ad95e0180..d64657afd7d8a4d1122132a833491455364ce0de 100644
--- a/content/shell/browser/layout_test/layout_test_notification_manager.cc
+++ b/content/shell/browser/layout_test/layout_test_notification_manager.cc
@@ -83,14 +83,14 @@ void LayoutTestNotificationManager::ClosePersistentNotification(
   persistent_notifications_.erase(notification_id);
 }
 
-bool LayoutTestNotificationManager::GetDisplayedNotifications(
+void LayoutTestNotificationManager::GetDisplayedNotifications(
     BrowserContext* browser_context,
-    std::set<std::string>* displayed_notifications) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(displayed_notifications);
-
-  // Notifications will never outlive the lifetime of running layout tests.
-  return false;
+    const DisplayedNotificationsCallback& callback) {
+  auto displayed_notifications = base::MakeUnique<std::set<std::string>>();
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(callback, base::Passed(&displayed_notifications),
+                 false /* supports getting displayed notifications */));

[Peter Beverloo, 2017/03/17 15:46:04]

nit: here and elsewhere - it would be much easier to keep things consistent if
you'd use an argument name instead of a textual description. (You use different
descriptions throughout the code.)

i.e.
  platform_notification_service.h
      "supports synchronization" --> "supports_synchronization"

  layout_test_notification_manager.cc
      "supports getting displayed notifications"
          --> "supports_synchronization"

Your argument names are also very inconsistent:

  supported
  sync_supported
  supports_synchronization
  notification_synchronization_supported

Would be cool if we can align all of that.

[Miguel Garcia, 2017/03/20 14:11:21]

Good idea, I changed it everywhere now.

On 2017/03/17 15:46:04, Peter Beverloo wrote:

 }
 
 void LayoutTestNotificationManager::SimulateClick(
@@ -113,10 +113,9 @@ void LayoutTestNotificationManager::SimulateClick(
     DCHECK(non_persistent_iter == non_persistent_notifications_.end());
 
     const PersistentNotification& notification = persistent_iter->second;
-    content::NotificationEventDispatcher::GetInstance()
-        ->DispatchNotificationClickEvent(
-            notification.browser_context, notification_id, notification.origin,
-            action_index, reply, base::Bind(&OnEventDispatchComplete));
+    NotificationEventDispatcher::GetInstance()->DispatchNotificationClickEvent(
+        notification.browser_context, notification_id, notification.origin,
+        action_index, reply, base::Bind(&OnEventDispatchComplete));
   } else if (non_persistent_iter != non_persistent_notifications_.end()) {
     DCHECK_EQ(action_index, -1) << "Action buttons are only supported for "
                                    "persistent notifications";
@@ -140,10 +139,9 @@ void LayoutTestNotificationManager::SimulateClose(const std::string& title,
     return;
 
   const PersistentNotification& notification = persistent_iter->second;
-  content::NotificationEventDispatcher::GetInstance()
-      ->DispatchNotificationCloseEvent(
-          notification.browser_context, notification_id, notification.origin,
-          by_user, base::Bind(&OnEventDispatchComplete));
+  NotificationEventDispatcher::GetInstance()->DispatchNotificationCloseEvent(
+      notification.browser_context, notification_id, notification.origin,
+      by_user, base::Bind(&OnEventDispatchComplete));
 }
 
 blink::mojom::PermissionStatus