From bb57de2959ca8d1391e9e6998afa1d6e56a7b32b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Tue, 11 Jan 2022 15:40:22 +0100 Subject: [PATCH] Extend make_ref_counted to interoperate with RefCountedNonVirtual Update RtpPacketInfos internals to use rtc::make_ref_counted, and a Data class with no virtual methods. Bug: webrtc:13464, webrtc:12701 Change-Id: I03f6bee69a9f060dcf287284fc779268d5eb433e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244505 Reviewed-by: Tomas Gunnarsson Reviewed-by: Danil Chapovalov Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/main@{#35660} --- api/DEPS | 3 ++ api/rtp_packet_infos.h | 11 +++--- rtc_base/ref_counted_object.h | 66 +++++++++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/api/DEPS b/api/DEPS index cadaaa2a7f..0b8a70eabf 100644 --- a/api/DEPS +++ b/api/DEPS @@ -168,6 +168,9 @@ specific_include_rules = { # For private member and constructor. "+rtc_base/system/file_wrapper.h", ], + "rtp_packet_infos\.h": [ + "+rtc_base/ref_counted_object.h", + ], "rtp_receiver_interface\.h": [ "+rtc_base/ref_count.h", ], diff --git a/api/rtp_packet_infos.h b/api/rtp_packet_infos.h index 2ca3174037..031e33332e 100644 --- a/api/rtp_packet_infos.h +++ b/api/rtp_packet_infos.h @@ -18,6 +18,7 @@ #include "api/ref_counted_base.h" #include "api/rtp_packet_info.h" #include "api/scoped_refptr.h" +#include "rtc_base/ref_counted_object.h" #include "rtc_base/system/rtc_export.h" namespace webrtc { @@ -79,7 +80,7 @@ class RTC_EXPORT RtpPacketInfos { size_type size() const { return entries().size(); } private: - class Data : public rtc::RefCountedBase { + class Data final : public rtc::RefCountedNonVirtual { public: static rtc::scoped_refptr Create(const vector_type& entries) { // Performance optimization for the empty case. @@ -87,7 +88,7 @@ class RTC_EXPORT RtpPacketInfos { return nullptr; } - return new Data(entries); + return rtc::make_ref_counted(entries); } static rtc::scoped_refptr Create(vector_type&& entries) { @@ -96,16 +97,16 @@ class RTC_EXPORT RtpPacketInfos { return nullptr; } - return new Data(std::move(entries)); + return rtc::make_ref_counted(std::move(entries)); } const vector_type& entries() const { return entries_; } - private: explicit Data(const vector_type& entries) : entries_(entries) {} explicit Data(vector_type&& entries) : entries_(std::move(entries)) {} - ~Data() override {} + ~Data() = default; + private: const vector_type entries_; }; diff --git a/rtc_base/ref_counted_object.h b/rtc_base/ref_counted_object.h index 2a55d863c1..37612add8c 100644 --- a/rtc_base/ref_counted_object.h +++ b/rtc_base/ref_counted_object.h @@ -20,6 +20,23 @@ namespace rtc { +namespace webrtc_make_ref_counted_internal { +// Determines if the given class has AddRef and Release methods. +template +class HasAddRefAndRelease { + private: + template ().AddRef())* = nullptr, + decltype(std::declval().Release())* = nullptr> + static int Test(int); + template + static char Test(...); + + public: + static constexpr bool value = std::is_same_v(0)), int>; +}; +} // namespace webrtc_make_ref_counted_internal + template class RefCountedObject : public T { public: @@ -64,10 +81,9 @@ template class FinalRefCountedObject final : public T { public: using T::T; - // Until c++17 compilers are allowed not to inherit the default constructors. - // Thus the default constructors are forwarded explicitly. - FinalRefCountedObject() = default; - explicit FinalRefCountedObject(const T& other) : T(other) {} + // Above using declaration propagates a default move constructor + // FinalRefCountedObject(FinalRefCountedObject&& other), but we also need + // move construction from T. explicit FinalRefCountedObject(T&& other) : T(std::move(other)) {} FinalRefCountedObject(const FinalRefCountedObject&) = delete; FinalRefCountedObject& operator=(const FinalRefCountedObject&) = delete; @@ -106,8 +122,13 @@ class FinalRefCountedObject final : public T { // // auto p = scoped_refptr(new RefCountedObject("bar", 123)); // -// If the class does not inherit from RefCountInterface, the example is -// equivalent to: +// If the class does not inherit from RefCountInterface, but does have +// AddRef/Release methods (so a T* is convertible to rtc::scoped_refptr), this +// is equivalent to just +// +// auto p = scoped_refptr(new Foo("bar", 123)); +// +// Otherwise, the example is equivalent to: // // auto p = scoped_refptr>( // new FinalRefCountedObject("bar", 123)); @@ -122,22 +143,37 @@ class FinalRefCountedObject final : public T { // needed. // `make_ref_counted` for classes that are convertible to RefCountInterface. -template < - typename T, - typename... Args, - typename std::enable_if::value, - T>::type* = nullptr> +template , + T>::type* = nullptr> scoped_refptr make_ref_counted(Args&&... args) { return new RefCountedObject(std::forward(args)...); } // `make_ref_counted` for complete classes that are not convertible to -// RefCountInterface. +// RefCountInterface and already carry a ref count. template < typename T, typename... Args, - typename std::enable_if::value, - T>::type* = nullptr> + typename std::enable_if< + !std::is_convertible_v && + webrtc_make_ref_counted_internal::HasAddRefAndRelease::value, + T>::type* = nullptr> +scoped_refptr make_ref_counted(Args&&... args) { + return scoped_refptr(new T(std::forward(args)...)); +} + +// `make_ref_counted` for complete classes that are not convertible to +// RefCountInterface and have no ref count of their own. +template < + typename T, + typename... Args, + typename std::enable_if< + !std::is_convertible_v && + !webrtc_make_ref_counted_internal::HasAddRefAndRelease::value, + + T>::type* = nullptr> scoped_refptr> make_ref_counted(Args&&... args) { return new FinalRefCountedObject(std::forward(args)...); } @@ -188,7 +224,7 @@ scoped_refptr> make_ref_counted(Args&&... args) { template struct Ref { typedef typename std::conditional< - std::is_convertible::value, + webrtc_make_ref_counted_internal::HasAddRefAndRelease::value, T, FinalRefCountedObject>::type Type;