Let Notification Handlers report when event handling has completed

This will significantly help reduce flakiness in testing (one deflake
included in this CL), and is required for migrating Android's intent
handling system to the BackgroundTaskScheduler which needs to know when
handling of the event has finished.

In addition, a hack where the NativeNotificationDisplayService invoked
the Close() method on a notification's delegate in order to fire the
event for non-persistent Web Notifications has been removed. We can just
fire the event from the NotificationMessageFilter instead.

BUG=766854, 784951

Change-Id: I57d0a446e6c47281bb7b5e001c449fadddeba571
Reviewed-on: https://chromium-review.googlesource.com/779722
Commit-Queue: Peter Beverloo <[email protected]>
Reviewed-by: Anita Woodruff <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Reviewed-by: David Trainor <[email protected]>
Cr-Commit-Position: refs/heads/master@{#518305}
diff --git a/chrome/browser/notifications/native_notification_display_service.cc b/chrome/browser/notifications/native_notification_display_service.cc
index 185ff5c..b4552ef 100644
--- a/chrome/browser/notifications/native_notification_display_service.cc
+++ b/chrome/browser/notifications/native_notification_display_service.cc
@@ -98,14 +98,6 @@
     const std::string& notification_id) {
   if (notification_bridge_ready_) {
     notification_bridge_->Close(GetProfileId(profile_), notification_id);
-
-    // TODO(miguelg): Figure out something better here, passing an empty
-    // origin works because only non persistent notifications care about
-    // this method for JS generated close calls and they don't require
-    // the origin.
-    NotificationHandler* handler = GetNotificationHandler(notification_type);
-    if (handler)
-      handler->OnClose(profile_, GURL(), notification_id, false /* by user */);
   } else if (message_center_display_service_) {
     message_center_display_service_->Close(notification_type, notification_id);
   } else {
diff --git a/chrome/browser/notifications/non_persistent_notification_handler.cc b/chrome/browser/notifications/non_persistent_notification_handler.cc
index 9380482..02cc38f 100644
--- a/chrome/browser/notifications/non_persistent_notification_handler.cc
+++ b/chrome/browser/notifications/non_persistent_notification_handler.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/notifications/non_persistent_notification_handler.h"
 
+#include "base/callback.h"
 #include "base/strings/nullable_string16.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "content/public/browser/notification_event_dispatcher.h"
@@ -22,9 +23,14 @@
     Profile* profile,
     const GURL& origin,
     const std::string& notification_id,
-    bool by_user) {
+    bool by_user,
+    base::OnceClosure completed_closure) {
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNonPersistentCloseEvent(notification_id);
+
+  // TODO(crbug.com/787459): Implement event acknowledgements once
+  // non-persistent notifications have updated to use Mojo instead of IPC.
+  std::move(completed_closure).Run();
 }
 
 void NonPersistentNotificationHandler::OnClick(
@@ -32,7 +38,8 @@
     const GURL& origin,
     const std::string& notification_id,
     const base::Optional<int>& action_index,
-    const base::Optional<base::string16>& reply) {
+    const base::Optional<base::string16>& reply,
+    base::OnceClosure completed_closure) {
   // Non persistent notifications don't allow buttons or replies.
   // https://notifications.spec.whatwg.org/#create-a-notification
   DCHECK(!action_index.has_value());
@@ -40,6 +47,10 @@
 
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNonPersistentClickEvent(notification_id);
+
+  // TODO(crbug.com/787459): Implement event acknowledgements once
+  // non-persistent notifications have updated to use Mojo instead of IPC.
+  std::move(completed_closure).Run();
 }
 
 void NonPersistentNotificationHandler::OpenSettings(Profile* profile) {
diff --git a/chrome/browser/notifications/non_persistent_notification_handler.h b/chrome/browser/notifications/non_persistent_notification_handler.h
index a7f5534f..c46b7a63 100644
--- a/chrome/browser/notifications/non_persistent_notification_handler.h
+++ b/chrome/browser/notifications/non_persistent_notification_handler.h
@@ -19,14 +19,14 @@
   void OnClose(Profile* profile,
                const GURL& origin,
                const std::string& notification_id,
-               bool by_user) override;
-
+               bool by_user,
+               base::OnceClosure completed_closure) override;
   void OnClick(Profile* profile,
                const GURL& origin,
                const std::string& notification_id,
                const base::Optional<int>& action_index,
-               const base::Optional<base::string16>& reply) override;
-
+               const base::Optional<base::string16>& reply,
+               base::OnceClosure completed_closure) override;
   void OpenSettings(Profile* profile) override;
 
  private:
diff --git a/chrome/browser/notifications/notification_display_service.cc b/chrome/browser/notifications/notification_display_service.cc
index 2ec41e1..a3b91e8 100644
--- a/chrome/browser/notifications/notification_display_service.cc
+++ b/chrome/browser/notifications/notification_display_service.cc
@@ -6,11 +6,15 @@
 
 #include <memory>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback.h"
 #include "base/strings/nullable_string16.h"
 #include "chrome/browser/notifications/non_persistent_notification_handler.h"
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/notifications/persistent_notification_handler.h"
+#include "content/public/browser/browser_thread.h"
 #include "extensions/features/features.h"
 #include "url/gurl.h"
 
@@ -18,8 +22,15 @@
 #include "chrome/browser/extensions/api/notifications/extension_notification_handler.h"
 #endif
 
-// static
+namespace {
 
+void OperationCompleted() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+}
+
+}  // namespace
+
+// static
 NotificationDisplayService* NotificationDisplayService::GetForProfile(
     Profile* profile) {
   return NotificationDisplayServiceFactory::GetForProfile(profile);
@@ -77,13 +88,20 @@
     LOG(ERROR) << "Unable to find a handler for " << notification_type;
     return;
   }
+
+  // TODO(crbug.com/766854): Plumb this through from the notification platform
+  // bridges so they can report completion of the operation as needed.
+  base::OnceClosure completed_closure = base::BindOnce(&OperationCompleted);
+
   switch (operation) {
     case NotificationCommon::CLICK:
-      handler->OnClick(profile_, origin, notification_id, action_index, reply);
+      handler->OnClick(profile_, origin, notification_id, action_index, reply,
+                       std::move(completed_closure));
       break;
     case NotificationCommon::CLOSE:
       DCHECK(by_user.has_value());
-      handler->OnClose(profile_, origin, notification_id, by_user.value());
+      handler->OnClose(profile_, origin, notification_id, by_user.value(),
+                       std::move(completed_closure));
       break;
     case NotificationCommon::SETTINGS:
       handler->OpenSettings(profile_);
diff --git a/chrome/browser/notifications/notification_handler.cc b/chrome/browser/notifications/notification_handler.cc
new file mode 100644
index 0000000..57463a0
--- /dev/null
+++ b/chrome/browser/notifications/notification_handler.cc
@@ -0,0 +1,35 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/notifications/notification_handler.h"
+
+#include "base/callback.h"
+
+NotificationHandler::~NotificationHandler() = default;
+
+void NotificationHandler::OnShow(Profile* profile,
+                                 const std::string& notification_id) {}
+
+void NotificationHandler::OnClose(Profile* profile,
+                                  const GURL& origin,
+                                  const std::string& notification_id,
+                                  bool by_user,
+                                  base::OnceClosure completed_closure) {
+  std::move(completed_closure).Run();
+}
+
+void NotificationHandler::OnClick(Profile* profile,
+                                  const GURL& origin,
+                                  const std::string& notification_id,
+                                  const base::Optional<int>& action_index,
+                                  const base::Optional<base::string16>& reply,
+                                  base::OnceClosure completed_closure) {
+  std::move(completed_closure).Run();
+}
+
+void NotificationHandler::OpenSettings(Profile* profile) {
+  // Notification types that display a settings button must override this method
+  // to handle user interaction with it.
+  NOTREACHED();
+}
diff --git a/chrome/browser/notifications/notification_handler.h b/chrome/browser/notifications/notification_handler.h
index 449fae1a..f5af3b43 100644
--- a/chrome/browser/notifications/notification_handler.h
+++ b/chrome/browser/notifications/notification_handler.h
@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "base/callback_forward.h"
 #include "base/optional.h"
 #include "base/strings/string16.h"
 
@@ -19,27 +20,31 @@
 // notification type.
 class NotificationHandler {
  public:
-  virtual ~NotificationHandler() {}
+  virtual ~NotificationHandler();
 
-  // Called after displaying a toast in case the caller needs some processing.
-  virtual void OnShow(Profile* profile, const std::string& notification_id) {}
+  // Called after a notification has been displayed.
+  virtual void OnShow(Profile* profile, const std::string& notification_id);
 
-  // Process notification close events.
+  // Process notification close events. The |completed_closure| must be invoked
+  // on the UI thread once processing of the close event has been finished.
   virtual void OnClose(Profile* profile,
                        const GURL& origin,
                        const std::string& notification_id,
-                       bool by_user) {}
+                       bool by_user,
+                       base::OnceClosure completed_closure);
 
-  // Process cliks to a notification or its buttons, depending on
-  // |action_index|.
+  // Process clicks on a notification or its buttons, depending on
+  // |action_index|. The |completed_closure| must be invoked on the UI thread
+  // once processing of the click event has been finished.
   virtual void OnClick(Profile* profile,
                        const GURL& origin,
                        const std::string& notification_id,
                        const base::Optional<int>& action_index,
-                       const base::Optional<base::string16>& reply) = 0;
+                       const base::Optional<base::string16>& reply,
+                       base::OnceClosure completed_closure);
 
   // Open notification settings.
-  virtual void OpenSettings(Profile* profile) {}
+  virtual void OpenSettings(Profile* profile);
 };
 
 #endif  // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_HANDLER_H_
diff --git a/chrome/browser/notifications/persistent_notification_handler.cc b/chrome/browser/notifications/persistent_notification_handler.cc
index 1a30f8c..898a6498 100644
--- a/chrome/browser/notifications/persistent_notification_handler.cc
+++ b/chrome/browser/notifications/persistent_notification_handler.cc
@@ -4,28 +4,29 @@
 
 #include "chrome/browser/notifications/persistent_notification_handler.h"
 
+#include "base/callback.h"
 #include "base/logging.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/profiles/profile.h"
 
-PersistentNotificationHandler::PersistentNotificationHandler() {}
-PersistentNotificationHandler::~PersistentNotificationHandler() {}
+PersistentNotificationHandler::PersistentNotificationHandler() = default;
+PersistentNotificationHandler::~PersistentNotificationHandler() = default;
 
-void PersistentNotificationHandler::OnShow(Profile* profile,
-                                           const std::string& notification_id) {
-}
-
-void PersistentNotificationHandler::OnClose(Profile* profile,
-                                            const GURL& origin,
-                                            const std::string& notification_id,
-                                            bool by_user) {
-  if (!by_user)
+void PersistentNotificationHandler::OnClose(
+    Profile* profile,
+    const GURL& origin,
+    const std::string& notification_id,
+    bool by_user,
+    base::OnceClosure completed_closure) {
+  if (!by_user) {
+    std::move(completed_closure).Run();
     return;  // no need to propagate back programmatic close events
+  }
 
   DCHECK(origin.is_valid());
 
   PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClose(
-      profile, notification_id, origin, by_user);
+      profile, notification_id, origin, by_user, std::move(completed_closure));
 }
 
 void PersistentNotificationHandler::OnClick(
@@ -33,11 +34,13 @@
     const GURL& origin,
     const std::string& notification_id,
     const base::Optional<int>& action_index,
-    const base::Optional<base::string16>& reply) {
+    const base::Optional<base::string16>& reply,
+    base::OnceClosure completed_closure) {
   DCHECK(origin.is_valid());
 
   PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick(
-      profile, notification_id, origin, action_index, reply);
+      profile, notification_id, origin, action_index, reply,
+      std::move(completed_closure));
 }
 
 void PersistentNotificationHandler::OpenSettings(Profile* profile) {
diff --git a/chrome/browser/notifications/persistent_notification_handler.h b/chrome/browser/notifications/persistent_notification_handler.h
index c4bfb90..848b2d1 100644
--- a/chrome/browser/notifications/persistent_notification_handler.h
+++ b/chrome/browser/notifications/persistent_notification_handler.h
@@ -16,16 +16,17 @@
   ~PersistentNotificationHandler() override;
 
   // NotificationHandler implementation.
-  void OnShow(Profile* profile, const std::string& notification_id) override;
   void OnClose(Profile* profile,
                const GURL& origin,
                const std::string& notification_id,
-               bool by_user) override;
+               bool by_user,
+               base::OnceClosure completed_closure) override;
   void OnClick(Profile* profile,
                const GURL& origin,
                const std::string& notification_id,
                const base::Optional<int>& action_index,
-               const base::Optional<base::string16>& reply) override;
+               const base::Optional<base::string16>& reply,
+               base::OnceClosure completed_closure) override;
   void OpenSettings(Profile* profile) override;
 
  private:
diff --git a/chrome/browser/notifications/platform_notification_service_impl.cc b/chrome/browser/notifications/platform_notification_service_impl.cc
index edb6b54..c9e5b305 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.cc
+++ b/chrome/browser/notifications/platform_notification_service_impl.cc
@@ -156,7 +156,8 @@
     const std::string& notification_id,
     const GURL& origin,
     const base::Optional<int>& action_index,
-    const base::Optional<base::string16>& reply) {
+    const base::Optional<base::string16>& reply,
+    base::OnceClosure completed_closure) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   blink::mojom::PermissionStatus permission_status =
       CheckPermissionOnUIThread(browser_context, origin,
@@ -168,6 +169,8 @@
   // Also change this method to be const again.
   if (permission_status != blink::mojom::PermissionStatus::GRANTED) {
     metrics_logger->LogPersistentNotificationClickWithoutPermission();
+
+    std::move(completed_closure).Run();
     return;
   }
 
@@ -190,9 +193,9 @@
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationClickEvent(
           browser_context, notification_id, origin, action_index, reply,
-          base::Bind(
+          base::BindOnce(
               &PlatformNotificationServiceImpl::OnClickEventDispatchComplete,
-              base::Unretained(this)));
+              base::Unretained(this), std::move(completed_closure)));
 }
 
 // TODO(miguelg): Move this to PersistentNotificationHandler
@@ -200,12 +203,15 @@
     BrowserContext* browser_context,
     const std::string& notification_id,
     const GURL& origin,
-    bool by_user) {
+    bool by_user,
+    base::OnceClosure completed_closure) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // If we programatically closed this notification, don't dispatch any event.
-  if (closed_notifications_.erase(notification_id) != 0)
+  if (closed_notifications_.erase(notification_id) != 0) {
+    std::move(completed_closure).Run();
     return;
+  }
 
   NotificationMetricsLogger* metrics_logger = GetMetricsLogger(browser_context);
   if (by_user)
@@ -216,9 +222,9 @@
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationCloseEvent(
           browser_context, notification_id, origin, by_user,
-          base::Bind(
+          base::BindOnce(
               &PlatformNotificationServiceImpl::OnCloseEventDispatchComplete,
-              base::Unretained(this)));
+              base::Unretained(this), std::move(completed_closure)));
 }
 
 blink::mojom::PermissionStatus
@@ -435,6 +441,7 @@
 }
 
 void PlatformNotificationServiceImpl::OnClickEventDispatchComplete(
+    base::OnceClosure completed_closure,
     content::PersistentNotificationStatus status) {
   UMA_HISTOGRAM_ENUMERATION(
       "Notifications.PersistentWebNotificationClickResult", status,
@@ -446,14 +453,19 @@
     click_dispatch_keep_alive_.reset();
   }
 #endif
+
+  std::move(completed_closure).Run();
 }
 
 void PlatformNotificationServiceImpl::OnCloseEventDispatchComplete(
+    base::OnceClosure completed_closure,
     content::PersistentNotificationStatus status) {
   UMA_HISTOGRAM_ENUMERATION(
       "Notifications.PersistentWebNotificationCloseResult", status,
       content::PersistentNotificationStatus::
           PERSISTENT_NOTIFICATION_STATUS_MAX);
+
+  std::move(completed_closure).Run();
 }
 
 message_center::Notification
diff --git a/chrome/browser/notifications/platform_notification_service_impl.h b/chrome/browser/notifications/platform_notification_service_impl.h
index 484bc76c..2b487e6 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.h
+++ b/chrome/browser/notifications/platform_notification_service_impl.h
@@ -13,6 +13,7 @@
 #include <string>
 #include <unordered_set>
 
+#include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
@@ -55,7 +56,8 @@
       const std::string& notification_id,
       const GURL& origin,
       const base::Optional<int>& action_index,
-      const base::Optional<base::string16>& reply);
+      const base::Optional<base::string16>& reply,
+      base::OnceClosure completed_closure);
 
   // To be called when a persistent notification has been closed. The data
   // associated with the notification has to be pruned from the database in this
@@ -64,7 +66,8 @@
   void OnPersistentNotificationClose(content::BrowserContext* browser_context,
                                      const std::string& notification_id,
                                      const GURL& origin,
-                                     bool by_user);
+                                     bool by_user,
+                                     base::OnceClosure completed_closure);
 
   // content::PlatformNotificationService implementation.
   blink::mojom::PermissionStatus CheckPermissionOnUIThread(
@@ -109,11 +112,11 @@
   PlatformNotificationServiceImpl();
   ~PlatformNotificationServiceImpl() override;
 
-  // Persistent notifications fired through the delegate do not care about the
-  // lifetime of the Service Worker responsible for executing the event.
   void OnClickEventDispatchComplete(
+      base::OnceClosure completed_closure,
       content::PersistentNotificationStatus status);
   void OnCloseEventDispatchComplete(
+      base::OnceClosure completed_closure,
       content::PersistentNotificationStatus status);
 
   // Creates a new Web Notification-based Notification object. Should only be
diff --git a/chrome/browser/notifications/platform_notification_service_interactive_uitest.cc b/chrome/browser/notifications/platform_notification_service_interactive_uitest.cc
index 7d0a5718..556f68f7 100644
--- a/chrome/browser/notifications/platform_notification_service_interactive_uitest.cc
+++ b/chrome/browser/notifications/platform_notification_service_interactive_uitest.cc
@@ -433,15 +433,8 @@
   ASSERT_EQ(0u, notifications.size());
 }
 
-// Flaky on Linux. http://crbug.com/784951
-#if defined(OS_LINUX)
-#define MAYBE_UserClosesPersistentNotification \
-  DISABLED_UserClosesPersistentNotification
-#else
-#define MAYBE_UserClosesPersistentNotification UserClosesPersistentNotification
-#endif
 IN_PROC_BROWSER_TEST_F(PlatformNotificationServiceBrowserTest,
-                       MAYBE_UserClosesPersistentNotification) {
+                       UserClosesPersistentNotification) {
   base::UserActionTester user_action_tester;
   ASSERT_NO_FATAL_FAILURE(GrantNotificationPermissionForTest());
 
@@ -457,7 +450,9 @@
       GetDisplayedNotifications(true /* is_persistent */);
   ASSERT_EQ(1u, notifications.size());
 
-  notifications[0].delegate()->Close(true /* by_user */);
+  display_service_tester_->RemoveNotification(NotificationCommon::PERSISTENT,
+                                              notifications[0].id(),
+                                              true /* by_user */);
 
   // The user closed this notification so the score should remain the same.
   EXPECT_DOUBLE_EQ(5.5, GetEngagementScore(GetLastCommittedURL()));
diff --git a/chrome/browser/notifications/platform_notification_service_unittest.cc b/chrome/browser/notifications/platform_notification_service_unittest.cc
index a805124c7..b3e97d5b 100644
--- a/chrome/browser/notifications/platform_notification_service_unittest.cc
+++ b/chrome/browser/notifications/platform_notification_service_unittest.cc
@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ -150,24 +151,24 @@
 
 TEST_F(PlatformNotificationServiceTest, OnPersistentNotificationClick) {
   EXPECT_CALL(*mock_logger_, LogPersistentNotificationClickWithoutPermission());
-  service()->OnPersistentNotificationClick(profile_, "jskdcjdslkcjlds",
-                                           GURL("https://example.com/"),
-                                           base::nullopt, base::nullopt);
+  service()->OnPersistentNotificationClick(
+      profile_, "jskdcjdslkcjlds", GURL("https://example.com/"), base::nullopt,
+      base::nullopt, base::BindOnce(&base::DoNothing));
 }
 
 TEST_F(PlatformNotificationServiceTest, OnPersistentNotificationClosedByUser) {
   EXPECT_CALL(*mock_logger_, LogPersistentNotificationClosedByUser());
-  service()->OnPersistentNotificationClose(profile_, "some_random_id_123",
-                                           GURL("https://example.com/"),
-                                           true /* by_user */);
+  service()->OnPersistentNotificationClose(
+      profile_, "some_random_id_123", GURL("https://example.com/"),
+      true /* by_user */, base::BindOnce(&base::DoNothing));
 }
 
 TEST_F(PlatformNotificationServiceTest,
        OnPersistentNotificationClosedProgrammatically) {
   EXPECT_CALL(*mock_logger_, LogPersistentNotificationClosedProgrammatically());
-  service()->OnPersistentNotificationClose(profile_, "some_random_id_738",
-                                           GURL("https://example.com/"),
-                                           false /* by_user */);
+  service()->OnPersistentNotificationClose(
+      profile_, "some_random_id_738", GURL("https://example.com/"),
+      false /* by_user */, base::BindOnce(&base::DoNothing));
 }
 
 TEST_F(PlatformNotificationServiceTest, DisplayNonPersistentPropertiesMatch) {
diff --git a/chrome/browser/notifications/stub_notification_display_service.cc b/chrome/browser/notifications/stub_notification_display_service.cc
index d57736b..eed7500 100644
--- a/chrome/browser/notifications/stub_notification_display_service.cc
+++ b/chrome/browser/notifications/stub_notification_display_service.cc
@@ -6,6 +6,10 @@
 
 #include <algorithm>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
+#include "base/run_loop.h"
 #include "chrome/browser/notifications/notification_handler.h"
 #include "chrome/browser/profiles/profile.h"
 
@@ -89,8 +93,10 @@
       DCHECK(!handler);
       iter->notification.delegate()->Close(by_user);
     } else {
+      base::RunLoop run_loop;
       handler->OnClose(profile_, iter->notification.origin_url(),
-                       notification_id, by_user);
+                       notification_id, by_user, run_loop.QuitClosure());
+      run_loop.Run();
     }
   }
 
@@ -105,8 +111,11 @@
   for (auto iter = notifications_.begin(); iter != notifications_.end();) {
     if (iter->type == notification_type) {
       if (handler) {
+        base::RunLoop run_loop;
         handler->OnClose(profile_, iter->notification.origin_url(),
-                         iter->notification.id(), by_user);
+                         iter->notification.id(), by_user,
+                         run_loop.QuitClosure());
+        run_loop.Run();
       } else {
         iter->notification.delegate()->Close(by_user);
       }
diff --git a/chrome/browser/notifications/stub_notification_display_service.h b/chrome/browser/notifications/stub_notification_display_service.h
index fabd936..7e91fdc 100644
--- a/chrome/browser/notifications/stub_notification_display_service.h
+++ b/chrome/browser/notifications/stub_notification_display_service.h
@@ -46,15 +46,16 @@
 
   // Simulates the notification identified by |notification_id| being closed due
   // to external events, such as the user dismissing it when |by_user| is set.
-  // When |silent| is set, the notification handlers won't be informed of the
-  // change to immitate behaviour of operating systems that don't inform apps
-  // about removed notifications.
+  // Will wait for the close event to complete. When |silent| is set, the
+  // notification handlers won't be informed of the change to immitate behaviour
+  // of operating systems that don't inform apps about removed notifications.
   void RemoveNotification(NotificationCommon::Type notification_type,
                           const std::string& notification_id,
                           bool by_user,
                           bool silent);
 
-  // Removes all notifications shown by this display service.
+  // Removes all notifications shown by this display service. Will wait for the
+  // close events to complete.
   void RemoveAllNotifications(NotificationCommon::Type notification_type,
                               bool by_user);