Commit da4d1914 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Notifications: Keep using MessageCenterDisplayService instead of the

NotificationPlatformBridge for TRANSIENT notifications until such time
that platform bridges can be updated to handle TRANSIENT notifications
via their delegates.

This should not have any immediate effect as would-be TRANSIENT
notifications do not use NotificationDisplayService yet, but it allows
them to be updated to use NDS while the platform bridges are still a
work in progress.

Bug: 783018
Change-Id: I224da7b03172152fbd9a5ee2cd4d5017a4de5f74
Reviewed-on: https://chromium-review.googlesource.com/779067Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518427}
parent 8c26d703
...@@ -54,12 +54,14 @@ NativeNotificationDisplayService::~NativeNotificationDisplayService() = default; ...@@ -54,12 +54,14 @@ NativeNotificationDisplayService::~NativeNotificationDisplayService() = default;
void NativeNotificationDisplayService::OnNotificationPlatformBridgeReady( void NativeNotificationDisplayService::OnNotificationPlatformBridgeReady(
bool success) { bool success) {
UMA_HISTOGRAM_BOOLEAN("Notifications.UsingNativeNotificationCenter", success); UMA_HISTOGRAM_BOOLEAN("Notifications.UsingNativeNotificationCenter", success);
if (success) { if (success)
notification_bridge_ready_ = true; notification_bridge_ready_ = true;
} else {
message_center_display_service_ = // TODO(estade): this shouldn't be necessary in the succesful case, but some
std::make_unique<MessageCenterDisplayService>(profile_); // notification bridges can't handle TRANSIENT notifications and still have to
} // fall back to the MessageCenter.
message_center_display_service_ =
std::make_unique<MessageCenterDisplayService>(profile_);
while (!actions_.empty()) { while (!actions_.empty()) {
std::move(actions_.front()).Run(); std::move(actions_.front()).Run();
...@@ -76,7 +78,7 @@ void NativeNotificationDisplayService::Display( ...@@ -76,7 +78,7 @@ void NativeNotificationDisplayService::Display(
if (notification_type == NotificationCommon::TRANSIENT) if (notification_type == NotificationCommon::TRANSIENT)
DCHECK(notification.delegate()); DCHECK(notification.delegate());
if (notification_bridge_ready_) { if (ShouldUsePlatformBridge(notification_type)) {
notification_bridge_->Display(notification_type, GetProfileId(profile_), notification_bridge_->Display(notification_type, GetProfileId(profile_),
profile_->IsOffTheRecord(), notification, profile_->IsOffTheRecord(), notification,
std::move(metadata)); std::move(metadata));
...@@ -96,7 +98,7 @@ void NativeNotificationDisplayService::Display( ...@@ -96,7 +98,7 @@ void NativeNotificationDisplayService::Display(
void NativeNotificationDisplayService::Close( void NativeNotificationDisplayService::Close(
NotificationCommon::Type notification_type, NotificationCommon::Type notification_type,
const std::string& notification_id) { const std::string& notification_id) {
if (notification_bridge_ready_) { if (ShouldUsePlatformBridge(notification_type)) {
notification_bridge_->Close(GetProfileId(profile_), notification_id); notification_bridge_->Close(GetProfileId(profile_), notification_id);
} else if (message_center_display_service_) { } else if (message_center_display_service_) {
message_center_display_service_->Close(notification_type, notification_id); message_center_display_service_->Close(notification_type, notification_id);
...@@ -120,3 +122,9 @@ void NativeNotificationDisplayService::GetDisplayed( ...@@ -120,3 +122,9 @@ void NativeNotificationDisplayService::GetDisplayed(
weak_factory_.GetWeakPtr(), callback)); weak_factory_.GetWeakPtr(), callback));
} }
} }
bool NativeNotificationDisplayService::ShouldUsePlatformBridge(
NotificationCommon::Type notification_type) {
return notification_bridge_ready_ &&
NotificationPlatformBridge::CanHandleType(notification_type);
}
...@@ -47,6 +47,8 @@ class NativeNotificationDisplayService : public NotificationDisplayService { ...@@ -47,6 +47,8 @@ class NativeNotificationDisplayService : public NotificationDisplayService {
// initializing. |success| indicates it is ready to be used. // initializing. |success| indicates it is ready to be used.
void OnNotificationPlatformBridgeReady(bool success); void OnNotificationPlatformBridgeReady(bool success);
bool ShouldUsePlatformBridge(NotificationCommon::Type notification_type);
Profile* profile_; Profile* profile_;
NotificationPlatformBridge* notification_bridge_; NotificationPlatformBridge* notification_bridge_;
......
...@@ -29,6 +29,11 @@ class NotificationPlatformBridge { ...@@ -29,6 +29,11 @@ class NotificationPlatformBridge {
static NotificationPlatformBridge* Create(); static NotificationPlatformBridge* Create();
// Returns whether a native bridge can handle a notification of the given
// type. Ideally, this would always return true, but for now some platforms
// can't handle TRANSIENT notifications.
static bool CanHandleType(NotificationCommon::Type notification_type);
virtual ~NotificationPlatformBridge() {} virtual ~NotificationPlatformBridge() {}
// Shows a toast on screen using the data passed in |notification|. // Shows a toast on screen using the data passed in |notification|.
......
...@@ -142,6 +142,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() { ...@@ -142,6 +142,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
return new NotificationPlatformBridgeAndroid(); return new NotificationPlatformBridgeAndroid();
} }
// static
bool NotificationPlatformBridge::CanHandleType(
NotificationCommon::Type notification_type) {
return notification_type != NotificationCommon::TRANSIENT;
}
NotificationPlatformBridgeAndroid::NotificationPlatformBridgeAndroid() { NotificationPlatformBridgeAndroid::NotificationPlatformBridgeAndroid() {
java_object_.Reset(Java_NotificationPlatformBridge_create( java_object_.Reset(Java_NotificationPlatformBridge_create(
AttachCurrentThread(), reinterpret_cast<intptr_t>(this))); AttachCurrentThread(), reinterpret_cast<intptr_t>(this)));
......
...@@ -33,6 +33,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() { ...@@ -33,6 +33,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
return new NotificationPlatformBridgeChromeOs(); return new NotificationPlatformBridgeChromeOs();
} }
// static
bool NotificationPlatformBridge::CanHandleType(
NotificationCommon::Type notification_type) {
return notification_type != NotificationCommon::TRANSIENT;
}
NotificationPlatformBridgeChromeOs::NotificationPlatformBridgeChromeOs() NotificationPlatformBridgeChromeOs::NotificationPlatformBridgeChromeOs()
: impl_(std::make_unique<ChromeAshMessageCenterClient>(this)) {} : impl_(std::make_unique<ChromeAshMessageCenterClient>(this)) {}
......
...@@ -249,6 +249,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() { ...@@ -249,6 +249,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
return new NotificationPlatformBridgeLinux(); return new NotificationPlatformBridgeLinux();
} }
// static
bool NotificationPlatformBridge::CanHandleType(
NotificationCommon::Type notification_type) {
return notification_type != NotificationCommon::TRANSIENT;
}
class NotificationPlatformBridgeLinuxImpl class NotificationPlatformBridgeLinuxImpl
: public NotificationPlatformBridge, : public NotificationPlatformBridge,
public content::NotificationObserver, public content::NotificationObserver,
......
...@@ -222,6 +222,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() { ...@@ -222,6 +222,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
alert_dispatcher.get()); alert_dispatcher.get());
} }
// static
bool NotificationPlatformBridge::CanHandleType(
NotificationCommon::Type notification_type) {
return notification_type != NotificationCommon::TRANSIENT;
}
void NotificationPlatformBridgeMac::Display( void NotificationPlatformBridgeMac::Display(
NotificationCommon::Type notification_type, NotificationCommon::Type notification_type,
const std::string& profile_id, const std::string& profile_id,
......
...@@ -145,6 +145,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() { ...@@ -145,6 +145,12 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
return new NotificationPlatformBridgeWin(); return new NotificationPlatformBridgeWin();
} }
// static
bool NotificationPlatformBridge::CanHandleType(
NotificationCommon::Type notification_type) {
return notification_type != NotificationCommon::TRANSIENT;
}
class NotificationPlatformBridgeWinImpl class NotificationPlatformBridgeWinImpl
: public base::RefCountedThreadSafe<NotificationPlatformBridgeWinImpl>, : public base::RefCountedThreadSafe<NotificationPlatformBridgeWinImpl>,
public content::BrowserThread::DeleteOnThread< public content::BrowserThread::DeleteOnThread<
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment