Fix deleting taskbar icon in wxMSW wxNotificationMessage

Previously, the taskbar icon could remain shown in the taskbar after
wxNotificationMessage was destroyed, as wxEVT_TASKBAR_BALLOON_TIMEOUT
handler was called on an already dead object.

Weirdly enough this didn't result in the crashes, but it definitely
didn't work correctly neither.

Fix this by binding a separate handler for this event, not using the
wxNotificationMessage object at all, which is responsible for destroying
the icon when it times out.

This also seems to make it completely unnecessary to have a separate
m_active flag in wxNotificationMessageImpl, as we can now just delete
the "impl" object too directly when the main object itself is deleted.

Closes #23432.

include/wx/private/notifmsg.h

@@ -14,8 +14,7 @@ class wxNotificationMessageImpl
 {
 public:
     wxNotificationMessageImpl(wxNotificationMessageBase* notification):
-        m_notification(notification),
-        m_active(false)
+        m_notification(notification)
     {
 
     }
@@ -38,20 +37,9 @@ public:
 
     virtual bool AddAction(wxWindowID actionid, const wxString &label) = 0;
 
-    virtual void Detach()
-    {
-        if (m_active)
-            m_notification = nullptr;
-        else
-            delete this;
-    }
-
     bool ProcessNotificationEvent(wxEvent& event)
     {
-        if (m_notification)
-            return m_notification->ProcessEvent(event);
-        else
-            return false;
+        return m_notification->ProcessEvent(event);
     }
 
     wxNotificationMessageBase* GetNotification() const
@@ -61,16 +49,6 @@ public:
 
 protected:
     wxNotificationMessageBase* m_notification;
-    bool m_active;
-
-    void SetActive(bool active)
-    {
-        m_active = active;
-
-        // Delete the implementation if the notification is detached
-        if (!m_notification && !active)
-            delete this;
-    }
 };
 
 #endif // _WX_PRIVATE_NOTIFMSG_H_

src/common/notifmsgcmn.cpp

@@ -35,7 +35,7 @@ wxDEFINE_EVENT( wxEVT_NOTIFICATION_MESSAGE_ACTION, wxCommandEvent );
 
 wxNotificationMessageBase::~wxNotificationMessageBase()
 {
-    m_impl->Detach();
+    delete m_impl;
 }
 
 bool wxNotificationMessageBase::Show(int timeout)

src/generic/notifmsgg.cpp

@@ -470,7 +470,6 @@ bool wxGenericNotificationMessageImpl::Show(int timeout)
         timeout = GetDefaultTimeout();
     }
 
-    SetActive(true);
     m_window->Set(timeout);
 
     m_window->ShowWithEffect(wxSHOW_EFFECT_BLEND);
@@ -485,8 +484,6 @@ bool wxGenericNotificationMessageImpl::Close()
 
     m_window->Hide();
 
-    SetActive(false);
-
     return true;
 }
 

src/msw/notifmsg.cpp

@@ -144,6 +144,7 @@ protected:
     // the icon is only destroyed when it reaches 0.
     static wxTaskBarIcon *ms_icon;
     static int ms_refCountIcon;
+
 private:
     wxString m_title;
     wxString m_message;
@@ -151,9 +152,16 @@ private:
     wxIcon m_icon;
     wxWindow* m_parent;
 
+    // Event handler not using this object which is called when the icon is
+    // dismissed even if it happens after this object was destroyed.
+    static void OnIconDismiss(wxTaskBarIconEvent& event);
+
+    // These event handlers are only used until this object is destroyed.
     void OnTimeout(wxTaskBarIconEvent& event);
     void OnClick(wxTaskBarIconEvent& event);
 
+    // Called when the icon is hidden and Unbind()s event handlers using this
+    // object from ms_icon.
     void OnIconHidden();
 };
 
@@ -184,19 +192,23 @@ wxTaskBarIcon *wxBalloonNotifMsgImpl::UseTaskBarIcon(wxTaskBarIcon *icon)
 
 wxBalloonNotifMsgImpl::~wxBalloonNotifMsgImpl()
 {
+    // Ensure no event handlers using this object remain bound.
+    OnIconHidden();
 }
 
 void wxBalloonNotifMsgImpl::OnIconHidden()
 {
-    SetActive(false);
     if ( ms_icon )
     {
         ms_icon->Unbind(wxEVT_TASKBAR_BALLOON_CLICK, &wxBalloonNotifMsgImpl::OnClick, this);
         ms_icon->Unbind(wxEVT_TASKBAR_BALLOON_TIMEOUT, &wxBalloonNotifMsgImpl::OnTimeout, this);
     }
+}
 
-    if ( IsUsingOwnIcon() )
-        wxBalloonNotifMsgImpl::ReleaseIcon();
+/* static */
+void wxBalloonNotifMsgImpl::OnIconDismiss(wxTaskBarIconEvent& WXUNUSED(event))
+{
+    wxBalloonNotifMsgImpl::ReleaseIcon();
 }
 
 void wxBalloonNotifMsgImpl::OnTimeout(wxTaskBarIconEvent& WXUNUSED(event))
@@ -252,6 +264,14 @@ void wxBalloonNotifMsgImpl::SetUpIcon(wxWindow *win)
         }
 
         ms_icon->SetIcon(icon);
+
+        // Ensure that the icon is dismissed when the timeout expires or it is
+        // clicked even if this object itself is destroyed before this happens
+        // (as is usually the case for the "fire and forget" notifications): we
+        // need a separate event handler for this, in addition to the
+        // event handlers using this object bound when showing the icon.
+        ms_icon->Bind(wxEVT_TASKBAR_BALLOON_CLICK, &wxBalloonNotifMsgImpl::OnIconDismiss);
+        ms_icon->Bind(wxEVT_TASKBAR_BALLOON_TIMEOUT, &wxBalloonNotifMsgImpl::OnIconDismiss);
     }
 }
 
@@ -302,7 +322,6 @@ wxBalloonNotifMsgImpl::Show(int timeout)
     {
         ms_icon->Bind(wxEVT_TASKBAR_BALLOON_CLICK, &wxBalloonNotifMsgImpl::OnClick, this);
         ms_icon->Bind(wxEVT_TASKBAR_BALLOON_TIMEOUT, &wxBalloonNotifMsgImpl::OnTimeout, this);
-        SetActive(true);
     }
 
     return res;