Merge pull request #214 from electron/fix-notification

Fix notification not showing on Windows

brightray/browser/linux/libnotify_notification.cc

@@ -136,19 +136,12 @@ void LibnotifyNotification::Dismiss() {
 
 void LibnotifyNotification::OnNotificationClosed(
     NotifyNotification* notification) {
-  delegate()->NotificationClosed();
+  NotificationDismissed();
-  Destroy();
 }
 
 void LibnotifyNotification::OnNotificationView(
     NotifyNotification* notification, char* action) {
-  delegate()->NotificationClick();
+  NotificationClicked();
-  Destroy();
-}
-
-void LibnotifyNotification::NotificationFailed() {
-  delegate()->NotificationFailed();
-  Destroy();
 }
 
 }  // namespace brightray

brightray/browser/linux/libnotify_notification.h

@@ -34,8 +34,6 @@ class LibnotifyNotification : public Notification {
   CHROMEG_CALLBACK_1(LibnotifyNotification, void, OnNotificationView,
                      NotifyNotification*, char*);
 
-  void NotificationFailed();
-
   NotifyNotification* notification_;
 
   DISALLOW_COPY_AND_ASSIGN(LibnotifyNotification);

brightray/browser/mac/cocoa_notification.h

@@ -28,8 +28,7 @@ class CocoaNotification : public Notification {
             const bool silent) override;
   void Dismiss() override;
 
-  void NotifyDisplayed();
+  void NotificationDisplayed();
-  void NotifyClick();
 
   NSUserNotification* notification() const { return notification_; }
 

brightray/browser/mac/cocoa_notification.mm

@@ -58,17 +58,11 @@ void CocoaNotification::Show(const base::string16& title,
 void CocoaNotification::Dismiss() {
   [NSUserNotificationCenter.defaultUserNotificationCenter
       removeDeliveredNotification:notification_];
-  delegate()->NotificationClosed();
+  NotificationDismissed();
-  Destroy();
 }
 
-void CocoaNotification::NotifyDisplayed() {
+void CocoaNotification::NotificationDisplayed() {
   delegate()->NotificationDisplayed();
 }
 
-void CocoaNotification::NotifyClick() {
-  delegate()->NotificationClick();
-  Destroy();
-}
-
 }  // namespace brightray

brightray/browser/mac/notification_center_delegate.mm

@@ -22,14 +22,14 @@
         didDeliverNotification:(NSUserNotification*)notif {
   auto notification = presenter_->GetNotification(notif);
   if (notification)
-    notification->NotifyDisplayed();
+    notification->NotificationDisplayed();
 }
 
 - (void)userNotificationCenter:(NSUserNotificationCenter*)center
        didActivateNotification:(NSUserNotification *)notif {
   auto notification = presenter_->GetNotification(notif);
   if (notification)
-    notification->NotifyClick();
+    notification->NotificationClicked();
 }
 
 - (BOOL)userNotificationCenter:(NSUserNotificationCenter*)center

brightray/browser/notification.cc

@@ -20,6 +20,21 @@ Notification::~Notification() {
   delegate()->NotificationDestroyed();
 }
 
+void Notification::NotificationClicked() {
+  delegate()->NotificationClick();
+  Destroy();
+}
+
+void Notification::NotificationDismissed() {
+  delegate()->NotificationClosed();
+  Destroy();
+}
+
+void Notification::NotificationFailed() {
+  delegate()->NotificationFailed();
+  Destroy();
+}
+
 void Notification::Destroy() {
   presenter()->RemoveNotification(this);
 }

brightray/browser/notification.h

@@ -29,6 +29,11 @@ class Notification {
   // notification gets closed.
   virtual void Dismiss() = 0;
 
+  // Should be called by derived classes.
+  void NotificationClicked();
+  void NotificationDismissed();
+  void NotificationFailed();
+
   base::WeakPtr<Notification> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }

brightray/browser/win/windows_toast_notification.cc

@@ -12,6 +12,7 @@
 #include "browser/win/scoped_hstring.h"
 #include "browser/win/notification_presenter_win.h"
 #include "common/application_info.h"
+#include "content/public/browser/browser_thread.h"
 
 using namespace ABI::Windows::Data::Xml::Dom;
 
@@ -90,16 +91,7 @@ void WindowsToastNotification::Show(
     const bool silent) {
   auto presenter_win = static_cast<NotificationPresenterWin*>(presenter());
   std::wstring icon_path = presenter_win->SaveIconToFilesystem(icon, icon_url);
-  
+
-  // Ask Windows for the current notification settings
-  // If not allowed, we return here (0 = enabled)
-  ABI::Windows::UI::Notifications::NotificationSetting setting;
-  toast_notifier_->get_Setting(&setting);
-  if (setting != 0) {
-    NotificationFailed();
-    return;
-  }
-  
   ComPtr<IXmlDocument> toast_xml;
   if(FAILED(GetToastXml(toast_manager_.Get(), title, msg, icon_path, silent, &toast_xml))) {
     NotificationFailed();
@@ -126,7 +118,7 @@ void WindowsToastNotification::Show(
     return;
   }
 
-  if (FAILED(SetupCallbacks(toast_notification_.Get()))) {
+  if (!SetupCallbacks(toast_notification_.Get())) {
     NotificationFailed();
     return;
   }
@@ -143,21 +135,6 @@ void WindowsToastNotification::Dismiss() {
   toast_notifier_->Hide(toast_notification_.Get());
 }
 
-void WindowsToastNotification::NotificationClicked() {
-  delegate()->NotificationClick();
-  Destroy();
-}
-
-void WindowsToastNotification::NotificationDismissed() {
-  delegate()->NotificationClosed();
-  Destroy();
-}
-
-void WindowsToastNotification::NotificationFailed() {
-  delegate()->NotificationFailed();
-  Destroy();
-}
-
 bool WindowsToastNotification::GetToastXml(
     ABI::Windows::UI::Notifications::IToastNotificationManagerStatics* toastManager,
     const std::wstring& title,
@@ -399,8 +376,8 @@ bool WindowsToastNotification::RemoveCallbacks(
 /*
 / Toast Event Handler
 */
-ToastEventHandler::ToastEventHandler(WindowsToastNotification* notification)
+ToastEventHandler::ToastEventHandler(Notification* notification)
-    : notification_(notification) {
+    : notification_(notification->GetWeakPtr()) {
 }
 
 ToastEventHandler::~ToastEventHandler() {
@@ -408,21 +385,27 @@ ToastEventHandler::~ToastEventHandler() {
 
 IFACEMETHODIMP ToastEventHandler::Invoke(
     ABI::Windows::UI::Notifications::IToastNotification* sender, IInspectable* args) {
-  notification_->NotificationClicked();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&Notification::NotificationClicked, notification_));
   return S_OK;
 }
 
 IFACEMETHODIMP ToastEventHandler::Invoke(
     ABI::Windows::UI::Notifications::IToastNotification* sender,
     ABI::Windows::UI::Notifications::IToastDismissedEventArgs* e) {
-  notification_->NotificationDismissed();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&Notification::NotificationDismissed, notification_));
   return S_OK;
 }
 
 IFACEMETHODIMP ToastEventHandler::Invoke(
     ABI::Windows::UI::Notifications::IToastNotification* sender,
     ABI::Windows::UI::Notifications::IToastFailedEventArgs* e) {
-  notification_->NotificationFailed();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&Notification::NotificationFailed, notification_));
   return S_OK;
 }
 

brightray/browser/win/windows_toast_notification.h

@@ -50,10 +50,6 @@ class WindowsToastNotification : public Notification {
  private:
   friend class ToastEventHandler;
 
-  void NotificationClicked();
-  void NotificationDismissed();
-  void NotificationFailed();
-
   bool GetToastXml(ABI::Windows::UI::Notifications::IToastNotificationManagerStatics* toastManager,
                    const std::wstring& title,
                    const std::wstring& msg,
@@ -97,7 +93,7 @@ class ToastEventHandler : public RuntimeClass<RuntimeClassFlags<ClassicCom>,
                                               DesktopToastDismissedEventHandler,
                                               DesktopToastFailedEventHandler> {
  public:
-  ToastEventHandler(WindowsToastNotification* notification);
+  ToastEventHandler(Notification* notification);
   ~ToastEventHandler();
 
   IFACEMETHODIMP Invoke(ABI::Windows::UI::Notifications::IToastNotification* sender, IInspectable* args);
@@ -105,7 +101,7 @@ class ToastEventHandler : public RuntimeClass<RuntimeClassFlags<ClassicCom>,
   IFACEMETHODIMP Invoke(ABI::Windows::UI::Notifications::IToastNotification* sender, ABI::Windows::UI::Notifications::IToastFailedEventArgs* e);
 
  private:
-  WindowsToastNotification* notification_;  // weak ref.
+  base::WeakPtr<Notification> notification_;  // weak ref.
 
   DISALLOW_COPY_AND_ASSIGN(ToastEventHandler);
 };