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_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) {
+// 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_ = 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