Make ChildThread::AllocateSharedMemory return a scoped_ptr.

The caller is taking ownership of the SharedMemory. Also, fix a bunch of leaks in the media code related to this.

Review URL: https://codereview.chromium.org/769563002

Cr-Commit-Position: refs/heads/master@{#306345}

15 files changed, 63 insertions, 53 deletions

diff --git a/content/child/child_shared_bitmap_manager.cc b/content/child/child_shared_bitmap_manager.cc
index a49e403..a62fd41 100644
--- a/content/child/child_shared_bitmap_manager.cc
+++ b/content/child/child_shared_bitmap_manager.cc
@@ -70,7 +70,7 @@ scoped_ptr<cc::SharedBitmap> ChildSharedBitmapManager::AllocateSharedBitmap(
   if (!memory->Map(memory_size))
     CHECK(false);
 #else
-  memory.reset(ChildThread::AllocateSharedMemory(memory_size, sender_.get()));
+  memory = ChildThread::AllocateSharedMemory(memory_size, sender_.get());
   CHECK(memory);
   if (!memory->Map(memory_size))
     CHECK(false);
diff --git a/content/child/child_thread.cc b/content/child/child_thread.cc
index 8e59add..aa92c6a 100644
--- a/content/child/child_thread.cc
+++ b/content/child/child_thread.cc
@@ -426,13 +426,14 @@ MessageRouter* ChildThread::GetRouter() {
   return &router_;
 }
 
-base::SharedMemory* ChildThread::AllocateSharedMemory(size_t buf_size) {
+scoped_ptr<base::SharedMemory> ChildThread::AllocateSharedMemory(
+    size_t buf_size) {
   DCHECK(base::MessageLoop::current() == message_loop());
   return AllocateSharedMemory(buf_size, this);
 }
 
 // static
-base::SharedMemory* ChildThread::AllocateSharedMemory(
+scoped_ptr<base::SharedMemory> ChildThread::AllocateSharedMemory(
     size_t buf_size,
     IPC::Sender* sender) {
   scoped_ptr<base::SharedMemory> shared_buf;
@@ -459,7 +460,7 @@ base::SharedMemory* ChildThread::AllocateSharedMemory(
     return NULL;
   }
 #endif
-  return shared_buf.release();
+  return shared_buf;
 }
 
 bool ChildThread::OnMessageReceived(const IPC::Message& msg) {
diff --git a/content/child/child_thread.h b/content/child/child_thread.h
index f66bc2f..7181db6 100644
--- a/content/child/child_thread.h
+++ b/content/child/child_thread.h
@@ -89,12 +89,13 @@ class CONTENT_EXPORT ChildThread : public IPC::Listener, public IPC::Sender {
   // failure.
   // Note: On posix, this requires a sync IPC to the browser process,
   // but on windows the child process directly allocates the block.
-  base::SharedMemory* AllocateSharedMemory(size_t buf_size);
+  scoped_ptr<base::SharedMemory> AllocateSharedMemory(size_t buf_size);
 
   // A static variant that can be called on background threads provided
   // the |sender| passed in is safe to use on background threads.
-  static base::SharedMemory* AllocateSharedMemory(size_t buf_size,
-                                                  IPC::Sender* sender);
+  static scoped_ptr<base::SharedMemory> AllocateSharedMemory(
+      size_t buf_size,
+      IPC::Sender* sender);
 
   ChildSharedBitmapManager* shared_bitmap_manager() const {
     return shared_bitmap_manager_.get();
diff --git a/content/renderer/media/renderer_gpu_video_accelerator_factories.cc b/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
index f81a7b3..9c24682 100644
--- a/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
+++ b/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
@@ -235,14 +235,14 @@ void RendererGpuVideoAcceleratorFactories::ReadPixels(
   gl_helper->DeleteTexture(tmp_texture);
 }
 
-base::SharedMemory* RendererGpuVideoAcceleratorFactories::CreateSharedMemory(
-    size_t size) {
+scoped_ptr<base::SharedMemory>
+RendererGpuVideoAcceleratorFactories::CreateSharedMemory(size_t size) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   scoped_ptr<base::SharedMemory> mem(
       ChildThread::AllocateSharedMemory(size, thread_safe_sender_.get()));
   if (mem && !mem->Map(size))
     return nullptr;
-  return mem.release();
+  return mem;
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
diff --git a/content/renderer/media/renderer_gpu_video_accelerator_factories.h b/content/renderer/media/renderer_gpu_video_accelerator_factories.h
index 4723230..dccabfa 100644
--- a/content/renderer/media/renderer_gpu_video_accelerator_factories.h
+++ b/content/renderer/media/renderer_gpu_video_accelerator_factories.h
@@ -61,7 +61,7 @@ class CONTENT_EXPORT RendererGpuVideoAcceleratorFactories
   void ReadPixels(uint32 texture_id,
                   const gfx::Rect& visible_rect,
                   const SkBitmap& pixels) override;
-  base::SharedMemory* CreateSharedMemory(size_t size) override;
+  scoped_ptr<base::SharedMemory> CreateSharedMemory(size_t size) override;
   scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() override;
   std::vector<media::VideoEncodeAccelerator::SupportedProfile>
   GetVideoEncodeAcceleratorSupportedProfiles() override;
diff --git a/content/renderer/media/rtc_video_decoder.cc b/content/renderer/media/rtc_video_decoder.cc
index 60ae893..0cc66a7 100644
--- a/content/renderer/media/rtc_video_decoder.cc
+++ b/content/renderer/media/rtc_video_decoder.cc
@@ -48,16 +48,19 @@ static const size_t kMaxNumOfPendingBuffers = 300;
 // of |shm|.
 class RTCVideoDecoder::SHMBuffer {
  public:
-  SHMBuffer(base::SharedMemory* shm, size_t size);
+  SHMBuffer(scoped_ptr<base::SharedMemory> shm, size_t size);
   ~SHMBuffer();
-  base::SharedMemory* const shm;
+  scoped_ptr<base::SharedMemory> const shm;
   const size_t size;
 };
 
-RTCVideoDecoder::SHMBuffer::SHMBuffer(base::SharedMemory* shm, size_t size)
-    : shm(shm), size(size) {}
+RTCVideoDecoder::SHMBuffer::SHMBuffer(scoped_ptr<base::SharedMemory> shm,
+                                      size_t size)
+    : shm(shm.Pass()), size(size) {
+}
 
-RTCVideoDecoder::SHMBuffer::~SHMBuffer() { shm->Close(); }
+RTCVideoDecoder::SHMBuffer::~SHMBuffer() {
+}
 
 RTCVideoDecoder::BufferData::BufferData(int32 bitstream_buffer_id,
                                         uint32_t timestamp,
@@ -761,12 +764,13 @@ void RTCVideoDecoder::CreateSHM(int number, size_t min_size) {
   }
   size_t size_to_allocate = std::max(min_size, kSharedMemorySegmentBytes);
   for (int i = 0; i < number_to_allocate; i++) {
-    base::SharedMemory* shm = factories_->CreateSharedMemory(size_to_allocate);
-    if (shm != NULL) {
+    scoped_ptr<base::SharedMemory> shm =
+        factories_->CreateSharedMemory(size_to_allocate);
+    if (shm) {
       base::AutoLock auto_lock(lock_);
       num_shm_buffers_++;
       PutSHM_Locked(
-          scoped_ptr<SHMBuffer>(new SHMBuffer(shm, size_to_allocate)));
+          scoped_ptr<SHMBuffer>(new SHMBuffer(shm.Pass(), size_to_allocate)));
     }
   }
   // Kick off the decoding.
diff --git a/content/renderer/media/rtc_video_decoder_unittest.cc b/content/renderer/media/rtc_video_decoder_unittest.cc
index da1804d..dcb9c4f 100644
--- a/content/renderer/media/rtc_video_decoder_unittest.cc
+++ b/content/renderer/media/rtc_video_decoder_unittest.cc
@@ -39,8 +39,6 @@ class RTCVideoDecoderTest : public ::testing::Test,
         .WillRepeatedly(Return(vda_task_runner_));
     EXPECT_CALL(*mock_gpu_factories_.get(), DoCreateVideoDecodeAccelerator())
         .WillRepeatedly(Return(mock_vda_));
-    EXPECT_CALL(*mock_gpu_factories_.get(), CreateSharedMemory(_))
-        .WillRepeatedly(Return(static_cast<base::SharedMemory*>(NULL)));
     EXPECT_CALL(*mock_vda_, Initialize(_, _))
         .Times(1)
         .WillRepeatedly(Return(true));
diff --git a/content/renderer/media/rtc_video_encoder.cc b/content/renderer/media/rtc_video_encoder.cc
index e1119d9..c64e9df 100644
--- a/content/renderer/media/rtc_video_encoder.cc
+++ b/content/renderer/media/rtc_video_encoder.cc
@@ -356,7 +356,7 @@ void RTCVideoEncoder::Impl::RequireBitstreamBuffers(
   input_frame_coded_size_ = input_coded_size;
 
   for (unsigned int i = 0; i < input_count + kInputBufferExtraCount; ++i) {
-    base::SharedMemory* shm =
+    scoped_ptr<base::SharedMemory> shm =
         gpu_factories_->CreateSharedMemory(media::VideoFrame::AllocationSize(
             media::VideoFrame::I420, input_coded_size));
     if (!shm) {
@@ -365,12 +365,12 @@ void RTCVideoEncoder::Impl::RequireBitstreamBuffers(
       NOTIFY_ERROR(media::VideoEncodeAccelerator::kPlatformFailureError);
       return;
     }
-    input_buffers_.push_back(shm);
+    input_buffers_.push_back(shm.release());
     input_buffers_free_.push_back(i);
   }
 
   for (int i = 0; i < kOutputBufferCount; ++i) {
-    base::SharedMemory* shm =
+    scoped_ptr<base::SharedMemory> shm =
         gpu_factories_->CreateSharedMemory(output_buffer_size);
     if (!shm) {
       DLOG(ERROR) << "Impl::RequireBitstreamBuffers(): "
@@ -378,7 +378,7 @@ void RTCVideoEncoder::Impl::RequireBitstreamBuffers(
       NOTIFY_ERROR(media::VideoEncodeAccelerator::kPlatformFailureError);
       return;
     }
-    output_buffers_.push_back(shm);
+    output_buffers_.push_back(shm.release());
   }
 
   // Immediately provide all output buffers to the VEA.
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 0aa7e55..7193aa7 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -1010,8 +1010,7 @@ void RenderThreadImpl::RecordComputedAction(const std::string& action) {
 
 scoped_ptr<base::SharedMemory>
     RenderThreadImpl::HostAllocateSharedMemoryBuffer(size_t size) {
-  return make_scoped_ptr(
-      ChildThread::AllocateSharedMemory(size, thread_safe_sender()));
+  return ChildThread::AllocateSharedMemory(size, thread_safe_sender());
 }
 
 void RenderThreadImpl::RegisterExtension(v8::Extension* extension) {
diff --git a/content/renderer/renderer_clipboard_delegate.cc b/content/renderer/renderer_clipboard_delegate.cc
index e09807e..4a184bed 100644
--- a/content/renderer/renderer_clipboard_delegate.cc
+++ b/content/renderer/renderer_clipboard_delegate.cc
@@ -143,7 +143,7 @@ bool RendererClipboardDelegate::WriteImage(ui::ClipboardType clipboard_type,
 
     // Allocate a shared memory buffer to hold the bitmap bits.
     uint32 buf_size = checked_buf_size.ValueOrDie();
-    shared_buf.reset(ChildThread::current()->AllocateSharedMemory(buf_size));
+    shared_buf = ChildThread::current()->AllocateSharedMemory(buf_size);
     if (!shared_buf)
       return false;
     if (!shared_buf->Map(buf_size))
diff --git a/media/filters/gpu_video_accelerator_factories.h b/media/filters/gpu_video_accelerator_factories.h
index 6ed04c7..497e0cc 100644
--- a/media/filters/gpu_video_accelerator_factories.h
+++ b/media/filters/gpu_video_accelerator_factories.h
@@ -66,9 +66,8 @@ class MEDIA_EXPORT GpuVideoAcceleratorFactories
                           const gfx::Rect& visible_rect,
                           const SkBitmap& pixels) = 0;
 
-  // Allocate & return a shared memory segment.  Caller is responsible for
-  // Close()ing the returned pointer.
-  virtual base::SharedMemory* CreateSharedMemory(size_t size) = 0;
+  // Allocate & return a shared memory segment.
+  virtual scoped_ptr<base::SharedMemory> CreateSharedMemory(size_t size) = 0;
 
   // Returns the task runner the video accelerator runs on.
   virtual scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() = 0;
diff --git a/media/filters/gpu_video_decoder.cc b/media/filters/gpu_video_decoder.cc
index aea4816..45adef7 100644
--- a/media/filters/gpu_video_decoder.cc
+++ b/media/filters/gpu_video_decoder.cc
@@ -38,8 +38,9 @@ enum { kMaxInFlightDecodes = 4 };
 // be on the beefy side.
 static const size_t kSharedMemorySegmentBytes = 100 << 10;
 
-GpuVideoDecoder::SHMBuffer::SHMBuffer(base::SharedMemory* m, size_t s)
-    : shm(m), size(s) {
+GpuVideoDecoder::SHMBuffer::SHMBuffer(scoped_ptr<base::SharedMemory> m,
+                                      size_t s)
+    : shm(m.Pass()), size(s) {
 }
 
 GpuVideoDecoder::SHMBuffer::~SHMBuffer() {}
@@ -253,7 +254,7 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   }
 
   size_t size = buffer->data_size();
-  SHMBuffer* shm_buffer = GetSHM(size);
+  scoped_ptr<SHMBuffer> shm_buffer = GetSHM(size);
   if (!shm_buffer) {
     bound_decode_cb.Run(kDecodeError);
     return;
@@ -265,9 +266,9 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   // Mask against 30 bits, to avoid (undefined) wraparound on signed integer.
   next_bitstream_buffer_id_ = (next_bitstream_buffer_id_ + 1) & 0x3FFFFFFF;
   DCHECK(!ContainsKey(bitstream_buffers_in_decoder_, bitstream_buffer.id()));
-  bitstream_buffers_in_decoder_.insert(
-      std::make_pair(bitstream_buffer.id(),
-                     PendingDecoderBuffer(shm_buffer, buffer, decode_cb)));
+  bitstream_buffers_in_decoder_.insert(std::make_pair(
+      bitstream_buffer.id(),
+      PendingDecoderBuffer(shm_buffer.release(), buffer, decode_cb)));
   DCHECK_LE(static_cast<int>(bitstream_buffers_in_decoder_.size()),
             kMaxInFlightDecodes);
   RecordBufferData(bitstream_buffer, *buffer.get());
@@ -531,25 +532,27 @@ void GpuVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id) {
     vda_->ReusePictureBuffer(picture_buffer_id);
 }
 
-GpuVideoDecoder::SHMBuffer* GpuVideoDecoder::GetSHM(size_t min_size) {
+scoped_ptr<GpuVideoDecoder::SHMBuffer> GpuVideoDecoder::GetSHM(
+    size_t min_size) {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   if (available_shm_segments_.empty() ||
       available_shm_segments_.back()->size < min_size) {
     size_t size_to_allocate = std::max(min_size, kSharedMemorySegmentBytes);
-    base::SharedMemory* shm = factories_->CreateSharedMemory(size_to_allocate);
+    scoped_ptr<base::SharedMemory> shm =
+        factories_->CreateSharedMemory(size_to_allocate);
     // CreateSharedMemory() can return NULL during Shutdown.
     if (!shm)
       return NULL;
-    return new SHMBuffer(shm, size_to_allocate);
+    return make_scoped_ptr(new SHMBuffer(shm.Pass(), size_to_allocate));
   }
-  SHMBuffer* ret = available_shm_segments_.back();
+  scoped_ptr<SHMBuffer> ret(available_shm_segments_.back());
   available_shm_segments_.pop_back();
-  return ret;
+  return ret.Pass();
 }
 
-void GpuVideoDecoder::PutSHM(SHMBuffer* shm_buffer) {
+void GpuVideoDecoder::PutSHM(scoped_ptr<SHMBuffer> shm_buffer) {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
-  available_shm_segments_.push_back(shm_buffer);
+  available_shm_segments_.push_back(shm_buffer.release());
 }
 
 void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) {
@@ -564,7 +567,7 @@ void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) {
     return;
   }
 
-  PutSHM(it->second.shm_buffer);
+  PutSHM(make_scoped_ptr(it->second.shm_buffer));
   it->second.done_cb.Run(state_ == kError ? kDecodeError : kOk);
   bitstream_buffers_in_decoder_.erase(it);
 }
@@ -578,7 +581,6 @@ GpuVideoDecoder::~GpuVideoDecoder() {
   DCHECK(assigned_picture_buffers_.empty());
 
   for (size_t i = 0; i < available_shm_segments_.size(); ++i) {
-    available_shm_segments_[i]->shm->Close();
     delete available_shm_segments_[i];
   }
   available_shm_segments_.clear();
@@ -586,7 +588,7 @@ GpuVideoDecoder::~GpuVideoDecoder() {
   for (std::map<int32, PendingDecoderBuffer>::iterator it =
            bitstream_buffers_in_decoder_.begin();
        it != bitstream_buffers_in_decoder_.end(); ++it) {
-    it->second.shm_buffer->shm->Close();
+    delete it->second.shm_buffer;
     it->second.done_cb.Run(kAborted);
   }
   bitstream_buffers_in_decoder_.clear();
diff --git a/media/filters/gpu_video_decoder.h b/media/filters/gpu_video_decoder.h
index 65c85d5..a4157b1 100644
--- a/media/filters/gpu_video_decoder.h
+++ b/media/filters/gpu_video_decoder.h
@@ -79,9 +79,9 @@ class MEDIA_EXPORT GpuVideoDecoder
 
   // A shared memory segment and its allocated size.
   struct SHMBuffer {
-    SHMBuffer(base::SharedMemory* m, size_t s);
+    SHMBuffer(scoped_ptr<base::SharedMemory> m, size_t s);
     ~SHMBuffer();
-    base::SharedMemory* shm;
+    scoped_ptr<base::SharedMemory> shm;
     size_t size;
   };
 
@@ -118,11 +118,11 @@ class MEDIA_EXPORT GpuVideoDecoder
   void DestroyVDA();
 
   // Request a shared-memory segment of at least |min_size| bytes.  Will
-  // allocate as necessary.  Caller does not own returned pointer.
-  SHMBuffer* GetSHM(size_t min_size);
+  // allocate as necessary.
+  scoped_ptr<SHMBuffer> GetSHM(size_t min_size);
 
   // Return a shared-memory segment to the available pool.
-  void PutSHM(SHMBuffer* shm_buffer);
+  void PutSHM(scoped_ptr<SHMBuffer> shm_buffer);
 
   // Destroy all PictureBuffers in |buffers|, and delete their textures.
   void DestroyPictureBuffers(PictureBufferMap* buffers);
diff --git a/media/filters/mock_gpu_video_accelerator_factories.cc b/media/filters/mock_gpu_video_accelerator_factories.cc
index eeb3ba6..1c4465a 100644
--- a/media/filters/mock_gpu_video_accelerator_factories.cc
+++ b/media/filters/mock_gpu_video_accelerator_factories.cc
@@ -10,6 +10,11 @@ MockGpuVideoAcceleratorFactories::MockGpuVideoAcceleratorFactories() {}
 
 MockGpuVideoAcceleratorFactories::~MockGpuVideoAcceleratorFactories() {}
 
+scoped_ptr<base::SharedMemory>
+MockGpuVideoAcceleratorFactories::CreateSharedMemory(size_t size) {
+  return nullptr;
+}
+
 scoped_ptr<VideoDecodeAccelerator>
 MockGpuVideoAcceleratorFactories::CreateVideoDecodeAccelerator() {
   return scoped_ptr<VideoDecodeAccelerator>(DoCreateVideoDecodeAccelerator());
diff --git a/media/filters/mock_gpu_video_accelerator_factories.h b/media/filters/mock_gpu_video_accelerator_factories.h
index 949f35d..c06c6c4 100644
--- a/media/filters/mock_gpu_video_accelerator_factories.h
+++ b/media/filters/mock_gpu_video_accelerator_factories.h
@@ -43,11 +43,12 @@ class MockGpuVideoAcceleratorFactories : public GpuVideoAcceleratorFactories {
                void(uint32 texture_id,
                     const gfx::Rect& visible_rect,
                     const SkBitmap& pixels));
-  MOCK_METHOD1(CreateSharedMemory, base::SharedMemory*(size_t size));
   MOCK_METHOD0(GetTaskRunner, scoped_refptr<base::SingleThreadTaskRunner>());
   MOCK_METHOD0(GetVideoEncodeAcceleratorSupportedProfiles,
                std::vector<VideoEncodeAccelerator::SupportedProfile>());
 
+  scoped_ptr<base::SharedMemory> CreateSharedMemory(size_t size) override;
+
   virtual scoped_ptr<VideoDecodeAccelerator> CreateVideoDecodeAccelerator()
       override;