From b6d07ad80c962ebc848a9aa1a9c0379e46f385ca Mon Sep 17 00:00:00 2001 From: Stephen L Peters Date: Wed, 21 Aug 2019 15:55:56 -0700 Subject: [PATCH] Calculator Swipe Crash (#1180) * Attempt to fix a rare crash in calculator app. * Do proper event token clean up. * Use the routedEventHelpers instead of doing these events by hand * Clean up * Fix a silly mistake * rename handler to revoker --- dev/SwipeControl/SwipeControl.cpp | 56 +++++++++++-------------------- dev/SwipeControl/SwipeControl.h | 10 +++--- dev/inc/RoutedEventHelpers.h | 8 +++++ 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/dev/SwipeControl/SwipeControl.cpp b/dev/SwipeControl/SwipeControl.cpp index 0fd7d8c529..0d294db27f 100644 --- a/dev/SwipeControl/SwipeControl.cpp +++ b/dev/SwipeControl/SwipeControl.cpp @@ -677,13 +677,24 @@ void SwipeControl::AttachDismissingHandlers() { if (auto xamlRoot = uiElement10.XamlRoot()) { - auto xamlRootContent = xamlRoot.Content(); - - m_onXamlRootPointerPressedEventHandler.set(winrt::box_value({ this, &SwipeControl::DismissSwipeOnAnExternalXamlRootTap })); - xamlRootContent.AddHandler(winrt::UIElement::PointerPressedEvent(), m_onXamlRootPointerPressedEventHandler.get(), true); - - m_onXamlRootKeyDownEventHandler.set(winrt::box_value({ this, &SwipeControl::DismissSwipeOnXamlRootKeyDown })); - xamlRootContent.AddHandler(winrt::UIElement::KeyDownEvent(), m_onXamlRootKeyDownEventHandler.get(), true); + if (auto&& xamlRootContent = xamlRoot.Content()) + { + m_xamlRootPointerPressedEventRevoker = AddRoutedEventHandler( + xamlRootContent, + [this](auto const&, auto const& args) + { + DismissSwipeOnAnExternalTap(args.GetCurrentPoint(nullptr).Position()); + }, + true /*handledEventsToo*/); + + m_xamlRootKeyDownEventRevoker = AddRoutedEventHandler( + xamlRootContent, + [this](auto const&, auto const& args) + { + CloseIfNotRemainOpenExecuteItem(); + }, + true /*handledEventsToo*/); + } m_xamlRootChangedRevoker = xamlRoot.Changed(winrt::auto_revoke, { this, &SwipeControl::CurrentXamlRootChanged }); } @@ -715,25 +726,8 @@ void SwipeControl::DetachDismissingHandlers() { SWIPECONTROL_TRACE_INFO(nullptr, TRACE_MSG_METH, METH_NAME, this); - if (winrt::IUIElement10 uiElement10 = *this) - { - if (auto xamlRoot = uiElement10.XamlRoot()) - { - auto xamlRootContent = xamlRoot.Content(); - - if (m_onXamlRootPointerPressedEventHandler) - { - xamlRootContent.RemoveHandler(winrt::UIElement::PointerPressedEvent(), m_onXamlRootPointerPressedEventHandler.get()); - m_onXamlRootPointerPressedEventHandler.set(nullptr); - } - - if (m_onXamlRootKeyDownEventHandler) - { - xamlRootContent.RemoveHandler(winrt::UIElement::KeyDownEvent(), m_onXamlRootKeyDownEventHandler.get()); - m_onXamlRootKeyDownEventHandler.set(nullptr); - } - } - } + m_xamlRootPointerPressedEventRevoker.revoke(); + m_xamlRootKeyDownEventRevoker.revoke(); m_xamlRootChangedRevoker.revoke(); m_acceleratorKeyActivatedRevoker.revoke(); @@ -748,21 +742,11 @@ void SwipeControl::DismissSwipeOnAcceleratorKeyActivator(const winrt::Windows::U CloseIfNotRemainOpenExecuteItem(); } -void SwipeControl::DismissSwipeOnXamlRootKeyDown(const winrt::IInspectable& sender, const winrt::KeyRoutedEventArgs& args) -{ - CloseIfNotRemainOpenExecuteItem(); -} - void SwipeControl::CurrentXamlRootChanged(const winrt::XamlRoot & /*sender*/, const winrt::XamlRootChangedEventArgs &/*args*/) { CloseIfNotRemainOpenExecuteItem(); } -void SwipeControl::DismissSwipeOnAnExternalXamlRootTap(const winrt::IInspectable& sender, const winrt::PointerRoutedEventArgs& args) -{ - DismissSwipeOnAnExternalTap(args.GetCurrentPoint(nullptr).Position()); -} - void SwipeControl::DismissSwipeOnCoreWindowKeyDown(const winrt::CoreWindow& sender, const winrt::KeyEventArgs& args) { CloseIfNotRemainOpenExecuteItem(); diff --git a/dev/SwipeControl/SwipeControl.h b/dev/SwipeControl/SwipeControl.h index dd2883eee4..8e194be64c 100644 --- a/dev/SwipeControl/SwipeControl.h +++ b/dev/SwipeControl/SwipeControl.h @@ -85,10 +85,8 @@ class SwipeControl : void DismissSwipeOnAcceleratorKeyActivator(const winrt::Windows::UI::Core::CoreDispatcher & sender, const winrt::AcceleratorKeyEventArgs & args); // Used on platforms where we have XamlRoot. - void DismissSwipeOnXamlRootKeyDown(const winrt::IInspectable & sender, const winrt::KeyRoutedEventArgs & args); void CurrentXamlRootChanged(const winrt::XamlRoot & sender, const winrt::XamlRootChangedEventArgs & args); - void DismissSwipeOnAnExternalXamlRootTap(const winrt::IInspectable& sender, const winrt::PointerRoutedEventArgs& args); - + // Used on platforms where we don't have XamlRoot. void DismissSwipeOnCoreWindowKeyDown(const winrt::CoreWindow & sender, const winrt::KeyEventArgs & args); @@ -179,9 +177,9 @@ class SwipeControl : tracker_ref m_onPointerPressedEventHandler{ this }; // Used on platforms where we have XamlRoot. - tracker_ref m_onXamlRootPointerPressedEventHandler{ this }; - tracker_ref m_onXamlRootKeyDownEventHandler{ this }; - winrt::IXamlRoot::Changed_revoker m_xamlRootChangedRevoker; + RoutedEventHandler_revoker m_xamlRootPointerPressedEventRevoker{}; + RoutedEventHandler_revoker m_xamlRootKeyDownEventRevoker{}; + winrt::IXamlRoot::Changed_revoker m_xamlRootChangedRevoker{}; // Used on platforms where we don't have XamlRoot. diff --git a/dev/inc/RoutedEventHelpers.h b/dev/inc/RoutedEventHelpers.h index 67f44623de..39eba7f63b 100644 --- a/dev/inc/RoutedEventHelpers.h +++ b/dev/inc/RoutedEventHelpers.h @@ -81,6 +81,7 @@ enum class RoutedEventType GettingFocus, LosingFocus, KeyDown, + PointerPressed }; template @@ -109,6 +110,13 @@ struct RoutedEventTraits using HandlerT = winrt::KeyEventHandler; }; +template <> +struct RoutedEventTraits +{ + static winrt::RoutedEvent Event() { return winrt::UIElement::PointerPressedEvent(); } + using HandlerT = winrt::PointerEventHandler; +}; + template> inline RoutedEventHandler_revoker AddRoutedEventHandler(winrt::UIElement const& object, typename traits::HandlerT const& callback, bool handledEventsToo) {