Return a shmem region from media::AudioSyncReader by value

AudioSyncReader::shared_memory_region() is never called more than once. If we
return a region from this function by value, we don't have to duplicate the
region.

This is a follow-up to https://crrev.com/c/1068928

Bug: 844508
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I22ac7f9bae926ed418c025ac0a8487b98659817a
Reviewed-on: https://chromium-review.googlesource.com/1069352
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561411}

content/browser/renderer_host/media/audio_output_delegate_impl.cc

@@ -205,7 +205,7 @@ void AudioOutputDelegateImpl::OnSetVolume(double volume) {
 
 void AudioOutputDelegateImpl::SendCreatedNotification() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  subscriber_->OnStreamCreated(stream_id_, reader_->shared_memory_region(),
+  subscriber_->OnStreamCreated(stream_id_, reader_->TakeSharedMemoryRegion(),
                                std::move(foreign_socket_));
 }
 

content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc

@@ -83,10 +83,10 @@ class MockEventHandler : public media::AudioOutputDelegate::EventHandler {
  public:
   void OnStreamCreated(
       int stream_id,
-      const base::UnsafeSharedMemoryRegion* shared_memory_region,
+      base::UnsafeSharedMemoryRegion shared_memory_region,
       std::unique_ptr<base::CancelableSyncSocket> socket) override {
     EXPECT_EQ(stream_id, kStreamId);
-    EXPECT_NE(shared_memory_region, nullptr);
+    EXPECT_TRUE(shared_memory_region.IsValid());
     EXPECT_NE(socket.get(), nullptr);
     GotOnStreamCreated();
   }

content/browser/renderer_host/media/old_render_frame_audio_output_stream_factory_unittest.cc

@@ -247,7 +247,7 @@ TEST(OldRenderFrameAudioOutputStreamFactoryTest, CreateStream) {
   auto remote = std::make_unique<base::CancelableSyncSocket>();
   ASSERT_TRUE(
       base::CancelableSyncSocket::CreatePair(local.get(), remote.get()));
-  event_handler->OnStreamCreated(kStreamId, &shared_memory_region,
+  event_handler->OnStreamCreated(kStreamId, std::move(shared_memory_region),
                                  std::move(remote));
 
   base::RunLoop().RunUntilIdle();

media/audio/audio_output_delegate.h

@@ -30,7 +30,7 @@ class MEDIA_EXPORT AudioOutputDelegate {
     // Called when the underlying stream is ready for playout.
     virtual void OnStreamCreated(
         int stream_id,
-        const base::UnsafeSharedMemoryRegion* shared_memory_region,
+        base::UnsafeSharedMemoryRegion shared_memory_region,
         std::unique_ptr<base::CancelableSyncSocket> socket) = 0;
 
     // Called if stream encounters an error and has become unusable.

media/audio/audio_output_device_unittest.cc

@@ -413,7 +413,7 @@ TEST_F(AudioOutputDeviceTest, VerifyDataFlow) {
   audio_device->OnDeviceAuthorized(OUTPUT_DEVICE_STATUS_OK, params,
                                    kDefaultDeviceId);
   audio_device->OnStreamCreated(
-      ToSharedMemoryHandle(env.reader->shared_memory_region()->Duplicate()),
+      ToSharedMemoryHandle(env.reader->TakeSharedMemoryRegion()),
       env.renderer_socket.Release(), /*playing_automatically*/ false);
 
   task_env_.RunUntilIdle();
@@ -474,7 +474,7 @@ TEST_F(AudioOutputDeviceTest, CreateNondefaultDevice) {
   audio_device->OnDeviceAuthorized(OUTPUT_DEVICE_STATUS_OK, params,
                                    kNonDefaultDeviceId);
   audio_device->OnStreamCreated(
-      ToSharedMemoryHandle(env.reader->shared_memory_region()->Duplicate()),
+      ToSharedMemoryHandle(env.reader->TakeSharedMemoryRegion()),
       env.renderer_socket.Release(), /*playing_automatically*/ false);
 
   audio_device->Stop();
@@ -510,7 +510,7 @@ TEST_F(AudioOutputDeviceTest, CreateBitStreamStream) {
   audio_device->OnDeviceAuthorized(OUTPUT_DEVICE_STATUS_OK, params,
                                    kNonDefaultDeviceId);
   audio_device->OnStreamCreated(
-      ToSharedMemoryHandle(env.reader->shared_memory_region()->Duplicate()),
+      ToSharedMemoryHandle(env.reader->TakeSharedMemoryRegion()),
       env.renderer_socket.Release(), /*playing_automatically*/ false);
 
   task_env_.RunUntilIdle();

media/audio/audio_sync_reader.cc

@@ -144,6 +144,11 @@ std::unique_ptr<AudioSyncReader> AudioSyncReader::Create(
       std::move(shared_memory_mapping), std::move(socket));
 }
 
+base::UnsafeSharedMemoryRegion AudioSyncReader::TakeSharedMemoryRegion() {
+  DCHECK(shared_memory_region_.IsValid());
+  return std::move(shared_memory_region_);
+}
+
 // AudioOutputController::SyncReader implementations.
 void AudioSyncReader::RequestMoreData(base::TimeDelta delay,
                                       base::TimeTicks delay_timestamp,

media/audio/audio_sync_reader.h

@@ -52,9 +52,9 @@ class MEDIA_EXPORT AudioSyncReader : public AudioOutputController::SyncReader {
       const AudioParameters& params,
       base::CancelableSyncSocket* foreign_socket);
 
-  const base::UnsafeSharedMemoryRegion* shared_memory_region() const {
-    return &shared_memory_region_;
-  }
+  // Transfers shared memory region ownership to a caller. It shouldn't be
+  // called more than once.
+  base::UnsafeSharedMemoryRegion TakeSharedMemoryRegion();
 
   void set_max_wait_timeout_for_test(base::TimeDelta time) {
     maximum_wait_time_ = time;

media/audio/audio_sync_reader_unittest.cc

@@ -62,7 +62,7 @@ TEST_P(AudioSyncReaderBitstreamTest, BitstreamBufferOverflow_DoesNotWriteOOB) {
   std::unique_ptr<AudioSyncReader> reader = AudioSyncReader::Create(
       base::BindRepeating(&NoLog), params, socket.get());
   const base::WritableSharedMemoryMapping shmem =
-      reader->shared_memory_region()->Map();
+      reader->TakeSharedMemoryRegion().Map();
   AudioOutputBuffer* buffer =
       reinterpret_cast<AudioOutputBuffer*>(shmem.memory());
   reader->RequestMoreData(base::TimeDelta(), base::TimeTicks(), 0);

media/mojo/services/mojo_audio_output_stream.cc

@@ -63,22 +63,19 @@ void MojoAudioOutputStream::SetVolume(double volume) {
 
 void MojoAudioOutputStream::OnStreamCreated(
     int stream_id,
-    const base::UnsafeSharedMemoryRegion* shared_memory_region,
+    base::UnsafeSharedMemoryRegion shared_memory_region,
     std::unique_ptr<base::CancelableSyncSocket> foreign_socket) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(stream_created_callback_);
-  DCHECK(shared_memory_region);
   DCHECK(foreign_socket);
 
-  base::UnsafeSharedMemoryRegion foreign_memory_region =
-      shared_memory_region->Duplicate();
-  if (!foreign_memory_region.IsValid()) {
+  if (!shared_memory_region.IsValid()) {
     OnStreamError(/*not used*/ 0);
     return;
   }
 
   mojo::ScopedSharedBufferHandle buffer_handle =
-      mojo::WrapUnsafeSharedMemoryRegion(std::move(foreign_memory_region));
+      mojo::WrapUnsafeSharedMemoryRegion(std::move(shared_memory_region));
   mojo::ScopedHandle socket_handle =
       mojo::WrapPlatformFile(foreign_socket->Release());
 

media/mojo/services/mojo_audio_output_stream.h

@@ -51,7 +51,7 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStream
   // AudioOutputDelegate::EventHandler implementation.
   void OnStreamCreated(
       int stream_id,
-      const base::UnsafeSharedMemoryRegion* shared_memory_region,
+      base::UnsafeSharedMemoryRegion shared_memory_region,
       std::unique_ptr<base::CancelableSyncSocket> foreign_socket) override;
   void OnStreamError(int stream_id) override;
 

media/mojo/services/mojo_audio_output_stream_unittest.cc

@@ -206,7 +206,7 @@ TEST_F(MojoAudioOutputStreamTest, Play_Plays) {
   EXPECT_CALL(client_, GotNotification());
   EXPECT_CALL(*delegate_, OnPlayStream());
 
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr->Play();
   base::RunLoop().RunUntilIdle();
@@ -218,7 +218,7 @@ TEST_F(MojoAudioOutputStreamTest, Pause_Pauses) {
   EXPECT_CALL(client_, GotNotification());
   EXPECT_CALL(*delegate_, OnPauseStream());
 
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr->Pause();
   base::RunLoop().RunUntilIdle();
@@ -230,7 +230,7 @@ TEST_F(MojoAudioOutputStreamTest, SetVolume_SetsVolume) {
   EXPECT_CALL(client_, GotNotification());
   EXPECT_CALL(*delegate_, OnSetVolume(kNewVolume));
 
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr->SetVolume(kNewVolume);
   base::RunLoop().RunUntilIdle();
@@ -243,7 +243,7 @@ TEST_F(MojoAudioOutputStreamTest, DestructWithCallPending_Safe) {
 
   ASSERT_NE(nullptr, delegate_event_handler_);
   foreign_socket_->ExpectOwnershipTransfer();
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr->Play();
   impl_.reset();
@@ -258,7 +258,7 @@ TEST_F(MojoAudioOutputStreamTest, Created_NotifiesClient) {
 
   ASSERT_NE(nullptr, delegate_event_handler_);
   foreign_socket_->ExpectOwnershipTransfer();
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
 
   base::RunLoop().RunUntilIdle();
@@ -269,7 +269,7 @@ TEST_F(MojoAudioOutputStreamTest, SetVolumeTooLarge_Error) {
   EXPECT_CALL(deleter_, Finished(true));
   EXPECT_CALL(client_, GotNotification());
 
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr->SetVolume(15);
   base::RunLoop().RunUntilIdle();
@@ -281,7 +281,7 @@ TEST_F(MojoAudioOutputStreamTest, SetVolumeNegative_Error) {
   EXPECT_CALL(deleter_, Finished(true));
   EXPECT_CALL(client_, GotNotification());
 
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr->SetVolume(-0.5);
   base::RunLoop().RunUntilIdle();
@@ -307,7 +307,7 @@ TEST_F(MojoAudioOutputStreamTest, DelegateErrorAfterCreated_PropagatesError) {
 
   ASSERT_NE(nullptr, delegate_event_handler_);
   foreign_socket_->ExpectOwnershipTransfer();
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   delegate_event_handler_->OnStreamError(kStreamId);
 
@@ -321,7 +321,7 @@ TEST_F(MojoAudioOutputStreamTest, RemoteEndGone_CallsDeleter) {
   EXPECT_CALL(client_, GotNotification());
   EXPECT_CALL(deleter_, Finished(false));
 
-  delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
+  delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
                                            std::move(foreign_socket_));
   audio_output_ptr.reset();
   base::RunLoop().RunUntilIdle();