Switch stream creation and closing in Chrome audio rendering from IPC to Mojo

content/browser/renderer_host/media/audio_renderer_host_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/audio_renderer_host.h"
 
 #include <stdint.h>
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/sync_socket.h"
+#include "content/browser/media/audio_output_impl.h"
 #include "content/browser/media/capture/audio_mirroring_manager.h"
 #include "content/browser/media/media_internals.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/common/media/audio_messages.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "ipc/ipc_message_utils.h"
 #include "media/audio/audio_manager.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/media_switches.h"
+#include "media/mojo/common/media_type_converters.h"
+#include "mojo/edk/embedder/embedder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::Assign;
 using ::testing::DoAll;
 using ::testing::NotNull;
 
 namespace {
 const int kRenderProcessId = 1;
@@ skipping 31 matching lines @@
                         MediaStreamManager* media_stream_manager,
                         const ResourceContext::SaltCallback& salt_callback)
       : AudioRendererHost(kRenderProcessId,
                           audio_manager,
                           mirroring_manager,
                           media_internals,
                           media_stream_manager,
                           salt_callback),
         shared_memory_length_(0) {}
 
+  AudioOutputImpl* Init(scoped_refptr<MockAudioRendererHost> host) {
+    media::interfaces::AudioOutputPtr* stream =
+        new media::interfaces::AudioOutputPtr();
+    audio_output_impl_ = (new AudioOutputImpl(0, mojo::GetProxy(stream)));
+    return audio_output_impl_;
+  }
+
   // A list of mock methods.
   MOCK_METHOD4(OnDeviceAuthorized,
                void(int stream_id,
                     media::OutputDeviceStatus device_status,
                     const media::AudioParameters& output_params,
                     const std::string& matched_device_id));
   MOCK_METHOD2(OnStreamCreated, void(int stream_id, int length));
   MOCK_METHOD1(OnStreamPlaying, void(int stream_id));
   MOCK_METHOD1(OnStreamPaused, void(int stream_id));
   MOCK_METHOD1(OnStreamError, void(int stream_id));
 
+  void OnNotifyStreamCreated(

[Henrik Grunell, 2016/05/02 12:12:23]

As we talked about offline, this function should either be moved to the test
class next to the CreateCallback function, or be merged with that function.
(Depending on how much code is still left after fixing the next comment.) If you
keep the function, it could be named something like "VerifyXyz...".

[rchtara, 2016/05/23 16:38:17]

Done.

+      int stream_id,
+      base::SharedMemoryHandle handle,
+      base::SyncSocket::TransitDescriptor socket_descriptor,
+      uint32_t length) {
+    // Maps the shared memory.
+    shared_memory_.reset(new base::SharedMemory(handle, false));

[Henrik Grunell, 2016/05/02 12:12:23]

As we talked about offline, storing in member variables in the original code
doesn't makes sense. They are not used anywhere. Remove the member variables
from the class and instead make sure to test whatever makes sense to test.

[rchtara, 2016/05/23 16:38:17]

Done.

+    CHECK(shared_memory_->Map(length));
+    CHECK(shared_memory_->memory());
+    shared_memory_length_ = length;
+
+    // Create the SyncSocket using the handle.
+    base::SyncSocket::Handle sync_socket_handle =
+        base::SyncSocket::UnwrapHandle(socket_descriptor);
+    sync_socket_.reset(new base::SyncSocket(sync_socket_handle));
+
+    // And then delegate the call to the mock method.
+    OnStreamCreated(stream_id, length);
+  }
+
  private:
   virtual ~MockAudioRendererHost() {
     // Make sure all audio streams have been deleted.
     EXPECT_TRUE(audio_entries_.empty());
   }
 
   // This method is used to dispatch IPC messages to the renderer. We intercept
   // these messages here and dispatch to our mock methods to verify the
   // conversation between this object and the renderer.
   virtual bool Send(IPC::Message* message) {
     CHECK(message);
 
     // In this method we dispatch the messages to the according handlers as if
     // we are the renderer.
     bool handled = true;
     IPC_BEGIN_MESSAGE_MAP(MockAudioRendererHost, *message)
       IPC_MESSAGE_HANDLER(AudioMsg_NotifyDeviceAuthorized,
                           OnNotifyDeviceAuthorized)
-      IPC_MESSAGE_HANDLER(AudioMsg_NotifyStreamCreated,
-                          OnNotifyStreamCreated)
       IPC_MESSAGE_HANDLER(AudioMsg_NotifyStreamStateChanged,
                           OnNotifyStreamStateChanged)
       IPC_MESSAGE_UNHANDLED(handled = false)
     IPC_END_MESSAGE_MAP()
     EXPECT_TRUE(handled);
 
     delete message;
     return true;
   }
 
   void OnNotifyDeviceAuthorized(int stream_id,
                                 media::OutputDeviceStatus device_status,
                                 const media::AudioParameters& output_params,
                                 const std::string& matched_device_id) {
     OnDeviceAuthorized(stream_id, device_status, output_params,
                        matched_device_id);
   }
 
-  void OnNotifyStreamCreated(
-      int stream_id,
-      base::SharedMemoryHandle handle,
-      base::SyncSocket::TransitDescriptor socket_descriptor,
-      uint32_t length) {
-    // Maps the shared memory.
-    shared_memory_.reset(new base::SharedMemory(handle, false));
-    CHECK(shared_memory_->Map(length));
-    CHECK(shared_memory_->memory());
-    shared_memory_length_ = length;
-
-    // Create the SyncSocket using the handle.
-    base::SyncSocket::Handle sync_socket_handle =
-        base::SyncSocket::UnwrapHandle(socket_descriptor);
-    sync_socket_.reset(new base::SyncSocket(sync_socket_handle));
-
-    // And then delegate the call to the mock method.
-    OnStreamCreated(stream_id, length);
-  }
-
   void OnNotifyStreamStateChanged(int stream_id,
                                   media::AudioOutputIPCDelegateState state) {
     switch (state) {
       case media::AUDIO_OUTPUT_IPC_DELEGATE_STATE_PLAYING:
         OnStreamPlaying(stream_id);
         break;
       case media::AUDIO_OUTPUT_IPC_DELEGATE_STATE_PAUSED:
         OnStreamPaused(stream_id);
         break;
       case media::AUDIO_OUTPUT_IPC_DELEGATE_STATE_ERROR:
@@ skipping 42 matching lines @@
     base::RunLoop run_loop;
     media_stream_manager_->audio_output_device_enumerator()->Enumerate(
         base::Bind(&WaitForEnumeration, &run_loop));
     run_loop.Run();
 
     host_ = new MockAudioRendererHost(audio_manager_.get(), &mirroring_manager_,
                                       MediaInternals::GetInstance(),
                                       media_stream_manager_.get(),
                                       GetMockSaltCallback());
 
+    audio_output_impl_.reset(host_->Init(host_.get()));
+
     // Simulate IPC channel connected.
     host_->set_peer_process_for_testing(base::Process::Current());
   }
 
   ~AudioRendererHostTest() override {
     // Simulate closing the IPC channel and give the audio thread time to close
     // the underlying streams.
     host_->OnChannelClosing();
     SyncWithAudioThread();
 
@@ skipping 35 matching lines @@
         media::AudioParameters::kAudioCDSampleRate / 10);
     int session_id = 0;
     if (unified_stream) {
       // Use AudioInputDeviceManager::kFakeOpenSessionId as the session id to
       // pass the permission check.
       session_id = AudioInputDeviceManager::kFakeOpenSessionId;
     }
     host_->OnRequestDeviceAuthorization(kStreamId, kRenderFrameId, session_id,
                                         device_id, security_origin);
     if (expected_device_status == media::OUTPUT_DEVICE_STATUS_OK) {
-      host_->OnCreateStream(kStreamId, kRenderFrameId, params);
+      host_->CreateStream(kStreamId, kRenderFrameId, params,
+                          base::Bind(&AudioRendererHostTest::CreateCallback,
+                                     base::Unretained(this)));
 
       // At some point in the future, a corresponding RemoveDiverter() call must
       // be made.
       EXPECT_CALL(mirroring_manager_, RemoveDiverter(NotNull()))
           .RetiresOnSaturation();
     }
     SyncWithAudioThread();
   }
 
+  void CreateCallback(int stream_id,
+                      media::interfaces::AudioOutputStreamPtr stream,
+                      mojo::ScopedSharedBufferHandle shared_buffer,
+                      mojo::ScopedHandle socket_descriptor) {
+    base::SharedMemoryHandle shared_memory_handle;
+    size_t length;
+    MojoResult pass_shared_memory_result = mojo::edk::PassSharedMemoryHandle(
+        shared_buffer.release().value(), &shared_memory_handle, &length,
+        nullptr);
+
+    if (pass_shared_memory_result != MOJO_RESULT_OK) {
+      LOG(ERROR) << "Failed to pass shared memory. Closing: "
+                 << pass_shared_memory_result;
+      return;
+    }
+    mojo::edk::ScopedPlatformHandle platform_handle;
+
+    MojoResult pass_platform_handle_result =
+        mojo::edk::PassWrappedPlatformHandle(
+            socket_descriptor.release().value(), &platform_handle);
+
+    if (pass_platform_handle_result != MOJO_RESULT_OK) {
+      LOG(ERROR) << "Failed to pass transit descriptor. Closing: "
+                 << pass_platform_handle_result;
+      return;
+    }
+
+    base::SyncSocket::TransitDescriptor descriptor;
+#if defined(OS_WIN)
+    descriptor = platform_handle.release().handle;
+#else
+    descriptor.fd = platform_handle.release().handle;
+#endif
+
+    host_->OnNotifyStreamCreated(stream_id, shared_memory_handle, descriptor,
+                                 length);
+    streams_[stream_id] = make_scoped_ptr(

[Henrik Grunell, 2016/05/02 12:12:23]

This isn't used anywhere. We talked about this offline but I don't remember why
this is needed. Could you explain that?

[rchtara, 2016/05/23 16:38:17]

it's not needed anymore.

+        new media::interfaces::AudioOutputStreamPtr(std::move(stream)));
+  }
+
   void Close() {
     // Send a message to AudioRendererHost to tell it we want to close the
     // stream.
     host_->OnCloseStream(kStreamId);
     SyncWithAudioThread();
   }
 
   void Play() {
     EXPECT_CALL(*host_.get(), OnStreamPlaying(kStreamId));
     host_->OnPlayStream(kStreamId);
@@ skipping 33 matching lines @@
   void SyncWithAudioThread() {
     base::RunLoop().RunUntilIdle();
 
     base::RunLoop run_loop;
     audio_manager_->GetTaskRunner()->PostTask(
         FROM_HERE, media::BindToCurrentLoop(run_loop.QuitClosure()));
     run_loop.Run();
   }
 
  private:
+  typedef std::map<int,
+                   std::unique_ptr<media::interfaces::AudioOutputStreamPtr>>
+      ScopedAudioOutputStreamPtrMap;
   // MediaStreamManager uses a DestructionObserver, so it must outlive the
   // TestBrowserThreadBundle.
   std::unique_ptr<MediaStreamManager> media_stream_manager_;
   TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<media::AudioManager> audio_manager_;
   MockAudioMirroringManager mirroring_manager_;
   scoped_refptr<MockAudioRendererHost> host_;
+  std::unique_ptr<AudioOutputImpl> audio_output_impl_;
+  ScopedAudioOutputStreamPtrMap streams_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioRendererHostTest);
 };
 
 TEST_F(AudioRendererHostTest, CreateAndClose) {
   Create();
   Close();
 }
 
 // Simulate the case where a stream is not properly closed.
@@ skipping 62 matching lines @@
 }
 
 TEST_F(AudioRendererHostTest, CreateInvalidDevice) {
   Create(false, kInvalidDeviceId, url::Origin(GURL(kSecurityOrigin)));
   Close();
 }
 
 // TODO(hclam): Add tests for data conversation in low latency mode.
 
 }  // namespace content