Make MojoAudioInputIPC a FactoryClient.

After https://chromium-review.googlesource.com/c/chromium/src/+/771196,
the interface MojoAudioInputIPC should conform to is slightly different
than originally expected. This CL updates it to fit the new interface.

Bug: 653871
Change-Id: Iea76d3033087742a93943fad94a8875c12459236
Reviewed-on: https://chromium-review.googlesource.com/806222
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524056}

content/renderer/media/mojo_audio_input_ipc.cc

@@ -14,8 +14,12 @@ namespace content {
 
 MojoAudioInputIPC::MojoAudioInputIPC(StreamCreatorCB stream_creator)
     : stream_creator_(std::move(stream_creator)),
-      client_binding_(this),
-      weak_factory_(this) {}
+      stream_client_binding_(this),
+      factory_client_binding_(this),
+      weak_factory_(this) {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+  DCHECK(stream_creator_);
+}
 
 MojoAudioInputIPC::~MojoAudioInputIPC() = default;
 
@@ -24,55 +28,57 @@ void MojoAudioInputIPC::CreateStream(media::AudioInputIPCDelegate* delegate,
                                      const media::AudioParameters& params,
                                      bool automatic_gain_control,
                                      uint32_t total_segments) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(delegate);
   DCHECK(!delegate_);
 
   delegate_ = delegate;
 
-  media::mojom::AudioInputStreamClientPtr client;
-  client_binding_.Bind(mojo::MakeRequest(&client));
-  client_binding_.set_connection_error_handler(base::BindOnce(
+  mojom::RendererAudioInputStreamFactoryClientPtr client;
+  factory_client_binding_.Bind(mojo::MakeRequest(&client));
+  factory_client_binding_.set_connection_error_handler(base::BindOnce(
       &media::AudioInputIPCDelegate::OnError, base::Unretained(delegate_)));
 
-  stream_creator_.Run(mojo::MakeRequest(&stream_), session_id, params,
-                      automatic_gain_control, total_segments, std::move(client),
-                      base::BindOnce(&MojoAudioInputIPC::StreamCreated,
-                                     weak_factory_.GetWeakPtr()));
-
-  // Unretained is safe since |delegate_| is required to remain valid until
-  // CloseStream is called, which closes the binding.
-  stream_.set_connection_error_handler(base::BindOnce(
-      &media::AudioInputIPCDelegate::OnError, base::Unretained(delegate_)));
+  stream_creator_.Run(std::move(client), session_id, params,
+                      automatic_gain_control, total_segments);
 }
 
 void MojoAudioInputIPC::RecordStream() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (stream_.is_bound())
-    stream_->Record();
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(stream_.is_bound());
+  stream_->Record();
 }
 
 void MojoAudioInputIPC::SetVolume(double volume) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (stream_.is_bound())
-    stream_->SetVolume(volume);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(stream_.is_bound());
+  stream_->SetVolume(volume);
 }
 
 void MojoAudioInputIPC::CloseStream() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  client_binding_.Close();
-  stream_.reset();
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   delegate_ = nullptr;
+  if (factory_client_binding_.is_bound())
+    factory_client_binding_.Unbind();
+  if (stream_client_binding_.is_bound())
+    stream_client_binding_.Unbind();
+  stream_.reset();
 }
 
 void MojoAudioInputIPC::StreamCreated(
+    media::mojom::AudioInputStreamPtr stream,
+    media::mojom::AudioInputStreamClientRequest stream_client_request,
     mojo::ScopedSharedBufferHandle shared_memory,
     mojo::ScopedHandle socket,
     bool initially_muted) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(delegate_);
   DCHECK(socket.is_valid());
   DCHECK(shared_memory.is_valid());
+  DCHECK(!stream_);
+  DCHECK(!stream_client_binding_.is_bound());
+  stream_ = std::move(stream);
+  stream_client_binding_.Bind(std::move(stream_client_request));
 
   base::PlatformFile socket_handle;
   auto result = mojo::UnwrapPlatformFile(std::move(socket), &socket_handle);
@@ -89,13 +95,13 @@ void MojoAudioInputIPC::StreamCreated(
 }
 
 void MojoAudioInputIPC::OnError() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(delegate_);
   delegate_->OnError();
 }
 
 void MojoAudioInputIPC::OnMutedStateChanged(bool is_muted) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(delegate_);
   delegate_->OnMuted(is_muted);
 }

content/renderer/media/mojo_audio_input_ipc.h

@@ -10,8 +10,9 @@
 #include "base/callback_helpers.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/thread_checker.h"
+#include "base/sequence_checker.h"
 #include "content/common/content_export.h"
+#include "content/common/media/renderer_audio_input_stream_factory.mojom.h"
 #include "media/audio/audio_input_ipc.h"
 #include "media/mojo/interfaces/audio_input_stream.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -23,24 +24,19 @@ namespace content {
 // thread.
 class CONTENT_EXPORT MojoAudioInputIPC
     : public media::AudioInputIPC,
+      public mojom::RendererAudioInputStreamFactoryClient,
       public media::mojom::AudioInputStreamClient {
  public:
-  using StreamCreatedCB =
-      base::OnceCallback<void(mojo::ScopedSharedBufferHandle shared_memory,
-                              mojo::ScopedHandle socket,
-                              bool initially_muted)>;
-
+  // This callback is used by MojoAudioInputIPC to create streams.
+  // It is expected that after calling, either client->StreamCreated() is
+  // called or |client| is destructed.
   using StreamCreatorCB = base::RepeatingCallback<void(
-      media::mojom::AudioInputStreamRequest,
-      int64_t session_id,
-      media::AudioParameters params,
+      mojom::RendererAudioInputStreamFactoryClientPtr client,
+      int32_t session_id,
+      const media::AudioParameters& params,
       bool automatic_gain_control,
-      uint32_t total_segments,
-      media::mojom::AudioInputStreamClientPtr client,
-      StreamCreatedCB on_stream_created)>;
+      uint32_t total_segments)>;
 
-  // |stream_creator| is required to create a stream and call on_stream_created.
-  // It will get posted on the |main_task_runner| during CreateStream.
   explicit MojoAudioInputIPC(StreamCreatorCB stream_creator);
   ~MojoAudioInputIPC() override;
 
@@ -55,19 +51,22 @@ class CONTENT_EXPORT MojoAudioInputIPC
   void CloseStream() override;
 
  private:
-  void StreamCreated(mojo::ScopedSharedBufferHandle shared_memory,
-                     mojo::ScopedHandle socket,
-                     bool initially_muted);
-
+  void StreamCreated(
+      media::mojom::AudioInputStreamPtr stream,
+      media::mojom::AudioInputStreamClientRequest stream_client_request,
+      mojo::ScopedSharedBufferHandle shared_memory,
+      mojo::ScopedHandle socket,
+      bool initially_muted) override;
   void OnError() override;
   void OnMutedStateChanged(bool is_muted) override;
 
   StreamCreatorCB stream_creator_;
 
-  THREAD_CHECKER(thread_checker_);
+  SEQUENCE_CHECKER(sequence_checker_);
 
   media::mojom::AudioInputStreamPtr stream_;
-  mojo::Binding<AudioInputStreamClient> client_binding_;
+  mojo::Binding<AudioInputStreamClient> stream_client_binding_;
+  mojo::Binding<RendererAudioInputStreamFactoryClient> factory_client_binding_;
   media::AudioInputIPCDelegate* delegate_ = nullptr;
 
   base::WeakPtrFactory<MojoAudioInputIPC> weak_factory_;

content/renderer/media/mojo_audio_input_ipc_unittest.cc

@@ -71,27 +71,26 @@ class FakeStreamCreator {
  public:
   FakeStreamCreator(media::mojom::AudioInputStream* stream,
                     bool initially_muted)
-      : stream_(stream), initially_muted_(initially_muted) {}
+      : stream_(stream), binding_(stream_), initially_muted_(initially_muted) {}
 
-  void Create(media::mojom::AudioInputStreamRequest stream_request,
-              int64_t session_id,
-              media::AudioParameters params,
+  void Create(mojom::RendererAudioInputStreamFactoryClientPtr factory_client,
+              int32_t session_id,
+              const media::AudioParameters& params,
               bool automatic_gain_control,
-              uint32_t total_segments,
-              media::mojom::AudioInputStreamClientPtr client,
-              MojoAudioInputIPC::StreamCreatedCB on_stream_created) {
-    EXPECT_EQ(binding_, base::nullopt);
+              uint32_t total_segments) {
+    EXPECT_FALSE(binding_.is_bound());
     EXPECT_NE(stream_, nullptr);
-    std::swap(client_, client);
-    binding_.emplace(stream_, std::move(stream_request));
+    std::swap(factory_client_, factory_client);
+    media::mojom::AudioInputStreamPtr stream_ptr;
+    binding_.Bind(mojo::MakeRequest(&stream_ptr));
     base::CancelableSyncSocket foreign_socket;
     EXPECT_TRUE(
         base::CancelableSyncSocket::CreatePair(&socket_, &foreign_socket));
-    std::move(on_stream_created)
-        .Run(mojo::SharedBufferHandle::Create(kMemoryLength)
-                 ->Clone(mojo::SharedBufferHandle::AccessMode::READ_ONLY),
-             mojo::WrapPlatformFile(foreign_socket.Release()),
-             initially_muted_);
+    factory_client_->StreamCreated(
+        std::move(stream_ptr), mojo::MakeRequest(&stream_client_),
+        mojo::SharedBufferHandle::Create(kMemoryLength)
+            ->Clone(mojo::SharedBufferHandle::AccessMode::READ_ONLY),
+        mojo::WrapPlatformFile(foreign_socket.Release()), initially_muted_);
   }
 
   MojoAudioInputIPC::StreamCreatorCB GetCallback() {
@@ -100,19 +99,21 @@ class FakeStreamCreator {
   }
 
   void Rearm() {
-    binding_.reset();
+    if (binding_.is_bound())
+      binding_.Unbind();
     socket_.Close();
   }
 
   void SignalError() {
-    ASSERT_TRUE(client_);
-    client_->OnError();
+    ASSERT_TRUE(stream_client_);
+    stream_client_->OnError();
   }
 
  private:
   media::mojom::AudioInputStream* stream_;
-  media::mojom::AudioInputStreamClientPtr client_;
-  base::Optional<mojo::Binding<media::mojom::AudioInputStream>> binding_;
+  media::mojom::AudioInputStreamClientPtr stream_client_;
+  mojom::RendererAudioInputStreamFactoryClientPtr factory_client_;
+  mojo::Binding<media::mojom::AudioInputStream> binding_;
   bool initially_muted_;
   base::CancelableSyncSocket socket_;
 };
@@ -137,6 +138,25 @@ TEST(MojoAudioInputIPC, OnStreamCreated_Propagates) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST(MojoAudioInputIPC, FactoryDisconnected_SendsError) {
+  base::MessageLoopForIO message_loop;
+  StrictMock<MockDelegate> delegate;
+
+  const std::unique_ptr<media::AudioInputIPC> ipc =
+      std::make_unique<MojoAudioInputIPC>(base::BindRepeating(
+          [](mojom::RendererAudioInputStreamFactoryClientPtr factory_client,
+             int32_t session_id, const media::AudioParameters& params,
+             bool automatic_gain_control, uint32_t total_segments) {}));
+
+  EXPECT_CALL(delegate, OnError());
+
+  ipc->CreateStream(&delegate, kSessionId, Params(), false, kTotalSegments);
+  base::RunLoop().RunUntilIdle();
+
+  ipc->CloseStream();
+  base::RunLoop().RunUntilIdle();
+}
+
 TEST(MojoAudioInputIPC, OnStreamCreated_PropagatesInitiallyMuted) {
   base::MessageLoopForIO message_loop;
   StrictMock<MockStream> stream;
@@ -219,6 +239,7 @@ TEST(MojoAudioInputIPC, Record_Records) {
   EXPECT_CALL(stream, Record());
 
   ipc->CreateStream(&delegate, kSessionId, Params(), false, kTotalSegments);
+  base::RunLoop().RunUntilIdle();
   ipc->RecordStream();
   base::RunLoop().RunUntilIdle();
 
@@ -239,6 +260,7 @@ TEST(MojoAudioInputIPC, SetVolume_SetsVolume) {
   EXPECT_CALL(stream, SetVolume(kNewVolume));
 
   ipc->CreateStream(&delegate, kSessionId, Params(), false, kTotalSegments);
+  base::RunLoop().RunUntilIdle();
   ipc->SetVolume(kNewVolume);
   base::RunLoop().RunUntilIdle();
 

content/renderer/media/mojo_audio_output_ipc.cc

@@ -96,14 +96,14 @@ void MojoAudioOutputIPC::CreateStream(media::AudioOutputIPCDelegate* delegate,
 
 void MojoAudioOutputIPC::PlayStream() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (stream_.is_bound())
-    stream_->Play();
+  DCHECK(stream_.is_bound());
+  stream_->Play();
 }
 
 void MojoAudioOutputIPC::PauseStream() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (stream_.is_bound())
-    stream_->Pause();
+  DCHECK(stream_.is_bound());
+  stream_->Pause();
 }
 
 void MojoAudioOutputIPC::CloseStream() {
@@ -119,8 +119,8 @@ void MojoAudioOutputIPC::CloseStream() {
 
 void MojoAudioOutputIPC::SetVolume(double volume) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (stream_.is_bound())
-    stream_->SetVolume(volume);
+  DCHECK(stream_.is_bound());
+  stream_->SetVolume(volume);
 }
 
 void MojoAudioOutputIPC::OnError() {