content: Cleanup GpuMemoryBuffer allocation.

content/common/gpu/client/gpu_memory_buffer_impl_shm.cc

 // Copyright 2013 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/common/gpu/client/gpu_memory_buffer_impl_shm.h"
 
-#include "base/logging.h"
+#include "base/numerics/safe_math.h"
 
 namespace content {
 
-GpuMemoryBufferImplShm::GpuMemoryBufferImplShm(gfx::Size size,
+GpuMemoryBufferImplShm::GpuMemoryBufferImplShm(const gfx::Size& size,
                                                unsigned internalformat)
     : GpuMemoryBufferImpl(size, internalformat) {}
 
 GpuMemoryBufferImplShm::~GpuMemoryBufferImplShm() {}
 
-bool GpuMemoryBufferImplShm::Initialize(gfx::GpuMemoryBufferHandle handle) {
+// static
+void GpuMemoryBufferImplShm::AllocateSharedMemoryForChildProcess(
+    const gfx::Size& size,
+    unsigned internalformat,
+    base::ProcessHandle child_process,
+    gfx::GpuMemoryBufferHandle* handle) {
+  base::CheckedNumeric<size_t> buffer_size = size.GetArea();

[piman, 2014/05/05 19:23:51]

Actually, GetArea can overflow too (int*int -> int).

[danakj, 2014/05/05 19:27:25]

We have some various helpers here:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/resources/share...

[reveman, 2014/05/05 20:41:23]

I made the caller responsible for passing a valid gfx::Size (where GetArea
doesn't overflow) to GpuMemoryBufferImpl::AllocateForChildProcess. See line 1265
in render_message_filter.cc. I think this more appropriate than having to check
it here.

[piman, 2014/05/05 21:02:16]

It's not obvious that it's the only caller (now, future...). Is there a way to
add a DCHECK to make it an explicit precondition?

[reveman, 2014/05/06 12:26:27]

Added a IsConfigurationSupported() function to latest patch and I'm now
verifying and failing properly in both AllocateForChildProcess() and
Initialize() instead of making the assumption that only one might be used in the
browser. IsConfigurationSupported doesn't make the assumption that GetArea will
not overflow.

+  buffer_size *= BytesPerPixel(internalformat);
+  if (!buffer_size.IsValid()) {
+    handle->type = gfx::EMPTY_BUFFER;
+    return;
+  }
+  base::SharedMemory shared_memory;
+  if (!shared_memory.CreateAnonymous(buffer_size.ValueOrDie())) {
+    handle->type = gfx::EMPTY_BUFFER;
+    return;
+  }
+  handle->type = gfx::SHARED_MEMORY_BUFFER;
+  shared_memory.GiveToProcess(child_process, &handle->handle);
+}
+
+bool GpuMemoryBufferImplShm::Initialize() {
+  scoped_ptr<base::SharedMemory> shared_memory(new base::SharedMemory());
+  if (!shared_memory->CreateAnonymous(size_.GetArea() *
+                                      BytesPerPixel(internalformat_)))
+    return false;
+  shared_memory_ = shared_memory.Pass();
+  DCHECK(!shared_memory_->memory());
+  return true;
+}
+
+bool GpuMemoryBufferImplShm::InitializeFromHandle(
+    gfx::GpuMemoryBufferHandle handle) {
   if (!base::SharedMemory::IsHandleValid(handle.handle))
     return false;
   shared_memory_.reset(new base::SharedMemory(handle.handle, false));
   DCHECK(!shared_memory_->memory());
   return true;
 }
-
-bool GpuMemoryBufferImplShm::InitializeFromSharedMemory(
-    scoped_ptr<base::SharedMemory> shared_memory) {
-  shared_memory_ = shared_memory.Pass();
-  DCHECK(!shared_memory_->memory());
-  return true;
-}
 
 void* GpuMemoryBufferImplShm::Map(AccessMode mode) {
   DCHECK(!mapped_);
   if (!shared_memory_->Map(size_.GetArea() * BytesPerPixel(internalformat_)))
     return NULL;
   mapped_ = true;
   return shared_memory_->memory();
 }
 
 void GpuMemoryBufferImplShm::Unmap() {
   DCHECK(mapped_);
   shared_memory_->Unmap();
   mapped_ = false;
 }
 
 gfx::GpuMemoryBufferHandle GpuMemoryBufferImplShm::GetHandle() const {
   gfx::GpuMemoryBufferHandle handle;
   handle.type = gfx::SHARED_MEMORY_BUFFER;
   handle.handle = shared_memory_->handle();
   return handle;
 }
 
 }  // namespace content