Commit a2c64d86 authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot

Support alerts on macOS 10.15.3+ again

The latest beta of 10.15.3 fixed the issue where an XPC service could
not show notifications. This removes the workaround we put in place in
https://crrev.com/c/1883648 for macOS 10.15.3+ but keeps it for earlier
versions of 10.15.

Bug: 1007418
Change-Id: I928dac7366c9e7faee793dd72d282b031e535092
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1991637
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730122}
parent d8b174f7
......@@ -53,6 +53,9 @@ class NotificationPlatformBridgeMac : public NotificationPlatformBridge {
// when a notification gets activated.
static bool VerifyNotificationData(NSDictionary* response) WARN_UNUSED_RESULT;
// Returns if alerts are supported on this machine.
static bool SupportsAlerts();
private:
// Cocoa class that receives callbacks from the NSUserNotificationCenter.
base::scoped_nsobject<NotificationCenterDelegate> delegate_;
......
......@@ -23,6 +23,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/sys_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/system/sys_info.h"
#include "base/task/post_task.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/notifications/notification_common.h"
......@@ -123,10 +124,7 @@ base::string16 CreateNotificationTitle(
bool IsPersistentNotification(
const message_center::Notification& notification) {
// TODO(crbug.com/1007418): Remove this and find a way to show alert style
// notifications in 10.15 and above. At least show them as banners until then
// as a temporary workaround.
if (base::mac::IsAtLeastOS10_15())
if (!NotificationPlatformBridgeMac::SupportsAlerts())
return false;
return notification.never_timeout() ||
......@@ -174,6 +172,19 @@ base::string16 CreateNotificationContext(
return etldplusone;
}
// Implements the version check to determine if alerts are supported. Do not
// call this method directly as SysInfo::OperatingSystemVersionNumbers might be
// an expensive call. Instead use NotificationPlatformBridgeMac::SupportsAlerts
// which caches this value.
bool SupportsAlertsImpl() {
int32_t major, minor, bugfix;
base::SysInfo::OperatingSystemVersionNumbers(&major, &minor, &bugfix);
// Allow alerts on all versions except 10.15.0, 10.15.1 & 10.15.2.
// See crbug.com/1007418 for details.
return major != 10 || minor != 15 || bugfix > 2;
}
} // namespace
// A Cocoa class that represents the delegate of NSUserNotificationCenter and
......@@ -462,6 +473,13 @@ bool NotificationPlatformBridgeMac::VerifyNotificationData(
return true;
}
// static
bool NotificationPlatformBridgeMac::SupportsAlerts() {
// Cache result as SysInfo::OperatingSystemVersionNumbers might be expensive.
static bool supports_alerts = SupportsAlertsImpl();
return supports_alerts;
}
// /////////////////////////////////////////////////////////////////////////////
@implementation NotificationCenterDelegate
- (void)userNotificationCenter:(NSUserNotificationCenter*)center
......
......@@ -281,8 +281,7 @@ TEST_F(NotificationPlatformBridgeMacTest, TestDisplayOneButton) {
}
TEST_F(NotificationPlatformBridgeMacTest, TestDisplayProgress) {
// TODO(crbug.com/1007418): Enable this when we support alerts on 10.15 again.
if (base::mac::IsAtLeastOS10_15())
if (!NotificationPlatformBridgeMac::SupportsAlerts())
return;
std::unique_ptr<Notification> notification =
......@@ -362,8 +361,7 @@ TEST_F(NotificationPlatformBridgeMacTest, TestQuitRemovesNotifications) {
}
TEST_F(NotificationPlatformBridgeMacTest, TestDisplayAlert) {
// TODO(crbug.com/1007418): Enable this when we support alerts on 10.15 again.
if (base::mac::IsAtLeastOS10_15())
if (!NotificationPlatformBridgeMac::SupportsAlerts())
return;
std::unique_ptr<Notification> alert =
......@@ -378,8 +376,7 @@ TEST_F(NotificationPlatformBridgeMacTest, TestDisplayAlert) {
}
TEST_F(NotificationPlatformBridgeMacTest, TestDisplayBannerAndAlert) {
// TODO(crbug.com/1007418): Enable this when we support alerts on 10.15 again.
if (base::mac::IsAtLeastOS10_15())
if (!NotificationPlatformBridgeMac::SupportsAlerts())
return;
std::unique_ptr<Notification> alert =
......@@ -398,8 +395,7 @@ TEST_F(NotificationPlatformBridgeMacTest, TestDisplayBannerAndAlert) {
}
TEST_F(NotificationPlatformBridgeMacTest, TestCloseAlert) {
// TODO(crbug.com/1007418): Enable this when we support alerts on 10.15 again.
if (base::mac::IsAtLeastOS10_15())
if (!NotificationPlatformBridgeMac::SupportsAlerts())
return;
std::unique_ptr<Notification> alert =
......@@ -417,8 +413,7 @@ TEST_F(NotificationPlatformBridgeMacTest, TestCloseAlert) {
}
TEST_F(NotificationPlatformBridgeMacTest, TestQuitRemovesBannersAndAlerts) {
// TODO(crbug.com/1007418): Enable this when we support alerts on 10.15 again.
if (base::mac::IsAtLeastOS10_15())
if (!NotificationPlatformBridgeMac::SupportsAlerts())
return;
std::unique_ptr<Notification> notification = CreateBanner(
......
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