From 102600eb379e630ad03a08d24eeb453d49f6ba64 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 8 Apr 2023 18:06:15 +0100 Subject: [PATCH] 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 | 26 ++------------------------ src/common/notifmsgcmn.cpp | 2 +- src/generic/notifmsgg.cpp | 3 --- src/msw/notifmsg.cpp | 27 +++++++++++++++++++++++---- 4 files changed, 26 insertions(+), 32 deletions(-) diff --git a/include/wx/private/notifmsg.h b/include/wx/private/notifmsg.h index f2fc42eee8..8d706b1bae 100644 --- a/include/wx/private/notifmsg.h +++ b/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_ diff --git a/src/common/notifmsgcmn.cpp b/src/common/notifmsgcmn.cpp index c3d47ec5e0..beaf03b415 100644 --- a/src/common/notifmsgcmn.cpp +++ b/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) diff --git a/src/generic/notifmsgg.cpp b/src/generic/notifmsgg.cpp index b0ef90c3c1..8566831470 100644 --- a/src/generic/notifmsgg.cpp +++ b/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; } diff --git a/src/msw/notifmsg.cpp b/src/msw/notifmsg.cpp index 483d9ac6e7..3a0d729a08 100644 --- a/src/msw/notifmsg.cpp +++ b/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;