From 953d65cc85d5fb75986c4bcdb024f4906a367ee0 Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Mon, 10 Jul 2023 18:16:10 -0400 Subject: [PATCH] Register event handlers with `shared_ptr` and `weak_ptr` (#1330) --- cppwinrt/code_writers.h | 36 ++++++++ strings/base_delegate.h | 10 +++ .../UnitTests/delegate_weak_strong.cpp | 85 ++++++++++++++++++- test/test/variadic_delegate.cpp | 52 ++++++++++++ 4 files changed, 180 insertions(+), 3 deletions(-) diff --git a/cppwinrt/code_writers.h b/cppwinrt/code_writers.h index e0162fe4..df5dd0c3 100644 --- a/cppwinrt/code_writers.h +++ b/cppwinrt/code_writers.h @@ -2446,6 +2446,8 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable template %(O* object, M method); template %(com_ptr&& object, M method); template %(weak_ref&& object, M method); + template %(std::shared_ptr&& object, M method); + template %(std::weak_ptr&& object, M method); auto operator()(%) const; }; )"; @@ -2462,6 +2464,8 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable type_name, type_name, type_name, + type_name, + type_name, bind(signature)); } @@ -2523,6 +2527,14 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable %([o = std::move(object), method](auto&&... args) { if (auto s = o.get()) { ((*s).*(method))(args...); } }) { } + template <%> template %<%>::%(std::shared_ptr&& object, M method) : + %([o = std::move(object), method](auto&&... args) { return ((*o).*(method))(args...); }) + { + } + template <%> template %<%>::%(std::weak_ptr&& object, M method) : + %([o = std::move(object), method](auto&&... args) { if (auto s = o.lock()) { ((*s).*(method))(args...); } }) + { + } template <%> auto %<%>::operator()(%) const {% check_hresult((*(impl::abi_t<%<%>>**)this)->Invoke(%));% @@ -2562,6 +2574,16 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable bind(generics), type_name, bind_list(", ", generics), + type_name, + type_name, + bind(generics), + type_name, + bind_list(", ", generics), + type_name, + type_name, + bind(generics), + type_name, + bind_list(", ", generics), bind(signature), bind(signature, true), type_name, @@ -2591,6 +2613,14 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable %([o = std::move(object), method](auto&&... args) { if (auto s = o.get()) { ((*s).*(method))(args...); } }) { } + template %::%(std::shared_ptr&& object, M method) : + %([o = std::move(object), method](auto&&... args) { return ((*o).*(method))(args...); }) + { + } + template %::%(std::weak_ptr&& object, M method) : + %([o = std::move(object), method](auto&&... args) { if (auto s = o.lock()) { ((*s).*(method))(args...); } }) + { + } inline auto %::operator()(%) const {% check_hresult((*(impl::abi_t<%>**)this)->Invoke(%));% @@ -2615,6 +2645,12 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable type_name, type_name, type_name, + type_name, + type_name, + type_name, + type_name, + type_name, + type_name, bind(signature), bind(signature, true), type_name, diff --git a/strings/base_delegate.h b/strings/base_delegate.h index e99fd481..59c4bfdb 100644 --- a/strings/base_delegate.h +++ b/strings/base_delegate.h @@ -179,6 +179,16 @@ namespace winrt::impl { } + template delegate_base(std::shared_ptr&& object, M method) : + delegate_base([o = std::move(object), method](auto&& ... args) { return ((*o).*(method))(args...); }) + { + } + + template delegate_base(std::weak_ptr&& object, M method) : + delegate_base([o = std::move(object), method](auto&& ... args) { if (auto s = o.lock()) { ((*s).*(method))(args...); } }) + { + } + auto operator()(Args const& ... args) const { return (*(variadic_delegate_abi * *)this)->invoke(args...); diff --git a/test/old_tests/UnitTests/delegate_weak_strong.cpp b/test/old_tests/UnitTests/delegate_weak_strong.cpp index b042d6da..30b02672 100644 --- a/test/old_tests/UnitTests/delegate_weak_strong.cpp +++ b/test/old_tests/UnitTests/delegate_weak_strong.cpp @@ -31,6 +31,25 @@ namespace } }; + template + struct ObjectStd : std::enable_shared_from_this> + { + ~ObjectStd() + { + destroyed = true; + } + + void StrongHandler(Sender const&, Args const&) + { + ++strong_count; + } + + void WeakHandler(Sender const&, Args const&) + { + ++weak_count; + } + }; + struct ReturnObject : implements { ~ReturnObject() @@ -44,8 +63,21 @@ namespace } }; + struct ReturnObjectStd : std::enable_shared_from_this + { + ~ReturnObjectStd() + { + destroyed = true; + } + + int Handler(int a, int b) + { + return a + b; + } + }; + template - void test_delegate() + void test_delegate_winrt() { auto object = make_self>(); @@ -81,6 +113,51 @@ namespace weak({}, {}); REQUIRE(weak_count == 2); } + + template + void test_delegate_std() + { + auto object = std::make_shared>(); + + Delegate strong{ object->shared_from_this(), &ObjectStd::StrongHandler }; + Delegate weak{ object->weak_from_this(), &ObjectStd::WeakHandler }; + + destroyed = false; + strong_count = 0; + weak_count = 0; + + // Both weak and strong handlers + strong({}, {}); + weak({}, {}); + REQUIRE(strong_count == 1); + REQUIRE(weak_count == 1); + + // Local 'object' strong reference is released + object = nullptr; + + // Still both since strong handler keeps object alive + strong({}, {}); + weak({}, {}); + REQUIRE(strong_count == 2); + REQUIRE(weak_count == 2); + + // ~Object is called since the strong delegate is destroyed + REQUIRE(!destroyed); + strong = nullptr; + REQUIRE(destroyed); + + // Weak delegate remains but no longer fires + REQUIRE(weak_count == 2); + weak({}, {}); + REQUIRE(weak_count == 2); + } + + template + void test_delegate() + { + test_delegate_winrt(); + test_delegate_std(); + } } TEST_CASE("delegate_weak_strong") @@ -111,8 +188,10 @@ TEST_CASE("delegate_weak_strong") // scenarios such as callbacks so weak support isn't interesting anyway, but it does work with get_strong. auto object = make_self(); - Component::TwoArgDelegateReturn strong{ object->get_strong(), &ReturnObject::Handler }; - REQUIRE(5 == strong(2, 3)); + + auto objectStd = std::make_shared(); + Component::TwoArgDelegateReturn strongStd{ objectStd->shared_from_this(), &ReturnObjectStd::Handler }; + REQUIRE(5 == strongStd(2, 3)); } diff --git a/test/test/variadic_delegate.cpp b/test/test/variadic_delegate.cpp index 6ffbe5ef..7291ddbe 100644 --- a/test/test/variadic_delegate.cpp +++ b/test/test/variadic_delegate.cpp @@ -40,6 +40,20 @@ namespace return L"Object"; } }; + + struct ObjectStd : std::enable_shared_from_this + { + int& m_count; + + ObjectStd(int& count) : m_count(count) + { + } + + void Callback() + { + ++m_count; + } + }; } TEST_CASE("variadic_delegate") @@ -100,6 +114,44 @@ TEST_CASE("variadic_delegate") REQUIRE(count == 2); // Unchanged } + // shared_from_this + { + int count{}; + auto object = std::make_shared(count); + + delegate<> up{ object->shared_from_this(), &ObjectStd::Callback }; + + REQUIRE(count == 0); + up(); + REQUIRE(count == 1); + up(); + REQUIRE(count == 2); + + object = nullptr; + + up(); + REQUIRE(count == 3); + } + + // weak_from_this + { + int count{}; + auto object = std::make_shared(count); + + delegate<> up{ object->weak_from_this(), &ObjectStd::Callback }; + + REQUIRE(count == 0); + up(); + REQUIRE(count == 1); + up(); + REQUIRE(count == 2); + + object = nullptr; + + up(); + REQUIRE(count == 2); // Unchanged + } + // Mixed arguments { int count{};