Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4590)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/extensions/api/notifications/notifications_apitest.cc

Issue 2703213004: Migrate extension notifications to the new NotificationDisplayService (Closed)
Patch Set: Finish functionality Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/browser/extensions/api/notifications/notifications_api.cc ('K') | « chrome/browser/extensions/api/notifications/notifications_api.cc ('k') | chrome/browser/notifications/native_notification_display_service.cc » ('j') | chrome/browser/notifications/native_notification_display_service.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/extensions/api/notifications/notifications_apitest.cc
diff --git a/chrome/browser/extensions/api/notifications/notifications_apitest.cc b/chrome/browser/extensions/api/notifications/notifications_apitest.cc
index 3d372dfb4f1d18255e89c22523cf77d51936c852..a8da55ddd0084b3c6f9a909ce68914dcbbe647c3 100644
--- a/chrome/browser/extensions/api/notifications/notifications_apitest.cc
+++ b/chrome/browser/extensions/api/notifications/notifications_apitest.cc
@@ -7,12 +7,17 @@
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/apps/app_browsertest_util.h"
-#include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/api/notifications/extension_notification_display_helper.h"
+#include "chrome/browser/extensions/api/notifications/extension_notification_display_helper_factory.h"
#include "chrome/browser/extensions/api/notifications/notifications_api.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_function_test_utils.h"
#include "chrome/browser/notifications/notification.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/notifications/notification_common.h"
+#include "chrome/browser/notifications/notification_display_service_factory.h"
+#include "chrome/browser/notifications/notifier_state_tracker.h"
+#include "chrome/browser/notifications/notifier_state_tracker_factory.h"
+#include "chrome/browser/notifications/stub_notification_display_service.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/extensions/app_launch_params.h"
#include "chrome/browser/ui/extensions/application_launch.h"
@@ -29,8 +34,6 @@
#include "extensions/common/test_util.h"
#include "extensions/test/extension_test_message_listener.h"
#include "extensions/test/result_catcher.h"
-#include "ui/message_center/message_center.h"
-#include "ui/message_center/notification_list.h"
#include "ui/message_center/notifier_settings.h"
#if defined(OS_MACOSX)
@@ -41,6 +44,8 @@
using extensions::AppWindow;
using extensions::AppWindowRegistry;
using extensions::Extension;
+using extensions::ExtensionNotificationDisplayHelper;
+using extensions::ExtensionNotificationDisplayHelperFactory;
using extensions::ResultCatcher;
namespace utils = extension_function_test_utils;
@@ -154,14 +159,30 @@ class NotificationsApiTest : public ExtensionApiTest {
return NULL;
}
+ ExtensionNotificationDisplayHelper* GetDisplayHelper() {
+ return ExtensionNotificationDisplayHelperFactory::GetForProfile(profile());
+ }
+
+ StubNotificationDisplayService* GetDisplayService() {
+ return reinterpret_cast<StubNotificationDisplayService*>(
+ NotificationDisplayServiceFactory::GetForProfile(profile()));
+ }
+
+ NotifierStateTracker* GetNotifierStateTracker() {
+ return NotifierStateTrackerFactory::GetForProfile(profile());
+ }
+
protected:
+ void SetUpOnMainThread() override {
+ ExtensionApiTest::SetUpOnMainThread();
+
+ DCHECK(profile());
+ NotificationDisplayServiceFactory::GetInstance()->SetTestingFactory(
+ profile(), &StubNotificationDisplayService::FactoryForTests);
+ }
+
std::string GetNotificationIdFromDelegateId(const std::string& delegate_id) {
- return g_browser_process->notification_ui_manager()
- ->FindById(
- delegate_id,
- NotificationUIManager::GetProfileID(
- g_browser_process->profile_manager()->GetLastUsedProfile()))
- ->id();
+ return GetDisplayHelper()->GetByNotificationId(delegate_id)->id();
}
void LaunchPlatformApp(const Extension* extension) {
@@ -215,8 +236,8 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestByUser) {
ResultCatcher catcher;
const std::string notification_id =
GetNotificationIdFromDelegateId(extension->id() + "-FOO");
- g_browser_process->message_center()->RemoveNotification(notification_id,
- false);
+ GetDisplayService()->RemoveNotification(
+ NotificationCommon::EXTENSION, notification_id, false /* by_user */);
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
@@ -224,21 +245,21 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestByUser) {
ResultCatcher catcher;
const std::string notification_id =
GetNotificationIdFromDelegateId(extension->id() + "-BAR");
- g_browser_process->message_center()->RemoveNotification(notification_id,
- true);
+ GetDisplayService()->RemoveNotification(
+ NotificationCommon::EXTENSION, notification_id, true /* by_user */);
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
{
ResultCatcher catcher;
- g_browser_process->message_center()->RemoveAllNotifications(
- false /* by_user */, message_center::MessageCenter::RemoveType::ALL);
+ GetDisplayService()->RemoveAllNotifications(NotificationCommon::EXTENSION,
+ false /* by_user */);
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
{
ResultCatcher catcher;
- g_browser_process->message_center()->RemoveAllNotifications(
- true /* by_user */, message_center::MessageCenter::RemoveType::ALL);
+ GetDisplayService()->RemoveAllNotifications(NotificationCommon::EXTENSION,
+ true /* by_user */);
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
}
@@ -253,10 +274,9 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestPartialUpdate) {
int kNewPriority = 2;
const char kButtonTitle[] = "NewButton";
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+ Notification* notification = notifications.front().get();
ASSERT_EQ(extension->url(), notification->origin_url());
LOG(INFO) << "Notification ID: " << notification->id();
@@ -302,9 +322,7 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestGetPermissionLevel) {
message_center::NotifierId notifier_id(
message_center::NotifierId::APPLICATION,
empty_extension->id());
- message_center::Notifier notifier(notifier_id, base::string16(), true);
- g_browser_process->message_center()->GetNotifierSettingsProvider()->
- SetNotifierEnabled(notifier, false);
+ GetNotifierStateTracker()->SetNotifierEnabled(notifier_id, false);
std::unique_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult(
notification_function.get(), "[]", browser(), utils::NONE));
@@ -328,9 +346,7 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestOnPermissionLevelChanged) {
message_center::NotifierId notifier_id(
message_center::NotifierId::APPLICATION,
extension->id());
- message_center::Notifier notifier(notifier_id, base::string16(), true);
- g_browser_process->message_center()->GetNotifierSettingsProvider()->
- SetNotifierEnabled(notifier, false);
+ GetNotifierStateTracker()->SetNotifierEnabled(notifier_id, false);
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
@@ -342,9 +358,7 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestOnPermissionLevelChanged) {
message_center::NotifierId notifier_id(
message_center::NotifierId::APPLICATION,
extension->id());
- message_center::Notifier notifier(notifier_id, base::string16(), false);
- g_browser_process->message_center()->GetNotifierSettingsProvider()->
- SetNotifierEnabled(notifier, true);
+ GetNotifierStateTracker()->SetNotifierEnabled(notifier_id, true);
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
@@ -355,10 +369,9 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestUserGesture) {
LoadExtensionAndWait("notifications/api/user_gesture");
ASSERT_TRUE(extension) << message_;
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+ Notification* notification = notifications.front().get();
ASSERT_EQ(extension->url(), notification->origin_url());
{
@@ -367,11 +380,13 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestUserGesture) {
EXPECT_TRUE(catcher.GetNextResult());
notification->Click();
EXPECT_TRUE(catcher.GetNextResult());
- notification->Close(true);
+ notification->Close(true /* by_user */);
EXPECT_TRUE(catcher.GetNextResult());
- notification->Close(false);
- EXPECT_FALSE(catcher.GetNextResult());
+
+ // Note that |notification| no longer points to valid memory.
}
+
+ ASSERT_EQ(0u, GetDisplayHelper()->GetNotificationsForTests().size());
}
IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestRequireInteraction) {
@@ -379,10 +394,9 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestRequireInteraction) {
LoadExtensionAndWait("notifications/api/require_interaction");
ASSERT_TRUE(extension) << message_;
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+ Notification* notification = notifications.front().get();
ASSERT_EQ(extension->url(), notification->origin_url());
EXPECT_TRUE(notification->never_timeout());
@@ -400,13 +414,12 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestShouldDisplayNormal) {
ASSERT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(
GetFirstAppWindow(extension->id())->GetNativeWindow()));
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+
// If the app hasn't created a fullscreen window, then its notifications
// shouldn't be displayed when a window is fullscreen.
- ASSERT_FALSE(notification->delegate()->ShouldDisplayOverFullscreen());
+ ASSERT_FALSE(notifications[0]->delegate()->ShouldDisplayOverFullscreen());
}
IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestShouldDisplayFullscreen) {
@@ -429,13 +442,12 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestShouldDisplayFullscreen) {
ASSERT_TRUE(GetFirstAppWindow(extension->id())->GetBaseWindow()->IsActive())
<< "Not Active";
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+
// If the app has created a fullscreen window, then its notifications should
// be displayed when a window is fullscreen.
- ASSERT_TRUE(notification->delegate()->ShouldDisplayOverFullscreen());
+ ASSERT_TRUE(notifications[0]->delegate()->ShouldDisplayOverFullscreen());
}
IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestShouldDisplayFullscreenOff) {
@@ -458,13 +470,12 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestShouldDisplayFullscreenOff) {
ASSERT_TRUE(GetFirstAppWindow(extension->id())->GetBaseWindow()->IsActive())
<< "Not Active";
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+
// When the experiment flag is off, then ShouldDisplayOverFullscreen should
// return false.
- ASSERT_FALSE(notification->delegate()->ShouldDisplayOverFullscreen());
+ ASSERT_FALSE(notifications[0]->delegate()->ShouldDisplayOverFullscreen());
}
// The Fake OSX fullscreen window doesn't like drawing a second fullscreen
@@ -492,13 +503,12 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestShouldDisplayMultiFullscreen) {
ASSERT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(
GetFirstAppWindow(extension2->id())->GetNativeWindow()));
- const message_center::NotificationList::Notifications& notifications =
- g_browser_process->message_center()->GetVisibleNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
- message_center::Notification* notification = *(notifications.begin());
+
// The first app window is superseded by the second window, so its
// notification shouldn't be displayed.
- ASSERT_FALSE(notification->delegate()->ShouldDisplayOverFullscreen());
+ ASSERT_FALSE(notifications[0]->delegate()->ShouldDisplayOverFullscreen());
}
#endif
@@ -525,7 +535,10 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest,
ASSERT_TRUE(GetFirstAppWindow(extension->id())->GetBaseWindow()->IsActive())
<< "Not Active";
- const message_center::NotificationList::PopupNotifications notifications =
- g_browser_process->message_center()->GetPopupNotifications();
+ const auto& notifications = GetDisplayHelper()->GetNotificationsForTests();
ASSERT_EQ(1u, notifications.size());
+
+ // The extension's window is being shown and focused, so its expected that
+ // the notification displays on top of it.
+ ASSERT_TRUE(notifications[0]->delegate()->ShouldDisplayOverFullscreen());
}
« chrome/browser/extensions/api/notifications/notifications_api.cc ('K') | « chrome/browser/extensions/api/notifications/notifications_api.cc ('k') | chrome/browser/notifications/native_notification_display_service.cc » ('j') | chrome/browser/notifications/native_notification_display_service.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698