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..d3f2090fb524b26a03ecdbdebe9e6ed37ca3a924 100644
--- a/content/browser/renderer_host/media/audio_renderer_host.cc
+++ b/content/browser/renderer_host/media/audio_renderer_host.cc
@@ -6,17 +6,20 @@
 
 #include <stdint.h>
 #include <utility>
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
-#include "base/memory/shared_memory.h"
 #include "base/metrics/histogram.h"
 #include "base/process/process.h"
 #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 +39,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;
@@ -117,62 +122,6 @@ void MaybeFixAudioParameters(media::AudioParameters* params) {
 
 }  // namespace
 
-class AudioRendererHost::AudioEntry
-    : public media::AudioOutputController::EventHandler {
- public:
-  AudioEntry(AudioRendererHost* host,
-             int stream_id,
-             int render_frame_id,
-             const media::AudioParameters& params,
-             const std::string& output_device_id,
-             std::unique_ptr<base::SharedMemory> shared_memory,
-             std::unique_ptr<media::AudioOutputController::SyncReader> reader);
-  ~AudioEntry() override;
-
-  int stream_id() const {
-    return stream_id_;
-  }
-
-  int render_frame_id() const { return render_frame_id_; }
-
-  media::AudioOutputController* controller() const { return controller_.get(); }
-
-  base::SharedMemory* shared_memory() {
-    return shared_memory_.get();
-  }
-
-  media::AudioOutputController::SyncReader* reader() const {
-    return reader_.get();
-  }
-
-  bool playing() const { return playing_; }
-  void set_playing(bool playing) { playing_ = playing; }
-
- private:
-  // media::AudioOutputController::EventHandler implementation.
-  void OnCreated() override;
-  void OnPlaying() override;
-  void OnPaused() override;
-  void OnError() override;
-
-  AudioRendererHost* const host_;
-  const int stream_id_;
-
-  // The routing ID of the source RenderFrame.
-  const int render_frame_id_;
-
-  // Shared memory for transmission of the audio data.  Used by |reader_|.
-  const std::unique_ptr<base::SharedMemory> shared_memory_;
-
-  // The synchronous reader to be used by |controller_|.
-  const std::unique_ptr<media::AudioOutputController::SyncReader> reader_;
-
-  // The AudioOutputController that manages the audio stream.
-  const scoped_refptr<media::AudioOutputController> controller_;
-
-  bool playing_;
-};
-
 AudioRendererHost::AudioEntry::AudioEntry(
     AudioRendererHost* host,
     int stream_id,
@@ -180,7 +129,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 +140,8 @@ AudioRendererHost::AudioEntry::AudioEntry(
                                                        this,
                                                        params,
                                                        output_device_id,
-                                                       reader_.get())),
+                                                       reader_.get(),
+                                                       callback)),
       playing_(false) {
   DCHECK(controller_.get());
 }
@@ -262,11 +213,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) {

[Henrik Grunell, 2016/04/19 15:36:09]

Can the callback be stored in the AudioEntry instead?

[rchtara, 2016/04/21 09:10:18]

it's possible to do that, but we are going to have soon other callbacks after we
mojofy play, pause, ...
So we need to store them somewhere, and they could come at the some time which
makes that even more complicated.
I think it's better to bind them to avoid issues.

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

Acknowledged.

+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioRendererHost::DoCompleteCreation,
+                                     host_, stream_id_, callback));
 }
 
 void AudioRendererHost::AudioEntry::OnPlaying() {
@@ -296,29 +247,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);
+    ReportErrorAndCloseMojo(stream_id, callback);
     return;
   }
 
   AudioEntry* const entry = LookupById(stream_id);
   if (!entry) {
-    ReportErrorAndClose(stream_id);
+    ReportErrorAndCloseMojo(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());
+  scoped_ptr<mojom::AudioOutputStreamPtr> stream_ptr(

[Henrik Grunell, 2016/04/19 15:36:09]

std::unique_ptr. Here and elsewhere.

[rchtara, 2016/04/21 09:10:17]

Done.

+      audio_output_impl_->StreamFactory(stream_id, entry, 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;
+    ReportErrorAndCloseMojo(entry->stream_id(), callback);
     return;
   }
 
@@ -329,13 +290,59 @@ 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());
+    ReportErrorAndCloseMojo(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;
+
+  // Mojo is going to close the socket handle when Mojo shutdowns.

[Henrik Grunell, 2016/04/19 15:36:08]

I'm not sure what Mojo means here. And what "when Mojo shutdowns" exactly means.
Could you rephrase this section?

[rchtara, 2016/04/21 09:10:17]

Done.

+  // This could create a conflict between Mojo and AudioOutputIPCDelegate where
+  // the socket is going to need to be closed too.

[Henrik Grunell, 2016/04/19 15:36:09]

This line is hard to parse.

[rchtara, 2016/04/21 09:10:18]

Done.

+  // This is why a duplicate of socket handler is going to be created, stored in
+  // |socket_descriptor_dup| and sent through Mojo to the renderer.
+
+#if defined(OS_WIN)

[Henrik Grunell, 2016/04/19 15:36:09]

This duplication looks like a good candidate for a separate function.

[rchtara, 2016/04/21 09:10:17]

Done.

+  base::SyncSocket::TransitDescriptor socket_descriptor_dup = nullptr;
+  if (::DuplicateHandle(GetCurrentProcess(),  // hSourceProcessHandle
+                        socket_descriptor,
+                        GetCurrentProcess(),  // hTargetProcessHandle
+                        &socket_descriptor_dup,
+                        0,  // dwDesiredAccess ignored due to SAME_ACCESS
+                        FALSE,  // !bInheritHandle
+                        DUPLICATE_SAME_ACCESS)) {
+    DLOG(WARNING) << "Failed to duplicate socket";
+    return;
+  }
+  platform_handle_result = mojo::edk::CreatePlatformHandleWrapper(
+      mojo::edk::ScopedPlatformHandle(
+          mojo::edk::PlatformHandle(socket_descriptor_dup)),
+      &socket_descriptor_handle);
+
+#else
+  int socket_descriptor_dup = dup(socket_descriptor.fd);
+  platform_handle_result = mojo::edk::CreatePlatformHandleWrapper(
+    mojo::edk::ScopedPlatformHandle(
+          mojo::edk::PlatformHandle(socket_descriptor_dup)),
+      &socket_descriptor_handle);
+#endif
+
+  if (platform_handle_result != MOJO_RESULT_OK) {
+    DLOG(WARNING) << "Failed to wrap platform handle. Closing: "
+                 << platform_handle_result;
+    ReportErrorAndCloseMojo(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(std::move(*stream_ptr), stream_id,
+               std::move(shared_buffer_handle), std::move(socket_handle));
+
 }
 
 void AudioRendererHost::DoNotifyStreamStateChanged(int stream_id,
@@ -386,7 +393,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,9 +531,11 @@ 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::OnCreateStream(
+    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"
            << "(stream_id=" << stream_id << ")";
@@ -536,19 +544,22 @@ void AudioRendererHost::OnCreateStream(int 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.
@@ -560,13 +571,13 @@ void AudioRendererHost::DoCreateStream(int stream_id,
   // Create the shared memory and share with the renderer process.
   uint32_t shared_memory_size = sizeof(media::AudioOutputBufferParameters) +
                                 AudioBus::CalculateMemorySize(params);
-  std::unique_ptr<base::SharedMemory> shared_memory(new base::SharedMemory());
+  scoped_ptr<base::SharedMemory> shared_memory(new base::SharedMemory());

[Henrik Grunell, 2016/04/19 15:36:09]

scoped_ptr is deprecated. Keep std::unique_ptr. (Not sure why you changed. :))

[rchtara, 2016/04/21 09:10:18]

Done.

   if (!shared_memory->CreateAndMapAnonymous(shared_memory_size)) {
     SendErrorMessage(stream_id);
     return;
   }
 
-  std::unique_ptr<AudioSyncReader> reader(
+  scoped_ptr<AudioSyncReader> reader(
       new AudioSyncReader(shared_memory.get(), params));
   if (!reader->Init()) {
     SendErrorMessage(stream_id);
@@ -578,9 +589,9 @@ void AudioRendererHost::DoCreateStream(int stream_id,
   if (media_observer)
     media_observer->OnCreatingAudioStream(render_process_id_, render_frame_id);
 
-  std::unique_ptr<AudioEntry> entry(
+  scoped_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 +704,28 @@ void AudioRendererHost::ReportErrorAndClose(int stream_id) {
   OnCloseStream(stream_id);
 }
 
+void AudioRendererHost::ReportErrorAndCloseMojo(

[Henrik Grunell, 2016/04/19 15:36:09]

"CloseMojo" sounds like the whole mojo infrastructure would be closed. :) It
seems to be the stream that is closed, right?

[rchtara, 2016/04/21 09:10:17]

:) yes exactly.
Done.

+    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(mojom::AudioOutputStreamPtr(), stream_id,
+               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);