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..1b400677261af78f8ee5bfa1fea7cedba4e4d3da 100644
--- a/content/common/gpu/client/gpu_memory_buffer_impl_shm.cc
+++ b/content/common/gpu/client/gpu_memory_buffer_impl_shm.cc
@@ -5,26 +5,61 @@
 #include "content/common/gpu/client/gpu_memory_buffer_impl_shm.h"
 
 #include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
+#include "content/common/child_process_host_impl.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) {
+  if (!IsFormatValid(internalformat)) {
+    handle->type = gfx::EMPTY_BUFFER;
+    return;
+  }
+
+  uint64 stride =
+      static_cast<uint64>(size.width()) * BytesPerPixel(internalformat);

[piman, 2014/05/02 22:26:51]

use base::CheckedNumeric here and below

[reveman, 2014/05/03 00:59:48]

Done. And moved some of these checks to where I think they really belong.

+  if (!base::IsValueInRangeForNumericType<uint32>(stride)) {
+    handle->type = gfx::EMPTY_BUFFER;
+    return;
+  }
+
+  uint64 buffer_size = stride * static_cast<uint64>(size.height());
+  if (!base::IsValueInRangeForNumericType<size_t>(buffer_size)) {
+    handle->type = gfx::EMPTY_BUFFER;
+    return;
+  }
+
+  handle->type = gfx::SHARED_MEMORY_BUFFER;
+  ChildProcessHostImpl::AllocateSharedMemory(

[piman, 2014/05/02 22:26:51]

This /looks/ wrong, because this class is in a directory that's on the client
side, and we're calling a host side function. I guess because
ChildProcessHostImpl is in content/common/ this doesn't cause deps issues?

In the future I was hoping to move some of the content/common/gpu/client classes
out of content/ and into gpu/ipc or something - so that I can use them from
ppapi/proxy, that also runs in NaCl and can't depend on content/.

It's kinda unfortunate to have both the host and client side in the same file,
because that prevents us from doing it (the host side will never work in nacl).
Do you have an idea how we should do that?

[reveman, 2014/05/03 00:59:48]

Yes, we shouldn't be using ChildProcessHostImpl here. I removed it from latest
patch.
Makes sense. I'll probably add some host interface to support GPU process side
allocation. I'm not sure exactly what that will look like but I'll make sure all
dependencies on content/ are properly separated from common/client code.

+      static_cast<size_t>(buffer_size), child_process, &handle->handle);
+}
+
+bool GpuMemoryBufferImplShm::Initialize() {
+  size_t size = size_.GetArea() * BytesPerPixel(internalformat_);
+  scoped_ptr<base::SharedMemory> shared_memory(new base::SharedMemory());
+  if (!shared_memory->CreateAnonymous(size))
     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;
 }