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
microsoft · Aug 21, 2019 · b6d07ad · b6d07ad
1 parent 893bb1c
commit b6d07ad
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 42 deletions.
diff --git 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<winrt::PointerEventHandler>({ this, &SwipeControl::DismissSwipeOnAnExternalXamlRootTap }));
-            xamlRootContent.AddHandler(winrt::UIElement::PointerPressedEvent(), m_onXamlRootPointerPressedEventHandler.get(), true);
-
-            m_onXamlRootKeyDownEventHandler.set(winrt::box_value<winrt::KeyEventHandler>({ this, &SwipeControl::DismissSwipeOnXamlRootKeyDown }));
-            xamlRootContent.AddHandler(winrt::UIElement::KeyDownEvent(), m_onXamlRootKeyDownEventHandler.get(), true);
+            if (auto&& xamlRootContent = xamlRoot.Content())
+            {
+                m_xamlRootPointerPressedEventRevoker = AddRoutedEventHandler<RoutedEventType::PointerPressed>(
+                    xamlRootContent,
+                    [this](auto const&, auto const& args)
+                    {
+                        DismissSwipeOnAnExternalTap(args.GetCurrentPoint(nullptr).Position());
+                    },
+                    true /*handledEventsToo*/);
+
+                m_xamlRootKeyDownEventRevoker = AddRoutedEventHandler<RoutedEventType::KeyDown>(
+                    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
@@ -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<winrt::IInspectable> m_onPointerPressedEventHandler{ this };
 
     // Used on platforms where we have XamlRoot.
-    tracker_ref<winrt::IInspectable> m_onXamlRootPointerPressedEventHandler{ this };
-    tracker_ref<winrt::IInspectable> 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
@@ -81,6 +81,7 @@ enum class RoutedEventType
     GettingFocus,
     LosingFocus,
     KeyDown,
+    PointerPressed
 };
 
 template<RoutedEventType eventType>
@@ -109,6 +110,13 @@ struct RoutedEventTraits<RoutedEventType::KeyDown>
     using HandlerT = winrt::KeyEventHandler;
 };
 
+template <>
+struct RoutedEventTraits<RoutedEventType::PointerPressed>
+{
+    static winrt::RoutedEvent Event() { return winrt::UIElement::PointerPressedEvent(); }
+    using HandlerT = winrt::PointerEventHandler;
+};
+
 template<RoutedEventType eventType, typename traits = RoutedEventTraits<eventType>>
 inline RoutedEventHandler_revoker AddRoutedEventHandler(winrt::UIElement const& object, typename traits::HandlerT const& callback, bool handledEventsToo)
 {