GPU:'Pass' SharedMemory when possible.

This avoids some duplication of shared memory handles just
for the purposes of passing them around. Instead we Map
immediately when the memory arrives in the process, and then
'Pass' the shared memory from there.

BUG=177063, 353822

Review URL: https://codereview.chromium.org/211703003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259772 0039d316-1c4b-4281-b951-d872f2087c98

content/common/gpu/gpu_command_buffer_stub.cc

@@ -686,9 +686,18 @@ void GpuCommandBufferStub::OnRegisterTransferBuffer(
     base::SharedMemoryHandle transfer_buffer,
     uint32 size) {
   TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnRegisterTransferBuffer");
-  base::SharedMemory shared_memory(transfer_buffer, false);
+
+  // Take ownership of the memory and map it into this process.
+  // This validates the size.
+  scoped_ptr<base::SharedMemory> shared_memory(
+      new base::SharedMemory(transfer_buffer, false));
+  if (!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, shared_memory.Pass(), size);
 }
 
 void GpuCommandBufferStub::OnDestroyTransferBuffer(int32 id) {

gpu/command_buffer/common/buffer.cc

@@ -4,12 +4,16 @@
 
 #include "gpu/command_buffer/common/buffer.h"
 
+#include "base/logging.h"
+
 namespace gpu {
 
 Buffer::Buffer(scoped_ptr<base::SharedMemory> shared_memory, size_t size)
     : shared_memory_(shared_memory.Pass()),
       memory_(shared_memory_->memory()),
-      size_(size) {}
+      size_(size) {
+  DCHECK(memory_) << "The memory must be mapped to create a Buffer";
+}
 
 Buffer::~Buffer() {}
 

gpu/command_buffer/common/buffer.h

@@ -17,7 +17,7 @@ namespace base {
 
 namespace gpu {
 
-// Buffer/ThreadSafeBuffer own a piece of shared-memory of a certain size.
+// Buffer owns a piece of shared-memory of a certain size.
 class GPU_EXPORT Buffer : public base::RefCountedThreadSafe<Buffer> {
  public:
   Buffer(scoped_ptr<base::SharedMemory> shared_memory, size_t size);

gpu/command_buffer/service/command_buffer_service.cc

@@ -123,14 +123,14 @@ scoped_refptr<Buffer> CommandBufferService::CreateTransferBuffer(size_t size,
                                                                  int32* id) {
   *id = -1;
 
-  SharedMemory buffer;
-  if (!buffer.CreateAnonymous(size))
+  scoped_ptr<SharedMemory> shared_memory(new SharedMemory());
+  if (!shared_memory->CreateAndMapAnonymous(size))
     return NULL;
 
   static int32 next_id = 1;
   *id = next_id++;
 
-  if (!RegisterTransferBuffer(*id, &buffer, size)) {
+  if (!RegisterTransferBuffer(*id, shared_memory.Pass(), size)) {
     *id = -1;
     return NULL;
   }
@@ -155,11 +155,10 @@ scoped_refptr<Buffer> CommandBufferService::GetTransferBuffer(int32 id) {
 
 bool CommandBufferService::RegisterTransferBuffer(
     int32 id,
-    base::SharedMemory* shared_memory,
+    scoped_ptr<base::SharedMemory> shared_memory,
     size_t size) {
-  return transfer_buffer_manager_->RegisterTransferBuffer(id,
-                                                          shared_memory,
-                                                          size);
+  return transfer_buffer_manager_->RegisterTransferBuffer(
+      id, shared_memory.Pass(), size);
 }
 
 void CommandBufferService::SetToken(int32 token) {

gpu/command_buffer/service/command_buffer_service.h

@@ -65,7 +65,7 @@ class GPU_EXPORT CommandBufferService : public CommandBuffer {
   // to identify it in the command buffer. Callee dups the handle until
   // DestroyTransferBuffer is called.
   bool RegisterTransferBuffer(int32 id,
-                              base::SharedMemory* shared_memory,
+                              scoped_ptr<base::SharedMemory> shared_memory,
                               size_t size);
 
  private:

gpu/command_buffer/service/transfer_buffer_manager.cc

@@ -40,7 +40,7 @@ bool TransferBufferManager::Initialize() {
 
 bool TransferBufferManager::RegisterTransferBuffer(
     int32 id,
-    base::SharedMemory* shared_memory,
+    scoped_ptr<base::SharedMemory> shared_memory,
     size_t size) {
   if (id <= 0) {
     DVLOG(0) << "Cannot register transfer buffer with non-positive ID.";
@@ -53,25 +53,8 @@ bool TransferBufferManager::RegisterTransferBuffer(
     return false;
   }
 
-  // Duplicate the handle.
-  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 false;
-  }
-  scoped_ptr<SharedMemory> duped_shared_memory(
-      new 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 false;
-  }
-
-  // If it could be mapped register the shared memory with the ID.
-  scoped_refptr<Buffer> buffer =
-      make_scoped_refptr(new gpu::Buffer(duped_shared_memory.Pass(), size));
+  // Register the shared memory with the ID.
+  scoped_refptr<Buffer> buffer = new gpu::Buffer(shared_memory.Pass(), size);
 
   // Check buffer alignment is sane.
   DCHECK(!(reinterpret_cast<uintptr_t>(buffer->memory()) &

gpu/command_buffer/service/transfer_buffer_manager.h

@@ -20,9 +20,10 @@ class GPU_EXPORT TransferBufferManagerInterface {
  public:
   virtual ~TransferBufferManagerInterface();
 
-  virtual bool RegisterTransferBuffer(int32 id,
-                                      base::SharedMemory* shared_memory,
-                                      size_t size) = 0;
+  virtual bool RegisterTransferBuffer(
+      int32 id,
+      scoped_ptr<base::SharedMemory> shared_memory,
+      size_t size) = 0;
   virtual void DestroyTransferBuffer(int32 id) = 0;
   virtual scoped_refptr<Buffer> GetTransferBuffer(int32 id) = 0;
 };
@@ -33,9 +34,10 @@ class GPU_EXPORT TransferBufferManager
   TransferBufferManager();
 
   bool Initialize();
-  virtual bool RegisterTransferBuffer(int32 id,
-                                      base::SharedMemory* shared_memory,
-                                      size_t size) OVERRIDE;
+  virtual bool RegisterTransferBuffer(
+      int32 id,
+      scoped_ptr<base::SharedMemory> shared_memory,
+      size_t size) OVERRIDE;
   virtual void DestroyTransferBuffer(int32 id) OVERRIDE;
   virtual scoped_refptr<Buffer> GetTransferBuffer(int32 id) OVERRIDE;
 

gpu/command_buffer/service/transfer_buffer_manager_unittest.cc

@@ -17,17 +17,11 @@ const static size_t kBufferSize = 1024;
 class TransferBufferManagerTest : public testing::Test {
  protected:
   virtual void SetUp() {
-    for (size_t i = 0; i < arraysize(buffers_); ++i) {
-      buffers_[i].CreateAnonymous(kBufferSize);
-      buffers_[i].Map(kBufferSize);
-    }
-
     TransferBufferManager* manager = new TransferBufferManager();
     transfer_buffer_manager_.reset(manager);
     ASSERT_TRUE(manager->Initialize());
   }
 
-  base::SharedMemory buffers_[3];
   scoped_ptr<TransferBufferManagerInterface> transfer_buffer_manager_;
 };
 
@@ -44,30 +38,23 @@ TEST_F(TransferBufferManagerTest, OutOfRangeHandleMapsToNull) {
 }
 
 TEST_F(TransferBufferManagerTest, CanRegisterTransferBuffer) {
-  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                               &buffers_[0],
-                                                               kBufferSize));
+  scoped_ptr<base::SharedMemory> shm(new base::SharedMemory());
+  shm->CreateAndMapAnonymous(kBufferSize);
+  base::SharedMemory* shm_raw_pointer = shm.get();
+  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm.Pass(), kBufferSize));
   scoped_refptr<Buffer> registered =
       transfer_buffer_manager_->GetTransferBuffer(1);
 
-  // Distinct memory range and shared memory handle from that originally
-  // registered.
-  scoped_refptr<Buffer> null_buffer;
-  EXPECT_NE(null_buffer, registered);
-  EXPECT_NE(buffers_[0].memory(), registered->memory());
-  EXPECT_EQ(kBufferSize, registered->size());
-  EXPECT_NE(&buffers_[0], registered->shared_memory());
-
-  // But maps to the same physical memory.
-  *static_cast<int*>(registered->memory()) = 7;
-  *static_cast<int*>(buffers_[0].memory()) = 8;
-  EXPECT_EQ(8, *static_cast<int*>(registered->memory()));
+  // Shared-memory ownership is transfered. It should be the same memory.
+  EXPECT_EQ(shm_raw_pointer, registered->shared_memory());
 }
 
 TEST_F(TransferBufferManagerTest, CanDestroyTransferBuffer) {
-  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                               &buffers_[0],
-                                                               kBufferSize));
+  scoped_ptr<base::SharedMemory> shm(new base::SharedMemory());
+  shm->CreateAndMapAnonymous(kBufferSize);
+  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm.Pass(), kBufferSize));
   transfer_buffer_manager_->DestroyTransferBuffer(1);
   scoped_refptr<Buffer> registered =
       transfer_buffer_manager_->GetTransferBuffer(1);
@@ -77,41 +64,49 @@ TEST_F(TransferBufferManagerTest, CanDestroyTransferBuffer) {
 }
 
 TEST_F(TransferBufferManagerTest, CannotRegregisterTransferBufferId) {
-  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                               &buffers_[0],
-                                                               kBufferSize));
-  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                                &buffers_[0],
-                                                                kBufferSize));
-  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                                &buffers_[1],
-                                                                kBufferSize));
+  scoped_ptr<base::SharedMemory> shm1(new base::SharedMemory());
+  scoped_ptr<base::SharedMemory> shm2(new base::SharedMemory());
+  scoped_ptr<base::SharedMemory> shm3(new base::SharedMemory());
+  shm1->CreateAndMapAnonymous(kBufferSize);
+  shm2->CreateAndMapAnonymous(kBufferSize);
+  shm3->CreateAndMapAnonymous(kBufferSize);
+
+  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm1.Pass(), kBufferSize));
+  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm2.Pass(), kBufferSize));
+  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm3.Pass(), kBufferSize));
 }
 
 TEST_F(TransferBufferManagerTest, CanReuseTransferBufferIdAfterDestroying) {
-  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                               &buffers_[0],
-                                                               kBufferSize));
+  scoped_ptr<base::SharedMemory> shm1(new base::SharedMemory());
+  scoped_ptr<base::SharedMemory> shm2(new base::SharedMemory());
+  shm1->CreateAndMapAnonymous(kBufferSize);
+  shm2->CreateAndMapAnonymous(kBufferSize);
+  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm1.Pass(), kBufferSize));
   transfer_buffer_manager_->DestroyTransferBuffer(1);
-  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1,
-                                                               &buffers_[1],
-                                                               kBufferSize));
+  EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(
+      1, shm2.Pass(), kBufferSize));
 }
 
 TEST_F(TransferBufferManagerTest, DestroyUnusedTransferBufferIdDoesNotCrash) {
   transfer_buffer_manager_->DestroyTransferBuffer(1);
 }
 
 TEST_F(TransferBufferManagerTest, CannotRegisterNullTransferBuffer) {
-  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(0,
-                                                                &buffers_[0],
-                                                                kBufferSize));
+  scoped_ptr<base::SharedMemory> shm(new base::SharedMemory());
+  shm->CreateAndMapAnonymous(kBufferSize);
+  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(
+      0, shm.Pass(), kBufferSize));
 }
 
 TEST_F(TransferBufferManagerTest, CannotRegisterNegativeTransferBufferId) {
-  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(-1,
-                                                                &buffers_[0],
-                                                                kBufferSize));
+  scoped_ptr<base::SharedMemory> shm(new base::SharedMemory());
+  shm->CreateAndMapAnonymous(kBufferSize);
+  EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(
+      -1, shm.Pass(), kBufferSize));
 }
 
 }  // namespace gpu

mojo/services/gles2/command_buffer_impl.cc

@@ -143,9 +143,16 @@ void CommandBufferImpl::MakeProgress(int32_t last_get_offset) {
 void CommandBufferImpl::RegisterTransferBuffer(int32_t id,
                                                const ShmHandle& transfer_buffer,
                                                uint32_t size) {
-  bool read_only = false;
-  base::SharedMemory shared_memory(transfer_buffer, read_only);
-  command_buffer_->RegisterTransferBuffer(id, &shared_memory, size);
+  // Take ownership of the memory and map it into this process.
+  // This validates the size.
+  scoped_ptr<base::SharedMemory> shared_memory(
+      new base::SharedMemory(transfer_buffer, false));
+  if (!shared_memory->Map(size)) {
+    DVLOG(0) << "Failed to map shared memory.";
+    return;
+  }
+
+  command_buffer_->RegisterTransferBuffer(id, shared_memory.Pass(), size);
 }
 
 void CommandBufferImpl::DestroyTransferBuffer(int32_t id) {