Add GenericSharedMemoryId and use w/ GpuMemoryBuffer

content/browser/gpu/browser_gpu_memory_buffer_manager.cc

 // Copyright 2014 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/browser/gpu/browser_gpu_memory_buffer_manager.h"
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/common/child_process_host_impl.h"
+#include "content/common/generic_shared_memory_id_generator.h"
 #include "content/common/gpu/client/gpu_memory_buffer_impl.h"
 #include "content/common/gpu/client/gpu_memory_buffer_impl_shared_memory.h"
 #include "content/common/gpu/gpu_memory_buffer_factory_shared_memory.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/GLES2/gl2extchromium.h"
 
 #if defined(OS_MACOSX)
 #include "content/common/gpu/gpu_memory_buffer_factory_io_surface.h"
 #endif
@@ skipping 86 matching lines @@
     if (IsGpuMemoryBufferFactoryConfigurationSupported(type, configuration))
       configurations.push_back(configuration);
   }
 #endif
 
   return configurations;
 }
 
 BrowserGpuMemoryBufferManager* g_gpu_memory_buffer_manager = nullptr;
 
-// Global atomic to generate gpu memory buffer unique IDs.
-base::StaticAtomicSequenceNumber g_next_gpu_memory_buffer_id;
-
 }  // namespace
 
 struct BrowserGpuMemoryBufferManager::AllocateGpuMemoryBufferRequest {
   AllocateGpuMemoryBufferRequest(const gfx::Size& size,
                                  gfx::BufferFormat format,
                                  gfx::BufferUsage usage,
                                  int client_id,
                                  int surface_id)
       : event(true, false),
         size(size),
@@ skipping 70 matching lines @@
 BrowserGpuMemoryBufferManager::AllocateGpuMemoryBufferForScanout(
     const gfx::Size& size,
     gfx::BufferFormat format,
     int32 surface_id) {
   DCHECK_GT(surface_id, 0);
   return AllocateGpuMemoryBufferForSurface(
       size, format, gfx::BufferUsage::SCANOUT, surface_id);
 }
 
 void BrowserGpuMemoryBufferManager::AllocateGpuMemoryBufferForChildProcess(
+    gfx::GpuMemoryBufferId id,
     const gfx::Size& size,
     gfx::BufferFormat format,
     gfx::BufferUsage usage,
     base::ProcessHandle child_process_handle,
     int child_client_id,
     const AllocationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  gfx::GpuMemoryBufferId new_id = g_next_gpu_memory_buffer_id.GetNext();
-
   // Use service side allocation if this is a supported configuration.
   if (IsGpuMemoryBufferConfigurationSupported(format, usage)) {
-    AllocateGpuMemoryBufferOnIO(new_id, size, format, usage, child_client_id, 0,
+    AllocateGpuMemoryBufferOnIO(id, size, format, usage, child_client_id, 0,
                                 false, callback);
     return;
   }
 
   // Early out if we cannot fallback to shared memory buffer.
   if (!GpuMemoryBufferImplSharedMemory::IsFormatSupported(format) ||
       !GpuMemoryBufferImplSharedMemory::IsUsageSupported(usage) ||
       !GpuMemoryBufferImplSharedMemory::IsSizeValidForFormat(size, format)) {
     callback.Run(gfx::GpuMemoryBufferHandle());
     return;
   }
 
   BufferMap& buffers = clients_[child_client_id];
-  DCHECK(buffers.find(new_id) == buffers.end());
+  if (buffers.find(id) != buffers.end()) {

[reveman, 2015/08/06 19:19:36]

nit: please save the iterator and use it to set BufferInfo below to avoid an
unnecessary map lookup in release code.

[ericrk, 2015/08/06 22:05:19]

At this point iterator shouldn't exist (we expect it to be buffers.end()). That
doesn't really have any re-use value?

+    LOG(ERROR) << "Client process attempted to allocate a GpuMemoryBuffer with "
+                  "an existing ID. Failing allocation.";
+    callback.Run(gfx::GpuMemoryBufferHandle());
+    return;
+  }
 
   // Allocate shared memory buffer as fallback.
-  buffers[new_id] =
-      BufferInfo(size, gfx::SHARED_MEMORY_BUFFER, format, usage, 0);
+  buffers[id] = BufferInfo(size, gfx::SHARED_MEMORY_BUFFER, format, usage, 0);
   callback.Run(GpuMemoryBufferImplSharedMemory::AllocateForChildProcess(
-      new_id, size, format, child_process_handle));
+      id, size, format, child_process_handle));
 }
 
 gfx::GpuMemoryBuffer*
 BrowserGpuMemoryBufferManager::GpuMemoryBufferFromClientBuffer(
     ClientBuffer buffer) {
   return GpuMemoryBufferImpl::FromClientBuffer(buffer);
 }
 
 void BrowserGpuMemoryBufferManager::SetDestructionSyncPoint(
     gfx::GpuMemoryBuffer* buffer,
@@ skipping 36 matching lines @@
       // corresponding dump for the same buffer, this will avoid to
       // double-count them in tracing. If, instead, no other process will emit a
       // dump with the same guid, the segment will be accounted to the browser.
       uint64 client_tracing_process_id = ClientIdToTracingProcessId(client_id);
 
       base::trace_event::MemoryAllocatorDumpGuid shared_buffer_guid =
           gfx::GetGpuMemoryBufferGUIDForTracing(client_tracing_process_id,
                                                 buffer_id);
       pmd->CreateSharedGlobalAllocatorDump(shared_buffer_guid);
       pmd->AddOwnershipEdge(dump->guid(), shared_buffer_guid);
+
+      if (buffer.second.type ==
+          gfx::GpuMemoryBufferType::SHARED_MEMORY_BUFFER) {
+        // Normally, the GpuMemoryBufferFactory handles establishing the link
+        // between a GpuMemoryBufferId and any GenericSharedMemoryId. However,
+        // there is no manager for SHARED_MEMORY_BUFFER buffers, so handle
+        // this here.

[reveman, 2015/08/06 19:19:36]

I was thinking that establishing the link is still up to the GMB manager code
here and all the factory does is make sure there's a guid for whatever memory is
really backing the buffer. Does that make sense?

[ericrk, 2015/08/06 22:05:19]

Using same GUID for both GpuMemoryBuffers and GenericSharedMemory, so edge is no
longer necessary. pulling this.

+        GpuMemoryBufferImplSharedMemory::DumpGenericSharedMemory(
+            pmd, client_tracing_process_id, buffer_id);

[reveman, 2015/08/06 19:19:36]

Can we move this into a static function as part
GpuMemoryBufferFactorySharedMemory? that way we can easily have the shared
memory factory that is currently only used for testing purposes do the right
thing if ever use it on the gpu process side.

[ericrk, 2015/08/06 22:05:19]

Pulled this for now, will re-add generic memory dumping in next CL.

+      }
     }
   }
 
   return true;
 }
 
 void BrowserGpuMemoryBufferManager::ChildProcessDeletedGpuMemoryBuffer(
     gfx::GpuMemoryBufferId id,
     base::ProcessHandle child_process_handle,
     int child_client_id,
@@ skipping 60 matching lines @@
     if (configuration.format == format && configuration.usage == usage)
       return true;
   }
   return false;
 }
 
 void BrowserGpuMemoryBufferManager::AllocateGpuMemoryBufferForSurfaceOnIO(
     AllocateGpuMemoryBufferRequest* request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  gfx::GpuMemoryBufferId new_id = g_next_gpu_memory_buffer_id.GetNext();
+  gfx::GpuMemoryBufferId new_id = content::GetNextGenericSharedMemoryId();
 
   // Use service side allocation if this is a supported configuration.
   if (IsGpuMemoryBufferConfigurationSupported(request->format,
                                               request->usage)) {
     // Note: Unretained is safe as this is only used for synchronous allocation
     // from a non-IO thread.
     AllocateGpuMemoryBufferOnIO(
         new_id, request->size, request->format, request->usage,
         request->client_id, request->surface_id, false,
         base::Bind(&BrowserGpuMemoryBufferManager::
@@ skipping 53 matching lines @@
     const gfx::Size& size,
     gfx::BufferFormat format,
     gfx::BufferUsage usage,
     int client_id,
     int surface_id,
     bool reused_gpu_process,
     const AllocationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   BufferMap& buffers = clients_[client_id];
-  DCHECK(buffers.find(id) == buffers.end());
+  if (buffers.find(id) != buffers.end()) {

[reveman, 2015/08/06 19:19:36]

same here. can we avoid two lookups?

[ericrk, 2015/08/06 22:05:19]

Same comment as above.

+    LOG(ERROR) << "Client process attempted to allocate a GpuMemoryBuffer with "
+                  "an existing ID. Failing allocation.";
+    callback.Run(gfx::GpuMemoryBufferHandle());
+    return;
+  }
 
   GpuProcessHost* host = GpuProcessHost::FromID(gpu_host_id_);
   if (!host) {
     host = GpuProcessHost::Get(GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED,
                                CAUSE_FOR_GPU_LAUNCH_GPU_MEMORY_BUFFER_ALLOCATE);
     if (!host) {
       LOG(ERROR) << "Failed to launch GPU process.";
       callback.Run(gfx::GpuMemoryBufferHandle());
       return;
     }
@@ skipping 128 matching lines @@
     return gpu_client_tracing_id_;
   }
 
   // In normal cases, |client_id| is a child process id, so we can perform
   // the standard conversion.
   return ChildProcessHostImpl::ChildProcessUniqueIdToTracingProcessId(
       client_id);
 }
 
 }  // namespace content