[Chromecast] Remove inactivity timer in CaptureServiceReceiver

So it won't lost connection due to timeout, but will still be closed if
the sender side crashes or send invalid message.

Meanwhile, fix a bug in the unittests that causes socket not destroyed
on the sequence by specifically calling Stop.

Bug: internal: 142752569
Bug: internal: 142443304
Test: On device and unittests.
Change-Id: I9a75d9727cf7abb3f0acc146b1028edfb3afb45a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865532Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Commit-Queue: Kehuang Li <kehuangli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706621}

chromecast/media/audio/capture_service/capture_service_receiver.cc

@@ -13,7 +13,6 @@
 #include "base/logging.h"
 #include "base/message_loop/message_pump_type.h"
 #include "base/threading/sequenced_task_runner_handle.h"
-#include "base/timer/timer.h"
 #include "chromecast/media/audio/audio_buildflags.h"
 #include "chromecast/media/audio/capture_service/constants.h"
 #include "chromecast/media/audio/capture_service/message_parsing_util.h"
@@ -66,7 +65,6 @@ class CaptureServiceReceiver::Socket : public SmallMessageSocket {
   const int channels_;
 
   ::media::AudioInputStream::AudioInputCallback* input_callback_;
-  base::OneShotTimer inactivity_timer_;
 
   DISALLOW_COPY_AND_ASSIGN(Socket);
 };
@@ -86,25 +84,16 @@ CaptureServiceReceiver::Socket::~Socket() = default;
 void CaptureServiceReceiver::Socket::Start(
     ::media::AudioInputStream::AudioInputCallback* input_callback) {
   input_callback_ = input_callback;
-  inactivity_timer_.Start(FROM_HERE, CaptureServiceReceiver::kInactivityTimeout,
-                          this,
-                          &CaptureServiceReceiver::Socket::OnInactivityTimeout);
   ReceiveMessages();
 }
 
 void CaptureServiceReceiver::Socket::ReportErrorAndStop() {
-  inactivity_timer_.Stop();
   if (input_callback_) {
     input_callback_->OnError();
   }
   input_callback_ = nullptr;
 }
 
-void CaptureServiceReceiver::Socket::OnInactivityTimeout() {
-  LOG(ERROR) << "Timed out " << this << " due to inactivity";
-  ReportErrorAndStop();
-}
-
 void CaptureServiceReceiver::Socket::OnError(int error) {
   LOG(INFO) << "Socket error from " << this << ": " << error;
   ReportErrorAndStop();
@@ -125,11 +114,6 @@ bool CaptureServiceReceiver::Socket::OnMessage(char* data, int size) {
     ReportErrorAndStop();
     return false;
   }
-
-  if (input_callback_) {
-    inactivity_timer_.Reset();
-  }
-
   return HandleAudio(std::move(audio.value()), timestamp);
 }
 
@@ -152,7 +136,6 @@ bool CaptureServiceReceiver::Socket::HandleAudio(
 
 // static
 constexpr base::TimeDelta CaptureServiceReceiver::kConnectTimeout;
-constexpr base::TimeDelta CaptureServiceReceiver::kInactivityTimeout;
 
 CaptureServiceReceiver::CaptureServiceReceiver(
     const ::media::AudioParameters& audio_params)

chromecast/media/audio/capture_service/capture_service_receiver.h

@@ -31,10 +31,6 @@ class CaptureServiceReceiver {
   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();
 

chromecast/media/audio/capture_service/capture_service_receiver_unittest.cc

@@ -108,9 +108,13 @@ TEST_F(CaptureServiceReceiverTest, ReceiveValidMessage) {
 
   receiver_.StartWithSocket(&audio_, std::move(socket));
   task_environment_.RunUntilIdle();
+  // Stop receiver to disconnect socket, since receiver doesn't own the IO
+  // task runner in unittests.
+  receiver_.Stop();
+  task_environment_.RunUntilIdle();
 }
 
-TEST_F(CaptureServiceReceiverTest, ReceiveInvalidMessage) {
+TEST_F(CaptureServiceReceiverTest, ReceiveEmptyMessage) {
   auto socket = std::make_unique<MockStreamSocket>();
   EXPECT_CALL(*socket, Connect(_)).WillOnce(Return(net::OK));
   EXPECT_CALL(*socket, Read(_, _, _))
@@ -128,35 +132,48 @@ TEST_F(CaptureServiceReceiverTest, ReceiveInvalidMessage) {
   task_environment_.RunUntilIdle();
 }
 
-TEST_F(CaptureServiceReceiverTest, ReceiveError) {
+TEST_F(CaptureServiceReceiverTest, ReceiveInvalidMessage) {
   auto socket = std::make_unique<MockStreamSocket>();
   EXPECT_CALL(*socket, Connect(_)).WillOnce(Return(net::OK));
   EXPECT_CALL(*socket, Read(_, _, _))
-      .WillOnce(Return(net::ERR_CONNECTION_RESET));
+      .WillOnce(Invoke([](net::IOBuffer* buf, int,
+                          net::CompletionOnceCallback) {
+        std::vector<char> header(16, 0);
+        base::BigEndianWriter data_writer(header.data(), header.size());
+        data_writer.WriteU16(334);  // 160 frames + header - data[0], in bytes.
+        data_writer.WriteU16(1);    // Mono channels.
+        data_writer.WriteU16(6);    // Invalid format.
+        data_writer.WriteU16(0);    // Padding zero.
+        data_writer.WriteU64(0);    // Timestamp.
+        std::copy(header.data(), header.data() + header.size(), buf->data());
+        // No need to fill audio frames.
+        return 336;
+      }));
   EXPECT_CALL(audio_, OnError());
 
   receiver_.StartWithSocket(&audio_, std::move(socket));
   task_environment_.RunUntilIdle();
 }
 
-TEST_F(CaptureServiceReceiverTest, ReceiveEosMessage) {
+TEST_F(CaptureServiceReceiverTest, ReceiveError) {
   auto socket = std::make_unique<MockStreamSocket>();
   EXPECT_CALL(*socket, Connect(_)).WillOnce(Return(net::OK));
-  EXPECT_CALL(*socket, Read(_, _, _)).WillOnce(Return(0));
+  EXPECT_CALL(*socket, Read(_, _, _))
+      .WillOnce(Return(net::ERR_CONNECTION_RESET));
   EXPECT_CALL(audio_, OnError());
 
   receiver_.StartWithSocket(&audio_, std::move(socket));
   task_environment_.RunUntilIdle();
 }
 
-TEST_F(CaptureServiceReceiverTest, ReceiveTimeout) {
+TEST_F(CaptureServiceReceiverTest, ReceiveEosMessage) {
   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(*socket, Read(_, _, _)).WillOnce(Return(0));
   EXPECT_CALL(audio_, OnError());
 
   receiver_.StartWithSocket(&audio_, std::move(socket));
-  task_environment_.FastForwardBy(CaptureServiceReceiver::kInactivityTimeout);
+  task_environment_.RunUntilIdle();
 }
 
 }  // namespace