diff options
29 files changed, 447 insertions, 313 deletions
diff --git a/content/common/gpu/client/command_buffer_proxy_impl.cc b/content/common/gpu/client/command_buffer_proxy_impl.cc index 6f3d234..c60d84c 100644 --- a/content/common/gpu/client/command_buffer_proxy_impl.cc +++ b/content/common/gpu/client/command_buffer_proxy_impl.cc @@ -52,7 +52,7 @@ CommandBufferProxyImpl::~CommandBufferProxyImpl() { it->second.shared_memory = NULL; } for (Decoders::iterator it = video_decoder_hosts_.begin(); - it != video_decoder_hosts_.end(); ++it) { + it != video_decoder_hosts_.end(); ++it) { if (it->second) it->second->OnChannelError(); } @@ -62,6 +62,8 @@ bool CommandBufferProxyImpl::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(CommandBufferProxyImpl, message) IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_Destroyed, OnDestroyed); + IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_NotifyRepaint, + OnNotifyRepaint); IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_EchoAck, OnEchoAck); IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_ConsoleMsg, OnConsoleMessage); IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_SetMemoryAllocation, @@ -151,18 +153,22 @@ bool CommandBufferProxyImpl::Initialize() { return false; } - gpu::Buffer buffer = CreateTransferBuffer(sizeof(*shared_state_), - &state_buffer_); + state_buffer_ = CreateTransferBuffer(sizeof *shared_state_, -1); + + if (state_buffer_ == -1) { + LOG(ERROR) << "Failed to create shared state transfer buffer."; + return false; + } + + gpu::Buffer buffer = GetTransferBuffer(state_buffer_); if (!buffer.ptr) { - LOG(ERROR) << "Failed to allocate shared state buffer."; + LOG(ERROR) << "Failed to get shared state transfer buffer"; return false; } shared_state_ = reinterpret_cast<gpu::CommandBufferSharedState*>(buffer.ptr); shared_state_->Initialize(); - // TODO(apatrick): Eliminate this. Pass the shared state buffer handle - // via the Initialize IPC. if (!Send(new GpuCommandBufferMsg_SetSharedStateBuffer(route_id_, state_buffer_))) { LOG(ERROR) << "Failed to initialize shared command buffer state."; @@ -241,53 +247,70 @@ void CommandBufferProxyImpl::SetGetOffset(int32 get_offset) { NOTREACHED(); } -gpu::Buffer CommandBufferProxyImpl::CreateTransferBuffer(size_t size, - int32* id) { - *id = -1; - +int32 CommandBufferProxyImpl::CreateTransferBuffer( + size_t size, int32 id_request) { if (last_state_.error != gpu::error::kNoError) - return gpu::Buffer(); + return -1; - int32 new_id = channel_->ReserveTransferBufferId(); - DCHECK(transfer_buffers_.find(new_id) == transfer_buffers_.end()); - - scoped_ptr<base::SharedMemory> shared_memory( + // Take ownership of shared memory. This will close the handle if Send below + // fails. Otherwise, callee takes ownership before this variable + // goes out of scope by duping the handle. + scoped_ptr<base::SharedMemory> shm( channel_->factory()->AllocateSharedMemory(size)); - if (!shared_memory.get()) - return gpu::Buffer(); - - if (!shared_memory->memory()) { - if (!shared_memory->Map(size)) - return Buffer(); - } + if (!shm.get()) + return -1; - // Duplicate handle to the GPU process. - base::SharedMemoryHandle handle = shared_memory->handle(); + base::SharedMemoryHandle handle = shm->handle(); #if defined(OS_WIN) // Windows needs to explicitly duplicate the handle out to another process. if (!BrokerDuplicateHandle(handle, channel_->gpu_pid(), &handle, FILE_MAP_WRITE, 0)) { - return Buffer(); + return -1; } -#else - shared_memory->ShareToProcess(channel_->gpu_pid(), &handle); +#elif defined(OS_POSIX) + DCHECK(!handle.auto_close); #endif + int32 id; if (!Send(new GpuCommandBufferMsg_RegisterTransferBuffer(route_id_, - new_id, handle, - size))) { - return Buffer(); + size, + id_request, + &id))) { + return -1; } - *id = new_id; - Buffer buffer; - buffer.ptr = shared_memory->memory(); - buffer.size = size; - buffer.shared_memory = shared_memory.release(); - transfer_buffers_[new_id] = buffer; + return id; +} - return buffer; +int32 CommandBufferProxyImpl::RegisterTransferBuffer( + base::SharedMemory* shared_memory, + size_t size, + int32 id_request) { + if (last_state_.error != gpu::error::kNoError) + return -1; + + // Returns FileDescriptor with auto_close off. + base::SharedMemoryHandle handle = shared_memory->handle(); +#if defined(OS_WIN) + // Windows needs to explicitly duplicate the handle out to another process. + if (!BrokerDuplicateHandle(handle, channel_->gpu_pid(), &handle, + FILE_MAP_WRITE, 0)) { + return -1; + } +#endif + + int32 id; + if (!Send(new GpuCommandBufferMsg_RegisterTransferBuffer( + route_id_, + handle, + size, + id_request, + &id))) { + return -1; + } + + return id; } void CommandBufferProxyImpl::DestroyTransferBuffer(int32 id) { @@ -327,19 +350,20 @@ Buffer CommandBufferProxyImpl::GetTransferBuffer(int32 id) { } // Cache the transfer buffer shared memory object client side. - scoped_ptr<base::SharedMemory> shared_memory( - new base::SharedMemory(handle, false)); + base::SharedMemory* shared_memory = new base::SharedMemory(handle, false); // Map the shared memory on demand. if (!shared_memory->memory()) { - if (!shared_memory->Map(size)) + if (!shared_memory->Map(size)) { + delete shared_memory; return Buffer(); + } } Buffer buffer; buffer.ptr = shared_memory->memory(); buffer.size = size; - buffer.shared_memory = shared_memory.release(); + buffer.shared_memory = shared_memory; transfer_buffers_[id] = buffer; return buffer; @@ -350,6 +374,13 @@ void CommandBufferProxyImpl::SetToken(int32 token) { NOTREACHED(); } +void CommandBufferProxyImpl::OnNotifyRepaint() { + if (!notify_repaint_task_.is_null()) + MessageLoop::current()->PostNonNestableTask( + FROM_HERE, notify_repaint_task_); + notify_repaint_task_.Reset(); +} + void CommandBufferProxyImpl::SetParseError( gpu::error::Error error) { // Not implemented in proxy. @@ -465,6 +496,10 @@ bool CommandBufferProxyImpl::SetParent( return result; } +void CommandBufferProxyImpl::SetNotifyRepaintTask(const base::Closure& task) { + notify_repaint_task_ = task; +} + GpuVideoDecodeAcceleratorHost* CommandBufferProxyImpl::CreateVideoDecoder( media::VideoCodecProfile profile, diff --git a/content/common/gpu/client/command_buffer_proxy_impl.h b/content/common/gpu/client/command_buffer_proxy_impl.h index 0db618c..73f53a7 100644 --- a/content/common/gpu/client/command_buffer_proxy_impl.h +++ b/content/common/gpu/client/command_buffer_proxy_impl.h @@ -76,10 +76,12 @@ class CommandBufferProxyImpl virtual State FlushSync(int32 put_offset, int32 last_known_get) OVERRIDE; virtual void SetGetBuffer(int32 shm_id) OVERRIDE; virtual void SetGetOffset(int32 get_offset) OVERRIDE; - virtual gpu::Buffer CreateTransferBuffer(size_t size, - int32* id) OVERRIDE; + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) OVERRIDE; + virtual int32 RegisterTransferBuffer(base::SharedMemory* shared_memory, + size_t size, + int32 id_request) OVERRIDE; virtual void DestroyTransferBuffer(int32 id) OVERRIDE; - virtual gpu::Buffer GetTransferBuffer(int32 id) OVERRIDE; + virtual gpu::Buffer GetTransferBuffer(int32 handle) OVERRIDE; virtual void SetToken(int32 token) OVERRIDE; virtual void SetParseError(gpu::error::Error error) OVERRIDE; virtual void SetContextLostReason( @@ -114,6 +116,10 @@ class CommandBufferProxyImpl // a finish. bool GenerateMailboxNames(unsigned num, std::vector<std::string>* names); + // Set a task that will be invoked the next time the window becomes invalid + // and needs to be repainted. Takes ownership of task. + void SetNotifyRepaintTask(const base::Closure& callback); + // Sends an IPC message with the new state of surface visibility. bool SetSurfaceVisible(bool visible); @@ -142,6 +148,7 @@ class CommandBufferProxyImpl // Message handlers: void OnUpdateState(const gpu::CommandBuffer::State& state); + void OnNotifyRepaint(); void OnDestroyed(gpu::error::ContextLostReason reason); void OnEchoAck(); void OnConsoleMessage(const GPUCommandBufferConsoleMessage& message); @@ -175,6 +182,8 @@ class CommandBufferProxyImpl // Tasks to be invoked in echo responses. std::queue<base::Closure> echo_tasks_; + base::Closure notify_repaint_task_; + base::Closure channel_error_callback_; base::Callback<void(const GpuMemoryAllocationForRenderer&)> @@ -186,9 +195,7 @@ class CommandBufferProxyImpl uint32 next_signal_id_; SignalTaskMap signal_tasks_; - // ID of transfer buffer containing shared state. TODO(apatrick): Eliminate - // this. The share state buffer handle can be passed to the GPU process as an - // argument to Initialize. + // ID of transfer buffer containing shared state. int32 state_buffer_; DISALLOW_COPY_AND_ASSIGN(CommandBufferProxyImpl); diff --git a/content/common/gpu/client/gpu_channel_host.cc b/content/common/gpu/client/gpu_channel_host.cc index 95cd75a..cb20909 100644 --- a/content/common/gpu/client/gpu_channel_host.cc +++ b/content/common/gpu/client/gpu_channel_host.cc @@ -29,7 +29,6 @@ GpuChannelHost::GpuChannelHost( client_id_(client_id), gpu_host_id_(gpu_host_id), state_(kUnconnected) { - next_transfer_buffer_id_.GetNext(); } void GpuChannelHost::Connect( @@ -290,10 +289,6 @@ void GpuChannelHost::OnGenerateMailboxNamesReply( names.end()); } -int32 GpuChannelHost::ReserveTransferBufferId() { - return next_transfer_buffer_id_.GetNext(); -} - GpuChannelHost::~GpuChannelHost() {} diff --git a/content/common/gpu/client/gpu_channel_host.h b/content/common/gpu/client/gpu_channel_host.h index 967fdd3..ab5b7a9 100644 --- a/content/common/gpu/client/gpu_channel_host.h +++ b/content/common/gpu/client/gpu_channel_host.h @@ -8,7 +8,6 @@ #include <string> #include <vector> -#include "base/atomic_sequence_num.h" #include "base/hash_tables.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" @@ -160,9 +159,6 @@ class GpuChannelHost : public IPC::Sender, // a finish. bool GenerateMailboxNames(unsigned num, std::vector<std::string>* names); - // Reserve one unused transfer buffer ID. - int32 ReserveTransferBufferId(); - private: friend class base::RefCountedThreadSafe<GpuChannelHost>; virtual ~GpuChannelHost(); @@ -219,9 +215,6 @@ class GpuChannelHost : public IPC::Sender, // A pool of valid mailbox names. std::vector<std::string> mailbox_name_pool_; - // Transfer buffer IDs are allocated in sequence. - base::AtomicSequenceNumber next_transfer_buffer_id_; - DISALLOW_COPY_AND_ASSIGN(GpuChannelHost); }; diff --git a/content/common/gpu/gpu_command_buffer_stub.cc b/content/common/gpu/gpu_command_buffer_stub.cc index d3abb59..2612db2 100644 --- a/content/common/gpu/gpu_command_buffer_stub.cc +++ b/content/common/gpu/gpu_command_buffer_stub.cc @@ -172,7 +172,8 @@ bool GpuCommandBufferStub::OnMessageReceived(const IPC::Message& message) { IPC_BEGIN_MESSAGE_MAP(GpuCommandBufferStub, message) IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_Initialize, OnInitialize); - IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_SetGetBuffer, OnSetGetBuffer); + IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_SetGetBuffer, + OnSetGetBuffer); IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_SetSharedStateBuffer, OnSetSharedStateBuffer); IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_SetParent, @@ -183,10 +184,12 @@ bool GpuCommandBufferStub::OnMessageReceived(const IPC::Message& message) { OnGetStateFast); IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_AsyncFlush, OnAsyncFlush); IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_Rescheduled, OnRescheduled); - IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_RegisterTransferBuffer, - OnRegisterTransferBuffer); - IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_DestroyTransferBuffer, - OnDestroyTransferBuffer); + IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_CreateTransferBuffer, + OnCreateTransferBuffer); + IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_RegisterTransferBuffer, + OnRegisterTransferBuffer); + IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_DestroyTransferBuffer, + OnDestroyTransferBuffer); IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_GetTransferBuffer, OnGetTransferBuffer); IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuCommandBufferMsg_CreateVideoDecoder, @@ -350,7 +353,8 @@ void GpuCommandBufferStub::OnInitializeFailed(IPC::Message* reply_message) { Send(reply_message); } -void GpuCommandBufferStub::OnInitialize(IPC::Message* reply_message) { +void GpuCommandBufferStub::OnInitialize( + IPC::Message* reply_message) { TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnInitialize"); DCHECK(!command_buffer_.get()); @@ -519,10 +523,16 @@ void GpuCommandBufferStub::OnInitialize(IPC::Message* reply_message) { } } -void GpuCommandBufferStub::OnSetGetBuffer(int32 shm_id) { +void GpuCommandBufferStub::OnSetGetBuffer( + int32 shm_id, IPC::Message* reply_message) { TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnSetGetBuffer"); - if (command_buffer_.get()) + if (command_buffer_.get()) { command_buffer_->SetGetBuffer(shm_id); + } else { + DLOG(ERROR) << "no command_buffer."; + reply_message->set_reply_error(); + } + Send(reply_message); } void GpuCommandBufferStub::OnSetSharedStateBuffer( @@ -634,22 +644,51 @@ void GpuCommandBufferStub::OnRescheduled() { ReportState(); } +void GpuCommandBufferStub::OnCreateTransferBuffer(uint32 size, + int32 id_request, + IPC::Message* reply_message) { + TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnCreateTransferBuffer"); + if (command_buffer_.get()) { + int32 id = command_buffer_->CreateTransferBuffer(size, id_request); + GpuCommandBufferMsg_CreateTransferBuffer::WriteReplyParams( + reply_message, id); + } else { + reply_message->set_reply_error(); + } + Send(reply_message); +} + void GpuCommandBufferStub::OnRegisterTransferBuffer( - int32 id, base::SharedMemoryHandle transfer_buffer, - uint32 size) { + uint32 size, + int32 id_request, + IPC::Message* reply_message) { TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnRegisterTransferBuffer"); base::SharedMemory shared_memory(transfer_buffer, false); - if (command_buffer_.get()) - command_buffer_->RegisterTransferBuffer(id, &shared_memory, size); + if (command_buffer_.get()) { + int32 id = command_buffer_->RegisterTransferBuffer(&shared_memory, + size, + id_request); + GpuCommandBufferMsg_RegisterTransferBuffer::WriteReplyParams(reply_message, + id); + } else { + reply_message->set_reply_error(); + } + + Send(reply_message); } -void GpuCommandBufferStub::OnDestroyTransferBuffer(int32 id) { +void GpuCommandBufferStub::OnDestroyTransferBuffer( + int32 id, + IPC::Message* reply_message) { TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnDestroyTransferBuffer"); - - if (command_buffer_.get()) + if (command_buffer_.get()) { command_buffer_->DestroyTransferBuffer(id); + } else { + reply_message->set_reply_error(); + } + Send(reply_message); } void GpuCommandBufferStub::OnGetTransferBuffer( diff --git a/content/common/gpu/gpu_command_buffer_stub.h b/content/common/gpu/gpu_command_buffer_stub.h index 57143a6..0519e8c 100644 --- a/content/common/gpu/gpu_command_buffer_stub.h +++ b/content/common/gpu/gpu_command_buffer_stub.h @@ -142,7 +142,7 @@ class GpuCommandBufferStub // Message handlers: void OnInitialize(IPC::Message* reply_message); - void OnSetGetBuffer(int32 shm_id); + void OnSetGetBuffer(int32 shm_id, IPC::Message* reply_message); void OnSetSharedStateBuffer(int32 shm_id, IPC::Message* reply_message); void OnSetParent(int32 parent_route_id, uint32 parent_texture_id, @@ -152,10 +152,14 @@ class GpuCommandBufferStub void OnAsyncFlush(int32 put_offset, uint32 flush_count); void OnEcho(const IPC::Message& message); void OnRescheduled(); - void OnRegisterTransferBuffer(int32 id, - base::SharedMemoryHandle transfer_buffer, - uint32 size); - void OnDestroyTransferBuffer(int32 id); + void OnCreateTransferBuffer(uint32 size, + int32 id_request, + IPC::Message* reply_message); + void OnRegisterTransferBuffer(base::SharedMemoryHandle transfer_buffer, + uint32 size, + int32 id_request, + IPC::Message* reply_message); + void OnDestroyTransferBuffer(int32 id, IPC::Message* reply_message); void OnGetTransferBuffer(int32 id, IPC::Message* reply_message); void OnCreateVideoDecoder( diff --git a/content/common/gpu/gpu_messages.h b/content/common/gpu/gpu_messages.h index 054ad49..f52cde0 100644 --- a/content/common/gpu/gpu_messages.h +++ b/content/common/gpu/gpu_messages.h @@ -492,8 +492,8 @@ IPC_SYNC_MESSAGE_ROUTED1_0(GpuCommandBufferMsg_SetSharedStateBuffer, int32 /* shm_id */) // Sets the shared memory buffer used for commands. -IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_SetGetBuffer, - int32 /* shm_id */) +IPC_SYNC_MESSAGE_ROUTED1_0(GpuCommandBufferMsg_SetGetBuffer, + int32 /* shm_id */) // Sets the parent command buffer. This allows the parent and child to share // textures. @@ -527,16 +527,24 @@ IPC_MESSAGE_ROUTED0(GpuCommandBufferMsg_Rescheduled) IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_ConsoleMsg, GPUCommandBufferConsoleMessage /* msg */) +// Create a shared memory transfer buffer. Returns an id that can be used to +// identify the transfer buffer from a comment. +IPC_SYNC_MESSAGE_ROUTED2_1(GpuCommandBufferMsg_CreateTransferBuffer, + uint32 /* size */, + int32 /* id_request (-1 means any) */, + int32 /* id */) + // Register an existing shared memory transfer buffer. Returns an id that can be // used to identify the transfer buffer from a command buffer. -IPC_MESSAGE_ROUTED3(GpuCommandBufferMsg_RegisterTransferBuffer, - int32 /* id */, - base::SharedMemoryHandle /* transfer_buffer */, - uint32 /* size */) +IPC_SYNC_MESSAGE_ROUTED3_1(GpuCommandBufferMsg_RegisterTransferBuffer, + base::SharedMemoryHandle /* transfer_buffer */, + uint32 /* size */, + int32 /* id_request (-1 means any) */, + int32 /* id */) // Destroy a previously created transfer buffer. -IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_DestroyTransferBuffer, - int32 /* id */) +IPC_SYNC_MESSAGE_ROUTED1_0(GpuCommandBufferMsg_DestroyTransferBuffer, + int32 /* id */) // Get the shared memory handle for a transfer buffer mapped to the callers // process. @@ -554,6 +562,10 @@ IPC_SYNC_MESSAGE_ROUTED1_1(GpuCommandBufferMsg_CreateVideoDecoder, IPC_SYNC_MESSAGE_ROUTED1_0(GpuCommandBufferMsg_DestroyVideoDecoder, int /* route_id */) +// Send from command buffer stub to proxy when window is invalid and must be +// repainted. +IPC_MESSAGE_ROUTED0(GpuCommandBufferMsg_NotifyRepaint) + // Tells the GPU process to resize an offscreen frame buffer. IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_ResizeOffscreenFrameBuffer, gfx::Size /* size */) diff --git a/gpu/command_buffer/client/client_test_helper.cc b/gpu/command_buffer/client/client_test_helper.cc index 92af99d..d9942ef 100644 --- a/gpu/command_buffer/client/client_test_helper.cc +++ b/gpu/command_buffer/client/client_test_helper.cc @@ -60,15 +60,16 @@ int32 MockCommandBufferBase::GetNextFreeTransferBufferId() { return -1; } -Buffer MockCommandBufferBase::CreateTransferBuffer(size_t size, int32* id) { - *id = GetNextFreeTransferBufferId(); - if (*id >= 0) { - int32 ndx = *id - kTransferBufferBaseId; +int32 MockCommandBufferBase::CreateTransferBuffer( + size_t size, int32 id_request) { + int32 id = GetNextFreeTransferBufferId(); + if (id >= 0) { + int32 ndx = id - kTransferBufferBaseId; transfer_buffers_[ndx].reset(new int8[size]); transfer_buffer_buffers_[ndx].ptr = transfer_buffers_[ndx].get(); transfer_buffer_buffers_[ndx].size = size; } - return GetTransferBuffer(*id); + return id; } void MockCommandBufferBase::DestroyTransferBufferHelper(int32 id) { @@ -85,6 +86,14 @@ Buffer MockCommandBufferBase::GetTransferBuffer(int32 id) { return transfer_buffer_buffers_[id - kTransferBufferBaseId]; } +int32 MockCommandBufferBase::RegisterTransferBuffer( + base::SharedMemory* shared_memory, + size_t size, + int32 id_request) { + GPU_NOTREACHED(); + return -1; +} + void MockCommandBufferBase::FlushHelper(int32 put_offset) { state_.put_offset = put_offset; } diff --git a/gpu/command_buffer/client/client_test_helper.h b/gpu/command_buffer/client/client_test_helper.h index 3302b47..25d1014 100644 --- a/gpu/command_buffer/client/client_test_helper.h +++ b/gpu/command_buffer/client/client_test_helper.h @@ -32,8 +32,11 @@ class MockCommandBufferBase : public CommandBuffer { virtual State FlushSync(int32 put_offset, int32 last_known_get) OVERRIDE; virtual void SetGetBuffer(int transfer_buffer_id) OVERRIDE; virtual void SetGetOffset(int32 get_offset) OVERRIDE; - virtual Buffer CreateTransferBuffer(size_t size, int32* id) OVERRIDE; + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) OVERRIDE; virtual Buffer GetTransferBuffer(int32 id) OVERRIDE; + virtual int32 RegisterTransferBuffer(base::SharedMemory* shared_memory, + size_t size, + int32 id_request) OVERRIDE; virtual void SetToken(int32 token) OVERRIDE; virtual void SetParseError(error::Error error) OVERRIDE; virtual void SetContextLostReason(error::ContextLostReason reason) OVERRIDE; diff --git a/gpu/command_buffer/client/cmd_buffer_helper.cc b/gpu/command_buffer/client/cmd_buffer_helper.cc index ad5447a..f7d31c5 100644 --- a/gpu/command_buffer/client/cmd_buffer_helper.cc +++ b/gpu/command_buffer/client/cmd_buffer_helper.cc @@ -52,13 +52,18 @@ bool CommandBufferHelper::AllocateRingBuffer() { return true; } - int32 id = -1; - ring_buffer_ = command_buffer_->CreateTransferBuffer(ring_buffer_size_, &id); + int32 id = command_buffer_->CreateTransferBuffer(ring_buffer_size_, -1); if (id < 0) { ClearUsable(); return false; } + ring_buffer_ = command_buffer_->GetTransferBuffer(id); + if (!ring_buffer_.ptr) { + ClearUsable(); + return false; + } + ring_buffer_id_ = id; command_buffer_->SetGetBuffer(id); diff --git a/gpu/command_buffer/client/fenced_allocator_test.cc b/gpu/command_buffer/client/fenced_allocator_test.cc index 2c6cdc6..44fb851 100644 --- a/gpu/command_buffer/client/fenced_allocator_test.cc +++ b/gpu/command_buffer/client/fenced_allocator_test.cc @@ -52,7 +52,7 @@ class BaseFencedAllocatorTest : public testing::Test { } command_buffer_.reset( new CommandBufferService(transfer_buffer_manager_.get())); - EXPECT_TRUE(command_buffer_->Initialize()); + command_buffer_->Initialize(); gpu_scheduler_.reset(new GpuScheduler( command_buffer_.get(), api_mock_.get(), NULL)); diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc index 6176525..581981e 100644 --- a/gpu/command_buffer/client/gles2_implementation_unittest.cc +++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc @@ -103,10 +103,10 @@ class MockTransferBuffer : public TransferBufferInterface { // we need to know their address before GLES2Implementation::Initialize // is called. for (int ii = 0; ii < kNumBuffers; ++ii) { - buffers_[ii] = command_buffer_->CreateTransferBuffer( - size_ + ii * alignment_, - &buffer_ids_[ii]); + buffer_ids_[ii] = command_buffer_->CreateTransferBuffer( + size_ + ii * alignment_, -1); EXPECT_NE(-1, buffer_ids_[ii]); + buffers_[ii] = command_buffer_->GetTransferBuffer(buffer_ids_[ii]); } } diff --git a/gpu/command_buffer/client/mapped_memory.cc b/gpu/command_buffer/client/mapped_memory.cc index 5278fc8..c8d400e 100644 --- a/gpu/command_buffer/client/mapped_memory.cc +++ b/gpu/command_buffer/client/mapped_memory.cc @@ -56,10 +56,11 @@ void* MappedMemoryManager::Alloc( unsigned int chunk_size = ((size + chunk_size_multiple_ - 1) / chunk_size_multiple_) * chunk_size_multiple_; - int32 id = -1; - gpu::Buffer shm = cmd_buf->CreateTransferBuffer(chunk_size, &id); - if (id < 0) + int32 id = cmd_buf->CreateTransferBuffer(chunk_size, -1); + if (id == -1) { return NULL; + } + gpu::Buffer shm = cmd_buf->GetTransferBuffer(id); MemoryChunk* mc = new MemoryChunk(id, shm, helper_); chunks_.push_back(mc); void* mem = mc->Alloc(size); diff --git a/gpu/command_buffer/client/transfer_buffer.cc b/gpu/command_buffer/client/transfer_buffer.cc index 7492159..7e0b25f 100644 --- a/gpu/command_buffer/client/transfer_buffer.cc +++ b/gpu/command_buffer/client/transfer_buffer.cc @@ -81,9 +81,9 @@ void TransferBuffer::FreePendingToken(void* p, unsigned int token) { void TransferBuffer::AllocateRingBuffer(unsigned int size) { for (;size >= min_buffer_size_; size /= 2) { - int32 id = -1; - buffer_ = helper_->command_buffer()->CreateTransferBuffer(size, &id); + int32 id = helper_->command_buffer()->CreateTransferBuffer(size, -1); if (id != -1) { + buffer_ = helper_->command_buffer()->GetTransferBuffer(id); ring_buffer_.reset(new AlignedRingBuffer( alignment_, id, diff --git a/gpu/command_buffer/client/transfer_buffer_unittest.cc b/gpu/command_buffer/client/transfer_buffer_unittest.cc index f40de23..469483b 100644 --- a/gpu/command_buffer/client/transfer_buffer_unittest.cc +++ b/gpu/command_buffer/client/transfer_buffer_unittest.cc @@ -16,7 +16,6 @@ using ::testing::_; using ::testing::Invoke; using ::testing::Return; -using ::testing::SetArgPointee; using ::testing::StrictMock; namespace gpu { @@ -212,10 +211,10 @@ class MockClientCommandBufferCanFail : public MockClientCommandBufferMockFlush { virtual ~MockClientCommandBufferCanFail() { } - MOCK_METHOD2(CreateTransferBuffer, Buffer(size_t size, int32* id)); + MOCK_METHOD2(CreateTransferBuffer, int32(size_t size, int32 id_request)); - Buffer RealCreateTransferBuffer(size_t size, int32* id) { - return MockCommandBufferBase::CreateTransferBuffer(size, id); + int32 RealCreateTransferBuffer(size_t size, int32 id_request) { + return MockCommandBufferBase::CreateTransferBuffer(size, id_request); } }; @@ -373,7 +372,7 @@ TEST_F(TransferBufferExpandContractTest, Contract) { // Try to allocate again, fail first request EXPECT_CALL(*command_buffer(), CreateTransferBuffer(kStartTransferBufferSize, _)) - .WillOnce(DoAll(SetArgPointee<1>(-1), Return(Buffer()))) + .WillOnce(Return(-1)) .RetiresOnSaturation(); EXPECT_CALL(*command_buffer(), CreateTransferBuffer(kMinTransferBufferSize, _)) @@ -425,9 +424,9 @@ TEST_F(TransferBufferExpandContractTest, OutOfMemory) { // Try to allocate again, fail both requests. EXPECT_CALL(*command_buffer(), CreateTransferBuffer(_, _)) - .WillOnce(DoAll(SetArgPointee<1>(-1), Return(Buffer()))) - .WillOnce(DoAll(SetArgPointee<1>(-1), Return(Buffer()))) - .WillOnce(DoAll(SetArgPointee<1>(-1), Return(Buffer()))) + .WillOnce(Return(-1)) + .WillOnce(Return(-1)) + .WillOnce(Return(-1)) .RetiresOnSaturation(); const size_t kSize1 = 512 - kStartingOffset; diff --git a/gpu/command_buffer/common/command_buffer.h b/gpu/command_buffer/common/command_buffer.h index e100792..6b19ea2 100644 --- a/gpu/command_buffer/common/command_buffer.h +++ b/gpu/command_buffer/common/command_buffer.h @@ -101,16 +101,28 @@ class GPU_EXPORT CommandBuffer { // Sets the current get offset. This can be called from any thread. virtual void SetGetOffset(int32 get_offset) = 0; - // Create a transfer buffer of the given size. Returns its ID or -1 on - // error. - virtual Buffer CreateTransferBuffer(size_t size, int32* id) = 0; - - // Destroy a transfer buffer. The ID must be positive. + // Create a transfer buffer and return a handle that uniquely + // identifies it or -1 on error. id_request lets the caller request a + // specific id for the transfer buffer, or -1 if the caller does not care. + // If the requested id can not be fulfilled, a different id will be returned. + // id_request must be either -1 or between 0 and 100. + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) = 0; + + // Register an existing shared memory object and get an ID that can be used + // to identify it in the command buffer. Callee dups the handle until + // DestroyTransferBuffer is called. id_request lets the caller request a + // specific id for the transfer buffer, or -1 if the caller does not care. + // If the requested id can not be fulfilled, a different id will be returned. + // id_request must be either -1 or between 0 and 100. + virtual int32 RegisterTransferBuffer(base::SharedMemory* shared_memory, + size_t size, + int32 id_request) = 0; + + // Destroy a transfer buffer and recycle the handle. virtual void DestroyTransferBuffer(int32 id) = 0; - // Get the transfer buffer associated with an ID. Returns a null buffer for - // ID 0. - virtual Buffer GetTransferBuffer(int32 id) = 0; + // Get the transfer buffer associated with a handle. + virtual Buffer GetTransferBuffer(int32 handle) = 0; // Allows the reader to update the current token value. virtual void SetToken(int32 token) = 0; diff --git a/gpu/command_buffer/common/command_buffer_mock.h b/gpu/command_buffer/common/command_buffer_mock.h index b2ee11f..0379340 100644 --- a/gpu/command_buffer/common/command_buffer_mock.h +++ b/gpu/command_buffer/common/command_buffer_mock.h @@ -28,9 +28,12 @@ class MockCommandBuffer : public CommandBuffer { MOCK_METHOD2(FlushSync, State(int32 put_offset, int32 last_known_get)); MOCK_METHOD1(SetGetBuffer, void(int32 transfer_buffer_id)); MOCK_METHOD1(SetGetOffset, void(int32 get_offset)); - MOCK_METHOD2(CreateTransferBuffer, Buffer(size_t size, int32* id)); - MOCK_METHOD1(DestroyTransferBuffer, void(int32 id)); - MOCK_METHOD1(GetTransferBuffer, Buffer(int32 id)); + MOCK_METHOD2(CreateTransferBuffer, int32(size_t size, int32 id_request)); + MOCK_METHOD1(DestroyTransferBuffer, void(int32 handle)); + MOCK_METHOD1(GetTransferBuffer, Buffer(int32 handle)); + MOCK_METHOD3(RegisterTransferBuffer, int32(base::SharedMemory* shared_memory, + size_t size, + int32 id_request)); MOCK_METHOD1(SetToken, void(int32 token)); MOCK_METHOD1(SetParseError, void(error::Error error)); MOCK_METHOD1(SetContextLostReason, diff --git a/gpu/command_buffer/service/command_buffer_service.cc b/gpu/command_buffer/service/command_buffer_service.cc index 391f335..8b00595 100644 --- a/gpu/command_buffer/service/command_buffer_service.cc +++ b/gpu/command_buffer/service/command_buffer_service.cc @@ -116,26 +116,20 @@ void CommandBufferService::SetGetOffset(int32 get_offset) { get_offset_ = get_offset; } -Buffer CommandBufferService::CreateTransferBuffer(size_t size, - int32* id) { - *id = -1; - - SharedMemory buffer; - if (!buffer.CreateAnonymous(size)) - return Buffer(); - - static int32 next_id = 1; - *id = next_id++; - - if (!RegisterTransferBuffer(*id, &buffer, size)) - return Buffer(); +int32 CommandBufferService::CreateTransferBuffer(size_t size, + int32 id_request) { + return transfer_buffer_manager_->CreateTransferBuffer(size, id_request); +} - return GetTransferBuffer(*id); +int32 CommandBufferService::RegisterTransferBuffer( + base::SharedMemory* shared_memory, size_t size, int32 id_request) { + return transfer_buffer_manager_->RegisterTransferBuffer( + shared_memory, size, id_request); } -void CommandBufferService::DestroyTransferBuffer(int32 id) { - transfer_buffer_manager_->DestroyTransferBuffer(id); - if (id == ring_buffer_id_) { +void CommandBufferService::DestroyTransferBuffer(int32 handle) { + transfer_buffer_manager_->DestroyTransferBuffer(handle); + if (handle == ring_buffer_id_) { ring_buffer_id_ = -1; ring_buffer_ = Buffer(); num_entries_ = 0; @@ -144,17 +138,8 @@ void CommandBufferService::DestroyTransferBuffer(int32 id) { } } -Buffer CommandBufferService::GetTransferBuffer(int32 id) { - return transfer_buffer_manager_->GetTransferBuffer(id); -} - -bool CommandBufferService::RegisterTransferBuffer( - int32 id, - base::SharedMemory* shared_memory, - size_t size) { - return transfer_buffer_manager_->RegisterTransferBuffer(id, - shared_memory, - size); +Buffer CommandBufferService::GetTransferBuffer(int32 handle) { + return transfer_buffer_manager_->GetTransferBuffer(handle); } void CommandBufferService::SetToken(int32 token) { diff --git a/gpu/command_buffer/service/command_buffer_service.h b/gpu/command_buffer/service/command_buffer_service.h index c53a4bc..c3bb841 100644 --- a/gpu/command_buffer/service/command_buffer_service.h +++ b/gpu/command_buffer/service/command_buffer_service.h @@ -31,9 +31,12 @@ class GPU_EXPORT CommandBufferService : public CommandBuffer { virtual State FlushSync(int32 put_offset, int32 last_known_get) OVERRIDE; virtual void SetGetBuffer(int32 transfer_buffer_id) OVERRIDE; virtual void SetGetOffset(int32 get_offset) OVERRIDE; - virtual Buffer CreateTransferBuffer(size_t size, int32* id) OVERRIDE; + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) OVERRIDE; + virtual int32 RegisterTransferBuffer(base::SharedMemory* shared_memory, + size_t size, + int32 id_request) OVERRIDE; virtual void DestroyTransferBuffer(int32 id) OVERRIDE; - virtual Buffer GetTransferBuffer(int32 id) OVERRIDE; + virtual Buffer GetTransferBuffer(int32 handle) OVERRIDE; virtual void SetToken(int32 token) OVERRIDE; virtual void SetParseError(error::Error error) OVERRIDE; virtual void SetContextLostReason(error::ContextLostReason) OVERRIDE; @@ -58,13 +61,6 @@ class GPU_EXPORT CommandBufferService : public CommandBuffer { // Copy the current state into the shared state transfer buffer. void UpdateState(); - // Register an existing shared memory object and get an ID that can be used - // to identify it in the command buffer. Callee dups the handle until - // DestroyTransferBuffer is called. - bool RegisterTransferBuffer(int32 id, - base::SharedMemory* shared_memory, - size_t size); - private: int32 ring_buffer_id_; Buffer ring_buffer_; diff --git a/gpu/command_buffer/service/command_buffer_service_unittest.cc b/gpu/command_buffer/service/command_buffer_service_unittest.cc index bf4d619..2c9093b 100644 --- a/gpu/command_buffer/service/command_buffer_service_unittest.cc +++ b/gpu/command_buffer/service/command_buffer_service_unittest.cc @@ -50,8 +50,7 @@ class CommandBufferServiceTest : public testing::Test { } bool Initialize(size_t size) { - int32 id; - command_buffer_->CreateTransferBuffer(size, &id); + int32 id = command_buffer_->CreateTransferBuffer(size, -1); EXPECT_GT(id, 0); command_buffer_->SetGetBuffer(id); return true; @@ -118,8 +117,7 @@ TEST_F(CommandBufferServiceTest, CanSyncGetAndPutOffset) { } TEST_F(CommandBufferServiceTest, SetGetBuffer) { - int32 ring_buffer_id; - command_buffer_->CreateTransferBuffer(1024, &ring_buffer_id); + int32 ring_buffer_id = command_buffer_->CreateTransferBuffer(1024, -1); EXPECT_GT(ring_buffer_id, 0); scoped_ptr<StrictMock<MockCallbackTest> > change_callback( diff --git a/gpu/command_buffer/service/transfer_buffer_manager.cc b/gpu/command_buffer/service/transfer_buffer_manager.cc index 12c9fb1..7bde12a 100644 --- a/gpu/command_buffer/service/transfer_buffer_manager.cc +++ b/gpu/command_buffer/service/transfer_buffer_manager.cc @@ -8,7 +8,6 @@ #include "base/process_util.h" #include "base/debug/trace_event.h" -#include "gpu/command_buffer/common/gles2_cmd_utils.h" using ::base::SharedMemory; @@ -19,15 +18,17 @@ TransferBufferManagerInterface::~TransferBufferManagerInterface() { TransferBufferManager::TransferBufferManager() : shared_memory_bytes_allocated_(0) { + // Element zero is always NULL. + registered_objects_.push_back(Buffer()); } TransferBufferManager::~TransferBufferManager() { - while (!registered_buffers_.empty()) { - BufferMap::iterator it = registered_buffers_.begin(); - DCHECK(shared_memory_bytes_allocated_ >= it->second.size); - shared_memory_bytes_allocated_ -= it->second.size; - delete it->second.shared_memory; - registered_buffers_.erase(it); + for (size_t i = 0; i < registered_objects_.size(); ++i) { + if (registered_objects_[i].shared_memory) { + DCHECK(shared_memory_bytes_allocated_ >= registered_objects_[i].size); + shared_memory_bytes_allocated_ -= registered_objects_[i].size; + delete registered_objects_[i].shared_memory; + } } DCHECK(!shared_memory_bytes_allocated_); } @@ -36,38 +37,42 @@ bool TransferBufferManager::Initialize() { return true; } -bool TransferBufferManager::RegisterTransferBuffer( - int32 id, - base::SharedMemory* shared_memory, - size_t size) { - if (id <= 0) { - DVLOG(ERROR) << "Cannot register transfer buffer with non-positive ID."; - return false; - } +int32 TransferBufferManager::CreateTransferBuffer( + size_t size, int32 id_request) { + SharedMemory buffer; + if (!buffer.CreateAnonymous(size)) + return -1; + + return RegisterTransferBuffer(&buffer, size, id_request); +} - // Fail if the ID is in use. - if (registered_buffers_.find(id) != registered_buffers_.end()) { - DVLOG(ERROR) << "Buffer ID already in use."; - return false; +int32 TransferBufferManager::RegisterTransferBuffer( + base::SharedMemory* shared_memory, size_t size, int32 id_request) { + // Check we haven't exceeded the range that fits in a 32-bit integer. + if (unused_registered_object_elements_.empty()) { + if (registered_objects_.size() > std::numeric_limits<uint32>::max()) + return -1; } + // Check that the requested ID is sane (not too large, or less than -1) + if (id_request != -1 && (id_request > 100 || id_request < -1)) + return -1; + // Duplicate the handle. base::SharedMemoryHandle duped_shared_memory_handle; if (!shared_memory->ShareToProcess(base::GetCurrentProcessHandle(), &duped_shared_memory_handle)) { - DVLOG(ERROR) << "Failed to duplicate shared memory handle."; - return false; + return -1; } 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(ERROR) << "Failed to map shared memory."; - return false; - } + if (!duped_shared_memory->Map(size)) + return -1; - // If it could be mapped register the shared memory with the ID. + // If it could be mapped, allocate an ID and register the shared memory with + // that ID. Buffer buffer; buffer.ptr = duped_shared_memory->memory(); buffer.size = size; @@ -77,36 +82,73 @@ bool TransferBufferManager::RegisterTransferBuffer( TRACE_COUNTER_ID1( "gpu", "GpuTransferBufferMemory", this, shared_memory_bytes_allocated_); - registered_buffers_[id] = buffer; + // If caller requested specific id, first try to use id_request. + if (id_request != -1) { + int32 cur_size = static_cast<int32>(registered_objects_.size()); + if (cur_size <= id_request) { + // Pad registered_objects_ to reach id_request. + registered_objects_.resize(static_cast<size_t>(id_request + 1)); + for (int32 id = cur_size; id < id_request; ++id) + unused_registered_object_elements_.insert(id); + registered_objects_[id_request] = buffer; + return id_request; + } else if (!registered_objects_[id_request].shared_memory) { + // id_request is already in free list. + registered_objects_[id_request] = buffer; + unused_registered_object_elements_.erase(id_request); + return id_request; + } + } - return true; + if (unused_registered_object_elements_.empty()) { + int32 handle = static_cast<int32>(registered_objects_.size()); + registered_objects_.push_back(buffer); + return handle; + } else { + int32 handle = *unused_registered_object_elements_.begin(); + unused_registered_object_elements_.erase( + unused_registered_object_elements_.begin()); + DCHECK(!registered_objects_[handle].shared_memory); + registered_objects_[handle] = buffer; + return handle; + } } -void TransferBufferManager::DestroyTransferBuffer(int32 id) { - BufferMap::iterator it = registered_buffers_.find(id); - if (it == registered_buffers_.end()) { - DVLOG(ERROR) << "Transfer buffer ID was not registered."; +void TransferBufferManager::DestroyTransferBuffer(int32 handle) { + if (handle <= 0) return; - } - DCHECK(shared_memory_bytes_allocated_ >= it->second.size); - shared_memory_bytes_allocated_ -= it->second.size; + if (static_cast<size_t>(handle) >= registered_objects_.size()) + return; + + DCHECK(shared_memory_bytes_allocated_ >= registered_objects_[handle].size); + shared_memory_bytes_allocated_ -= registered_objects_[handle].size; TRACE_COUNTER_ID1( "CommandBuffer", "SharedMemory", this, shared_memory_bytes_allocated_); - delete it->second.shared_memory; - registered_buffers_.erase(it); + delete registered_objects_[handle].shared_memory; + registered_objects_[handle] = Buffer(); + unused_registered_object_elements_.insert(handle); + + // Remove all null objects from the end of the vector. This allows the vector + // to shrink when, for example, all objects are unregistered. Note that this + // loop never removes element zero, which is always NULL. + while (registered_objects_.size() > 1 && + !registered_objects_.back().shared_memory) { + registered_objects_.pop_back(); + unused_registered_object_elements_.erase( + static_cast<int32>(registered_objects_.size())); + } } -Buffer TransferBufferManager::GetTransferBuffer(int32 id) { - if (id == 0) +Buffer TransferBufferManager::GetTransferBuffer(int32 handle) { + if (handle < 0) return Buffer(); - BufferMap::iterator it = registered_buffers_.find(id); - if (it == registered_buffers_.end()) + if (static_cast<size_t>(handle) >= registered_objects_.size()) return Buffer(); - return it->second; + return registered_objects_[handle]; } } // namespace gpu diff --git a/gpu/command_buffer/service/transfer_buffer_manager.h b/gpu/command_buffer/service/transfer_buffer_manager.h index b50a5ae..9f24e2c 100644 --- a/gpu/command_buffer/service/transfer_buffer_manager.h +++ b/gpu/command_buffer/service/transfer_buffer_manager.h @@ -10,7 +10,6 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" -#include "base/hash_tables.h" #include "base/shared_memory.h" #include "gpu/command_buffer/common/command_buffer_shared.h" @@ -20,11 +19,13 @@ class GPU_EXPORT TransferBufferManagerInterface { public: virtual ~TransferBufferManagerInterface(); - virtual bool RegisterTransferBuffer(int32 id, - base::SharedMemory* shared_memory, - size_t size) = 0; + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) = 0; + virtual int32 RegisterTransferBuffer( + base::SharedMemory* shared_memory, + size_t size, + int32 id_request) = 0; virtual void DestroyTransferBuffer(int32 id) = 0; - virtual Buffer GetTransferBuffer(int32 id) = 0; + virtual Buffer GetTransferBuffer(int32 handle) = 0; }; @@ -34,17 +35,19 @@ class GPU_EXPORT TransferBufferManager TransferBufferManager(); bool Initialize(); - virtual bool RegisterTransferBuffer(int32 id, - base::SharedMemory* shared_memory, - size_t size) OVERRIDE; + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) OVERRIDE; + virtual int32 RegisterTransferBuffer( + base::SharedMemory* shared_memory, + size_t size, + int32 id_request) OVERRIDE; virtual void DestroyTransferBuffer(int32 id) OVERRIDE; - virtual Buffer GetTransferBuffer(int32 id) OVERRIDE; + virtual Buffer GetTransferBuffer(int32 handle) OVERRIDE; private: virtual ~TransferBufferManager(); - typedef base::hash_map<int32, Buffer> BufferMap; - BufferMap registered_buffers_; + std::set<int32> unused_registered_object_elements_; + std::vector<Buffer> registered_objects_; size_t shared_memory_bytes_allocated_; DISALLOW_COPY_AND_ASSIGN(TransferBufferManager); diff --git a/gpu/command_buffer/service/transfer_buffer_manager_unittest.cc b/gpu/command_buffer/service/transfer_buffer_manager_unittest.cc index 68c5c2e..029cb0e 100644 --- a/gpu/command_buffer/service/transfer_buffer_manager_unittest.cc +++ b/gpu/command_buffer/service/transfer_buffer_manager_unittest.cc @@ -12,22 +12,14 @@ using base::SharedMemory; namespace gpu { -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()); + manager->Initialize(); } - base::SharedMemory buffers_[3]; scoped_ptr<TransferBufferManagerInterface> transfer_buffer_manager_; }; @@ -43,73 +35,59 @@ TEST_F(TransferBufferManagerTest, OutOfRangeHandleMapsToNull) { EXPECT_TRUE(NULL == transfer_buffer_manager_->GetTransferBuffer(1).ptr); } -TEST_F(TransferBufferManagerTest, CanRegisterTransferBuffer) { - EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1, - &buffers_[0], - kBufferSize)); - Buffer registered = transfer_buffer_manager_->GetTransferBuffer(1); - - // Distinct memory range and shared memory handle from that originally - // registered. - EXPECT_NE(static_cast<void*>(NULL), registered.ptr); - EXPECT_NE(buffers_[0].memory(), registered.ptr); - EXPECT_EQ(kBufferSize, registered.size); - EXPECT_NE(&buffers_[0], registered.shared_memory); - - // But maps to the same physical memory. - *static_cast<int*>(registered.ptr) = 7; - *static_cast<int*>(buffers_[0].memory()) = 8; - EXPECT_EQ(8, *static_cast<int*>(registered.ptr)); +TEST_F(TransferBufferManagerTest, CanCreateTransferBuffers) { + int32 handle = transfer_buffer_manager_->CreateTransferBuffer(1024, -1); + EXPECT_EQ(1, handle); + Buffer buffer = transfer_buffer_manager_->GetTransferBuffer(handle); + ASSERT_TRUE(NULL != buffer.ptr); + EXPECT_EQ(1024u, buffer.size); } -TEST_F(TransferBufferManagerTest, CanDestroyTransferBuffer) { - EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1, - &buffers_[0], - kBufferSize)); - transfer_buffer_manager_->DestroyTransferBuffer(1); - Buffer registered = transfer_buffer_manager_->GetTransferBuffer(1); +TEST_F(TransferBufferManagerTest, CreateTransferBufferReturnsDistinctHandles) { + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); +} - EXPECT_EQ(static_cast<void*>(NULL), registered.ptr); - EXPECT_EQ(0U, registered.size); - EXPECT_EQ(static_cast<base::SharedMemory*>(NULL), registered.shared_memory); +TEST_F(TransferBufferManagerTest, + CreateTransferBufferReusesUnregisteredHandles) { + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); + EXPECT_EQ(2, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); + transfer_buffer_manager_->DestroyTransferBuffer(1); + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); + EXPECT_EQ(3, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); } -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)); +TEST_F(TransferBufferManagerTest, CannotUnregisterHandleZero) { + transfer_buffer_manager_->DestroyTransferBuffer(0); + EXPECT_TRUE(NULL == transfer_buffer_manager_->GetTransferBuffer(0).ptr); + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); } -TEST_F(TransferBufferManagerTest, CanReuseTransferBufferIdAfterDestroying) { - EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1, - &buffers_[0], - kBufferSize)); - transfer_buffer_manager_->DestroyTransferBuffer(1); - EXPECT_TRUE(transfer_buffer_manager_->RegisterTransferBuffer(1, - &buffers_[1], - kBufferSize)); +TEST_F(TransferBufferManagerTest, CannotUnregisterNegativeHandles) { + transfer_buffer_manager_->DestroyTransferBuffer(-1); + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); } -TEST_F(TransferBufferManagerTest, DestroyUnusedTransferBufferIdDoesNotCrash) { +TEST_F(TransferBufferManagerTest, CannotUnregisterUnregisteredHandles) { transfer_buffer_manager_->DestroyTransferBuffer(1); + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); } -TEST_F(TransferBufferManagerTest, CannotRegisterNullTransferBuffer) { - EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(0, - &buffers_[0], - kBufferSize)); +// Testing this case specifically because there is an optimization that takes +// a different code path in this case. +TEST_F(TransferBufferManagerTest, UnregistersLastRegisteredHandle) { + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); + transfer_buffer_manager_->DestroyTransferBuffer(1); + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); } -TEST_F(TransferBufferManagerTest, CannotRegisterNegativeTransferBufferId) { - EXPECT_FALSE(transfer_buffer_manager_->RegisterTransferBuffer(-1, - &buffers_[0], - kBufferSize)); +// Testing this case specifically because there is an optimization that takes +// a different code path in this case. +TEST_F(TransferBufferManagerTest, UnregistersTwoLastRegisteredHandles) { + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); + EXPECT_EQ(2, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); + transfer_buffer_manager_->DestroyTransferBuffer(2); + transfer_buffer_manager_->DestroyTransferBuffer(1); + EXPECT_EQ(1, transfer_buffer_manager_->CreateTransferBuffer(1024, -1)); } } // namespace gpu diff --git a/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.cc b/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.cc index eef8f0f..95b7028 100644 --- a/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.cc +++ b/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.cc @@ -104,20 +104,18 @@ void CommandBufferNacl::SetGetOffset(int32 get_offset) { GPU_NOTREACHED(); } -gpu::Buffer CommandBufferNacl::CreateTransferBuffer(size_t size, int32* id) { +int32 CommandBufferNacl::CreateTransferBuffer(size_t size, int32 id_request) { DebugPrintf("CommandBufferNacl::CreateTransferBuffer\n"); - *id = -1; + int32_t id; NaClSrpcChannel* channel = ppapi_proxy::GetMainSrpcChannel(); NaClSrpcError retval = PpbGraphics3DRpcClient::PPB_Graphics3DTrusted_CreateTransferBuffer( - channel, graphics_3d_, size, -1, id); + channel, graphics_3d_, size, id_request, &id); if (NACL_SRPC_RESULT_OK != retval) - return BufferFromShm(-1, 0); - if ((*id) <= 0) - return BufferFromShm(-1, 0); + return 0; - return GetTransferBuffer(*id); + return id; } void CommandBufferNacl::DestroyTransferBuffer(int32 id) { diff --git a/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.h b/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.h index eadd374..8806289 100644 --- a/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.h +++ b/ppapi/native_client/src/shared/ppapi_proxy/command_buffer_nacl.h @@ -28,9 +28,15 @@ class CommandBufferNacl : public gpu::CommandBuffer { virtual State FlushSync(int32 put_offset, int32 last_known_get); virtual void SetGetBuffer(int32 transfer_buffer_id); virtual void SetGetOffset(int32 get_offset); - virtual gpu::Buffer CreateTransferBuffer(size_t size, int32* id); + virtual int32 CreateTransferBuffer(size_t size, int32 id_request); + virtual int32 RegisterTransferBuffer(base::SharedMemory* buffer, + size_t size, + int32 id_request) { + // TODO(neb): support for nacl if neccessary + return -1; + } virtual void DestroyTransferBuffer(int32 id); - virtual gpu::Buffer GetTransferBuffer(int32 id); + virtual gpu::Buffer GetTransferBuffer(int32 handle); virtual void SetToken(int32 token); virtual void SetParseError(gpu::error::Error error); virtual void SetContextLostReason(gpu::error::ContextLostReason); diff --git a/ppapi/proxy/ppapi_command_buffer_proxy.cc b/ppapi/proxy/ppapi_command_buffer_proxy.cc index 175b8de..ad80a27 100644 --- a/ppapi/proxy/ppapi_command_buffer_proxy.cc +++ b/ppapi/proxy/ppapi_command_buffer_proxy.cc @@ -129,22 +129,26 @@ void PpapiCommandBufferProxy::SetGetOffset(int32 get_offset) { NOTREACHED(); } -gpu::Buffer PpapiCommandBufferProxy::CreateTransferBuffer(size_t size, - int32* id) { - *id = -1; - - if (last_state_.error != gpu::error::kNoError) - return gpu::Buffer(); - - if (!Send(new PpapiHostMsg_PPBGraphics3D_CreateTransferBuffer( - ppapi::API_ID_PPB_GRAPHICS_3D, resource_, size, id))) { - return gpu::Buffer(); +int32 PpapiCommandBufferProxy::CreateTransferBuffer( + size_t size, + int32 id_request) { + if (last_state_.error == gpu::error::kNoError) { + int32 id; + if (Send(new PpapiHostMsg_PPBGraphics3D_CreateTransferBuffer( + ppapi::API_ID_PPB_GRAPHICS_3D, resource_, size, &id))) { + return id; + } } + return -1; +} - if ((*id) <= 0) - return gpu::Buffer(); - - return GetTransferBuffer(*id); +int32 PpapiCommandBufferProxy::RegisterTransferBuffer( + base::SharedMemory* shared_memory, + size_t size, + int32 id_request) { + // Not implemented in proxy. + NOTREACHED(); + return -1; } void PpapiCommandBufferProxy::DestroyTransferBuffer(int32 id) { diff --git a/ppapi/proxy/ppapi_command_buffer_proxy.h b/ppapi/proxy/ppapi_command_buffer_proxy.h index d1aac55..716d3f3 100644 --- a/ppapi/proxy/ppapi_command_buffer_proxy.h +++ b/ppapi/proxy/ppapi_command_buffer_proxy.h @@ -44,9 +44,12 @@ class PPAPI_PROXY_EXPORT PpapiCommandBufferProxy : public CommandBufferProxy { virtual State FlushSync(int32 put_offset, int32 last_known_get); virtual void SetGetBuffer(int32 transfer_buffer_id); virtual void SetGetOffset(int32 get_offset); - virtual gpu::Buffer CreateTransferBuffer(size_t size, int32* id); + virtual int32 CreateTransferBuffer(size_t size, int32 id_request); + virtual int32 RegisterTransferBuffer(base::SharedMemory* shared_memory, + size_t size, + int32 id_request); virtual void DestroyTransferBuffer(int32 id); - virtual gpu::Buffer GetTransferBuffer(int32 id); + virtual gpu::Buffer GetTransferBuffer(int32 handle); virtual void SetToken(int32 token); virtual void SetParseError(gpu::error::Error error); virtual void SetContextLostReason(gpu::error::ContextLostReason reason); diff --git a/ppapi/proxy/ppb_graphics_3d_proxy.cc b/ppapi/proxy/ppb_graphics_3d_proxy.cc index 03d1ca2..cef181a 100644 --- a/ppapi/proxy/ppb_graphics_3d_proxy.cc +++ b/ppapi/proxy/ppb_graphics_3d_proxy.cc @@ -146,18 +146,24 @@ class Graphics3D::LockingCommandBuffer : public gpu::CommandBuffer { MaybeLock lock(need_to_lock_); gpu_command_buffer_->SetGetOffset(get_offset); } - virtual gpu::Buffer CreateTransferBuffer(size_t size, - int32* id) OVERRIDE { + virtual int32 CreateTransferBuffer(size_t size, int32 id_request) OVERRIDE { MaybeLock lock(need_to_lock_); - return gpu_command_buffer_->CreateTransferBuffer(size, id); + return gpu_command_buffer_->CreateTransferBuffer(size, id_request); + } + virtual int32 RegisterTransferBuffer(base::SharedMemory* shared_memory, + size_t size, + int32 id_request) OVERRIDE { + MaybeLock lock(need_to_lock_); + return gpu_command_buffer_->RegisterTransferBuffer(shared_memory, size, + id_request); } virtual void DestroyTransferBuffer(int32 id) OVERRIDE { MaybeLock lock(need_to_lock_); gpu_command_buffer_->DestroyTransferBuffer(id); } - virtual gpu::Buffer GetTransferBuffer(int32 id) OVERRIDE { + virtual gpu::Buffer GetTransferBuffer(int32 handle) OVERRIDE { MaybeLock lock(need_to_lock_); - return gpu_command_buffer_->GetTransferBuffer(id); + return gpu_command_buffer_->GetTransferBuffer(handle); } virtual void SetToken(int32 token) OVERRIDE { MaybeLock lock(need_to_lock_); @@ -449,7 +455,7 @@ void PPB_Graphics3D_Proxy::OnMsgCreateTransferBuffer( if (enter.succeeded()) *id = enter.object()->CreateTransferBuffer(size); else - *id = -1; + *id = 0; } void PPB_Graphics3D_Proxy::OnMsgDestroyTransferBuffer( diff --git a/webkit/plugins/ppapi/ppb_graphics_3d_impl.cc b/webkit/plugins/ppapi/ppb_graphics_3d_impl.cc index 5c6e24a..5e6592c 100644 --- a/webkit/plugins/ppapi/ppb_graphics_3d_impl.cc +++ b/webkit/plugins/ppapi/ppb_graphics_3d_impl.cc @@ -141,9 +141,7 @@ PP_Graphics3DTrustedState PPB_Graphics3D_Impl::GetState() { } int32_t PPB_Graphics3D_Impl::CreateTransferBuffer(uint32_t size) { - int32_t id = -1; - GetCommandBuffer()->CreateTransferBuffer(size, &id); - return id; + return GetCommandBuffer()->CreateTransferBuffer(size, -1); } PP_Bool PPB_Graphics3D_Impl::DestroyTransferBuffer(int32_t id) { |