Change sync primitives ownership for audio rendering.

content/browser/renderer_host/media/audio_renderer_host.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 <utility>
 
 #include "base/bind.h"
@@ skipping 116 matching lines @@
       !!RenderFrameHost::FromID(render_process_id, render_frame_id);
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(callback, frame_exists));
 }
 
 }  // 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;
 
+  // Returns nullptr on failure.
+  static std::unique_ptr<AudioRendererHost::AudioEntry> Create(
+      AudioRendererHost* host,
+      int stream_id,
+      int render_frame_id,
+      const media::AudioParameters& params,
+      const std::string& output_device_id);
+
   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();
-  }
+  AudioSyncReader* reader() const { return reader_.get(); }
 
-  media::AudioOutputController::SyncReader* reader() const {
-    return reader_.get();
-  }
-
+  // Used by ARH to track the number of active streams for UMA stats.
   bool playing() const { return playing_; }
   void set_playing(bool playing) { playing_ = playing; }
 
  private:
+  AudioEntry(AudioRendererHost* host,
+             int stream_id,
+             int render_frame_id,
+             const media::AudioParameters& params,
+             const std::string& output_device_id,
+             std::unique_ptr<AudioSyncReader> reader);
+
   // 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_;
+  const std::unique_ptr<AudioSyncReader> reader_;
 
   // The AudioOutputController that manages the audio stream.
   const scoped_refptr<media::AudioOutputController> controller_;
 
   bool playing_;
+
+  DISALLOW_COPY_AND_ASSIGN(AudioEntry);
 };
 
 AudioRendererHost::AudioEntry::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)
+    std::unique_ptr<AudioSyncReader> reader)
     : host_(host),
       stream_id_(stream_id),
       render_frame_id_(render_frame_id),
-      shared_memory_(std::move(shared_memory)),
       reader_(std::move(reader)),
       controller_(media::AudioOutputController::Create(host->audio_manager_,
                                                        this,
                                                        params,
                                                        output_device_id,
                                                        reader_.get())),
       playing_(false) {
-  DCHECK(controller_.get());
+  DCHECK(controller_);
 }
 
 AudioRendererHost::AudioEntry::~AudioEntry() {}
 
+// static
+std::unique_ptr<AudioRendererHost::AudioEntry>
+AudioRendererHost::AudioEntry::Create(AudioRendererHost* host,
+                                      int stream_id,
+                                      int render_frame_id,
+                                      const media::AudioParameters& params,
+                                      const std::string& output_device_id) {
+  std::unique_ptr<AudioSyncReader> reader(AudioSyncReader::Create(params));
+  if (!reader) {
+    return {};
+  }
+  return std::unique_ptr<AudioEntry>(
+      new AudioEntry(host, stream_id, render_frame_id, params, output_device_id,

[o1ka, 2016/09/16 10:51:59]

(you can use base::WrapUnique instead as well
https://cs.chromium.org/chromium/src/base/memory/ptr_util.h?type=cs&sq=packag...)

[Max Morin, 2016/09/16 12:46:15]

Done.

+                     std::move(reader)));
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // AudioRendererHost implementations.
 
 AudioRendererHost::AudioRendererHost(int render_process_id,
                                      media::AudioManager* audio_manager,
                                      AudioMirroringManager* mirroring_manager,
                                      MediaInternals* media_internals,
                                      MediaStreamManager* media_stream_manager,
                                      const std::string& salt)
     : BrowserMessageFilter(AudioMsgStart),
@@ skipping 95 matching lines @@
     ReportErrorAndClose(stream_id);
     return;
   }
 
   AudioEntry* const entry = LookupById(stream_id);
   if (!entry) {
     ReportErrorAndClose(stream_id);
     return;
   }
 
-  // Once the audio stream is created then complete the creation process by
-  // mapping shared memory and sharing with the renderer process.

[o1ka, 2016/09/16 10:51:59]

Could you transit all the comments to the new code?

[Max Morin, 2016/09/16 12:46:15]

Done.

-  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());
-    return;
-  }
-
-  AudioSyncReader* reader = static_cast<AudioSyncReader*>(entry->reader());
-
+  base::SharedMemory* shared_memory = entry->reader()->shared_memory();
+  base::CancelableSyncSocket* foreign_socket =
+      entry->reader()->foreign_socket();
+  base::SharedMemoryHandle memory_handle;

[o1ka, 2016/09/16 10:51:59]

nit: I think foreign_memory_handle was more descriptive

[Max Morin, 2016/09/16 12:46:15]

Done.

   base::SyncSocket::TransitDescriptor socket_descriptor;
-
-  // 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)) {
+  size_t shared_memory_size = shared_memory->requested_size();
+  if (!(shared_memory->ShareToProcess(PeerHandle(), &memory_handle) &&
+        foreign_socket->PrepareTransitDescriptor(PeerHandle(),
+                                                 &socket_descriptor))) {
     ReportErrorAndClose(entry->stream_id());
     return;
   }
 
   Send(new AudioMsg_NotifyStreamCreated(
-      entry->stream_id(), foreign_memory_handle, socket_descriptor,
-      entry->shared_memory()->requested_size()));
+      stream_id, memory_handle, socket_descriptor,
+      static_cast<uint32_t>(shared_memory_size)));

[o1ka, 2016/09/16 10:51:59]

checked_cast?
(https://cs.chromium.org/chromium/src/base/numerics/safe_conversions.h?l=48)
also see
https://www.chromium.org/Home/chromium-security/education/security-tips-for-ipc

[Max Morin, 2016/09/16 12:46:15]

Done.

 }
 
 void AudioRendererHost::DidValidateRenderFrame(int stream_id, bool is_valid) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
- if (!is_valid) {
-   DLOG(WARNING) << "Render frame for stream (id=" << stream_id
-                 << ") no longer exists.";
-   ReportErrorAndClose(stream_id);
- }
+  if (!is_valid) {
+    DLOG(WARNING) << "Render frame for stream (id=" << stream_id
+                  << ") no longer exists.";
+    ReportErrorAndClose(stream_id);
+  }
 }
 
 void AudioRendererHost::DoNotifyStreamStateChanged(int stream_id,
                                                    bool is_playing) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   AudioEntry* const entry = LookupById(stream_id);
   if (!entry)
     return;
 
@@ skipping 227 matching lines @@
   // a valid render frame. This validation is important for all the reasons
   // stated in the comments above. This does not block stream creation, but will
   // force-close the stream later if validation fails.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(validate_render_frame_id_function_, render_process_id_,
                  render_frame_id,
                  base::Bind(&AudioRendererHost::DidValidateRenderFrame, this,
                             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());
-  if (!shared_memory->CreateAndMapAnonymous(shared_memory_size)) {
+  std::unique_ptr<AudioEntry> entry = AudioEntry::Create(
+      this, stream_id, render_frame_id, params, device_unique_id);
+  if (!entry) {
     SendErrorMessage(stream_id);
     return;
   }
-
-  std::unique_ptr<AudioSyncReader> reader(
-      new AudioSyncReader(shared_memory.get(), params));
-  if (!reader->Init()) {
-    SendErrorMessage(stream_id);
-    return;
-  }
 
   MediaObserver* const media_observer =
       GetContentClient()->browser()->GetMediaObserver();
   if (media_observer)
     media_observer->OnCreatingAudioStream(render_process_id_, render_frame_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)));
   if (mirroring_manager_) {
     mirroring_manager_->AddDiverter(
         render_process_id_, entry->render_frame_id(), entry->controller());
   }
   audio_entries_.insert(std::make_pair(stream_id, entry.release()));
   g_audio_streams_tracker.Get().IncreaseStreamCount();
 
   audio_log_->OnCreated(stream_id, params, device_unique_id);
   MediaInternals::GetInstance()->SetWebContentsTitleForAudioLogEntry(
       stream_id, render_process_id_, render_frame_id, audio_log_.get());
@@ skipping 210 matching lines @@
   callback.Run(false, device_info);
 }
 
 bool AudioRendererHost::IsAuthorizationStarted(int stream_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   const auto& i = authorizations_.find(stream_id);
   return i != authorizations_.end();
 }
 
 }  // namespace content