Make MojoAudioInputStream share read only memory.

This includes making SharedMemory::GetReadOnlyHandle() const.
Getting a handle with write permissions is currently const, so it makes
no sense at all that getting a handle with read only permissions is not
const :).

Bug: 653871
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie98fde38535f5ffe0ae51949ae043c38c9791eb8
Reviewed-on: https://chromium-review.googlesource.com/806655
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522064}

base/memory/shared_memory.h

@@ -208,7 +208,7 @@ class BASE_EXPORT SharedMemory {
   // that takes ownership of the handle. As such, it's not valid to pass the
   // sample handle to the IPC subsystem twice. Returns an invalid handle on
   // failure.
-  SharedMemoryHandle GetReadOnlyHandle();
+  SharedMemoryHandle GetReadOnlyHandle() const;
 
   // Returns an ID for the mapped region. This is ID of the SharedMemoryHandle
   // that was mapped. The ID is valid even after the SharedMemoryHandle is

base/memory/shared_memory_fuchsia.cc

@@ -149,7 +149,7 @@ SharedMemoryHandle SharedMemory::DuplicateHandle(
   return handle.Duplicate();
 }
 
-SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const {
   zx_handle_t duped_handle;
   const int kNoWriteOrExec =
       ZX_DEFAULT_VMO_RIGHTS &

base/memory/shared_memory_mac.cc

@@ -244,7 +244,7 @@ void SharedMemory::Close() {
   }
 }
 
-SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const {
   if (shm_.type_ == SharedMemoryHandle::POSIX) {
     // We could imagine re-opening the file from /dev/fd, but that can't make it
     // readonly on Mac: https://codereview.chromium.org/27265002/#msg10.

base/memory/shared_memory_nacl.cc

@@ -130,7 +130,7 @@ void SharedMemory::Close() {
   }
 }
 
-SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const {
   // Untrusted code can't create descriptors or handles, which is needed to
   // drop permissions.
   return SharedMemoryHandle();

base/memory/shared_memory_posix.cc

@@ -363,7 +363,7 @@ bool SharedMemory::FilePathForMemoryName(const std::string& mem_name,
 }
 #endif  // !defined(OS_ANDROID)
 
-SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const {
   CHECK(readonly_shm_.IsValid());
   return readonly_shm_.Duplicate();
 }

base/memory/shared_memory_win.cc

@@ -338,7 +338,7 @@ bool SharedMemory::Unmap() {
   return true;
 }
 
-SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const {
   HANDLE result;
   ProcessHandle process = GetCurrentProcess();
   if (!::DuplicateHandle(process, shm_.GetHandle(), process, &result,

content/browser/renderer_host/media/audio_input_sync_writer.cc

@@ -145,10 +145,16 @@ std::unique_ptr<AudioInputSyncWriter> AudioInputSyncWriter::Create(
       media::ComputeAudioInputBufferSizeChecked(params,
                                                 shared_memory_segment_count);
 
+  if (!requested_memory_size.IsValid())
+    return nullptr;
+
+  // Make sure we can share the memory read-only with the client.
+  base::SharedMemoryCreateOptions shmem_options;
+  shmem_options.size = requested_memory_size.ValueOrDie();
+  shmem_options.share_read_only = true;
   auto shared_memory = std::make_unique<base::SharedMemory>();
-  if (!requested_memory_size.IsValid() ||
-      !shared_memory->CreateAndMapAnonymous(
-          requested_memory_size.ValueOrDie())) {
+  if (!shared_memory->Create(shmem_options) ||
+      !shared_memory->Map(shmem_options.size)) {
     return nullptr;
   }
 

content/renderer/media/mojo_audio_input_ipc_unittest.cc

@@ -56,7 +56,7 @@ class MockDelegate : public media::AudioInputIPCDelegate {
                        base::SyncSocket::Handle socket_handle,
                        bool initially_muted) override {
     base::SharedMemory sh_mem(
-        mem_handle, /*read_only*/ false);  // Releases the shared memory handle.
+        mem_handle, /*read_only*/ true);  // Releases the shared memory handle.
     base::SyncSocket socket(socket_handle);  // Releases the socket descriptor.
     GotOnStreamCreated(initially_muted);
   }

media/audio/audio_device_thread.cc

@@ -31,6 +31,7 @@ namespace media {
 
 AudioDeviceThread::Callback::Callback(const AudioParameters& audio_parameters,
                                       base::SharedMemoryHandle memory,
+                                      bool read_only_memory,
                                       uint32_t segment_length,
                                       uint32_t total_segments)
     : audio_parameters_(audio_parameters),
@@ -41,7 +42,7 @@ AudioDeviceThread::Callback::Callback(const AudioParameters& audio_parameters,
       // CHECK that the shared memory is large enough. The memory allocated
       // must be at least as large as expected.
       shared_memory_((CHECK(memory_length_ <= memory.GetSize()), memory),
-                     false) {
+                     read_only_memory) {
   CHECK_GT(total_segments_, 0u);
   thread_checker_.DetachFromThread();
 }

media/audio/audio_device_thread.h

@@ -31,6 +31,7 @@ class MEDIA_EXPORT AudioDeviceThread : public base::PlatformThread::Delegate {
    public:
     Callback(const AudioParameters& audio_parameters,
              base::SharedMemoryHandle memory,
+             bool read_only_memory,
              uint32_t segment_length,
              uint32_t total_segments);
 

media/audio/audio_input_device.cc

@@ -68,7 +68,7 @@ class AudioInputDevice::AudioThreadCallback
   const double bytes_per_ms_;
   size_t current_segment_id_;
   uint32_t last_buffer_id_;
-  std::vector<std::unique_ptr<media::AudioBus>> audio_buses_;
+  std::vector<std::unique_ptr<const media::AudioBus>> audio_buses_;
   CaptureCallback* capture_callback_;
 
   // Used for informing AudioInputDevice that we have gotten data, i.e. the
@@ -369,6 +369,7 @@ AudioInputDevice::AudioThreadCallback::AudioThreadCallback(
     : AudioDeviceThread::Callback(
           audio_parameters,
           memory,
+          /*read only*/ true,
           ComputeAudioInputBufferSize(audio_parameters, 1u),
           total_segments),
       bytes_per_ms_(static_cast<double>(audio_parameters.GetBytesPerSecond()) /
@@ -387,12 +388,12 @@ void AudioInputDevice::AudioThreadCallback::MapSharedMemory() {
   shared_memory_.Map(memory_length_);
 
   // Create vector of audio buses by wrapping existing blocks of memory.
-  uint8_t* ptr = static_cast<uint8_t*>(shared_memory_.memory());
+  const uint8_t* ptr = static_cast<const uint8_t*>(shared_memory_.memory());
   for (uint32_t i = 0; i < total_segments_; ++i) {
-    media::AudioInputBuffer* buffer =
-        reinterpret_cast<media::AudioInputBuffer*>(ptr);
+    const media::AudioInputBuffer* buffer =
+        reinterpret_cast<const media::AudioInputBuffer*>(ptr);
     audio_buses_.push_back(
-        media::AudioBus::WrapMemory(audio_parameters_, buffer->audio));
+        media::AudioBus::WrapReadOnlyMemory(audio_parameters_, buffer->audio));
     ptr += segment_length_;
   }
 
@@ -407,9 +408,10 @@ void AudioInputDevice::AudioThreadCallback::Process(uint32_t pending_data) {
   // The shared memory represents parameters, size of the data buffer and the
   // actual data buffer containing audio data. Map the memory into this
   // structure and parse out parameters and the data area.
-  uint8_t* ptr = static_cast<uint8_t*>(shared_memory_.memory());
+  const uint8_t* ptr = static_cast<const uint8_t*>(shared_memory_.memory());
   ptr += current_segment_id_ * segment_length_;
-  AudioInputBuffer* buffer = reinterpret_cast<AudioInputBuffer*>(ptr);
+  const AudioInputBuffer* buffer =
+      reinterpret_cast<const AudioInputBuffer*>(ptr);
 
   // Usually this will be equal but in the case of low sample rate (e.g. 8kHz,
   // the buffer may be bigger (on mac at least)).
@@ -434,7 +436,7 @@ void AudioInputDevice::AudioThreadCallback::Process(uint32_t pending_data) {
   last_buffer_id_ = buffer->params.id;
 
   // Use pre-allocated audio bus wrapping existing block of shared memory.
-  media::AudioBus* audio_bus = audio_buses_[current_segment_id_].get();
+  const media::AudioBus* audio_bus = audio_buses_[current_segment_id_].get();
 
   // Regularly inform that we have gotten data.
   frames_since_last_got_data_callback_ += audio_bus->frames();

media/audio/audio_output_device.cc

@@ -461,8 +461,9 @@ AudioOutputDevice::AudioThreadCallback::AudioThreadCallback(
     : AudioDeviceThread::Callback(
           audio_parameters,
           memory,
+          /*read only*/ false,
           ComputeAudioOutputBufferSize(audio_parameters),
-          1),
+          /*segment count*/ 1),
       render_callback_(render_callback),
       callback_num_(0) {}
 

media/base/audio_bus.cc

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <limits>
+#include <utility>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ -148,6 +149,16 @@ std::unique_ptr<AudioBus> AudioBus::WrapMemory(const AudioParameters& params,
                                        static_cast<float*>(data)));
 }
 
+std::unique_ptr<const AudioBus> AudioBus::WrapReadOnlyMemory(
+    const AudioParameters& params,
+    const void* data) {
+  // Note: const_cast is generally dangerous but is used in this case since
+  // AudioBus accomodates both read-only and read/write use cases. A const
+  // AudioBus object is returned to ensure noone accidentally writes to the
+  // read-only data.
+  return WrapMemory(params, const_cast<void*>(data));
+}
+
 void AudioBus::SetChannelData(int channel, float* data) {
   CHECK(can_set_channel_data_);
   CHECK(data);

media/base/audio_bus.h

@@ -55,6 +55,9 @@ class MEDIA_SHMEM_EXPORT AudioBus {
                                               void* data);
   static std::unique_ptr<AudioBus> WrapMemory(const AudioParameters& params,
                                               void* data);
+  static std::unique_ptr<const AudioBus> WrapReadOnlyMemory(
+      const AudioParameters& params,
+      const void* data);
 
   // Based on the given number of channels and frames, calculates the minimum
   // required size in bytes of a contiguous block of memory to be passed to

media/mojo/services/mojo_audio_input_stream.cc

@@ -76,14 +76,15 @@ void MojoAudioInputStream::OnStreamCreated(
   DCHECK(foreign_socket);
 
   base::SharedMemoryHandle foreign_memory_handle =
-      base::SharedMemory::DuplicateHandle(shared_memory->handle());
+      shared_memory->GetReadOnlyHandle();
   if (!base::SharedMemory::IsHandleValid(foreign_memory_handle)) {
     OnStreamError(/*not used*/ 0);
     return;
   }
 
   mojo::ScopedSharedBufferHandle buffer_handle = mojo::WrapSharedMemoryHandle(
-      foreign_memory_handle, shared_memory->requested_size(), false);
+      foreign_memory_handle, shared_memory->requested_size(),
+      /*read_only*/ true);
   mojo::ScopedHandle socket_handle =
       mojo::WrapPlatformFile(foreign_socket->Release());
 

media/mojo/services/mojo_audio_input_stream_unittest.cc

@@ -114,7 +114,7 @@ class MockClient : public mojom::AudioInputStreamClient {
         mojo::UnwrapSharedMemoryHandle(std::move(shared_buffer), &shmem_handle,
                                        &memory_length, &read_only),
         MOJO_RESULT_OK);
-    EXPECT_FALSE(read_only);
+    EXPECT_TRUE(read_only);
     buffer_ = base::MakeUnique<base::SharedMemory>(shmem_handle, read_only);
 
     GotNotification(initially_muted);
@@ -173,7 +173,10 @@ class MojoAudioInputStreamTest : public Test {
         base::WrapUnique(delegate_));
     EXPECT_TRUE(
         base::CancelableSyncSocket::CreatePair(&local_, foreign_socket_.get()));
-    EXPECT_TRUE(mem_.CreateAnonymous(kShmemSize));
+    base::SharedMemoryCreateOptions shmem_options;
+    shmem_options.size = kShmemSize;
+    shmem_options.share_read_only = true;
+    EXPECT_TRUE(mem_.Create(shmem_options));
     EXPECT_CALL(mock_delegate_factory_, MockCreateDelegate(NotNull()))
         .WillOnce(SaveArg<0>(&delegate_event_handler_));
   }