From ee4442f964a78e2425bb1b680c9bed0b7bd0f4ee Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 25 Oct 2016 19:02:06 +0530 Subject: [PATCH 1/3] set notification close callback before requesting user permission --- .../browser/linux/libnotify_notification.cc | 6 ++- brightray/browser/mac/cocoa_notification.mm | 5 +- brightray/browser/notification.h | 6 +-- .../browser/platform_notification_service.cc | 51 +++++++++---------- 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/brightray/browser/linux/libnotify_notification.cc b/brightray/browser/linux/libnotify_notification.cc index a3f273c1c0..6946abbc93 100644 --- a/brightray/browser/linux/libnotify_notification.cc +++ b/brightray/browser/linux/libnotify_notification.cc @@ -82,8 +82,10 @@ LibnotifyNotification::LibnotifyNotification(NotificationDelegate* delegate, } LibnotifyNotification::~LibnotifyNotification() { - g_signal_handlers_disconnect_by_data(notification_, this); - g_object_unref(notification_); + if (notification_) { + g_signal_handlers_disconnect_by_data(notification_, this); + g_object_unref(notification_); + } } void LibnotifyNotification::Show(const base::string16& title, diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index e7ed61dbf3..fe21000e37 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -24,8 +24,9 @@ CocoaNotification::CocoaNotification(NotificationDelegate* delegate, } CocoaNotification::~CocoaNotification() { - [NSUserNotificationCenter.defaultUserNotificationCenter - removeDeliveredNotification:notification_]; + if (notification_) + [NSUserNotificationCenter.defaultUserNotificationCenter + removeDeliveredNotification:notification_]; } void CocoaNotification::Show(const base::string16& title, diff --git a/brightray/browser/notification.h b/brightray/browser/notification.h index afacd50c19..87096bbe26 100644 --- a/brightray/browser/notification.h +++ b/brightray/browser/notification.h @@ -34,6 +34,9 @@ class Notification { void NotificationDismissed(); void NotificationFailed(); + // delete this. + void Destroy(); + base::WeakPtr GetWeakPtr() { return weak_factory_.GetWeakPtr(); } @@ -46,9 +49,6 @@ class Notification { NotificationPresenter* presenter); virtual ~Notification(); - // delete this. - void Destroy(); - private: friend class NotificationPresenter; diff --git a/brightray/browser/platform_notification_service.cc b/brightray/browser/platform_notification_service.cc index 77ca31a5bb..5fabac82c3 100644 --- a/brightray/browser/platform_notification_service.cc +++ b/brightray/browser/platform_notification_service.cc @@ -21,28 +21,17 @@ void RemoveNotification(base::WeakPtr notification) { notification->Dismiss(); } -void OnWebNotificationAllowed( - brightray::BrowserClient* browser_client, - const SkBitmap& icon, - const content::PlatformNotificationData& data, - std::unique_ptr delegate, - base::Closure* cancel_callback, - bool audio_muted, - bool allowed) { - if (!allowed) +void OnWebNotificationAllowed(base::WeakPtr notification, + const SkBitmap& icon, + const content::PlatformNotificationData& data, + bool audio_muted, + bool allowed) { + if (!allowed) { + notification->Destroy(); return; - auto presenter = browser_client->GetNotificationPresenter(); - if (!presenter) - return; - std::unique_ptr adapter( - new NotificationDelegateAdapter(std::move(delegate))); - auto notification = presenter->CreateNotification(adapter.get()); - if (notification) { - ignore_result(adapter.release()); // it will release itself automatically. - notification->Show(data.title, data.body, data.tag, data.icon, icon, - audio_muted ? true : data.silent); - *cancel_callback = base::Bind(&RemoveNotification, notification); } + notification->Show(data.title, data.body, data.tag, data.icon, icon, + audio_muted ? true : data.silent); } } // namespace @@ -77,14 +66,20 @@ void PlatformNotificationService::DisplayNotification( const content::NotificationResources& notification_resources, std::unique_ptr delegate, base::Closure* cancel_callback) { - browser_client_->WebNotificationAllowed( - render_process_id_, - base::Bind(&OnWebNotificationAllowed, - browser_client_, - notification_resources.notification_icon, - notification_data, - base::Passed(&delegate), - cancel_callback)); + auto presenter = browser_client_->GetNotificationPresenter(); + if (!presenter) + return; + std::unique_ptr adapter( + new NotificationDelegateAdapter(std::move(delegate))); + auto notification = presenter->CreateNotification(adapter.get()); + if (notification) { + ignore_result(adapter.release()); // it will release itself automatically. + *cancel_callback = base::Bind(&RemoveNotification, notification); + browser_client_->WebNotificationAllowed( + render_process_id_, base::Bind(&OnWebNotificationAllowed, notification, + notification_resources.notification_icon, + notification_data)); + } } void PlatformNotificationService::DisplayPersistentNotification( From 0a5f7171d4bd6aae36d6690b1046a4f16bf5f254 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 28 Oct 2016 10:54:55 -0700 Subject: [PATCH 2/3] Add more guards around null notification --- brightray/browser/mac/cocoa_notification.mm | 5 +++-- brightray/browser/platform_notification_service.cc | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index fe21000e37..4e9ee3238b 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -57,8 +57,9 @@ void CocoaNotification::Show(const base::string16& title, } void CocoaNotification::Dismiss() { - [NSUserNotificationCenter.defaultUserNotificationCenter - removeDeliveredNotification:notification_]; + if (notification_) + [NSUserNotificationCenter.defaultUserNotificationCenter + removeDeliveredNotification:notification_]; NotificationDismissed(); } diff --git a/brightray/browser/platform_notification_service.cc b/brightray/browser/platform_notification_service.cc index 5fabac82c3..c9f2e5a1f9 100644 --- a/brightray/browser/platform_notification_service.cc +++ b/brightray/browser/platform_notification_service.cc @@ -26,12 +26,13 @@ void OnWebNotificationAllowed(base::WeakPtr notification, const content::PlatformNotificationData& data, bool audio_muted, bool allowed) { - if (!allowed) { - notification->Destroy(); + if (!notification) return; - } - notification->Show(data.title, data.body, data.tag, data.icon, icon, - audio_muted ? true : data.silent); + if (allowed) + notification->Show(data.title, data.body, data.tag, data.icon, icon, + audio_muted ? true : data.silent); + else + notification->Destroy(); } } // namespace From 7cb8c1fdb969226f410b54ccc9e1194289f36116 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 28 Oct 2016 11:24:47 -0700 Subject: [PATCH 3/3] Add dismiss notification guard on Linux --- brightray/browser/linux/libnotify_notification.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/brightray/browser/linux/libnotify_notification.cc b/brightray/browser/linux/libnotify_notification.cc index 6946abbc93..0fd71faaf1 100644 --- a/brightray/browser/linux/libnotify_notification.cc +++ b/brightray/browser/linux/libnotify_notification.cc @@ -146,6 +146,11 @@ void LibnotifyNotification::Show(const base::string16& title, } void LibnotifyNotification::Dismiss() { + if (!notification_) { + Destroy(); + return; + } + GError* error = nullptr; libnotify_loader_.notify_notification_close(notification_, &error); if (error) {