GPU: 'Pass' SharedMemory when possible.

content/common/gpu/gpu_command_buffer_stub.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/hash.h"
 #include "base/memory/shared_memory.h"
@@ skipping 665 matching lines @@
   gpu::CommandBuffer::State pre_state = command_buffer_->GetLastState();
   command_buffer_->Flush(pre_state.put_offset);
   gpu::CommandBuffer::State post_state = command_buffer_->GetLastState();
 
   if (pre_state.get_offset != post_state.get_offset)
     ReportState();
 }
 
 void GpuCommandBufferStub::OnRegisterTransferBuffer(
     int32 id,
     base::SharedMemoryHandle transfer_buffer,

[epenner, 2014/03/25 22:14:52]

Since this is just a handle, the app will crash if we simply wrap it in a
SharedMemory and 'Pass' it. Duplicating here replaces the dupe the we used to do
in RegisterXferBuffer (with the exception that we at least don't need to dupe
handles in CreateXferBuffer anymore - we can just Create/Map/'Pass').

If we want to remove this duplication (and the one in mojo), I think I need to
understand these handles better. Given this function just gives us a handle, it
would make sense to me that we don't own it. For example since we duplicated it
before, this handle must be getting destroyed somewhere?

However if we do own it, perhaps we just need to 'Map' the memory here, like I
did in CreateTransferBuffer.

[epenner, 2014/03/25 22:25:37]

Hmm, is there even a possibility we were leaking file-descriptors here? If this
is legit file descriptor, we would need to close it if we don't use it. But I
don't see us doing that anywhere.

[epenner, 2014/03/25 23:02:13]

Actually, forget this I'm stupid :P. We were closing it before by passing the
handle to the SharedMemory, which then closes it in it's destructor. I'll update
this to simply Map/Pass, and we should be good.

     uint32 size) {
   TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnRegisterTransferBuffer");
+
+  // Duplicate the shared memory for this process.
   base::SharedMemory shared_memory(transfer_buffer, false);
+  base::SharedMemoryHandle duped_shared_memory_handle;
+  if (!shared_memory.ShareToProcess(base::GetCurrentProcessHandle(),
+                                    &duped_shared_memory_handle)) {
+    DVLOG(0) << "Failed to duplicate shared memory handle.";
+    return;
+  }
+  scoped_ptr<base::SharedMemory> duped_shared_memory(
+      new base::SharedMemory(duped_shared_memory_handle, false));
+
+  // Map the shared memory into this process. This validates the size.
+  if (!duped_shared_memory->Map(size)) {
+    DVLOG(0) << "Failed to map shared memory.";
+    return;
+  }
+
   if (command_buffer_)
-    command_buffer_->RegisterTransferBuffer(id, &shared_memory, size);
+    command_buffer_->RegisterTransferBuffer(
+        id, duped_shared_memory.Pass(), size);
 }
 
 void GpuCommandBufferStub::OnDestroyTransferBuffer(int32 id) {
   TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnDestroyTransferBuffer");
 
   if (command_buffer_)
     command_buffer_->DestroyTransferBuffer(id);
 }
 
 void GpuCommandBufferStub::OnGetTransferBuffer(
@@ skipping 275 matching lines @@
   if (decoder_)
     decoder_->LoseContext(GL_UNKNOWN_CONTEXT_RESET_ARB);
   command_buffer_->SetParseError(gpu::error::kLostContext);
 }
 
 uint64 GpuCommandBufferStub::GetMemoryUsage() const {
   return GetMemoryManager()->GetClientMemoryUsage(this);
 }
 
 }  // namespace content