Browser side OZONE_NATIVE_BUFFER allocation

content/common/gpu/client/gpu_memory_buffer_impl.h

 // 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.
 
 #ifndef CONTENT_COMMON_GPU_CLIENT_GPU_MEMORY_BUFFER_IMPL_H_
 #define CONTENT_COMMON_GPU_CLIENT_GPU_MEMORY_BUFFER_IMPL_H_
 
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 #include "ui/gfx/size.h"
 
 namespace content {
+class GpuMemoryBufferFactoryHost;
 
 // Provides common implementation of a GPU memory buffer.
 class GpuMemoryBufferImpl : public gfx::GpuMemoryBuffer {
  public:
   typedef base::Callback<void(const gfx::GpuMemoryBufferHandle& handle)>
       AllocationCallback;
 
   virtual ~GpuMemoryBufferImpl();
 
   // Creates a GPU memory buffer instance with |size| and |internalformat| for
   // |usage|.
   static scoped_ptr<GpuMemoryBufferImpl> Create(const gfx::Size& size,
                                                 unsigned internalformat,
                                                 unsigned usage);
 
   // Allocates a GPU memory buffer with |size| and |internalformat| for |usage|
   // by |child_process|. The |handle| returned can be used by the
   // |child_process| to create an instance of this class.
+  // The buffer is being allocated for |child_id| and |factory_host| can
+  // be used to talk to the allocator.

[reveman, 2014/08/13 08:03:11]

Can you merge the mention of |child_id| with the text above instead? ie. "for
|usage| by |child_process| with |child_id|"...

[achaulk, 2014/08/13 15:53:19]

Done.

   static void AllocateForChildProcess(const gfx::Size& size,
                                       unsigned internalformat,
                                       unsigned usage,
                                       base::ProcessHandle child_process,
+                                      int child_id,
+                                      GpuMemoryBufferFactoryHost* factory_host,

[reveman, 2014/08/13 08:03:11]

It's weird that we pass a factory to this function. Makes it seem like we might
need to be able to pass different factory instances to this function. The use of
the factory is supposed to be an implementation detail of this function. I would
prefer if we add a GpuMemoryBufferFactoryHost::GetInstance() function that can
be used when necessary instead. If we want to avoid global variables, then we'd
need to add a GpuMemoryBufferImplFactory and pass the FactoryHost to the ctor of
this new factory instance. Adding GpuMemoryBufferFactoryHost::GetInstance()
function is fine with me for now though.

[achaulk, 2014/08/13 15:53:19]

Sure, I can do that. The only implementation is a singleton anyway.

                                       const AllocationCallback& callback);
 
   // Notify that GPU memory buffer has been deleted by |child_process|.
   static void DeletedByChildProcess(gfx::GpuMemoryBufferType type,
                                     const gfx::GpuMemoryBufferId& id,
                                     base::ProcessHandle child_process);
 
   // Creates an instance from the given |handle|. |size| and |internalformat|
   // should match what was used to allocate the |handle|.
   static scoped_ptr<GpuMemoryBufferImpl> CreateFromHandle(
@@ skipping 20 matching lines @@
   const gfx::Size size_;
   const unsigned internalformat_;
   bool mapped_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_COMMON_GPU_CLIENT_GPU_MEMORY_BUFFER_IMPL_H_