Mojo interfaces needed for switching audio rendering stream creation and closing from IPC to Mojo

content/browser/renderer_host/media/audio_renderer_host.cc

Index: content/browser/renderer_host/media/audio_renderer_host.cc
diff --git a/content/browser/renderer_host/media/audio_renderer_host.cc b/content/browser/renderer_host/media/audio_renderer_host.cc
index ae580222c519763e30ebe1715654343a94c4bea8..49de17d8702ca19505b4723e6ba653c2250254cd 100644
--- a/content/browser/renderer_host/media/audio_renderer_host.cc
+++ b/content/browser/renderer_host/media/audio_renderer_host.cc
@@ -6,6 +6,9 @@
 
 #include <stdint.h>
 #include <utility>
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -17,6 +20,7 @@
 #include "content/browser/bad_message.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/media/audio_output_impl.h"
 #include "content/browser/media/audio_stream_monitor.h"
 #include "content/browser/media/capture/audio_mirroring_manager.h"
 #include "content/browser/media/media_internals.h"
@@ -36,6 +40,8 @@
 #include "media/audio/audio_streams_tracker.h"
 #include "media/base/audio_bus.h"
 #include "media/base/limits.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "mojo/public/cpp/system/handle.h"
 
 using media::AudioBus;
 using media::AudioManager;
@@ -61,6 +67,25 @@ GURL ConvertToGURL(const url::Origin& origin) {
   return origin.unique() ? GURL() : GURL(origin.Serialize());
 }
 
+base::SyncSocket::TransitDescriptor DuplicateSocket(
+    base::SyncSocket::TransitDescriptor socket_descriptor) {
+  base::SyncSocket::TransitDescriptor socket_descriptor_dup;

[tommi (sloooow) - chröme, 2016/04/26 15:29:53]

Initialize please. As is, you might be returning a  bogus handle on windows.

[rchtara, 2016/04/29 12:54:46]

Done.

+
+#if defined(OS_WIN)
+  ::DuplicateHandle(GetCurrentProcess(),  // hSourceProcessHandle

[tommi (sloooow) - chröme, 2016/04/26 15:29:53]

check the return value?

[rchtara, 2016/04/29 12:54:46]

Done.

+                    socket_descriptor,
+                    GetCurrentProcess(),  // hTargetProcessHandle
+                    &socket_descriptor_dup,
+                    0,  // dwDesiredAccess ignored due to SAME_ACCESS
+                    FALSE,  // !bInheritHandle
+                    DUPLICATE_SAME_ACCESS);
+
+#else
+  socket_descriptor_dup.fd = dup(socket_descriptor.fd);
+#endif
+  return socket_descriptor_dup;
+}
+
 bool IsValidDeviceId(const std::string& device_id) {
   static const std::string::size_type kValidLength = 64;
 
@@ -126,7 +151,8 @@ class AudioRendererHost::AudioEntry
              const media::AudioParameters& params,
              const std::string& output_device_id,
              std::unique_ptr<base::SharedMemory> shared_memory,
-             std::unique_ptr<media::AudioOutputController::SyncReader> reader);
+             std::unique_ptr<media::AudioOutputController::SyncReader> reader,
+             const mojom::AudioOutput::CreateStreamCallback& callback);
   ~AudioEntry() override;
 
   int stream_id() const {
@@ -150,7 +176,8 @@ class AudioRendererHost::AudioEntry
 
  private:
   // media::AudioOutputController::EventHandler implementation.
-  void OnCreated() override;
+  void OnCreated(
+      const mojom::AudioOutput::CreateStreamCallback& callback) override;
   void OnPlaying() override;
   void OnPaused() override;
   void OnError() override;
@@ -180,7 +207,8 @@ AudioRendererHost::AudioEntry::AudioEntry(
     const media::AudioParameters& params,
     const std::string& output_device_id,
     std::unique_ptr<base::SharedMemory> shared_memory,
-    std::unique_ptr<media::AudioOutputController::SyncReader> reader)
+    std::unique_ptr<media::AudioOutputController::SyncReader> reader,
+    const mojom::AudioOutput::CreateStreamCallback& callback)
     : host_(host),
       stream_id_(stream_id),
       render_frame_id_(render_frame_id),
@@ -190,7 +218,8 @@ AudioRendererHost::AudioEntry::AudioEntry(
                                                        this,
                                                        params,
                                                        output_device_id,
-                                                       reader_.get())),
+                                                       reader_.get(),
+                                                       callback)),
       playing_(false) {
   DCHECK(controller_.get());
 }
@@ -262,11 +291,11 @@ void AudioRendererHost::OnDestruct() const {
   BrowserThread::DeleteOnIOThread::Destruct(this);
 }
 
-void AudioRendererHost::AudioEntry::OnCreated() {
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&AudioRendererHost::DoCompleteCreation, host_, stream_id_));
+void AudioRendererHost::AudioEntry::OnCreated(
+    const mojom::AudioOutput::CreateStreamCallback& callback) {
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioRendererHost::DoCompleteCreation,
+                                     host_, stream_id_, callback));
 }
 
 void AudioRendererHost::AudioEntry::OnPlaying() {
@@ -296,29 +325,39 @@ void AudioRendererHost::AudioEntry::OnError() {
       base::Bind(&AudioRendererHost::ReportErrorAndClose, host_, stream_id_));
 }
 
-void AudioRendererHost::DoCompleteCreation(int stream_id) {
+void AudioRendererHost::DoCompleteCreation(
+    int stream_id,
+    const mojom::AudioOutput::CreateStreamCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (!PeerHandle()) {
     DLOG(WARNING) << "Renderer process handle is invalid.";
-    ReportErrorAndClose(stream_id);
+    ReportErrorAndCloseStream(stream_id, callback);
     return;
   }
 
   AudioEntry* const entry = LookupById(stream_id);
   if (!entry) {
-    ReportErrorAndClose(stream_id);
+    ReportErrorAndCloseStream(stream_id, callback);
     return;
   }
 
-  // Once the audio stream is created then complete the creation process by
-  // mapping shared memory and sharing with the renderer process.
-  base::SharedMemoryHandle foreign_memory_handle;
-  if (!entry->shared_memory()->ShareToProcess(PeerHandle(),
-                                              &foreign_memory_handle)) {
-    // If we failed to map and share the shared memory then close the audio
-    // stream and send an error message.
-    ReportErrorAndClose(entry->stream_id());
+  std::unique_ptr<mojom::AudioOutputStreamPtr> stream_ptr(

[xhwang, 2016/04/26 22:54:42]

AudioOutputStreamPtr is an InterfacePtr which is already move-only. No need for
std::unique_ptr here?

[rchtara, 2016/04/29 12:54:46]

Done.

+      audio_output_impl_->StreamFactory(stream_id, this));
+
+  base::SharedMemoryHandle shared_memory_handle =
+      base::SharedMemory::DuplicateHandle(entry->shared_memory()->handle());
+
+  MojoHandle mojo_foreign_memory_handle;
+
+  MojoResult shared_buffer_result = mojo::edk::CreateSharedBufferWrapper(
+      shared_memory_handle, entry->shared_memory()->requested_size(), false,
+      &mojo_foreign_memory_handle);
+
+  if (shared_buffer_result != MOJO_RESULT_OK) {
+    DLOG(WARNING) << "Failed to wrap transit descriptor. Closing: "
+                 << shared_buffer_result;
+    ReportErrorAndCloseStream(entry->stream_id(), callback);
     return;
   }
 
@@ -329,13 +368,50 @@ void AudioRendererHost::DoCompleteCreation(int stream_id) {
   // If we failed to prepare the sync socket for the renderer then we fail
   // the construction of audio stream.
   if (!reader->PrepareForeignSocket(PeerHandle(), &socket_descriptor)) {
-    ReportErrorAndClose(entry->stream_id());
+    ReportErrorAndCloseStream(entry->stream_id(), callback);
+    return;
+  }
+  mojo::ScopedSharedBufferHandle shared_buffer_handle =
+      mojo::ScopedSharedBufferHandle(
+          mojo::SharedBufferHandle(mojo_foreign_memory_handle));
+
+  MojoHandle socket_descriptor_handle;
+  MojoResult platform_handle_result;
+
+  // The socket handle is going to be closed when |mojo_application_host_| is

[Henrik Grunell, 2016/04/22 08:23:04]

Why do we need two handles actually? Only one is used in the end, right? Can it
be passed instead?

[rchtara, 2016/04/29 12:54:46]

yes.
if we dont do that, both mojo and AudioOutputIPCDelegate are going to need to
close the socket descriptor which causes an error.

+  // reset. AudioOutputIPCDelegate is going to need to close the socket handle
+  // too when the steam it controls is closed. This is going to cause an issue
+  // because both of them are going to close the same socket handle.
+  // This is why a duplicate of the socket handler is going to be created,
+  // stored in |socket_descriptor_dup| and sent through Mojo to the renderer so
+  // it can be used by AudioOutputIPCDelegate.
+
+  base::SyncSocket::TransitDescriptor socket_descriptor_dup =
+      DuplicateSocket(socket_descriptor);
+#if defined(OS_WIN)
+  platform_handle_result = mojo::edk::CreatePlatformHandleWrapper(
+      mojo::edk::ScopedPlatformHandle(
+          mojo::edk::PlatformHandle(socket_descriptor_dup),
+      &socket_descriptor_handle);
+#else
+  platform_handle_result = mojo::edk::CreatePlatformHandleWrapper(
+    mojo::edk::ScopedPlatformHandle(
+          mojo::edk::PlatformHandle(socket_descriptor_dup.fd)),
+      &socket_descriptor_handle);
+#endif
+
+  if (platform_handle_result != MOJO_RESULT_OK) {
+    DLOG(WARNING) << "Failed to wrap platform handle. Closing: "
+                 << platform_handle_result;
+    ReportErrorAndCloseStream(stream_id, callback);
     return;
   }
 
-  Send(new AudioMsg_NotifyStreamCreated(
-      entry->stream_id(), foreign_memory_handle, socket_descriptor,
-      entry->shared_memory()->requested_size()));
+  mojo::ScopedHandle socket_handle =
+      mojo::ScopedHandle(mojo::Handle(socket_descriptor_handle));
+  callback.Run(stream_id, std::move(*stream_ptr),
+               std::move(shared_buffer_handle), std::move(socket_handle));
+
 }
 
 void AudioRendererHost::DoNotifyStreamStateChanged(int stream_id,
@@ -386,7 +462,6 @@ bool AudioRendererHost::OnMessageReceived(const IPC::Message& message) {
   IPC_BEGIN_MESSAGE_MAP(AudioRendererHost, message)
     IPC_MESSAGE_HANDLER(AudioHostMsg_RequestDeviceAuthorization,
                         OnRequestDeviceAuthorization)
-    IPC_MESSAGE_HANDLER(AudioHostMsg_CreateStream, OnCreateStream)
     IPC_MESSAGE_HANDLER(AudioHostMsg_PlayStream, OnPlayStream)
     IPC_MESSAGE_HANDLER(AudioHostMsg_PauseStream, OnPauseStream)
     IPC_MESSAGE_HANDLER(AudioHostMsg_CloseStream, OnCloseStream)
@@ -525,30 +600,35 @@ void AudioRendererHost::OnDeviceIDTranslated(
       stream_id, media::OUTPUT_DEVICE_STATUS_OK, output_params, std::string()));
 }
 
-void AudioRendererHost::OnCreateStream(int stream_id,
-                                       int render_frame_id,
-                                       const media::AudioParameters& params) {
+void AudioRendererHost::CreateStream(
+    int stream_id,
+    int render_frame_id,
+    const media::AudioParameters& params,
+    const mojom::AudioOutput::CreateStreamCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DVLOG(1) << "AudioRendererHost@" << this << "::OnCreateStream"
+  DVLOG(1) << "AudioRendererHost@" << this << "::CreateStream"
            << "(stream_id=" << stream_id << ")";
 
   const auto& auth_data = authorizations_.find(stream_id);
 
   // If no previous authorization requested, assume default device
   if (auth_data == authorizations_.end()) {
-    DoCreateStream(stream_id, render_frame_id, params, std::string());
+    DoCreateStream(stream_id, render_frame_id, params, std::string(), callback);
     return;
   }
 
   CHECK(auth_data->second.first);
-  DoCreateStream(stream_id, render_frame_id, params, auth_data->second.second);
+  DoCreateStream(stream_id, render_frame_id, params, auth_data->second.second,
+                 callback);
   authorizations_.erase(auth_data);
 }
 
-void AudioRendererHost::DoCreateStream(int stream_id,
-                                       int render_frame_id,
-                                       const media::AudioParameters& params,
-                                       const std::string& device_unique_id) {
+void AudioRendererHost::DoCreateStream(
+    int stream_id,
+    int render_frame_id,
+    const media::AudioParameters& params,
+    const std::string& device_unique_id,
+    const mojom::AudioOutput::CreateStreamCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // media::AudioParameters is validated in the deserializer.
@@ -580,7 +660,7 @@ void AudioRendererHost::DoCreateStream(int stream_id,
 
   std::unique_ptr<AudioEntry> entry(
       new AudioEntry(this, stream_id, render_frame_id, params, device_unique_id,
-                     std::move(shared_memory), std::move(reader)));
+                     std::move(shared_memory), std::move(reader), callback));
   if (mirroring_manager_) {
     mirroring_manager_->AddDiverter(
         render_process_id_, entry->render_frame_id(), entry->controller());
@@ -693,6 +773,28 @@ void AudioRendererHost::ReportErrorAndClose(int stream_id) {
   OnCloseStream(stream_id);
 }
 
+void AudioRendererHost::ReportErrorAndCloseStream(
+    int stream_id,
+    const mojom::AudioOutput::CreateStreamCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // Make sure this isn't a stray callback executing after the stream has been
+  // closed, so error notifications aren't sent after clients believe the stream
+  // is closed.
+  if (!LookupById(stream_id))
+    return;
+  mojo::ScopedSharedBufferHandle shared_buffer_handle =
+      mojo::ScopedSharedBufferHandle(mojo::SharedBufferHandle());
+
+  mojo::ScopedHandle socket_handle = mojo::ScopedHandle(mojo::Handle());
+
+  callback.Run(stream_id, mojom::AudioOutputStreamPtr(),
+               std::move(shared_buffer_handle), std::move(socket_handle));
+
+  audio_log_->OnError(stream_id);
+  OnCloseStream(stream_id);
+}
+
 AudioRendererHost::AudioEntry* AudioRendererHost::LookupById(int stream_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);