Merge pull request #258 from deepak1556/notification_close_patch

set notification close callback before requesting user permission

brightray/browser/linux/libnotify_notification.cc

@@ -82,9 +82,11 @@ LibnotifyNotification::LibnotifyNotification(NotificationDelegate* delegate,
 }
 
 LibnotifyNotification::~LibnotifyNotification() {
+  if (notification_) {
     g_signal_handlers_disconnect_by_data(notification_, this);
     g_object_unref(notification_);
   }
+}
 
 void LibnotifyNotification::Show(const base::string16& title,
                                  const base::string16& body,
@@ -144,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) {

brightray/browser/mac/cocoa_notification.mm

@@ -24,6 +24,7 @@ CocoaNotification::CocoaNotification(NotificationDelegate* delegate,
 }
 
 CocoaNotification::~CocoaNotification() {
+  if (notification_)
     [NSUserNotificationCenter.defaultUserNotificationCenter
         removeDeliveredNotification:notification_];
 }
@@ -56,6 +57,7 @@ void CocoaNotification::Show(const base::string16& title,
 }
 
 void CocoaNotification::Dismiss() {
+  if (notification_)
     [NSUserNotificationCenter.defaultUserNotificationCenter
         removeDeliveredNotification:notification_];
   NotificationDismissed();

brightray/browser/notification.h

@@ -34,6 +34,9 @@ class Notification {
   void NotificationDismissed();
   void NotificationFailed();
 
+  // delete this.
+  void Destroy();
+
   base::WeakPtr<Notification> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
@@ -46,9 +49,6 @@ class Notification {
                NotificationPresenter* presenter);
   virtual ~Notification();
 
-  // delete this.
-  void Destroy();
-
  private:
   friend class NotificationPresenter;
 

brightray/browser/platform_notification_service.cc

@@ -21,28 +21,18 @@ void RemoveNotification(base::WeakPtr<Notification> notification) {
     notification->Dismiss();
 }
 
-void OnWebNotificationAllowed(
-    brightray::BrowserClient* browser_client,
+void OnWebNotificationAllowed(base::WeakPtr<Notification> notification,
                               const SkBitmap& icon,
                               const content::PlatformNotificationData& data,
-    std::unique_ptr<content::DesktopNotificationDelegate> delegate,
-    base::Closure* cancel_callback,
                               bool audio_muted,
                               bool allowed) {
-  if (!allowed)
+  if (!notification)
     return;
-  auto presenter = browser_client->GetNotificationPresenter();
-  if (!presenter)
-    return;
-  std::unique_ptr<NotificationDelegateAdapter> adapter(
-      new NotificationDelegateAdapter(std::move(delegate)));
-  auto notification = presenter->CreateNotification(adapter.get());
-  if (notification) {
-    ignore_result(adapter.release());  // it will release itself automatically.
+  if (allowed)
     notification->Show(data.title, data.body, data.tag, data.icon, icon,
                        audio_muted ? true : data.silent);
-    *cancel_callback = base::Bind(&RemoveNotification, notification);
-  }
+  else
+    notification->Destroy();
 }
 
 }  // namespace
@@ -77,14 +67,20 @@ void PlatformNotificationService::DisplayNotification(
     const content::NotificationResources& notification_resources,
     std::unique_ptr<content::DesktopNotificationDelegate> delegate,
     base::Closure* cancel_callback) {
+  auto presenter = browser_client_->GetNotificationPresenter();
+  if (!presenter)
+    return;
+  std::unique_ptr<NotificationDelegateAdapter> 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,
-                 browser_client_,
+        render_process_id_, base::Bind(&OnWebNotificationAllowed, notification,
                                        notification_resources.notification_icon,
-                 notification_data,
-                 base::Passed(&delegate),
-                 cancel_callback));
+                                       notification_data));
+  }
 }
 
 void PlatformNotificationService::DisplayPersistentNotification(