content: Cleanup GpuMemoryBuffer allocation.

content/common/gpu/client/gpu_memory_buffer_impl_shm.cc

Index: content/common/gpu/client/gpu_memory_buffer_impl_shm.cc
diff --git a/content/common/gpu/client/gpu_memory_buffer_impl_shm.cc b/content/common/gpu/client/gpu_memory_buffer_impl_shm.cc
index c550bf65f7ec2b660303dc5cf5bb1496127d48a8..f4779c9f36cca2a4fd41308f70e7d2ac3adc77e3 100644
--- a/content/common/gpu/client/gpu_memory_buffer_impl_shm.cc
+++ b/content/common/gpu/client/gpu_memory_buffer_impl_shm.cc
@@ -4,27 +4,52 @@
 
 #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) {
-  if (!base::SharedMemory::IsHandleValid(handle.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_.reset(new base::SharedMemory(handle.handle, false));
+  shared_memory_ = shared_memory.Pass();
   DCHECK(!shared_memory_->memory());
   return true;
 }
 
-bool GpuMemoryBufferImplShm::InitializeFromSharedMemory(
-    scoped_ptr<base::SharedMemory> shared_memory) {
-  shared_memory_ = shared_memory.Pass();
+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;
 }