[Chromecast] Update capture service receiver tests

As crbug/946657 has been fixed, fulfill the TODO in the unittest.

Bug: internal: 133880006
Test: Run unit tests.
Change-Id: I7cbc5ac0ea18b19cf584c8842b1d3405fb663e63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724930Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Kehuang Li <kehuangli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682354}

chromecast/media/audio/capture_service/capture_service_receiver.cc

@@ -9,12 +9,10 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/callback.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_pump_type.h"
 #include "base/threading/sequenced_task_runner_handle.h"
-#include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chromecast/media/audio/capture_service/capture_service_buildflags.h"
 #include "chromecast/media/audio/capture_service/constants.h"
@@ -47,13 +45,6 @@
 namespace chromecast {
 namespace media {
 
-namespace {
-
-constexpr base::TimeDelta kConnectTimeout = base::TimeDelta::FromSeconds(1);
-constexpr base::TimeDelta kInactivityTimeout = base::TimeDelta::FromSeconds(5);
-
-}  // namespace
-
 class CaptureServiceReceiver::Socket : public SmallMessageSocket {
  public:
   Socket(std::unique_ptr<net::StreamSocket> socket, int channels);
@@ -95,7 +86,8 @@ CaptureServiceReceiver::Socket::~Socket() = default;
 void CaptureServiceReceiver::Socket::Start(
     ::media::AudioInputStream::AudioInputCallback* input_callback) {
   input_callback_ = input_callback;
-  inactivity_timer_.Start(FROM_HERE, kInactivityTimeout, this,
+  inactivity_timer_.Start(FROM_HERE, CaptureServiceReceiver::kInactivityTimeout,
+                          this,
                           &CaptureServiceReceiver::Socket::OnInactivityTimeout);
   ReceiveMessages();
 }
@@ -158,6 +150,10 @@ bool CaptureServiceReceiver::Socket::HandleAudio(
   return true;
 }
 
+// static
+constexpr base::TimeDelta CaptureServiceReceiver::kConnectTimeout;
+constexpr base::TimeDelta CaptureServiceReceiver::kInactivityTimeout;
+
 CaptureServiceReceiver::CaptureServiceReceiver(
     const ::media::AudioParameters& audio_params)
     : audio_params_(audio_params), io_thread_(__func__) {
@@ -241,10 +237,6 @@ void CaptureServiceReceiver::OnConnected(
     input_callback->OnError();
     connecting_socket_.reset();
   }
-
-  if (!connected_cb_.is_null()) {
-    std::move(connected_cb_).Run();
-  }
 }
 
 void CaptureServiceReceiver::OnConnectTimeout(
@@ -264,11 +256,6 @@ void CaptureServiceReceiver::Stop() {
   socket_.reset();
 }
 
-void CaptureServiceReceiver::SetConnectClosureForTest(
-    base::OnceClosure connected_cb) {
-  connected_cb_ = std::move(connected_cb);
-}
-
 void CaptureServiceReceiver::SetTaskRunnerForTest(
     scoped_refptr<base::SequencedTaskRunner> task_runner) {
   task_runner_ = std::move(task_runner);

chromecast/media/audio/capture_service/capture_service_receiver.h

@@ -7,10 +7,10 @@
 
 #include <memory>
 
-#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/threading/thread.h"
+#include "base/time/time.h"
 #include "media/audio/audio_io.h"
 #include "media/base/audio_parameters.h"
 
@@ -27,6 +27,14 @@ namespace media {
 
 class CaptureServiceReceiver {
  public:
+  // The timeout for a connecting socket to stop waiting and report error.
+  static constexpr base::TimeDelta kConnectTimeout =
+      base::TimeDelta::FromSeconds(1);
+
+  // The timeout for a connected socket to disconnect due to inactivity.
+  static constexpr base::TimeDelta kInactivityTimeout =
+      base::TimeDelta::FromSeconds(5);
+
   explicit CaptureServiceReceiver(const ::media::AudioParameters& audio_params);
   ~CaptureServiceReceiver();
 
@@ -39,18 +47,8 @@ class CaptureServiceReceiver {
       ::media::AudioInputStream::AudioInputCallback* input_callback,
       std::unique_ptr<net::StreamSocket> connecting_socket);
 
-  // Unit test can wait for the closure as a async way to run to idle. E.g.,
-  //
-  //   base::RunLoop run_loop;
-  //   SetConnectClosureForTest(run_loop.QuitClosure());
-  //   StartWithSocket(...);
-  //   run_loop.Run();
-  //
-  void SetConnectClosureForTest(base::OnceClosure connected_cb);
-
   // Unit test can set test task runner so as to run test in sync. Must be
-  // called after construction and before any other methods. Do not mix with the
-  // use of SetConnectClosureForTest().
+  // called after construction and before any other methods.
   void SetTaskRunnerForTest(
       scoped_refptr<base::SequencedTaskRunner> task_runner);
 
@@ -75,8 +73,6 @@ class CaptureServiceReceiver {
   std::unique_ptr<net::StreamSocket> connecting_socket_;
   std::unique_ptr<Socket> socket_;
 
-  base::OnceClosure connected_cb_;
-
   DISALLOW_COPY_AND_ASSIGN(CaptureServiceReceiver);
 };
 

chromecast/media/audio/capture_service/capture_service_receiver_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/big_endian.h"
 #include "base/run_loop.h"
+#include "base/task/post_task.h"
 #include "base/test/scoped_task_environment.h"
 #include "chromecast/media/audio/mock_audio_input_callback.h"
 #include "chromecast/net/mock_stream_socket.h"
@@ -36,11 +37,15 @@ class CaptureServiceReceiverTest : public ::testing::Test {
             ::media::AudioParameters::AUDIO_PCM_LINEAR,
             ::media::ChannelLayout::CHANNEL_LAYOUT_MONO,
             16000,
-            160)) {}
+            160)) {
+    receiver_.SetTaskRunnerForTest(base::CreateSequencedTaskRunner(
+        {base::ThreadPool(), base::TaskPriority::USER_BLOCKING}));
+  }
   ~CaptureServiceReceiverTest() override = default;
 
  protected:
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  base::test::ScopedTaskEnvironment scoped_task_environment_{
+      base::test::ScopedTaskEnvironment::TimeSource::MOCK_TIME};
   chromecast::MockAudioInputCallback audio_;
   CaptureServiceReceiver receiver_;
 };
@@ -53,15 +58,14 @@ TEST_F(CaptureServiceReceiverTest, StartStop) {
   EXPECT_CALL(*socket2, Connect(_)).WillOnce(Return(net::OK));
 
   // Sync.
-  base::RunLoop run_loop;
-  receiver_.SetConnectClosureForTest(run_loop.QuitClosure());
   receiver_.StartWithSocket(&audio_, std::move(socket1));
-  run_loop.Run();
+  scoped_task_environment_.RunUntilIdle();
   receiver_.Stop();
 
   // Async.
   receiver_.StartWithSocket(&audio_, std::move(socket2));
   receiver_.Stop();
+  scoped_task_environment_.RunUntilIdle();
 }
 
 TEST_F(CaptureServiceReceiverTest, ConnectFailed) {
@@ -69,15 +73,19 @@ TEST_F(CaptureServiceReceiverTest, ConnectFailed) {
   EXPECT_CALL(*socket, Connect(_)).WillOnce(Return(net::ERR_FAILED));
   EXPECT_CALL(audio_, OnError());
 
-  base::RunLoop run_loop;
-  receiver_.SetConnectClosureForTest(run_loop.QuitClosure());
   receiver_.StartWithSocket(&audio_, std::move(socket));
-  run_loop.Run();
+  scoped_task_environment_.RunUntilIdle();
 }
 
-// TODO(https://crbug.com/946657): Add unit tests for timeout once supporting of
-// MOCK_TIME for threads other than the main thread is available. Also, update
-// the use of task runner in the following tests.
+TEST_F(CaptureServiceReceiverTest, ConnectTimeout) {
+  auto socket = std::make_unique<MockStreamSocket>();
+  EXPECT_CALL(*socket, Connect(_)).WillOnce(Return(net::ERR_IO_PENDING));
+  EXPECT_CALL(audio_, OnError());
+
+  receiver_.StartWithSocket(&audio_, std::move(socket));
+  scoped_task_environment_.FastForwardBy(
+      CaptureServiceReceiver::kConnectTimeout);
+}
 
 TEST_F(CaptureServiceReceiverTest, ReceiveValidMessage) {
   auto socket = std::make_unique<MockStreamSocket>();
@@ -99,8 +107,6 @@ TEST_F(CaptureServiceReceiverTest, ReceiveValidMessage) {
       .WillOnce(Return(net::ERR_IO_PENDING));
   EXPECT_CALL(audio_, OnData(_, _, 1.0 /* volume */));
 
-  receiver_.SetTaskRunnerForTest(
-      scoped_task_environment_.GetMainThreadTaskRunner());
   receiver_.StartWithSocket(&audio_, std::move(socket));
   scoped_task_environment_.RunUntilIdle();
 }
@@ -119,8 +125,6 @@ TEST_F(CaptureServiceReceiverTest, ReceiveInvalidMessage) {
       }));
   EXPECT_CALL(audio_, OnError());
 
-  receiver_.SetTaskRunnerForTest(
-      scoped_task_environment_.GetMainThreadTaskRunner());
   receiver_.StartWithSocket(&audio_, std::move(socket));
   scoped_task_environment_.RunUntilIdle();
 }
@@ -132,8 +136,6 @@ TEST_F(CaptureServiceReceiverTest, ReceiveError) {
       .WillOnce(Return(net::ERR_CONNECTION_RESET));
   EXPECT_CALL(audio_, OnError());
 
-  receiver_.SetTaskRunnerForTest(
-      scoped_task_environment_.GetMainThreadTaskRunner());
   receiver_.StartWithSocket(&audio_, std::move(socket));
   scoped_task_environment_.RunUntilIdle();
 }
@@ -144,12 +146,21 @@ TEST_F(CaptureServiceReceiverTest, ReceiveEosMessage) {
   EXPECT_CALL(*socket, Read(_, _, _)).WillOnce(Return(0));
   EXPECT_CALL(audio_, OnError());
 
-  receiver_.SetTaskRunnerForTest(
-      scoped_task_environment_.GetMainThreadTaskRunner());
   receiver_.StartWithSocket(&audio_, std::move(socket));
   scoped_task_environment_.RunUntilIdle();
 }
 
+TEST_F(CaptureServiceReceiverTest, ReceiveTimeout) {
+  auto socket = std::make_unique<MockStreamSocket>();
+  EXPECT_CALL(*socket, Connect(_)).WillOnce(Return(net::OK));
+  EXPECT_CALL(*socket, Read(_, _, _)).WillOnce(Return(net::ERR_IO_PENDING));
+  EXPECT_CALL(audio_, OnError());
+
+  receiver_.StartWithSocket(&audio_, std::move(socket));
+  scoped_task_environment_.FastForwardBy(
+      CaptureServiceReceiver::kInactivityTimeout);
+}
+
 }  // namespace
 }  // namespace capture_service
 }  // namespace media