Use AlwaysValidPointer in connection_context

This extends AlwaysValidPointer to take a lambda for its default
rather than requesting a constructor.

Bug: none
Change-Id: Ied97968c3f511af15422a1eef9801d14d4ec5b96
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/260580
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36745}

pc/BUILD.gn

@@ -956,6 +956,7 @@ rtc_library("connection_context") {
     "../rtc_base:socket_server",
     "../rtc_base:threading",
     "../rtc_base:timeutils",
+    "../rtc_base/memory:always_valid_pointer",
     "../rtc_base/task_utils:to_queued_task",
   ]
 }

pc/connection_context.cc

@@ -43,18 +43,6 @@ rtc::Thread* MaybeStartNetworkThread(
   return thread_holder.get();
 }
 
-rtc::Thread* MaybeStartWorkerThread(
-    rtc::Thread* old_thread,
-    std::unique_ptr<rtc::Thread>& thread_holder) {
-  if (old_thread) {
-    return old_thread;
-  }
-  thread_holder = rtc::Thread::Create();
-  thread_holder->SetName("pc_worker_thread", nullptr);
-  thread_holder->Start();
-  return thread_holder.get();
-}
-
 rtc::Thread* MaybeWrapThread(rtc::Thread* signaling_thread,
                              bool& wraps_current_thread) {
   wraps_current_thread = false;
@@ -99,8 +87,13 @@ ConnectionContext::ConnectionContext(
     : network_thread_(MaybeStartNetworkThread(dependencies->network_thread,
                                               owned_socket_factory_,
                                               owned_network_thread_)),
-      worker_thread_(MaybeStartWorkerThread(dependencies->worker_thread,
+      worker_thread_(dependencies->worker_thread,
-                                            owned_worker_thread_)),
+                     []() {
+                       auto thread_holder = rtc::Thread::Create();
+                       thread_holder->SetName("pc_worker_thread", nullptr);
+                       thread_holder->Start();
+                       return thread_holder;
+                     }),
       signaling_thread_(MaybeWrapThread(dependencies->signaling_thread,
                                         wraps_current_thread_)),
       trials_(dependencies->trials ? std::move(dependencies->trials)
@@ -112,7 +105,7 @@ ConnectionContext::ConnectionContext(
           MaybeCreateSctpFactory(std::move(dependencies->sctp_factory),
                                  network_thread(),
                                  *trials_.get())) {
-  signaling_thread_->AllowInvokesToThread(worker_thread_);
+  signaling_thread_->AllowInvokesToThread(worker_thread());
   signaling_thread_->AllowInvokesToThread(network_thread_);
   worker_thread_->AllowInvokesToThread(network_thread_);
   if (network_thread_->IsCurrent()) {

pc/connection_context.h

@@ -70,8 +70,8 @@ class ConnectionContext final
 
   rtc::Thread* signaling_thread() { return signaling_thread_; }
   const rtc::Thread* signaling_thread() const { return signaling_thread_; }
-  rtc::Thread* worker_thread() { return worker_thread_; }
+  rtc::Thread* worker_thread() { return worker_thread_.get(); }
-  const rtc::Thread* worker_thread() const { return worker_thread_; }
+  const rtc::Thread* worker_thread() const { return worker_thread_.get(); }
   rtc::Thread* network_thread() { return network_thread_; }
   const rtc::Thread* network_thread() const { return network_thread_; }
 
@@ -91,7 +91,7 @@ class ConnectionContext final
     return default_socket_factory_.get();
   }
   CallFactoryInterface* call_factory() {
-    RTC_DCHECK_RUN_ON(worker_thread_);
+    RTC_DCHECK_RUN_ON(worker_thread());
     return call_factory_.get();
   }
 
@@ -105,16 +105,11 @@ class ConnectionContext final
   // The following three variables are used to communicate between the
   // constructor and the destructor, and are never exposed externally.
   bool wraps_current_thread_;
-  // Note: Since owned_network_thread_ and owned_worker_thread_ are used
-  // in the initialization of network_thread_ and worker_thread_, they
-  // must be declared before them, so that they are initialized first.
   std::unique_ptr<rtc::SocketFactory> owned_socket_factory_;
   std::unique_ptr<rtc::Thread> owned_network_thread_
       RTC_GUARDED_BY(signaling_thread_);
-  std::unique_ptr<rtc::Thread> owned_worker_thread_
-      RTC_GUARDED_BY(signaling_thread_);
   rtc::Thread* const network_thread_;
-  rtc::Thread* const worker_thread_;
+  AlwaysValidPointer<rtc::Thread> const worker_thread_;
   rtc::Thread* const signaling_thread_;
 
   // Accessed both on signaling thread and worker thread.
@@ -127,7 +122,7 @@ class ConnectionContext final
   std::unique_ptr<rtc::BasicNetworkManager> default_network_manager_
       RTC_GUARDED_BY(signaling_thread_);
   std::unique_ptr<webrtc::CallFactoryInterface> const call_factory_
-      RTC_GUARDED_BY(worker_thread_);
+      RTC_GUARDED_BY(worker_thread());
 
   std::unique_ptr<rtc::BasicPacketSocketFactory> default_socket_factory_
       RTC_GUARDED_BY(signaling_thread_);

rtc_base/memory/always_valid_pointer.h

@@ -29,14 +29,39 @@ class AlwaysValidPointer {
     RTC_DCHECK(pointer_);
   }
 
-  template <typename... Args>
+  template <typename Arg,
-  AlwaysValidPointer(Interface* pointer, Args... args)
+            typename std::enable_if<!(std::is_invocable<Arg>::value),
-      : owned_instance_(
+                                    bool>::type = true>
-            pointer ? nullptr : std::make_unique<Default>(std::move(args...))),
+  AlwaysValidPointer(Interface* pointer, Arg arg)
+      : owned_instance_(pointer ? nullptr
+                                : std::make_unique<Default>(std::move(arg))),
         pointer_(pointer ? pointer : owned_instance_.get()) {
     RTC_DCHECK(pointer_);
   }
 
+  // Multiple arguments
+  template <typename Arg1, typename... Args>
+  AlwaysValidPointer(Interface* pointer, Arg1 arg1, Args... args)
+      : owned_instance_(pointer
+                            ? nullptr
+                            : std::make_unique<Default>(std::move(arg1),
+                                                        std::move(args...))),
+        pointer_(pointer ? pointer : owned_instance_.get()) {
+    RTC_DCHECK(pointer_);
+  }
+
+  // Create a pointer by
+  // a) using |pointer|, without taking ownership
+  // b) calling |function| and taking ownership of the result
+  template <typename Func,
+            typename std::enable_if<std::is_invocable<Func>::value,
+                                    bool>::type = true>
+  AlwaysValidPointer(Interface* pointer, Func function)
+      : owned_instance_(pointer ? nullptr : function()),
+        pointer_(owned_instance_ ? owned_instance_.get() : pointer) {
+    RTC_DCHECK(pointer_);
+  }
+
   // Create a pointer by
   // a) taking over ownership of |instance|
   // b) or fallback to |pointer|, without taking ownership.

rtc_base/memory/always_valid_pointer_unittest.cc

@@ -84,4 +84,10 @@ TEST(AlwaysValidPointerTest, TakeOverOwnershipDoesNotForwardDefaultArguments) {
   EXPECT_EQ(*str2, "anka");
 }
 
+TEST(AlwaysValidPointerTest, DefaultToLambda) {
+  AlwaysValidPointer<std::string> ptr(
+      nullptr, []() { return std::make_unique<std::string>("onkel skrue"); });
+  EXPECT_EQ(*ptr, "onkel skrue");
+}
+
 }  // namespace webrtc