Clean-ups for callbacks in remoting.

Changes in response to post-commit comments on: * https://crrev.com/c/2050212 * https://crrev.com/c/2049003 * https://crrev.com/c/2050049 Bug: 1007822,1007821,1007823,1007825,1007826,1007828 Change-Id: I5124ef2169f62c14f00fd451ae8f2f40c5739d79 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063613 Commit-Queue: Joey Scarr <jsca@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: Joe Downing <joedow@chromium.org> Cr-Commit-Position: refs/heads/master@{#742872}
qweri0p · Feb 20, 2020 · 21dd6fd · 21dd6fd
1 parent cd6cf02
commit 21dd6fd
Show file tree

Hide file tree

Showing 16 changed files with 61 additions and 55 deletions.
diff --git a/remoting/client/dual_buffer_frame_consumer.cc b/remoting/client/dual_buffer_frame_consumer.cc
@@ -67,10 +67,12 @@ std::unique_ptr<webrtc::DesktopFrame> PaddedDesktopFrame::CopyOf(
 }  // namespace
 
 DualBufferFrameConsumer::DualBufferFrameConsumer(
-    const RenderCallback& callback,
+    RenderCallback callback,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     protocol::FrameConsumer::PixelFormat format)
-    : callback_(callback), task_runner_(task_runner), pixel_format_(format) {
+    : callback_(std::move(callback)),
+      task_runner_(task_runner),
+      pixel_format_(format) {
   weak_ptr_ = weak_factory_.GetWeakPtr();
   thread_checker_.DetachFromThread();
 }

diff --git a/remoting/client/dual_buffer_frame_consumer.h b/remoting/client/dual_buffer_frame_consumer.h
@@ -31,10 +31,10 @@ class DualBufferFrameConsumer : public protocol::FrameConsumer {
   // RenderCallback(decoded_frame, done)
   // |done| should be run after it is rendered. Can be called on any thread.
   using RenderCallback =
-      base::Callback<void(std::unique_ptr<webrtc::DesktopFrame>,
-                          base::OnceClosure)>;
+      base::RepeatingCallback<void(std::unique_ptr<webrtc::DesktopFrame>,
+                                   base::OnceClosure)>;
   DualBufferFrameConsumer(
-      const RenderCallback& callback,
+      RenderCallback callback,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       PixelFormat format);
   ~DualBufferFrameConsumer() override;

diff --git a/remoting/client/dual_buffer_frame_consumer_unittest.cc b/remoting/client/dual_buffer_frame_consumer_unittest.cc
@@ -88,7 +88,7 @@ TEST_F(DualBufferFrameConsumerTest, AllocateOneFrame) {
       consumer_->AllocateFrame(webrtc::DesktopSize(16, 16));
   ASSERT_TRUE(frame->size().equals(webrtc::DesktopSize(16, 16)));
   webrtc::DesktopFrame* raw_frame = frame.get();
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
   EXPECT_EQ(raw_frame, received_frame_.get());
 }
 
@@ -98,22 +98,22 @@ TEST_F(DualBufferFrameConsumerTest, BufferRotation) {
   std::unique_ptr<webrtc::DesktopFrame> frame =
       consumer_->AllocateFrame(size16x16);
   webrtc::DesktopFrame* underlying_frame_1 = GetUnderlyingFrame(frame);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   frame = consumer_->AllocateFrame(size16x16);
   webrtc::DesktopFrame* underlying_frame_2 = GetUnderlyingFrame(frame);
   EXPECT_NE(underlying_frame_1, underlying_frame_2);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   frame = consumer_->AllocateFrame(size16x16);
   webrtc::DesktopFrame* underlying_frame_3 = GetUnderlyingFrame(frame);
   EXPECT_EQ(underlying_frame_1, underlying_frame_3);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   frame = consumer_->AllocateFrame(size16x16);
   webrtc::DesktopFrame* underlying_frame_4 = GetUnderlyingFrame(frame);
   EXPECT_EQ(underlying_frame_2, underlying_frame_4);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 }
 
 TEST_F(DualBufferFrameConsumerTest, DrawAndMergeFrames) {
@@ -128,15 +128,15 @@ TEST_F(DualBufferFrameConsumerTest, DrawAndMergeFrames) {
       consumer_->AllocateFrame(size2x2);
   FillRGBARect(0xff, 0, 0, 0xff, webrtc::DesktopRect::MakeXYWH(0, 0, 2, 2),
                frame.get());
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   // Frame 2:
   // GG
   // XX
   frame = consumer_->AllocateFrame(size2x2);
   FillRGBARect(0, 0xff, 0, 0xff, webrtc::DesktopRect::MakeXYWH(0, 0, 2, 1),
                frame.get());
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   // Merged Frame:
   // GG
@@ -159,7 +159,7 @@ TEST_F(DualBufferFrameConsumerTest, DrawAndMergeFrames) {
   frame = consumer_->AllocateFrame(size2x2);
   FillRGBARect(0, 0, 0xff, 0xff, webrtc::DesktopRect::MakeXYWH(0, 0, 1, 2),
                frame.get());
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   // Merged Frame:
   // BG
@@ -183,24 +183,24 @@ TEST_F(DualBufferFrameConsumerTest, ChangeScreenSizeAndReallocateBuffers) {
   std::unique_ptr<webrtc::DesktopFrame> frame =
       consumer_->AllocateFrame(size16x16);
   webrtc::DesktopFrame* underlying_frame_1 = GetUnderlyingFrame(frame);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   frame = consumer_->AllocateFrame(size16x16);
   webrtc::DesktopFrame* underlying_frame_2 = GetUnderlyingFrame(frame);
   EXPECT_NE(underlying_frame_1, underlying_frame_2);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   webrtc::DesktopSize size32x32(32, 32);
 
   frame = consumer_->AllocateFrame(size32x32);
   webrtc::DesktopFrame* underlying_frame_3 = GetUnderlyingFrame(frame);
   EXPECT_NE(underlying_frame_1, underlying_frame_3);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 
   frame = consumer_->AllocateFrame(size32x32);
   webrtc::DesktopFrame* underlying_frame_4 = GetUnderlyingFrame(frame);
   EXPECT_NE(underlying_frame_2, underlying_frame_4);
-  consumer_->DrawFrame(std::move(frame), base::Closure());
+  consumer_->DrawFrame(std::move(frame), base::NullCallback());
 }
 
 }  // namespace remoting
diff --git a/remoting/client/jni/jni_gl_display_handler.cc b/remoting/client/jni/jni_gl_display_handler.cc
@@ -91,10 +91,11 @@ JniGlDisplayHandler::Core::Core(base::WeakPtr<JniGlDisplayHandler> shell)
                                 base::Unretained(this)));
 
   // Do not bind GlRenderer::OnFrameReceived. |renderer_| is not ready yet.
-  owned_frame_consumer_.reset(new DualBufferFrameConsumer(
-      base::Bind(&JniGlDisplayHandler::Core::OnFrameReceived, weak_ptr_),
+  owned_frame_consumer_ = std::make_unique<DualBufferFrameConsumer>(
+      base::BindRepeating(&JniGlDisplayHandler::Core::OnFrameReceived,
+                          weak_ptr_),
       runtime_->display_task_runner(),
-      protocol::FrameConsumer::PixelFormat::FORMAT_RGBA));
+      protocol::FrameConsumer::PixelFormat::FORMAT_RGBA);
   frame_consumer_ = owned_frame_consumer_->GetWeakPtr();
 }
 

diff --git a/remoting/client/software_video_renderer.cc b/remoting/client/software_video_renderer.cc
@@ -177,8 +177,8 @@ void SoftwareVideoRenderer::RenderFrame(
 
   consumer_->DrawFrame(
       std::move(frame),
-      base::Bind(&SoftwareVideoRenderer::OnFrameRendered,
-                 weak_factory_.GetWeakPtr(), base::Passed(&stats), done));
+      base::BindOnce(&SoftwareVideoRenderer::OnFrameRendered,
+                     weak_factory_.GetWeakPtr(), base::Passed(&stats), done));
 }
 
 void SoftwareVideoRenderer::OnFrameRendered(

diff --git a/remoting/codec/audio_encoder_opus.cc b/remoting/codec/audio_encoder_opus.cc
@@ -74,11 +74,11 @@ void AudioEncoderOpus::InitEncoder() {
         new char[kFrameSamples * kBytesPerSample * channels_]);
     // TODO(sergeyu): Figure out the right buffer size to use per packet instead
     // of using media::SincResampler::kDefaultRequestSize.
-    resampler_.reset(new media::MultiChannelResampler(
+    resampler_ = std::make_unique<media::MultiChannelResampler>(
         channels_, static_cast<double>(sampling_rate_) / kOpusSamplingRate,
         media::SincResampler::kDefaultRequestSize,
         base::BindRepeating(&AudioEncoderOpus::FetchBytesToResample,
-                            base::Unretained(this))));
+                            base::Unretained(this)));
     resampler_bus_ = media::AudioBus::Create(channels_, kFrameSamples);
   }
 

diff --git a/remoting/host/fake_desktop_environment.h b/remoting/host/fake_desktop_environment.h
@@ -86,7 +86,7 @@ class FakeDesktopEnvironment : public DesktopEnvironment {
   // by FakeDesktopEnvironment.
   void set_frame_generator(
       protocol::FakeDesktopCapturer::FrameGenerator frame_generator) {
-    frame_generator_ = frame_generator;
+    frame_generator_ = std::move(frame_generator);
   }
 
   const DesktopEnvironmentOptions& options() const;
@@ -138,7 +138,7 @@ class FakeDesktopEnvironmentFactory : public DesktopEnvironmentFactory {
   // by FakeDesktopEnvironment.
   void set_frame_generator(
       protocol::FakeDesktopCapturer::FrameGenerator frame_generator) {
-    frame_generator_ = frame_generator;
+    frame_generator_ = std::move(frame_generator);
   }
 
   // DesktopEnvironmentFactory implementation.

diff --git a/remoting/ios/display/gl_display_handler.mm b/remoting/ios/display/gl_display_handler.mm
@@ -97,10 +97,10 @@ void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
   weak_ptr_ = weak_factory_.GetWeakPtr();
 
   // Do not bind GlRenderer::OnFrameReceived. |renderer_| is not ready yet.
-  owned_frame_consumer_.reset(new remoting::DualBufferFrameConsumer(
+  owned_frame_consumer_ = std::make_unique<remoting::DualBufferFrameConsumer>(
       base::BindRepeating(&Core::OnFrameReceived, weak_ptr_),
       runtime_->display_task_runner(),
-      protocol::FrameConsumer::PixelFormat::FORMAT_RGBA));
+      protocol::FrameConsumer::PixelFormat::FORMAT_RGBA);
   frame_consumer_ = owned_frame_consumer_->GetWeakPtr();
 
   renderer_proxy_ =

diff --git a/remoting/protocol/fake_desktop_capturer.cc b/remoting/protocol/fake_desktop_capturer.cc
@@ -122,16 +122,16 @@ std::unique_ptr<webrtc::DesktopFrame> DefaultFrameGenerator::GenerateFrame(
 
 FakeDesktopCapturer::FakeDesktopCapturer()
     : callback_(nullptr) {
-  frame_generator_ = base::Bind(&DefaultFrameGenerator::GenerateFrame,
-                                base::MakeRefCounted<DefaultFrameGenerator>());
+  frame_generator_ =
+      base::BindRepeating(&DefaultFrameGenerator::GenerateFrame,
+                          base::MakeRefCounted<DefaultFrameGenerator>());
 }
 
 FakeDesktopCapturer::~FakeDesktopCapturer() = default;
 
-void FakeDesktopCapturer::set_frame_generator(
-    const FrameGenerator& frame_generator) {
+void FakeDesktopCapturer::set_frame_generator(FrameGenerator frame_generator) {
   DCHECK(!callback_);
-  frame_generator_ = frame_generator;
+  frame_generator_ = std::move(frame_generator);
 }
 
 void FakeDesktopCapturer::Start(Callback* callback) {

diff --git a/remoting/protocol/fake_desktop_capturer.h b/remoting/protocol/fake_desktop_capturer.h
@@ -27,14 +27,14 @@ class FakeDesktopCapturer : public webrtc::DesktopCapturer {
   static const int kWidth = 800;
   static const int kHeight = 600;
 
-  typedef base::Callback<std::unique_ptr<webrtc::DesktopFrame>(
-      webrtc::SharedMemoryFactory* shared_memory_factory)>
-      FrameGenerator;
+  using FrameGenerator =
+      base::RepeatingCallback<std::unique_ptr<webrtc::DesktopFrame>(
+          webrtc::SharedMemoryFactory* shared_memory_factory)>;
 
   FakeDesktopCapturer();
   ~FakeDesktopCapturer() override;
 
-  void set_frame_generator(const FrameGenerator& frame_generator);
+  void set_frame_generator(FrameGenerator frame_generator);
 
   // webrtc::DesktopCapturer interface.
   void Start(Callback* callback) override;

diff --git a/remoting/protocol/fake_video_renderer.cc b/remoting/protocol/fake_video_renderer.cc
@@ -51,9 +51,9 @@ void FakeFrameConsumer::DrawFrame(std::unique_ptr<webrtc::DesktopFrame> frame,
                                   base::OnceClosure done) {
   CHECK(thread_checker_.CalledOnValidThread());
   received_frames_.push_back(std::move(frame));
-  if (!done.is_null())
+  if (done)
     std::move(done).Run();
-  if (!on_frame_callback_.is_null())
+  if (on_frame_callback_)
     on_frame_callback_.Run();
 }
 

diff --git a/remoting/protocol/frame_consumer.h b/remoting/protocol/frame_consumer.h
@@ -32,7 +32,7 @@ class FrameConsumer {
       const webrtc::DesktopSize& size) = 0;
 
   virtual void DrawFrame(std::unique_ptr<webrtc::DesktopFrame> frame,
-                         const base::OnceClosure done) = 0;
+                         base::OnceClosure done) = 0;
 
   // Returns the preferred pixel encoding for the platform.
   virtual PixelFormat GetPixelFormat() = 0;

diff --git a/remoting/protocol/webrtc_video_renderer_adapter.cc b/remoting/protocol/webrtc_video_renderer_adapter.cc
@@ -194,8 +194,9 @@ void WebrtcVideoRendererAdapter::DrawFrame(
   stats->time_decoded = base::TimeTicks::Now();
   video_renderer_->GetFrameConsumer()->DrawFrame(
       std::move(frame),
-      base::Bind(&WebrtcVideoRendererAdapter::FrameRendered,
-                 weak_factory_.GetWeakPtr(), frame_id, base::Passed(&stats)));
+      base::BindOnce(&WebrtcVideoRendererAdapter::FrameRendered,
+                     weak_factory_.GetWeakPtr(), frame_id,
+                     base::Passed(&stats)));
 }
 
 void WebrtcVideoRendererAdapter::FrameRendered(

diff --git a/remoting/protocol/webrtc_video_stream.cc b/remoting/protocol/webrtc_video_stream.cc
@@ -87,15 +87,17 @@ WebrtcVideoStream::WebrtcVideoStream(const SessionOptions& session_options)
   // Unretained(this) is safe because |encoder_selector_| is owned by this
   // object.
   encoder_selector_.RegisterEncoder(
-      base::Bind(&WebrtcVideoEncoderVpx::IsSupportedByVP8),
-      base::Bind(&WebrtcVideoStream::CreateVP8Encoder, base::Unretained(this)));
+      base::BindRepeating(&WebrtcVideoEncoderVpx::IsSupportedByVP8),
+      base::BindRepeating(&WebrtcVideoStream::CreateVP8Encoder,
+                          base::Unretained(this)));
   encoder_selector_.RegisterEncoder(
-      base::Bind(&WebrtcVideoEncoderVpx::IsSupportedByVP9),
-      base::Bind(&WebrtcVideoStream::CreateVP9Encoder, base::Unretained(this)));
+      base::BindRepeating(&WebrtcVideoEncoderVpx::IsSupportedByVP9),
+      base::BindRepeating(&WebrtcVideoStream::CreateVP9Encoder,
+                          base::Unretained(this)));
 #if defined(USE_H264_ENCODER)
   encoder_selector_.RegisterEncoder(
-      base::Bind(&WebrtcVideoEncoderGpu::IsSupportedByH264),
-      base::Bind(&WebrtcVideoEncoderGpu::CreateForH264));
+      base::BindRepeating(&WebrtcVideoEncoderGpu::IsSupportedByH264),
+      base::BindRepeating(&WebrtcVideoEncoderGpu::CreateForH264));
 #endif
 }
 

diff --git a/remoting/test/fake_socket_factory.h b/remoting/test/fake_socket_factory.h
@@ -109,11 +109,11 @@ class FakePacketSocketFactory : public rtc::PacketSocketFactory,
     int data_size;
   };
 
-  typedef base::RepeatingCallback<void(const rtc::SocketAddress& from,
-                                       const rtc::SocketAddress& to,
-                                       const scoped_refptr<net::IOBuffer>& data,
-                                       int data_size)>
-      ReceiveCallback;
+  using ReceiveCallback =
+      base::RepeatingCallback<void(const rtc::SocketAddress& from,
+                                   const rtc::SocketAddress& to,
+                                   const scoped_refptr<net::IOBuffer>& data,
+                                   int data_size)>;
   typedef std::map<uint16_t, ReceiveCallback> UdpSocketsMap;
 
   void DoReceivePacket();

diff --git a/remoting/test/protocol_perftest.cc b/remoting/test/protocol_perftest.cc
@@ -168,9 +168,9 @@ class ProtocolPerfTest
   void DrawFrame(std::unique_ptr<webrtc::DesktopFrame> frame,
                  base::OnceClosure done) override {
     last_video_frame_ = std::move(frame);
-    if (!on_frame_task_.is_null())
+    if (on_frame_task_)
       on_frame_task_.Run();
-    if (!done.is_null())
+    if (done)
       std::move(done).Run();
   }