Cleaning up C++14 move into lambda TODOs.

Bug: webrtc:10945
Change-Id: I4d2f358b0e33b37e4b4f7bfcf3f6cd55e8d46bf9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/153240
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29212}

common_video/incoming_video_stream.cc

@@ -38,18 +38,14 @@ void IncomingVideoStream::OnFrame(const VideoFrame& video_frame) {
   TRACE_EVENT0("webrtc", "IncomingVideoStream::OnFrame");
   RTC_CHECK_RUNS_SERIALIZED(&decoder_race_checker_);
   RTC_DCHECK(!incoming_render_queue_.IsCurrent());
-  // TODO(srte): This struct should be replaced by a lambda with move capture
+  // TODO(srte): Using video_frame = std::move(video_frame) would move the frame
-  // when C++14 lambdas are allowed.
+  // into the lambda instead of copying it, but it doesn't work unless we change
-  struct NewFrameTask {
+  // OnFrame to take its frame argument by value instead of const reference.
-    void operator()() {
+  incoming_render_queue_.PostTask([this, video_frame = video_frame]() mutable {
-      RTC_DCHECK(stream->incoming_render_queue_.IsCurrent());
+    RTC_DCHECK(incoming_render_queue_.IsCurrent());
-      if (stream->render_buffers_.AddFrame(std::move(frame)) == 1)
+    if (render_buffers_.AddFrame(std::move(video_frame)) == 1)
-        stream->Dequeue();
+      Dequeue();
-    }
+  });
-    IncomingVideoStream* stream;
-    VideoFrame frame;
-  };
-  incoming_render_queue_.PostTask(NewFrameTask{this, std::move(video_frame)});
 }
 
 void IncomingVideoStream::Dequeue() {

logging/rtc_event_log/rtc_event_log_impl.cc

@@ -36,26 +36,6 @@ constexpr size_t kMaxEventsInHistory = 10000;
 // to prevent an attack via unreasonable memory use.
 constexpr size_t kMaxEventsInConfigHistory = 1000;
 
-// TODO(eladalon): This class exists because C++11 doesn't allow transferring a
-// unique_ptr to a lambda (a copy constructor is required). We should get
-// rid of this when we move to C++14.
-template <typename T>
-class ResourceOwningTask final : public QueuedTask {
- public:
-  ResourceOwningTask(std::unique_ptr<T> resource,
-                     std::function<void(std::unique_ptr<T>)> handler)
-      : resource_(std::move(resource)), handler_(handler) {}
-
-  bool Run() override {
-    handler_(std::move(resource_));
-    return true;
-  }
-
- private:
-  std::unique_ptr<T> resource_;
-  std::function<void(std::unique_ptr<T>)> handler_;
-};
-
 std::unique_ptr<RtcEventLogEncoder> CreateEncoder(
     RtcEventLog::EncodingType type) {
   switch (type) {
@@ -113,9 +93,11 @@ bool RtcEventLogImpl::StartLogging(std::unique_ptr<RtcEventLogOutput> output,
   RTC_LOG(LS_INFO) << "Starting WebRTC event log. (Timestamp, UTC) = "
                    << "(" << timestamp_us << ", " << utc_time_us << ").";
 
+  RTC_DCHECK_RUN_ON(&logging_state_checker_);
+  logging_state_started_ = true;
   // Binding to |this| is safe because |this| outlives the |task_queue_|.
-  auto start = [this, output_period_ms, timestamp_us,
+  task_queue_->PostTask([this, output_period_ms, timestamp_us, utc_time_us,
-                utc_time_us](std::unique_ptr<RtcEventLogOutput> output) {
+                         output = std::move(output)]() mutable {
     RTC_DCHECK_RUN_ON(task_queue_.get());
     RTC_DCHECK(output->IsActive());
     output_period_ms_ = output_period_ms;
@@ -123,13 +105,7 @@ bool RtcEventLogImpl::StartLogging(std::unique_ptr<RtcEventLogOutput> output,
     num_config_events_written_ = 0;
     WriteToOutput(event_encoder_->EncodeLogStart(timestamp_us, utc_time_us));
     LogEventsFromMemoryToOutput();
-  };
+  });
-
-  RTC_DCHECK_RUN_ON(&logging_state_checker_);
-  logging_state_started_ = true;
-
-  task_queue_->PostTask(std::make_unique<ResourceOwningTask<RtcEventLogOutput>>(
-      std::move(output), start));
 
   return true;
 }
@@ -168,15 +144,12 @@ void RtcEventLogImpl::Log(std::unique_ptr<RtcEvent> event) {
   RTC_CHECK(event);
 
   // Binding to |this| is safe because |this| outlives the |task_queue_|.
-  auto event_handler = [this](std::unique_ptr<RtcEvent> unencoded_event) {
+  task_queue_->PostTask([this, event = std::move(event)]() mutable {
     RTC_DCHECK_RUN_ON(task_queue_.get());
-    LogToMemory(std::move(unencoded_event));
+    LogToMemory(std::move(event));
     if (event_output_)
       ScheduleOutput();
-  };
+  });
-
-  task_queue_->PostTask(std::make_unique<ResourceOwningTask<RtcEvent>>(
-      std::move(event), event_handler));
 }
 
 void RtcEventLogImpl::ScheduleOutput() {

rtc_base/task_utils/repeating_task_unittest.cc

@@ -60,18 +60,6 @@ class MoveOnlyClosure {
  private:
   MockClosure* mock_;
 };
-
-// Helper closure class to stop repeating task on a task queue. This is
-// equivalent to [handle{move(handle)}] { handle.Stop(); } in c++14.
-class TaskHandleStopper {
- public:
-  explicit TaskHandleStopper(RepeatingTaskHandle handle)
-      : handle_(std::move(handle)) {}
-  void operator()() { handle_.Stop(); }
-
- private:
-  RepeatingTaskHandle handle_;
-};
 }  // namespace
 
 TEST(RepeatingTaskTest, TaskIsStoppedOnStop) {
@@ -91,7 +79,8 @@ TEST(RepeatingTaskTest, TaskIsStoppedOnStop) {
   Sleep(kShortInterval * (kShortIntervalCount + kMargin));
   EXPECT_EQ(counter.load(), kShortIntervalCount);
 
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
   // Sleep long enough that the task would run at least once more if not
   // stopped.
   Sleep(kLongInterval * 2);
@@ -144,7 +133,8 @@ TEST(RepeatingTaskTest, CancelDelayedTaskBeforeItRuns) {
   TaskQueueForTest task_queue("queue");
   auto handle = RepeatingTaskHandle::DelayedStart(
       task_queue.Get(), TimeDelta::ms(100), MoveOnlyClosure(&mock));
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
   EXPECT_TRUE(done.Wait(kTimeout.ms()));
 }
 
@@ -156,7 +146,8 @@ TEST(RepeatingTaskTest, CancelTaskAfterItRuns) {
   TaskQueueForTest task_queue("queue");
   auto handle =
       RepeatingTaskHandle::Start(task_queue.Get(), MoveOnlyClosure(&mock));
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
   EXPECT_TRUE(done.Wait(kTimeout.ms()));
 }
 
@@ -223,9 +214,11 @@ TEST(RepeatingTaskTest, Example) {
   RepeatingTaskHandle handle;
   object->StartPeriodicTask(&handle, task_queue.Get());
   // Restart the task
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
   object->StartPeriodicTask(&handle, task_queue.Get());
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
   struct Destructor {
     void operator()() { object.reset(); }
     std::unique_ptr<ObjectOnTaskQueue> object;

test/network/network_emulation_manager.cc

@@ -27,29 +27,6 @@ namespace {
 constexpr uint32_t kMinIPv4Address = 0xC0A80000;
 // uint32_t representation of 192.168.255.255 address
 constexpr uint32_t kMaxIPv4Address = 0xC0A8FFFF;
-
-template <typename T, typename Closure>
-class ResourceOwningTask final : public QueuedTask {
- public:
-  ResourceOwningTask(T&& resource, Closure&& handler)
-      : resource_(std::move(resource)),
-        handler_(std::forward<Closure>(handler)) {}
-
-  bool Run() override {
-    handler_(std::move(resource_));
-    return true;
-  }
-
- private:
-  T resource_;
-  Closure handler_;
-};
-template <typename T, typename Closure>
-std::unique_ptr<QueuedTask> CreateResourceOwningTask(T resource,
-                                                     Closure&& closure) {
-  return std::make_unique<ResourceOwningTask<T, Closure>>(
-      std::forward<T>(resource), std::forward<Closure>(closure));
-}
 }  // namespace
 
 NetworkEmulationManagerImpl::NetworkEmulationManagerImpl()
@@ -79,10 +56,9 @@ EmulatedNetworkNode* NetworkEmulationManagerImpl::CreateEmulatedNode(
   auto node = std::make_unique<EmulatedNetworkNode>(
       clock_, &task_queue_, std::move(network_behavior));
   EmulatedNetworkNode* out = node.get();
-  task_queue_.PostTask(CreateResourceOwningTask(
+  task_queue_.PostTask([this, node = std::move(node)]() mutable {
-      std::move(node), [this](std::unique_ptr<EmulatedNetworkNode> node) {
+    network_nodes_.push_back(std::move(node));
-        network_nodes_.push_back(std::move(node));
+  });
-      }));
   return out;
 }
 
@@ -203,9 +179,8 @@ NetworkEmulationManagerImpl::CreateRandomWalkCrossTraffic(
       std::make_unique<RandomWalkCrossTraffic>(config, traffic_route);
   RandomWalkCrossTraffic* out = traffic.get();
 
-  task_queue_.PostTask(CreateResourceOwningTask(
+  task_queue_.PostTask(
-      std::move(traffic),
+      [this, config, traffic = std::move(traffic)]() mutable {
-      [this, config](std::unique_ptr<RandomWalkCrossTraffic> traffic) {
         auto* traffic_ptr = traffic.get();
         random_cross_traffics_.push_back(std::move(traffic));
         RepeatingTaskHandle::Start(task_queue_.Get(),
@@ -213,7 +188,7 @@ NetworkEmulationManagerImpl::CreateRandomWalkCrossTraffic(
                                      traffic_ptr->Process(Now());
                                      return config.min_packet_interval;
                                    });
-      }));
+      });
   return out;
 }
 
@@ -224,9 +199,8 @@ NetworkEmulationManagerImpl::CreatePulsedPeaksCrossTraffic(
   auto traffic =
       std::make_unique<PulsedPeaksCrossTraffic>(config, traffic_route);
   PulsedPeaksCrossTraffic* out = traffic.get();
-  task_queue_.PostTask(CreateResourceOwningTask(
+  task_queue_.PostTask(
-      std::move(traffic),
+      [this, config, traffic = std::move(traffic)]() mutable {
-      [this, config](std::unique_ptr<PulsedPeaksCrossTraffic> traffic) {
         auto* traffic_ptr = traffic.get();
         pulsed_cross_traffics_.push_back(std::move(traffic));
         RepeatingTaskHandle::Start(task_queue_.Get(),
@@ -234,7 +208,7 @@ NetworkEmulationManagerImpl::CreatePulsedPeaksCrossTraffic(
                                      traffic_ptr->Process(Now());
                                      return config.min_packet_interval;
                                    });
-      }));
+      });
   return out;
 }
 

test/peer_scenario/scenario_connection.cc

@@ -138,28 +138,26 @@ void ScenarioIceConnectionImpl::SendRtpPacket(
     rtc::ArrayView<const uint8_t> packet_view) {
   rtc::CopyOnWriteBuffer packet(packet_view.data(), packet_view.size(),
                                 ::cricket::kMaxRtpPacketLen);
-  // TODO(srte): Move |packet| into lambda when we have c++14.
+  network_thread_->PostTask(
-  network_thread_->PostTask(RTC_FROM_HERE, [this, packet]() mutable {
+      RTC_FROM_HERE, [this, packet = std::move(packet)]() mutable {
-    RTC_DCHECK_RUN_ON(network_thread_);
+        RTC_DCHECK_RUN_ON(network_thread_);
-    if (rtp_transport_ == nullptr)
+        if (rtp_transport_ != nullptr)
-      return;
+          rtp_transport_->SendRtpPacket(&packet, rtc::PacketOptions(),
-    rtp_transport_->SendRtpPacket(&packet, rtc::PacketOptions(),
+                                        cricket::PF_SRTP_BYPASS);
-                                  cricket::PF_SRTP_BYPASS);
+      });
-  });
 }
 
 void ScenarioIceConnectionImpl::SendRtcpPacket(
     rtc::ArrayView<const uint8_t> packet_view) {
   rtc::CopyOnWriteBuffer packet(packet_view.data(), packet_view.size(),
                                 ::cricket::kMaxRtpPacketLen);
-  // TODO(srte): Move |packet| into lambda when we have c++14.
+  network_thread_->PostTask(
-  network_thread_->PostTask(RTC_FROM_HERE, [this, packet]() mutable {
+      RTC_FROM_HERE, [this, packet = std::move(packet)]() mutable {
-    RTC_DCHECK_RUN_ON(network_thread_);
+        RTC_DCHECK_RUN_ON(network_thread_);
-    if (rtp_transport_ == nullptr)
+        if (rtp_transport_ != nullptr)
-      return;
+          rtp_transport_->SendRtcpPacket(&packet, rtc::PacketOptions(),
-    rtp_transport_->SendRtcpPacket(&packet, rtc::PacketOptions(),
+                                         cricket::PF_SRTP_BYPASS);
-                                   cricket::PF_SRTP_BYPASS);
+      });
-  });
 }
 void ScenarioIceConnectionImpl::SetRemoteSdp(SdpType type,
                                              const std::string& remote_sdp) {

test/time_controller/simulated_time_controller_unittest.cc

@@ -28,18 +28,6 @@ using ::testing::MockFunction;
 using ::testing::NiceMock;
 using ::testing::Return;
 constexpr Timestamp kStartTime = Timestamp::Seconds<1000>();
-
-// Helper closure class to stop repeating task on a task queue. This is
-// equivalent to [handle{move(handle)}] { handle.Stop(); } in c++14.
-class TaskHandleStopper {
- public:
-  explicit TaskHandleStopper(RepeatingTaskHandle handle)
-      : handle_(std::move(handle)) {}
-  void operator()() { handle_.Stop(); }
-
- private:
-  RepeatingTaskHandle handle_;
-};
 }  // namespace
 
 TEST(SimulatedTimeControllerTest, TaskIsStoppedOnStop) {
@@ -61,7 +49,9 @@ TEST(SimulatedTimeControllerTest, TaskIsStoppedOnStop) {
   time_simulation.Sleep(kShortInterval * (kShortIntervalCount + kMargin));
   EXPECT_EQ(counter.load(), kShortIntervalCount);
 
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
+
   // Sleep long enough that the task would run at least once more if not
   // stopped.
   time_simulation.Sleep(kLongInterval * 2);
@@ -108,9 +98,12 @@ TEST(SimulatedTimeControllerTest, Example) {
   RepeatingTaskHandle handle;
   object->StartPeriodicTask(&handle, &task_queue);
   // Restart the task
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
   object->StartPeriodicTask(&handle, &task_queue);
-  task_queue.PostTask(TaskHandleStopper(std::move(handle)));
+  task_queue.PostTask(
+      [handle = std::move(handle)]() mutable { handle.Stop(); });
+
   struct Destructor {
     void operator()() { object.reset(); }
     std::unique_ptr<ObjectOnTaskQueue> object;

video/video_stream_encoder.cc

@@ -630,49 +630,35 @@ void VideoStreamEncoder::SetStartBitrate(int start_bitrate_bps) {
 
 void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config,
                                           size_t max_data_payload_length) {
-  // TODO(srte): This struct should be replaced by a lambda with move capture
-  // when C++14 lambda is allowed.
-  struct ConfigureEncoderTask {
-    void operator()() {
-      encoder->ConfigureEncoderOnTaskQueue(std::move(config),
-                                           max_data_payload_length);
-    }
-    VideoStreamEncoder* encoder;
-    VideoEncoderConfig config;
-    size_t max_data_payload_length;
-  };
   encoder_queue_.PostTask(
-      ConfigureEncoderTask{this, std::move(config), max_data_payload_length});
+      [this, config = std::move(config), max_data_payload_length]() mutable {
-}
+        RTC_DCHECK_RUN_ON(&encoder_queue_);
+        RTC_DCHECK(sink_);
+        RTC_LOG(LS_INFO) << "ConfigureEncoder requested.";
 
-void VideoStreamEncoder::ConfigureEncoderOnTaskQueue(
+        pending_encoder_creation_ =
-    VideoEncoderConfig config,
+            (!encoder_ || encoder_config_.video_format != config.video_format ||
-    size_t max_data_payload_length) {
+             max_data_payload_length_ != max_data_payload_length);
-  RTC_DCHECK_RUN_ON(&encoder_queue_);
+        encoder_config_ = std::move(config);
-  RTC_DCHECK(sink_);
+        max_data_payload_length_ = max_data_payload_length;
-  RTC_LOG(LS_INFO) << "ConfigureEncoder requested.";
+        pending_encoder_reconfiguration_ = true;
 
-  pending_encoder_creation_ =
+        // Reconfigure the encoder now if the encoder has an internal source or
-      (!encoder_ || encoder_config_.video_format != config.video_format ||
+        // if the frame resolution is known. Otherwise, the reconfiguration is
-       max_data_payload_length_ != max_data_payload_length);
+        // deferred until the next frame to minimize the number of
-  encoder_config_ = std::move(config);
+        // reconfigurations. The codec configuration depends on incoming video
-  max_data_payload_length_ = max_data_payload_length;
+        // frame size.
-  pending_encoder_reconfiguration_ = true;
+        if (last_frame_info_) {
-
+          ReconfigureEncoder();
-  // Reconfigure the encoder now if the encoder has an internal source or
+        } else {
-  // if the frame resolution is known. Otherwise, the reconfiguration is
+          codec_info_ = settings_.encoder_factory->QueryVideoEncoder(
-  // deferred until the next frame to minimize the number of reconfigurations.
+              encoder_config_.video_format);
-  // The codec configuration depends on incoming video frame size.
+          if (HasInternalSource()) {
-  if (last_frame_info_) {
+            last_frame_info_ = VideoFrameInfo(176, 144, false);
-    ReconfigureEncoder();
+            ReconfigureEncoder();
-  } else {
+          }
-    codec_info_ = settings_.encoder_factory->QueryVideoEncoder(
+        }
-        encoder_config_.video_format);
+      });
-    if (HasInternalSource()) {
-      last_frame_info_ = VideoFrameInfo(176, 144, false);
-      ReconfigureEncoder();
-    }
-  }
 }
 
 static absl::optional<VideoEncoder::ResolutionBitrateLimits>

video/video_stream_encoder.h

@@ -142,8 +142,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
     DataRate stable_encoder_target;
   };
 
-  void ConfigureEncoderOnTaskQueue(VideoEncoderConfig config,
-                                   size_t max_data_payload_length);
   void ReconfigureEncoder() RTC_RUN_ON(&encoder_queue_);
 
   void ConfigureQualityScaler(const VideoEncoder::EncoderInfo& encoder_info);