Nuke NotificationUIManager from PlatformNotificationServiceImpl

chrome/browser/notifications/platform_notification_service_browsertest.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 <string>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "chrome/browser/notifications/chrome_notification_display_service.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_permission_context.h"
 #include "chrome/browser/notifications/notification_permission_context_factory.h"
 #include "chrome/browser/notifications/notification_test_util.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_manager.h"
 #include "chrome/grit/generated_resources.h"
@@ skipping 62 matching lines @@
   std::string RequestAndRespondToPermission(
       PermissionBubbleManager::AutoResponseType bubble_response);
 
   content::WebContents* GetActiveWebContents(Browser* browser) {
     return browser->tab_strip_model()->GetActiveWebContents();
   }
 
   const base::FilePath server_root_;
   const std::string test_page_url_;
   scoped_ptr<StubNotificationUIManager> ui_manager_;
+  scoped_ptr<ChromeNotificationDisplayService> display_service_;
   scoped_ptr<net::EmbeddedTestServer> https_server_;
 };
 
 // -----------------------------------------------------------------------------
 
 namespace {
 const char kTestFileName[] = "notifications/platform_notification_service.html";
 }
 
 PlatformNotificationServiceBrowserTest::PlatformNotificationServiceBrowserTest()
     : server_root_(FILE_PATH_LITERAL("chrome/test/data")),
       // The test server has a base directory that doesn't exist in the
       // filesystem.
       test_page_url_(std::string("/") + kTestFileName) {}
 
 void PlatformNotificationServiceBrowserTest::SetUpCommandLine(
     base::CommandLine* command_line) {
   command_line->AppendSwitch(switches::kEnableExperimentalWebPlatformFeatures);
   command_line->AppendSwitch(switches::kEnableNotificationActionIcons);
 
   InProcessBrowserTest::SetUpCommandLine(command_line);
 }
 
 void PlatformNotificationServiceBrowserTest::SetUp() {
   ui_manager_.reset(new StubNotificationUIManager);
   https_server_.reset(
       new net::EmbeddedTestServer(net::EmbeddedTestServer::TYPE_HTTPS));
   https_server_->ServeFilesFromSourceDirectory(server_root_);
   ASSERT_TRUE(https_server_->Start());
-
-  service()->SetNotificationUIManagerForTesting(ui_manager_.get());
-
   InProcessBrowserTest::SetUp();
 }
 
 void PlatformNotificationServiceBrowserTest::SetUpOnMainThread() {
   NavigateToTestPage(test_page_url_);
-
+  display_service_.reset(new ChromeNotificationDisplayService(
+      browser()->profile(), ui_manager_.get()));
+  service()->SetNotificationDisplayServiceForTesting(display_service_.get());

[Peter Beverloo, 2016/04/18 14:57:10]

Why can't we use KeyedServiceFactory::SetTestingFactory()? That way
PlatformNotificationServiceImpl wouldn't have to care at all.

[Miguel Garcia, 2016/04/19 14:24:58]

Looked into it, we can for the unit test but the browser tests still need this
method so it seems better to just use the same mechanism in all tests.

   InProcessBrowserTest::SetUpOnMainThread();
 }
 
 void PlatformNotificationServiceBrowserTest::TearDown() {
-  service()->SetNotificationUIManagerForTesting(nullptr);
+  service()->SetNotificationDisplayServiceForTesting(nullptr);
 }
 
 void PlatformNotificationServiceBrowserTest::
     GrantNotificationPermissionForTest() const {
   GURL origin = TestPageUrl().GetOrigin();
 
   DesktopNotificationProfileUtil::GrantPermission(browser()->profile(), origin);
   ASSERT_EQ(CONTENT_SETTING_ALLOW,
             DesktopNotificationProfileUtil::GetContentSetting(
                 browser()->profile(), origin));
@@ skipping 338 matching lines @@
   EXPECT_EQ("actionTitle2", base::UTF16ToUTF8(notification.buttons()[1].title));
 
   notification.delegate()->ButtonClick(0);
   ASSERT_TRUE(RunScript("GetMessageFromWorker()", &script_result));
   EXPECT_EQ("action_button_click actionId1", script_result);
 
   notification.delegate()->ButtonClick(1);
   ASSERT_TRUE(RunScript("GetMessageFromWorker()", &script_result));
   EXPECT_EQ("action_button_click actionId2", script_result);
 }