Update more tests to use NotificationDisplayServiceTester. (try 2)

For the changes to FileManagerBrowserTestBase, you can run affected
tests with --gtest_filter=DriveSpecific*

This is identical to 2a3c7c84 but should work now after the fix
which landed as 7a379f73

Bug: 783018
Change-Id: Ie77f44e2e7a5af15e24e630c412bebe5788cb5a0
Reviewed-on: https://chromium-review.googlesource.com/881681Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532050}

chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager_unittest.cc

@@ -11,10 +11,8 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
-#include "chrome/browser/notifications/notification_test_util.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/grit/generated_resources.h"
-#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/fake_auth_policy_client.h"
@@ -52,15 +50,14 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
     chromeos::NetworkHandler::Initialize();
     fake_auth_policy_client()->DisableOperationDelayForTesting();
 
-    TestingBrowserProcess::GetGlobal()->SetNotificationUIManager(
-        std::make_unique<StubNotificationUIManager>());
-
     TestingProfile::Builder profile_builder;
     profile_builder.SetProfileName("user@gmail.com");
     profile_ = profile_builder.Build();
     account_id_ = AccountId::AdFromUserEmailObjGuid(
         profile()->GetProfileUserName(), "1234567890");
     mock_user_manager()->AddUser(account_id_);
+    display_service_ =
+        std::make_unique<NotificationDisplayServiceTester>(profile());
 
     base::RunLoop run_loop;
     fake_auth_policy_client()->set_on_get_status_closure(
@@ -99,19 +96,18 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
   }
 
   int GetNumberOfNotifications() {
-    return TestingBrowserProcess::GetGlobal()
-        ->notification_ui_manager()
-        ->GetAllIdsByProfile(profile())
+    return display_service_
+        ->GetDisplayedNotificationsForType(NotificationHandler::Type::TRANSIENT)
         .size();
   }
 
-  bool CancelNotificationById(int message_id) {
+  void CancelNotificationById(int message_id) {
     const std::string notification_id = kProfileSigninNotificationId +
                                         profile()->GetProfileUserName() +
                                         std::to_string(message_id);
-    return TestingBrowserProcess::GetGlobal()
-        ->notification_ui_manager()
-        ->CancelById(notification_id, profile());
+    EXPECT_TRUE(display_service_->GetNotification(notification_id));
+    display_service_->RemoveNotification(NotificationHandler::Type::TRANSIENT,
+                                         notification_id, false);
   }
 
   void CallGetUserStatusAndWait() {
@@ -123,7 +119,6 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
     testing::Mock::VerifyAndClearExpectations(mock_user_manager());
   }
 
- private:
   content::TestBrowserThreadBundle thread_bundle_;
   AccountId account_id_;
   std::unique_ptr<TestingProfile> profile_;
@@ -132,6 +127,9 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
   AuthPolicyCredentialsManager* auth_policy_credentials_manager_;
   user_manager::ScopedUserManager user_manager_enabler_;
 
+  std::unique_ptr<NotificationDisplayServiceTester> display_service_;
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(AuthPolicyCredentialsManagerTest);
 };
 
@@ -162,7 +160,7 @@ TEST_F(AuthPolicyCredentialsManagerTest, ShowSameNotificationOnce) {
   EXPECT_CALL(*mock_user_manager(), SaveForceOnlineSignin(account_id(), true));
   CallGetUserStatusAndWait();
   EXPECT_EQ(1, GetNumberOfNotifications());
-  EXPECT_TRUE(CancelNotificationById(IDS_ACTIVE_DIRECTORY_PASSWORD_EXPIRED));
+  CancelNotificationById(IDS_ACTIVE_DIRECTORY_PASSWORD_EXPIRED);
 
   // Do not show the same notification twice.
   EXPECT_CALL(*mock_user_manager(), SaveForceOnlineSignin(account_id(), true));
@@ -181,8 +179,8 @@ TEST_F(AuthPolicyCredentialsManagerTest, ShowDifferentNotifications) {
   EXPECT_CALL(*mock_user_manager(), SaveForceOnlineSignin(account_id(), true));
   CallGetUserStatusAndWait();
   EXPECT_EQ(2, GetNumberOfNotifications());
-  EXPECT_TRUE(CancelNotificationById(IDS_ACTIVE_DIRECTORY_PASSWORD_CHANGED));
-  EXPECT_TRUE(CancelNotificationById(IDS_ACTIVE_DIRECTORY_REFRESH_AUTH_TOKEN));
+  CancelNotificationById(IDS_ACTIVE_DIRECTORY_PASSWORD_CHANGED);
+  CancelNotificationById(IDS_ACTIVE_DIRECTORY_REFRESH_AUTH_TOKEN);
   EXPECT_EQ(0, GetNumberOfNotifications());
 }
 

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -25,7 +25,7 @@
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/common/chrome_switches.h"
 #include "chromeos/chromeos_switches.h"
 #include "components/drive/chromeos/file_system_interface.h"
@@ -549,6 +549,9 @@ void FileManagerBrowserTestBase::SetUpOnMainThread() {
     drive_volume_->ConfigureShareUrlBase(share_url_base);
     test_util::WaitUntilDriveMountPointIsAdded(profile());
   }
+
+  display_service_ =
+      std::make_unique<NotificationDisplayServiceTester>(profile());
 }
 
 void FileManagerBrowserTestBase::SetUpCommandLine(
@@ -729,12 +732,13 @@ void FileManagerBrowserTestBase::OnMessage(const std::string& name,
     ASSERT_TRUE(value.GetInteger("index", &index));
 
     const std::string delegate_id = extension_id + "-" + notification_id;
-    const message_center::Notification* notification =
-        g_browser_process->notification_ui_manager()->FindById(delegate_id,
-                                                               profile());
-    ASSERT_TRUE(notification);
 
-    notification->delegate()->ButtonClick(index);
+    base::Optional<message_center::Notification> notification =
+        display_service_->GetNotification(delegate_id);
+    EXPECT_TRUE(notification);
+
+    display_service_->SimulateClick(NotificationHandler::Type::EXTENSION,
+                                    delegate_id, index, base::nullopt);
     return;
   }
 

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

@@ -20,13 +20,15 @@
 #define DISABLE_SLOW_FILESAPP_TESTS
 #endif
 
+class NotificationDisplayServiceTester;
+
 namespace file_manager {
 
 enum GuestMode { NOT_IN_GUEST_MODE, IN_GUEST_MODE, IN_INCOGNITO };
 
-class LocalTestVolume;
 class DriveTestVolume;
 class FakeTestVolume;
+class LocalTestVolume;
 
 // The base test class.
 class FileManagerBrowserTestBase : public ExtensionApiTest {
@@ -72,6 +74,7 @@ class FileManagerBrowserTestBase : public ExtensionApiTest {
       create_drive_integration_service_;
   std::unique_ptr<drive::DriveIntegrationServiceFactory::ScopedFactoryForTest>
       service_factory_for_test_;
+  std::unique_ptr<NotificationDisplayServiceTester> display_service_;
 };
 
 }  // namespace file_manager

chrome/browser/chromeos/hats/hats_notification_controller_unittest.cc

@@ -8,7 +8,6 @@
 #include "base/strings/string_split.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/notifications/notification_handler.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/testing_browser_process.h"

chrome/browser/extensions/extension_crash_recovery_browsertest.cc

@@ -9,7 +9,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -30,21 +30,21 @@
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/constants.h"
 #include "extensions/test/background_page_watcher.h"
-#include "ui/message_center/message_center.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_delegate.h"
-#include "ui/message_center/notification_list.h"
 
 using content::NavigationController;
 using content::WebContents;
 using extensions::Extension;
 using extensions::ExtensionRegistry;
 
-class ExtensionCrashRecoveryTestBase : public ExtensionBrowserTest {
+class ExtensionCrashRecoveryTest : public ExtensionBrowserTest {
  protected:
-  virtual void AcceptNotification(size_t index) = 0;
-  virtual void CancelNotification(size_t index) = 0;
-  virtual size_t CountBalloons() = 0;
+  void SetUpOnMainThread() override {
+    ExtensionBrowserTest::SetUpOnMainThread();
+    display_service_ =
+        std::make_unique<NotificationDisplayServiceTester>(profile());
+  }
 
   ExtensionService* GetExtensionService() {
     return extensions::ExtensionSystem::Get(browser()->profile())->
@@ -122,44 +122,25 @@ class ExtensionCrashRecoveryTestBase : public ExtensionBrowserTest {
     CheckExtensionConsistency(second_extension_id_);
   }
 
-  std::string first_extension_id_;
-  std::string second_extension_id_;
-};
-
-class ExtensionCrashRecoveryTest : public ExtensionCrashRecoveryTestBase {
- protected:
-  void AcceptNotification(size_t index) override {
-    message_center::MessageCenter* message_center =
-        message_center::MessageCenter::Get();
-    ASSERT_GT(message_center->NotificationCount(), index);
-    message_center::NotificationList::Notifications::reverse_iterator it =
-        message_center->GetVisibleNotifications().rbegin();
-    for (size_t i = 0; i < index; ++i)
-      ++it;
-    std::string id = (*it)->id();
+  void AcceptNotification(const std::string& extension_id) {
     extensions::TestExtensionRegistryObserver observer(GetExtensionRegistry());
-    message_center->ClickOnNotification(id);
+    display_service_->SimulateClick(NotificationHandler::Type::TRANSIENT,
+                                    "app.background.crashed." + extension_id,
+                                    base::nullopt, base::nullopt);
     auto* extension = observer.WaitForExtensionLoaded();
     extensions::BackgroundPageWatcher(GetProcessManager(), extension)
         .WaitForOpen();
   }
 
-  void CancelNotification(size_t index) override {
-    message_center::MessageCenter* message_center =
-        message_center::MessageCenter::Get();
-    ASSERT_GT(message_center->NotificationCount(), index);
-    message_center::NotificationList::Notifications::reverse_iterator it =
-        message_center->GetVisibleNotifications().rbegin();
-    for (size_t i = 0; i < index; ++i)
-      ++it;
-    ASSERT_TRUE(g_browser_process->notification_ui_manager()->CancelById(
-        (*it)->id(),
-        NotificationUIManager::GetProfileID(browser()->profile())));
+  size_t CountNotifications() {
+    return display_service_
+        ->GetDisplayedNotificationsForType(NotificationHandler::Type::TRANSIENT)
+        .size();
   }
 
-  size_t CountBalloons() override {
-    return message_center::MessageCenter::Get()->NotificationCount();
-  }
+  std::string first_extension_id_;
+  std::string second_extension_id_;
+  std::unique_ptr<NotificationDisplayServiceTester> display_service_;
 };
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, Basic) {
@@ -169,7 +150,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, Basic) {
   CrashExtension(first_extension_id_);
   ASSERT_EQ(count_before, GetEnabledExtensionCount());
   ASSERT_EQ(crash_count_before + 1, GetTerminatedExtensionCount());
-  ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+  ASSERT_NO_FATAL_FAILURE(AcceptNotification(first_extension_id_));
 
   SCOPED_TRACE("after clicking the balloon");
   CheckExtensionConsistency(first_extension_id_);
@@ -186,7 +167,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, DISABLED_CloseAndReload) {
   ASSERT_EQ(count_before, GetEnabledExtensionCount());
   ASSERT_EQ(crash_count_before + 1, GetTerminatedExtensionCount());
 
-  ASSERT_NO_FATAL_FAILURE(CancelNotification(0));
+  // In 2013, when this test became flaky, this line was part of the test.
+  // CancelNotification() no longer exists.
+  // ASSERT_NO_FATAL_FAILURE(CancelNotification(0));
   ReloadExtension(first_extension_id_);
 
   SCOPED_TRACE("after reloading");
@@ -211,7 +194,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, ReloadIndependently) {
 
   // The balloon should automatically hide after the extension is successfully
   // reloaded.
-  ASSERT_EQ(0U, CountBalloons());
+  ASSERT_EQ(0U, CountNotifications());
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
@@ -224,7 +207,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
   WebContents* original_tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(original_tab);
-  ASSERT_EQ(1U, CountBalloons());
+  ASSERT_EQ(1U, CountNotifications());
 
   // Open a new tab, but the balloon will still be there.
   chrome::NewTab(browser());
@@ -232,7 +215,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(new_current_tab);
   ASSERT_NE(new_current_tab, original_tab);
-  ASSERT_EQ(1U, CountBalloons());
+  ASSERT_EQ(1U, CountNotifications());
 
   ReloadExtension(first_extension_id_);
 
@@ -241,7 +224,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
 
   // The balloon should automatically hide after the extension is successfully
   // reloaded.
-  ASSERT_EQ(0U, CountBalloons());
+  ASSERT_EQ(0U, CountNotifications());
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
@@ -254,14 +237,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
   WebContents* current_tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(current_tab);
-  ASSERT_EQ(1U, CountBalloons());
+  ASSERT_EQ(1U, CountNotifications());
 
   // Navigate to another page.
   ui_test_utils::NavigateToURL(
       browser(), ui_test_utils::GetTestUrl(
                      base::FilePath(base::FilePath::kCurrentDirectory),
                      base::FilePath(FILE_PATH_LITERAL("title1.html"))));
-  ASSERT_EQ(1U, CountBalloons());
+  ASSERT_EQ(1U, CountNotifications());
 
   ReloadExtension(first_extension_id_);
 
@@ -270,7 +253,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
 
   // The balloon should automatically hide after the extension is successfully
   // reloaded.
-  ASSERT_EQ(0U, CountBalloons());
+  ASSERT_EQ(0U, CountNotifications());
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, ShutdownWhileCrashed) {
@@ -286,7 +269,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsCrashFirst) {
   LoadSecondExtension();
   CrashExtension(first_extension_id_);
   ASSERT_EQ(count_before + 1, GetEnabledExtensionCount());
-  ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+  ASSERT_NO_FATAL_FAILURE(AcceptNotification(first_extension_id_));
 
   SCOPED_TRACE("after clicking the balloon");
   CheckExtensionConsistency(first_extension_id_);
@@ -299,7 +282,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsCrashSecond) {
   LoadSecondExtension();
   CrashExtension(second_extension_id_);
   ASSERT_EQ(count_before + 1, GetEnabledExtensionCount());
-  ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+  ASSERT_NO_FATAL_FAILURE(AcceptNotification(second_extension_id_));
 
   SCOPED_TRACE("after clicking the balloon");
   CheckExtensionConsistency(first_extension_id_);
@@ -321,13 +304,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
 
   {
     SCOPED_TRACE("first balloon");
-    ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+    ASSERT_NO_FATAL_FAILURE(AcceptNotification(first_extension_id_));
     CheckExtensionConsistency(first_extension_id_);
   }
 
   {
     SCOPED_TRACE("second balloon");
-    ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+    ASSERT_NO_FATAL_FAILURE(AcceptNotification(second_extension_id_));
     CheckExtensionConsistency(first_extension_id_);
     CheckExtensionConsistency(second_extension_id_);
   }
@@ -344,13 +327,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsOneByOne) {
 
   {
     SCOPED_TRACE("first balloon");
-    ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+    ASSERT_NO_FATAL_FAILURE(AcceptNotification(first_extension_id_));
     CheckExtensionConsistency(first_extension_id_);
   }
 
   {
     SCOPED_TRACE("second balloon");
-    ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+    ASSERT_NO_FATAL_FAILURE(AcceptNotification(second_extension_id_));
     CheckExtensionConsistency(first_extension_id_);
     CheckExtensionConsistency(second_extension_id_);
   }
@@ -384,8 +367,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
   // Accept notification 1 before canceling notification 0.
   // Otherwise, on Linux and Windows, there is a race here, in which
   // canceled notifications do not immediately go away.
-  ASSERT_NO_FATAL_FAILURE(AcceptNotification(1));
-  ASSERT_NO_FATAL_FAILURE(CancelNotification(0));
+  ASSERT_NO_FATAL_FAILURE(AcceptNotification(first_extension_id_));
+  // In 2013, when this test became flaky, these lines were part of the test.
+  // CancelNotification() no longer exists.
+  // ASSERT_NO_FATAL_FAILURE(CancelNotification(0));
 
   SCOPED_TRACE("balloons done");
   ASSERT_EQ(count_before + 1, GetEnabledExtensionCount());
@@ -408,16 +393,16 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
         browser()->tab_strip_model()->GetActiveWebContents();
     ASSERT_TRUE(current_tab);
     // At the beginning we should have one balloon displayed for each extension.
-    ASSERT_EQ(2U, CountBalloons());
+    ASSERT_EQ(2U, CountNotifications());
     ReloadExtension(first_extension_id_);
     // One of the balloons should hide after the extension is reloaded.
-    ASSERT_EQ(1U, CountBalloons());
+    ASSERT_EQ(1U, CountNotifications());
     CheckExtensionConsistency(first_extension_id_);
   }
 
   {
     SCOPED_TRACE("second: balloon");
-    ASSERT_NO_FATAL_FAILURE(AcceptNotification(0));
+    ASSERT_NO_FATAL_FAILURE(AcceptNotification(second_extension_id_));
     CheckExtensionConsistency(first_extension_id_);
     CheckExtensionConsistency(second_extension_id_);
   }
@@ -432,14 +417,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CrashAndUninstall) {
   ASSERT_EQ(count_before + 1, GetEnabledExtensionCount());
   ASSERT_EQ(crash_count_before + 1, GetTerminatedExtensionCount());
 
-  ASSERT_EQ(1U, CountBalloons());
+  ASSERT_EQ(1U, CountNotifications());
   UninstallExtension(first_extension_id_);
   base::RunLoop().RunUntilIdle();
 
   SCOPED_TRACE("after uninstalling");
   ASSERT_EQ(count_before + 1, GetEnabledExtensionCount());
   ASSERT_EQ(crash_count_before, GetTerminatedExtensionCount());
-  ASSERT_EQ(0U, CountBalloons());
+  ASSERT_EQ(0U, CountNotifications());
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CrashAndUnloadAll) {
@@ -480,18 +465,23 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
   EXPECT_EQ(count_before, GetEnabledExtensionCount());
   EXPECT_EQ(crash_count_before + 1, GetTerminatedExtensionCount());
 
+  extensions::TestExtensionRegistryObserver observer(GetExtensionRegistry());
   {
     content::WindowedNotificationObserver observer(
         content::NOTIFICATION_LOAD_STOP,
-        content::Source<NavigationController>(
-            &browser()->tab_strip_model()->GetActiveWebContents()->
-                GetController()));
+        content::Source<NavigationController>(&browser()
+                                                   ->tab_strip_model()
+                                                   ->GetActiveWebContents()
+                                                   ->GetController()));
     chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB);
     observer.Wait();
   }
+  auto* extension = observer.WaitForExtensionLoaded();
+  EXPECT_EQ(first_extension_id_, extension->id());
+
   // Extension should now be loaded.
   SCOPED_TRACE("after reloading the tab");
   CheckExtensionConsistency(first_extension_id_);
   ASSERT_EQ(count_before + 1, GetEnabledExtensionCount());
-  ASSERT_EQ(0U, CountBalloons());
+  ASSERT_EQ(0U, CountNotifications());
 }