PlatformNotificationService layering cleanup.

chrome/browser/notifications/platform_notification_service_unittest.cc

 // Copyright 2014 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 <stdint.h>
 
 #include <utility>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/notifications/message_center_display_service.h"
 #include "chrome/browser/notifications/notification_delegate.h"
+#include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/notifications/notification_test_util.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
+#include "chrome/browser/notifications/stub_notification_platform_bridge.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/desktop_notification_delegate.h"
 #include "content/public/common/notification_resources.h"
 #include "content/public/common/platform_notification_data.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 48 matching lines @@
  private:
   bool displayed_;
   bool clicked_;
 };
 
 }  // namespace
 
 class PlatformNotificationServiceTest : public testing::Test {
  public:
   void SetUp() override {
-    profile_.reset(new TestingProfile());
-    ui_manager_.reset(new StubNotificationUIManager);
-    display_service_.reset(
-        new MessageCenterDisplayService(profile_.get(), ui_manager_.get()));
-    service()->SetNotificationDisplayServiceForTesting(display_service_.get());
     profile_manager_.reset(
         new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
     ASSERT_TRUE(profile_manager_->SetUp());
+    profile_ = profile_manager_->CreateTestingProfile("Miguel");
+    std::unique_ptr<NotificationUIManager> ui_manager(
+        new StubNotificationUIManager);
+    std::unique_ptr<NotificationPlatformBridge> notification_bridge(
+        new StubNotificationPlatformBridge());
+
+    TestingBrowserProcess::GetGlobal()->SetNotificationUIManager(
+        std::move(ui_manager));
+    TestingBrowserProcess::GetGlobal()->SetNotificationPlatformBridge(
+        std::move(notification_bridge));
   }
 
   void TearDown() override {
-    service()->SetNotificationDisplayServiceForTesting(nullptr);
-    display_service_.reset();
-    ui_manager_.reset();
-    profile_.reset();
     profile_manager_.reset();
     TestingBrowserProcess::DeleteInstance();
   }
 
  protected:
   // Displays a simple, fake notifications and returns a weak pointer to the
   // delegate receiving events for it (ownership is transferred to the service).
   MockDesktopNotificationDelegate* CreateSimplePageNotification() const {
     return CreateSimplePageNotificationWithCloseClosure(nullptr);
   }
@@ skipping 16 matching lines @@
 
     return delegate;
   }
 
   // Returns the Platform Notification Service these unit tests are for.
   PlatformNotificationServiceImpl* service() const {
     return PlatformNotificationServiceImpl::GetInstance();
   }
 
   // Returns the Profile to be used for these tests.
-  Profile* profile() const { return profile_.get(); }
+  Profile* profile() const { return profile_; }
 
-  // Returns the UI Manager on which notifications will be displayed.
-  StubNotificationUIManager* ui_manager() const { return ui_manager_.get(); }
+  size_t GetNotificationCount() const {
+    std::set<std::string> notifications;
+    EXPECT_TRUE(display_service()->GetDisplayed(&notifications));
+    return notifications.size();
+  }
+
+  Notification GetDisplayedNotification() {
+#if defined(OS_ANDROID)
+    return static_cast<StubNotificationPlatformBridge*>(
+               g_browser_process->notification_platform_bridge())
+        ->GetNotificationAt(0);
+#else
+    return static_cast<StubNotificationUIManager*>(
+               g_browser_process->notification_ui_manager())
+        ->GetNotificationAt(0);
+#endif

[Peter Beverloo, 2016/04/25 13:37:29]

This is why I'd like this test to use a mocked NotificationDisplayService
instead, it shouldn't have to care about these sorts of differences. (That's an
implementation detail of the NotificationDisplayService.)

That said, we're still in the same boat for the PNSI as well, so we should
figure out a plan and address all cases at once.

[Miguel Garcia, 2016/04/25 17:19:59]

Yeah I knew you would not like this one. I was hoping I would be able to change
the test just from the public interface (like I did with GetNotificationCount)
but all in all this seems like a net improvement.

On 2016/04/25 13:37:29, Peter Beverloo wrote:

+  }
 
  private:
-  std::unique_ptr<TestingProfile> profile_;
-  std::unique_ptr<StubNotificationUIManager> ui_manager_;
-  std::unique_ptr<MessageCenterDisplayService> display_service_;
+  NotificationDisplayService* display_service() const {
+    return NotificationDisplayServiceFactory::GetForProfile(profile_);
+  }
 
-  std::unique_ptr<TestingProfileManager> profile_manager_;
+  scoped_ptr<TestingProfileManager> profile_manager_;
+  TestingProfile* profile_;
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
 TEST_F(PlatformNotificationServiceTest, DisplayPageDisplayedEvent) {
   auto* delegate = CreateSimplePageNotification();
 
-  EXPECT_EQ(1u, ui_manager()->GetNotificationCount());
+  EXPECT_EQ(1u, GetNotificationCount());
   EXPECT_TRUE(delegate->displayed());
 }
 
 TEST_F(PlatformNotificationServiceTest, DisplayPageCloseClosure) {
   base::Closure close_closure;
   CreateSimplePageNotificationWithCloseClosure(&close_closure);
 
-  EXPECT_EQ(1u, ui_manager()->GetNotificationCount());
+  EXPECT_EQ(1u, GetNotificationCount());
 
   ASSERT_FALSE(close_closure.is_null());
   close_closure.Run();
 
-  EXPECT_EQ(0u, ui_manager()->GetNotificationCount());
-
+  EXPECT_EQ(0u, GetNotificationCount());
   // Note that we cannot verify whether the closed event was called on the
   // delegate given that it'd result in a use-after-free.
 }
 
 // TODO(peter): Re-enable this test when //content is responsible for creating
 // the notification delegate ids.
 #if !defined(OS_ANDROID)
 TEST_F(PlatformNotificationServiceTest, PersistentNotificationDisplay) {
   PlatformNotificationData notification_data;
   notification_data.title = base::ASCIIToUTF16("My notification's title");
   notification_data.body = base::ASCIIToUTF16("Hello, world!");
 
   service()->DisplayPersistentNotification(
       profile(), kPersistentNotificationId, GURL("https://chrome.com/"),
       notification_data, NotificationResources());
 
-  ASSERT_EQ(1u, ui_manager()->GetNotificationCount());
+  ASSERT_EQ(1u, GetNotificationCount());
 
-  const Notification& notification = ui_manager()->GetNotificationAt(0);
+  const Notification& notification = GetDisplayedNotification();

[Peter Beverloo, 2016/04/25 13:37:29]

nit: no const& (GetDisplayedNotification returns by value)

elsewhere too

[Miguel Garcia, 2016/04/25 17:19:59]

Done.

   EXPECT_EQ("https://chrome.com/", notification.origin_url().spec());
   EXPECT_EQ("My notification's title",
       base::UTF16ToUTF8(notification.title()));
   EXPECT_EQ("Hello, world!",
       base::UTF16ToUTF8(notification.message()));
 
   service()->ClosePersistentNotification(profile(), kPersistentNotificationId);
-  EXPECT_EQ(0u, ui_manager()->GetNotificationCount());
+  EXPECT_EQ(0u, GetNotificationCount());
 }
 #endif  // !defined(OS_ANDROID)
 
 TEST_F(PlatformNotificationServiceTest, DisplayPageNotificationMatches) {
   std::vector<int> vibration_pattern(
       kNotificationVibrationPattern,
       kNotificationVibrationPattern + arraysize(kNotificationVibrationPattern));
 
   PlatformNotificationData notification_data;
   notification_data.title = base::ASCIIToUTF16("My notification's title");
   notification_data.body = base::ASCIIToUTF16("Hello, world!");
   notification_data.vibration_pattern = vibration_pattern;
   notification_data.silent = true;
 
   MockDesktopNotificationDelegate* delegate
       = new MockDesktopNotificationDelegate();
   service()->DisplayNotification(profile(), GURL("https://chrome.com/"),
                                  notification_data, NotificationResources(),
                                  base::WrapUnique(delegate), nullptr);
 
-  ASSERT_EQ(1u, ui_manager()->GetNotificationCount());
+  ASSERT_EQ(1u, GetNotificationCount());
 
-  const Notification& notification = ui_manager()->GetNotificationAt(0);
+  const Notification& notification = GetDisplayedNotification();
   EXPECT_EQ("https://chrome.com/", notification.origin_url().spec());
   EXPECT_EQ("My notification's title",
       base::UTF16ToUTF8(notification.title()));
   EXPECT_EQ("Hello, world!",
       base::UTF16ToUTF8(notification.message()));
 
   EXPECT_THAT(notification.vibration_pattern(),
       testing::ElementsAreArray(kNotificationVibrationPattern));
 
   EXPECT_TRUE(notification.silent());
@@ skipping 13 matching lines @@
   notification_data.actions[0].title = base::ASCIIToUTF16("Button 1");
   notification_data.actions[1].title = base::ASCIIToUTF16("Button 2");
 
   NotificationResources notification_resources;
   notification_resources.action_icons.resize(notification_data.actions.size());
 
   service()->DisplayPersistentNotification(
       profile(), 0u /* persistent notification */, GURL("https://chrome.com/"),
       notification_data, notification_resources);
 
-  ASSERT_EQ(1u, ui_manager()->GetNotificationCount());
+  ASSERT_EQ(1u, GetNotificationCount());
 
-  const Notification& notification = ui_manager()->GetNotificationAt(0);
+  const Notification& notification = GetDisplayedNotification();
   EXPECT_EQ("https://chrome.com/", notification.origin_url().spec());
   EXPECT_EQ("My notification's title", base::UTF16ToUTF8(notification.title()));
   EXPECT_EQ("Hello, world!", base::UTF16ToUTF8(notification.message()));
 
   EXPECT_THAT(notification.vibration_pattern(),
               testing::ElementsAreArray(kNotificationVibrationPattern));
 
   EXPECT_TRUE(notification.silent());
 
   const auto& buttons = notification.buttons();
@@ skipping 145 matching lines @@
   notification = service()->CreateNotificationFromData(
       profile(),
       GURL("chrome-extension://honijodknafkokifofgiaalefdiedpko/main.html"),
       notification_data, NotificationResources(),
       new MockNotificationDelegate("hello"));
   EXPECT_EQ("NotificationTest",
             base::UTF16ToUTF8(notification.context_message()));
 }
 
 #endif  // defined(ENABLE_EXTENSIONS)