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

content/browser/renderer_host/media/audio_renderer_host_unittest.cc

Index: content/browser/renderer_host/media/audio_renderer_host_unittest.cc
diff --git a/content/browser/renderer_host/media/audio_renderer_host_unittest.cc b/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
index a0cdfdb4dafea0c650105473b642152f361f5a8e..46703d4b19b378b81580fc2c43818fe73ec3bebd 100644
--- a/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
+++ b/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
@@ -13,6 +13,7 @@
 #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"
@@ -24,6 +25,8 @@
 #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"
 
@@ -75,6 +78,13 @@ class MockAudioRendererHost : public AudioRendererHost {
                           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,
@@ -86,6 +96,26 @@ class MockAudioRendererHost : public AudioRendererHost {
   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.
@@ -104,8 +134,6 @@ class MockAudioRendererHost : public AudioRendererHost {
     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)
@@ -124,26 +152,6 @@ class MockAudioRendererHost : public AudioRendererHost {
                        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) {
@@ -206,6 +214,8 @@ class AudioRendererHostTest : public testing::Test {
                                       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());
   }
@@ -261,7 +271,9 @@ class AudioRendererHostTest : public testing::Test {
     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.
@@ -271,6 +283,46 @@ class AudioRendererHostTest : public testing::Test {
     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.
@@ -324,6 +376,9 @@ class AudioRendererHostTest : public testing::Test {
   }
 
  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_;
@@ -331,6 +386,8 @@ class AudioRendererHostTest : public testing::Test {
   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);
 };