From ce3d97d450e6271e0599d450c4fe7d394c521c38 Mon Sep 17 00:00:00 2001 From: bridiver Date: Sat, 9 Jul 2016 14:13:04 -0700 Subject: [PATCH] set cancel callback synchronously fixes https://github.com/brave/browser-laptop/issues/1931 --- .../platform_notification_service_impl.cc | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/brave/browser/notifications/platform_notification_service_impl.cc b/brave/browser/notifications/platform_notification_service_impl.cc index 70cd61252..54afc87ab 100644 --- a/brave/browser/notifications/platform_notification_service_impl.cc +++ b/brave/browser/notifications/platform_notification_service_impl.cc @@ -20,6 +20,24 @@ namespace brave { namespace { +class NotificationProxy : public base::SupportsWeakPtr { + public: + explicit NotificationProxy() {} + + void Dismiss() { + if (notification_) + notification_->Dismiss(); + } + + void set_notification(base::WeakPtr notification) { + notification_ = notification; + } + private: + base::WeakPtr notification_; + + DISALLOW_COPY_AND_ASSIGN(NotificationProxy); +}; + void OnPermissionResponse(const base::Callback& callback, blink::mojom::PermissionStatus status) { if (status == blink::mojom::PermissionStatus::GRANTED) @@ -28,9 +46,8 @@ void OnPermissionResponse(const base::Callback& callback, callback.Run(false); } -void RemoveNotification(base::WeakPtr notification) { - if (notification) - notification->Dismiss(); +void RemoveNotification(std::unique_ptr notification_proxy) { + notification_proxy->Dismiss(); } void OnWebNotificationAllowed( @@ -38,7 +55,7 @@ void OnWebNotificationAllowed( const SkBitmap& icon, const content::PlatformNotificationData& data, std::unique_ptr delegate, - base::Closure* cancel_callback, + const base::WeakPtr notification_proxy, bool allowed) { if (!allowed) return; @@ -51,8 +68,8 @@ void OnWebNotificationAllowed( if (notification) { ignore_result(adapter.release()); // it will release itself automatically. notification->Show(data.title, data.body, data.tag, data.icon, icon, data.silent); - if (cancel_callback) - *cancel_callback = base::Bind(&RemoveNotification, notification); + if (notification_proxy) + notification_proxy->set_notification(notification); } } @@ -89,12 +106,22 @@ void PlatformNotificationServiceImpl::DisplayNotification( const content::NotificationResources& notification_resources, std::unique_ptr delegate, base::Closure* cancel_callback) { + // cancel_callback must be set when this method returns, but the + // RequestPermission callback may run asynchronously so we use + // the notification proxy as a placeholder until the actual + // notification is available + std::unique_ptr notification_proxy(new NotificationProxy); + auto callback = base::Bind(&OnWebNotificationAllowed, BraveContentBrowserClient::Get(), notification_resources.notification_icon, notification_data, base::Passed(&delegate), - cancel_callback); + notification_proxy->AsWeakPtr()); + + // TODO(bridiver) - there is a memory leak on mac because CocoaNotification never sends + // NotificationClosed to the PageNotificationDelegate + *cancel_callback = base::Bind(&RemoveNotification, base::Passed(¬ification_proxy)); auto permission_manager = browser_context->GetPermissionManager(); permission_manager->RequestPermission(