Simplify reference counting implementation of PendingTaskSafetyFlag.

On a 32bit system, this reduces the allocation size of the flag
down from 12 bytes to 8, and removes the need for a vtable (the extra
4 bytes are the vtable pointer).

The downside is that this change makes the binary layout of the
flag, less compatible with RefCountedObject<> based reference counting
objects and thus we don't immediately get the benefits of identical
COMDAT folding and subsequently there's a slight binary size increase.
With wider use, the binary size benefits will come.

Bug: none
Change-Id: I04129771790a3258d6accaf0ab1258b7a798a55e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215681
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33793}

api/BUILD.gn

@@ -561,6 +561,7 @@ rtc_source_set("rtc_stats_api") {
 
   deps = [
     ":scoped_refptr",
+    "../api:refcountedbase",
     "../rtc_base:checks",
     "../rtc_base:rtc_base_approved",
     "../rtc_base/system:rtc_export",
@@ -682,7 +683,10 @@ rtc_source_set("array_view") {
 rtc_source_set("refcountedbase") {
   visibility = [ "*" ]
   sources = [ "ref_counted_base.h" ]
-  deps = [ "../rtc_base:rtc_base_approved" ]
+  deps = [
+    "../rtc_base:macromagic",
+    "../rtc_base:refcount",
+  ]
 }
 
 rtc_library("ice_transport_factory") {

api/ref_counted_base.h

@@ -10,8 +10,9 @@
 #ifndef API_REF_COUNTED_BASE_H_
 #define API_REF_COUNTED_BASE_H_
 
+#include <type_traits>
+
 #include "rtc_base/constructor_magic.h"
-#include "rtc_base/ref_count.h"
 #include "rtc_base/ref_counter.h"
 
 namespace rtc {
@@ -38,6 +39,51 @@ class RefCountedBase {
   RTC_DISALLOW_COPY_AND_ASSIGN(RefCountedBase);
 };
 
+// Template based version of `RefCountedBase` for simple implementations that do
+// not need (or want) destruction via virtual destructor or the overhead of a
+// vtable.
+//
+// To use:
+//   struct MyInt : public rtc::RefCountedNonVirtual<MyInt>  {
+//     int foo_ = 0;
+//   };
+//
+//   rtc::scoped_refptr<MyInt> my_int(new MyInt());
+//
+// sizeof(MyInt) on a 32 bit system would then be 8, int + refcount and no
+// vtable generated.
+template <typename T>
+class RefCountedNonVirtual {
+ public:
+  RefCountedNonVirtual() = default;
+
+  void AddRef() const { ref_count_.IncRef(); }
+  RefCountReleaseStatus Release() const {
+    // If you run into this assert, T has virtual methods. There are two
+    // options:
+    // 1) The class doesn't actually need virtual methods, the type is complete
+    //    so the virtual attribute(s) can be removed.
+    // 2) The virtual methods are a part of the design of the class. In this
+    //    case you can consider using `RefCountedBase` instead or alternatively
+    //    use `rtc::RefCountedObject`.
+    static_assert(!std::is_polymorphic<T>::value,
+                  "T has virtual methods. RefCountedBase is a better fit.");
+    const auto status = ref_count_.DecRef();
+    if (status == RefCountReleaseStatus::kDroppedLastRef) {
+      delete static_cast<const T*>(this);
+    }
+    return status;
+  }
+
+ protected:
+  ~RefCountedNonVirtual() = default;
+
+ private:
+  mutable webrtc::webrtc_impl::RefCounter ref_count_{0};
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(RefCountedNonVirtual);
+};
+
 }  // namespace rtc
 
 #endif  // API_REF_COUNTED_BASE_H_

api/stats/rtc_stats_report.h

@@ -19,9 +19,11 @@
 #include <string>
 #include <vector>
 
+#include "api/ref_counted_base.h"
 #include "api/scoped_refptr.h"
 #include "api/stats/rtc_stats.h"
-#include "rtc_base/ref_count.h"
+// TODO(tommi): Remove this include after fixing iwyu issue in chromium.
+// See: third_party/blink/renderer/platform/peerconnection/rtc_stats.cc
 #include "rtc_base/ref_counted_object.h"
 #include "rtc_base/system/rtc_export.h"
 
@@ -29,7 +31,8 @@ namespace webrtc {
 
 // A collection of stats.
 // This is accessible as a map from |RTCStats::id| to |RTCStats|.
-class RTC_EXPORT RTCStatsReport : public rtc::RefCountInterface {
+class RTC_EXPORT RTCStatsReport final
+    : public rtc::RefCountedNonVirtual<RTCStatsReport> {
  public:
   typedef std::map<std::string, std::unique_ptr<const RTCStats>> StatsMap;
 
@@ -107,11 +110,11 @@ class RTC_EXPORT RTCStatsReport : public rtc::RefCountInterface {
   // listing all of its stats objects.
   std::string ToJson() const;
 
-  friend class rtc::RefCountedObject<RTCStatsReport>;
+ protected:
+  friend class rtc::RefCountedNonVirtual<RTCStatsReport>;
+  ~RTCStatsReport() = default;
 
  private:
-  ~RTCStatsReport() override;
-
   int64_t timestamp_us_;
   StatsMap stats_;
 };

modules/desktop_capture/full_screen_window_detector.h

@@ -32,7 +32,8 @@ namespace webrtc {
 // window using criteria provided by application specific
 // FullScreenApplicationHandler.
 
-class FullScreenWindowDetector : public rtc::RefCountedBase {
+class FullScreenWindowDetector
+    : public rtc::RefCountedNonVirtual<FullScreenWindowDetector> {
  public:
   using ApplicationHandlerFactory =
       std::function<std::unique_ptr<FullScreenApplicationHandler>(

modules/desktop_capture/linux/shared_x_display.h

@@ -28,7 +28,8 @@ typedef union _XEvent XEvent;
 namespace webrtc {
 
 // A ref-counted object to store XDisplay connection.
-class RTC_EXPORT SharedXDisplay : public rtc::RefCountedBase {
+class RTC_EXPORT SharedXDisplay
+    : public rtc::RefCountedNonVirtual<SharedXDisplay> {
  public:
   class XEventHandler {
    public:
@@ -38,9 +39,6 @@ class RTC_EXPORT SharedXDisplay : public rtc::RefCountedBase {
     virtual bool HandleXEvent(const XEvent& event) = 0;
   };
 
-  // Takes ownership of |display|.
-  explicit SharedXDisplay(Display* display);
-
   // Creates a new X11 Display for the |display_name|. NULL is returned if X11
   // connection failed. Equivalent to CreateDefault() when |display_name| is
   // empty.
@@ -65,8 +63,11 @@ class RTC_EXPORT SharedXDisplay : public rtc::RefCountedBase {
 
   void IgnoreXServerGrabs();
 
+  ~SharedXDisplay();
+
  protected:
-  ~SharedXDisplay() override;
+  // Takes ownership of |display|.
+  explicit SharedXDisplay(Display* display);
 
  private:
   typedef std::map<int, std::vector<XEventHandler*> > EventHandlersMap;

modules/desktop_capture/mac/desktop_configuration_monitor.h

@@ -25,15 +25,15 @@ namespace webrtc {
 
 // The class provides functions to synchronize capturing and display
 // reconfiguring across threads, and the up-to-date MacDesktopConfiguration.
-class DesktopConfigurationMonitor : public rtc::RefCountedBase {
+class DesktopConfigurationMonitor final
+    : public rtc::RefCountedNonVirtual<DesktopConfigurationMonitor> {
  public:
   DesktopConfigurationMonitor();
+  ~DesktopConfigurationMonitor();
+
   // Returns the current desktop configuration.
   MacDesktopConfiguration desktop_configuration();
 
- protected:
-  ~DesktopConfigurationMonitor() override;
-
  private:
   static void DisplaysReconfiguredCallback(CGDirectDisplayID display,
                                            CGDisplayChangeSummaryFlags flags,

pc/BUILD.gn

@@ -329,6 +329,7 @@ rtc_library("connection_context") {
     "../api:callfactory_api",
     "../api:libjingle_peerconnection_api",
     "../api:media_stream_interface",
+    "../api:refcountedbase",
     "../api:scoped_refptr",
     "../api:sequence_checker",
     "../api/neteq:neteq_api",

pc/connection_context.cc

@@ -17,7 +17,6 @@
 #include "api/transport/field_trial_based_config.h"
 #include "media/sctp/sctp_transport_factory.h"
 #include "rtc_base/helpers.h"
-#include "rtc_base/ref_counted_object.h"
 #include "rtc_base/task_utils/to_queued_task.h"
 #include "rtc_base/time_utils.h"
 
@@ -76,7 +75,7 @@ std::unique_ptr<SctpTransportFactoryInterface> MaybeCreateSctpFactory(
 // Static
 rtc::scoped_refptr<ConnectionContext> ConnectionContext::Create(
     PeerConnectionFactoryDependencies* dependencies) {
-  return new rtc::RefCountedObject<ConnectionContext>(dependencies);
+  return new ConnectionContext(dependencies);
 }
 
 ConnectionContext::ConnectionContext(

pc/connection_context.h

@@ -17,6 +17,7 @@
 #include "api/call/call_factory_interface.h"
 #include "api/media_stream_interface.h"
 #include "api/peer_connection_interface.h"
+#include "api/ref_counted_base.h"
 #include "api/scoped_refptr.h"
 #include "api/sequence_checker.h"
 #include "api/transport/sctp_transport_factory_interface.h"
@@ -27,7 +28,6 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/network.h"
 #include "rtc_base/network_monitor_factory.h"
-#include "rtc_base/ref_count.h"
 #include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/thread.h"
 #include "rtc_base/thread_annotations.h"
@@ -47,7 +47,8 @@ class RtcEventLog;
 // interferes with the operation of other PeerConnections.
 //
 // This class must be created and destroyed on the signaling thread.
-class ConnectionContext : public rtc::RefCountInterface {
+class ConnectionContext final
+    : public rtc::RefCountedNonVirtual<ConnectionContext> {
  public:
   // Creates a ConnectionContext. May return null if initialization fails.
   // The Dependencies class allows simple management of all new dependencies
@@ -92,7 +93,8 @@ class ConnectionContext : public rtc::RefCountInterface {
  protected:
   explicit ConnectionContext(PeerConnectionFactoryDependencies* dependencies);
 
-  virtual ~ConnectionContext();
+  friend class rtc::RefCountedNonVirtual<ConnectionContext>;
+  ~ConnectionContext();
 
  private:
   // The following three variables are used to communicate between the

rtc_base/BUILD.gn

@@ -916,6 +916,7 @@ rtc_library("rtc_base") {
     ":threading",
     "../api:array_view",
     "../api:function_view",
+    "../api:refcountedbase",
     "../api:scoped_refptr",
     "../api:sequence_checker",
     "../api/numerics",

rtc_base/rtc_certificate.cc

@@ -13,7 +13,6 @@
 #include <memory>
 
 #include "rtc_base/checks.h"
-#include "rtc_base/ref_counted_object.h"
 #include "rtc_base/ssl_certificate.h"
 #include "rtc_base/ssl_identity.h"
 #include "rtc_base/time_utils.h"
@@ -22,14 +21,14 @@ namespace rtc {
 
 scoped_refptr<RTCCertificate> RTCCertificate::Create(
     std::unique_ptr<SSLIdentity> identity) {
-  return new RefCountedObject<RTCCertificate>(identity.release());
+  return new RTCCertificate(identity.release());
 }
 
 RTCCertificate::RTCCertificate(SSLIdentity* identity) : identity_(identity) {
   RTC_DCHECK(identity_);
 }
 
-RTCCertificate::~RTCCertificate() {}
+RTCCertificate::~RTCCertificate() = default;
 
 uint64_t RTCCertificate::Expires() const {
   int64_t expires = GetSSLCertificate().CertificateExpirationTime();
@@ -67,7 +66,7 @@ scoped_refptr<RTCCertificate> RTCCertificate::FromPEM(
       SSLIdentity::CreateFromPEMStrings(pem.private_key(), pem.certificate()));
   if (!identity)
     return nullptr;
-  return new RefCountedObject<RTCCertificate>(identity.release());
+  return new RTCCertificate(identity.release());
 }
 
 bool RTCCertificate::operator==(const RTCCertificate& certificate) const {

rtc_base/rtc_certificate.h

@@ -16,8 +16,8 @@
 #include <memory>
 #include <string>
 
+#include "api/ref_counted_base.h"
 #include "api/scoped_refptr.h"
-#include "rtc_base/ref_count.h"
 #include "rtc_base/system/rtc_export.h"
 
 namespace rtc {
@@ -49,7 +49,8 @@ class RTCCertificatePEM {
 // A thin abstraction layer between "lower level crypto stuff" like
 // SSLCertificate and WebRTC usage. Takes ownership of some lower level objects,
 // reference counting protects these from premature destruction.
-class RTC_EXPORT RTCCertificate : public RefCountInterface {
+class RTC_EXPORT RTCCertificate final
+    : public RefCountedNonVirtual<RTCCertificate> {
  public:
   // Takes ownership of |identity|.
   static scoped_refptr<RTCCertificate> Create(
@@ -82,7 +83,9 @@ class RTC_EXPORT RTCCertificate : public RefCountInterface {
 
  protected:
   explicit RTCCertificate(SSLIdentity* identity);
-  ~RTCCertificate() override;
+
+  friend class RefCountedNonVirtual<RTCCertificate>;
+  ~RTCCertificate();
 
  private:
   // The SSLIdentity is the owner of the SSLCertificate. To protect our

rtc_base/task_utils/BUILD.gn

@@ -33,7 +33,7 @@ rtc_library("pending_task_safety_flag") {
   ]
   deps = [
     "..:checks",
-    "..:refcount",
+    "../../api:refcountedbase",
     "../../api:scoped_refptr",
     "../../api:sequence_checker",
     "../system:no_unique_address",

rtc_base/task_utils/pending_task_safety_flag.cc

@@ -10,19 +10,17 @@
 
 #include "rtc_base/task_utils/pending_task_safety_flag.h"
 
-#include "rtc_base/ref_counted_object.h"
-
 namespace webrtc {
 
 // static
 rtc::scoped_refptr<PendingTaskSafetyFlag> PendingTaskSafetyFlag::Create() {
-  return new rtc::RefCountedObject<PendingTaskSafetyFlag>(true);
+  return new PendingTaskSafetyFlag(true);
 }
 
 rtc::scoped_refptr<PendingTaskSafetyFlag>
 PendingTaskSafetyFlag::CreateDetached() {
   rtc::scoped_refptr<PendingTaskSafetyFlag> safety_flag(
-      new rtc::RefCountedObject<PendingTaskSafetyFlag>(true));
+      new PendingTaskSafetyFlag(true));
   safety_flag->main_sequence_.Detach();
   return safety_flag;
 }
@@ -30,7 +28,7 @@ PendingTaskSafetyFlag::CreateDetached() {
 rtc::scoped_refptr<PendingTaskSafetyFlag>
 PendingTaskSafetyFlag::CreateDetachedInactive() {
   rtc::scoped_refptr<PendingTaskSafetyFlag> safety_flag(
-      new rtc::RefCountedObject<PendingTaskSafetyFlag>(false));
+      new PendingTaskSafetyFlag(false));
   safety_flag->main_sequence_.Detach();
   return safety_flag;
 }

rtc_base/task_utils/pending_task_safety_flag.h

@@ -11,10 +11,10 @@
 #ifndef RTC_BASE_TASK_UTILS_PENDING_TASK_SAFETY_FLAG_H_
 #define RTC_BASE_TASK_UTILS_PENDING_TASK_SAFETY_FLAG_H_
 
+#include "api/ref_counted_base.h"
 #include "api/scoped_refptr.h"
 #include "api/sequence_checker.h"
 #include "rtc_base/checks.h"
-#include "rtc_base/ref_count.h"
 #include "rtc_base/system/no_unique_address.h"
 
 namespace webrtc {
@@ -55,7 +55,8 @@ namespace webrtc {
 //    my_task_queue_->PostTask(ToQueuedTask(pending_task_safety_flag_,
 //        [this]() { MyMethod(); }));
 //
-class PendingTaskSafetyFlag : public rtc::RefCountInterface {
+class PendingTaskSafetyFlag final
+    : public rtc::RefCountedNonVirtual<PendingTaskSafetyFlag> {
  public:
   static rtc::scoped_refptr<PendingTaskSafetyFlag> Create();
 
@@ -113,7 +114,7 @@ class PendingTaskSafetyFlag : public rtc::RefCountInterface {
 // This should be used by the class that wants tasks dropped after destruction.
 // The requirement is that the instance has to be constructed and destructed on
 // the same thread as the potentially dropped tasks would be running on.
-class ScopedTaskSafety {
+class ScopedTaskSafety final {
  public:
   ScopedTaskSafety() = default;
   ~ScopedTaskSafety() { flag_->SetNotAlive(); }
@@ -128,7 +129,7 @@ class ScopedTaskSafety {
 
 // Like ScopedTaskSafety, but allows construction on a different thread than
 // where the flag will be used.
-class ScopedTaskSafetyDetached {
+class ScopedTaskSafetyDetached final {
  public:
   ScopedTaskSafetyDetached() = default;
   ~ScopedTaskSafetyDetached() { flag_->SetNotAlive(); }

sdk/android/src/jni/audio_device/opensles_common.cc

@@ -106,8 +106,6 @@ OpenSLEngineManager::OpenSLEngineManager() {
   thread_checker_.Detach();
 }
 
-OpenSLEngineManager::~OpenSLEngineManager() = default;
-
 SLObjectItf OpenSLEngineManager::GetOpenSLEngine() {
   RTC_LOG(INFO) << "GetOpenSLEngine";
   RTC_DCHECK(thread_checker_.IsCurrent());

sdk/android/src/jni/audio_device/opensles_common.h

@@ -68,10 +68,11 @@ typedef ScopedSLObject<SLObjectItf, const SLObjectItf_*> ScopedSLObjectItf;
 // Subsequent calls returns the already created engine.
 // Note: This class must be used single threaded and this is enforced by a
 // thread checker.
-class OpenSLEngineManager : public rtc::RefCountedBase {
+class OpenSLEngineManager
+    : public rtc::RefCountedNonVirtual<OpenSLEngineManager> {
  public:
   OpenSLEngineManager();
-  ~OpenSLEngineManager() override;
+  ~OpenSLEngineManager() = default;
   SLObjectItf GetOpenSLEngine();
 
  private:

sdk/android/src/jni/pc/add_ice_candidate_observer.h

@@ -20,10 +20,11 @@
 namespace webrtc {
 namespace jni {
 
-class AddIceCandidateObserverJni final : public rtc::RefCountedBase {
+class AddIceCandidateObserverJni final
+    : public rtc::RefCountedNonVirtual<AddIceCandidateObserverJni> {
  public:
   AddIceCandidateObserverJni(JNIEnv* env, const JavaRef<jobject>& j_observer);
-  ~AddIceCandidateObserverJni() override = default;
+  ~AddIceCandidateObserverJni() = default;
 
   void OnComplete(RTCError error);
 

stats/rtc_stats_report.cc

@@ -56,15 +56,12 @@ bool RTCStatsReport::ConstIterator::operator!=(
 
 rtc::scoped_refptr<RTCStatsReport> RTCStatsReport::Create(
     int64_t timestamp_us) {
-  return rtc::scoped_refptr<RTCStatsReport>(
+  return rtc::scoped_refptr<RTCStatsReport>(new RTCStatsReport(timestamp_us));
-      new rtc::RefCountedObject<RTCStatsReport>(timestamp_us));
 }
 
 RTCStatsReport::RTCStatsReport(int64_t timestamp_us)
     : timestamp_us_(timestamp_us) {}
 
-RTCStatsReport::~RTCStatsReport() {}
-
 rtc::scoped_refptr<RTCStatsReport> RTCStatsReport::Copy() const {
   rtc::scoped_refptr<RTCStatsReport> copy = Create(timestamp_us_);
   for (auto it = stats_.begin(); it != stats_.end(); ++it) {