Step 1: Changing CommandBufferHelper to only use a portion

of the command buffer. This brought out the fact that
there were lots of places in the code mixing size in bytes
with num command buffer entries.

This fixes most of those issues. No public interface
uses num command buffer entries except 
gpu::CommandBuffer::State where it makes sense.

TEST=relying on unit tests, conformance tests and chrome
BUG=none

Review URL: http://codereview.chromium.org/2008014

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

19 files changed, 61 insertions, 56 deletions

diff --git a/chrome/common/gpu_messages.h b/chrome/common/gpu_messages.h
index f3e16a3..7b37731 100644
--- a/chrome/common/gpu_messages.h
+++ b/chrome/common/gpu_messages.h
@@ -22,7 +22,7 @@ template <>
 struct ParamTraits<gpu::CommandBuffer::State> {
   typedef gpu::CommandBuffer::State param_type;
   static void Write(Message* m, const param_type& p) {
-    m->WriteInt(p.size);
+    m->WriteInt(p.num_entries);
     m->WriteInt(p.get_offset);
     m->WriteInt(p.put_offset);
     m->WriteInt(p.token);
@@ -30,7 +30,7 @@ struct ParamTraits<gpu::CommandBuffer::State> {
   }
   static bool Read(const Message* m, void** iter, param_type* p) {
     int32 temp;
-    if (m->ReadInt(iter, &p->size) &&
+    if (m->ReadInt(iter, &p->num_entries) &&
         m->ReadInt(iter, &p->get_offset) &&
         m->ReadInt(iter, &p->put_offset) &&
         m->ReadInt(iter, &p->token) &&
diff --git a/chrome/renderer/command_buffer_proxy.cc b/chrome/renderer/command_buffer_proxy.cc
index 76fea60..f561be9 100644
--- a/chrome/renderer/command_buffer_proxy.cc
+++ b/chrome/renderer/command_buffer_proxy.cc
@@ -15,7 +15,7 @@ using gpu::Buffer;
 CommandBufferProxy::CommandBufferProxy(
     IPC::Channel::Sender* channel,
     int route_id)
-    : size_(0),
+    : num_entries_(0),
       channel_(channel),
       route_id_(route_id) {
 }
@@ -69,8 +69,8 @@ bool CommandBufferProxy::Initialize(int32 size) {
   if (Send(new GpuCommandBufferMsg_Initialize(route_id_, size, &handle)) &&
       base::SharedMemory::IsHandleValid(handle)) {
     ring_buffer_.reset(new base::SharedMemory(handle, false));
-    if (ring_buffer_->Map(size * sizeof(int32))) {
-      size_ = size;
+    if (ring_buffer_->Map(size)) {
+      num_entries_ = size / sizeof(gpu::CommandBufferEntry);
       return true;
     }
 
@@ -84,7 +84,7 @@ Buffer CommandBufferProxy::GetRingBuffer() {
   // Return locally cached ring buffer.
   Buffer buffer;
   buffer.ptr = ring_buffer_->memory();
-  buffer.size = size_ * sizeof(gpu::CommandBufferEntry);
+  buffer.size = num_entries_ * sizeof(gpu::CommandBufferEntry);
   buffer.shared_memory = ring_buffer_.get();
   return buffer;
 }
@@ -96,8 +96,8 @@ gpu::CommandBuffer::State CommandBufferProxy::GetState() {
 
 gpu::CommandBuffer::State CommandBufferProxy::Flush(int32 put_offset) {
   Send(new GpuCommandBufferMsg_Flush(route_id_,
-                                  put_offset,
-                                  &last_state_));
+                                     put_offset,
+                                     &last_state_));
   return last_state_;
 }
 
@@ -140,9 +140,9 @@ Buffer CommandBufferProxy::GetTransferBuffer(int32 id) {
   base::SharedMemoryHandle handle;
   uint32 size;
   if (!Send(new GpuCommandBufferMsg_GetTransferBuffer(route_id_,
-                                                   id,
-                                                   &handle,
-                                                   &size))) {
+                                                      id,
+                                                      &handle,
+                                                      &size))) {
     return Buffer();
   }
 
diff --git a/chrome/renderer/command_buffer_proxy.h b/chrome/renderer/command_buffer_proxy.h
index 20275b8..31b5319 100644
--- a/chrome/renderer/command_buffer_proxy.h
+++ b/chrome/renderer/command_buffer_proxy.h
@@ -84,7 +84,7 @@ class CommandBufferProxy : public gpu::CommandBuffer,
   void OnNotifyRepaint();
 
   // As with the service, the client takes ownership of the ring buffer.
-  int32 size_;
+  int32 num_entries_;
   scoped_ptr<base::SharedMemory> ring_buffer_;
 
   // Local cache of id to transfer buffer mapping.
diff --git a/chrome/renderer/ggl/ggl.cc b/chrome/renderer/ggl/ggl.cc
index da52114..9ab30c4 100644
--- a/chrome/renderer/ggl/ggl.cc
+++ b/chrome/renderer/ggl/ggl.cc
@@ -147,7 +147,7 @@ bool Context::Initialize(gfx::NativeViewId view, const gfx::Size& size) {
 
   // Create the GLES2 helper, which writes the command buffer protocol.
   gles2_helper_ = new gpu::gles2::GLES2CmdHelper(command_buffer_);
-  if (!gles2_helper_->Initialize()) {
+  if (!gles2_helper_->Initialize(kCommandBufferSize)) {
     Destroy();
     return false;
   }
diff --git a/chrome/renderer/webplugin_delegate_pepper.cc b/chrome/renderer/webplugin_delegate_pepper.cc
index c545c87..54f7ff5 100644
--- a/chrome/renderer/webplugin_delegate_pepper.cc
+++ b/chrome/renderer/webplugin_delegate_pepper.cc
@@ -71,7 +71,7 @@ struct Device3DImpl {
 
 const int32 kDefaultCommandBufferSize = 1024 * 1024;
 
-} // namespace
+}  // namespace
 
 WebPluginDelegatePepper* WebPluginDelegatePepper::Create(
     const FilePath& filename,
@@ -526,7 +526,7 @@ NPError WebPluginDelegatePepper::Device3DInitializeContext(
         context->waitForProgress = true;
         Buffer ring_buffer = command_buffer_->GetRingBuffer();
         context->commandBuffer = ring_buffer.ptr;
-        context->commandBufferSize = state.size;
+        context->commandBufferSize = state.num_entries;
         context->repaintCallback = NULL;
         Synchronize3DContext(context, state);
 
diff --git a/gpu/command_buffer/client/cmd_buffer_helper.cc b/gpu/command_buffer/client/cmd_buffer_helper.cc
index 4f9d725..72724d7 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper.cc
+++ b/gpu/command_buffer/client/cmd_buffer_helper.cc
@@ -19,14 +19,18 @@ CommandBufferHelper::CommandBufferHelper(CommandBuffer* command_buffer)
       put_(0) {
 }
 
-bool CommandBufferHelper::Initialize() {
+bool CommandBufferHelper::Initialize(int32 ring_buffer_size) {
   ring_buffer_ = command_buffer_->GetRingBuffer();
   if (!ring_buffer_.ptr)
     return false;
 
   CommandBuffer::State state = command_buffer_->GetState();
   entries_ = static_cast<CommandBufferEntry*>(ring_buffer_.ptr);
-  entry_count_ = state.size;
+  int32 num_ring_buffer_entries = ring_buffer_size / sizeof(CommandBufferEntry);
+  if (num_ring_buffer_entries > state.num_entries) {
+    return false;
+  }
+  entry_count_ = num_ring_buffer_entries;
   put_ = state.put_offset;
   SynchronizeState(state);
   return true;
diff --git a/gpu/command_buffer/client/cmd_buffer_helper.h b/gpu/command_buffer/client/cmd_buffer_helper.h
index af18226..cc93057 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper.h
+++ b/gpu/command_buffer/client/cmd_buffer_helper.h
@@ -36,7 +36,11 @@ class CommandBufferHelper {
   explicit CommandBufferHelper(CommandBuffer* command_buffer);
   virtual ~CommandBufferHelper();
 
-  bool Initialize();
+  // Initializes the CommandBufferHelper.
+  // Parameters:
+  //   ring_buffer_size: The size of the ring buffer portion of the command
+  //       buffer.
+  bool Initialize(int32 ring_buffer_size);
 
   // Flushes the commands, setting the put pointer to let the buffer interface
   // know that new commands have been added. After a flush returns, the command
diff --git a/gpu/command_buffer/client/cmd_buffer_helper_test.cc b/gpu/command_buffer/client/cmd_buffer_helper_test.cc
index 2f1d4af..a61d83b 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper_test.cc
+++ b/gpu/command_buffer/client/cmd_buffer_helper_test.cc
@@ -41,7 +41,7 @@ class CommandBufferHelperTest : public testing::Test {
         .WillRepeatedly(Return(error::kNoError));
 
     command_buffer_.reset(new CommandBufferService);
-    command_buffer_->Initialize(kNumCommandEntries);
+    command_buffer_->Initialize(kCommandBufferSizeBytes);
     Buffer ring_buffer = command_buffer_->GetRingBuffer();
 
     parser_ = new CommandParser(ring_buffer.ptr,
@@ -59,7 +59,7 @@ class CommandBufferHelperTest : public testing::Test {
     api_mock_->set_engine(gpu_processor_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
-    helper_->Initialize();
+    helper_->Initialize(kCommandBufferSizeBytes);
   }
 
   virtual void TearDown() {
diff --git a/gpu/command_buffer/client/fenced_allocator_test.cc b/gpu/command_buffer/client/fenced_allocator_test.cc
index 1d5c970..21b511a 100644
--- a/gpu/command_buffer/client/fenced_allocator_test.cc
+++ b/gpu/command_buffer/client/fenced_allocator_test.cc
@@ -42,7 +42,7 @@ class BaseFencedAllocatorTest : public testing::Test {
                               Return(error::kNoError)));
 
     command_buffer_.reset(new CommandBufferService);
-    command_buffer_->Initialize(kBufferSize / sizeof(CommandBufferEntry));
+    command_buffer_->Initialize(kBufferSize);
     Buffer ring_buffer = command_buffer_->GetRingBuffer();
 
     parser_ = new CommandParser(ring_buffer.ptr,
@@ -60,7 +60,7 @@ class BaseFencedAllocatorTest : public testing::Test {
     api_mock_->set_engine(gpu_processor_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
-    helper_->Initialize();
+    helper_->Initialize(kBufferSize);
   }
 
   int32 GetToken() {
diff --git a/gpu/command_buffer/client/gles2_demo.cc b/gpu/command_buffer/client/gles2_demo.cc
index b2a0497..365cf53 100644
--- a/gpu/command_buffer/client/gles2_demo.cc
+++ b/gpu/command_buffer/client/gles2_demo.cc
@@ -66,7 +66,7 @@ bool GLES2Demo::Setup(void* hwnd, int32 size) {
       NewCallback(gpu_processor, &GPUProcessor::ProcessCommands));
 
   GLES2CmdHelper* helper = new GLES2CmdHelper(command_buffer.get());
-  if (!helper->Initialize()) {
+  if (!helper->Initialize(size)) {
     // TODO(gman): cleanup.
     return false;
   }
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index b73b889..78a0682 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -22,8 +22,8 @@ class GLES2MockCommandBufferHelper : public CommandBuffer {
   virtual bool Initialize(int32 size) {
     ring_buffer_.reset(new CommandBufferEntry[size]);
     ring_buffer_buffer_.ptr = ring_buffer_.get();
-    ring_buffer_buffer_.size = size * sizeof(ring_buffer_[0]);
-    state_.size = size;
+    ring_buffer_buffer_.size = size;
+    state_.num_entries = size / sizeof(ring_buffer_[0]);
     state_.token = 10000;  // All token checks in the tests should pass.
     return true;
   }
@@ -150,7 +150,7 @@ class GLES2ImplementationTest : public testing::Test {
 
   virtual void SetUp() {
     command_buffer_.reset(new MockGLES2CommandBuffer());
-    command_buffer_->Initialize(kNumCommandEntries);
+    command_buffer_->Initialize(kCommandBufferSizeBytes);
 
     EXPECT_EQ(kTransferBufferId,
               command_buffer_->CreateTransferBuffer(kTransferBufferSize));
@@ -158,7 +158,7 @@ class GLES2ImplementationTest : public testing::Test {
     ClearTransferBuffer();
 
     helper_.reset(new GLES2CmdHelper(command_buffer_.get()));
-    helper_->Initialize();
+    helper_->Initialize(kCommandBufferSizeBytes);
 
     #if defined(GLES2_SUPPORT_CLIENT_SIDE_BUFFERS)
       EXPECT_CALL(*command_buffer_, OnFlush(_))
@@ -681,8 +681,8 @@ TEST_F(GLES2ImplementationTest, ReadPixels2Reads) {
   scoped_array<int8> buffer(new int8[kWidth * kHeight * kBytesPerPixel]);
 
   EXPECT_CALL(*command_buffer_, OnFlush(_))
-      .WillOnce(SetMemory(uint32(1)))
-      .WillOnce(SetMemory(uint32(1)))
+      .WillOnce(SetMemory(static_cast<uint32>(1)))
+      .WillOnce(SetMemory(static_cast<uint32>(1)))
       .RetiresOnSaturation();
 
   gl_->ReadPixels(0, 0, kWidth, kHeight, kFormat, kType, buffer.get());
diff --git a/gpu/command_buffer/client/ring_buffer_test.cc b/gpu/command_buffer/client/ring_buffer_test.cc
index 8af4eb5..b2ffb01 100644
--- a/gpu/command_buffer/client/ring_buffer_test.cc
+++ b/gpu/command_buffer/client/ring_buffer_test.cc
@@ -43,7 +43,7 @@ class BaseRingBufferTest : public testing::Test {
                               Return(error::kNoError)));
 
     command_buffer_.reset(new CommandBufferService);
-    command_buffer_->Initialize(kBufferSize / sizeof(CommandBufferEntry));
+    command_buffer_->Initialize(kBufferSize);
     Buffer ring_buffer = command_buffer_->GetRingBuffer();
 
     parser_ = new CommandParser(ring_buffer.ptr,
@@ -61,7 +61,7 @@ class BaseRingBufferTest : public testing::Test {
     api_mock_->set_engine(gpu_processor_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
-    helper_->Initialize();
+    helper_->Initialize(kBufferSize);
   }
 
   int32 GetToken() {
diff --git a/gpu/command_buffer/common/command_buffer.h b/gpu/command_buffer/common/command_buffer.h
index 106d9b3..41eda77 100644
--- a/gpu/command_buffer/common/command_buffer.h
+++ b/gpu/command_buffer/common/command_buffer.h
@@ -19,7 +19,7 @@ class CommandBuffer {
 
   struct State {
     State()
-        : size(0),
+        : num_entries(0),
           get_offset(0),
           put_offset(0),
           token(-1),
@@ -27,7 +27,7 @@ class CommandBuffer {
     }
 
     // Size of the command buffer in command buffer entries.
-    int32 size;
+    int32 num_entries;
 
     // The offset (in entries) from which the reader is reading.
     int32 get_offset;
@@ -52,8 +52,7 @@ class CommandBuffer {
   virtual ~CommandBuffer() {
   }
 
-  // Initialize the command buffer with the given size (number of command
-  // entries).
+  // Initialize the command buffer with the given size.
   virtual bool Initialize(int32 size) = 0;
 
   // Gets the ring buffer for the command buffer.
diff --git a/gpu/command_buffer/service/command_buffer_service.cc b/gpu/command_buffer/service/command_buffer_service.cc
index 8dca2f6..b692701 100644
--- a/gpu/command_buffer/service/command_buffer_service.cc
+++ b/gpu/command_buffer/service/command_buffer_service.cc
@@ -14,7 +14,7 @@ using ::base::SharedMemory;
 namespace gpu {
 
 CommandBufferService::CommandBufferService()
-    : size_(0),
+    : num_entries_(0),
       get_offset_(0),
       put_offset_(0),
       token_(0),
@@ -34,16 +34,15 @@ bool CommandBufferService::Initialize(int32 size) {
   if (size <= 0 || size > kMaxCommandBufferSize)
     return false;
 
-  size_ = size;
+  num_entries_ = size / sizeof(CommandBufferEntry);
 
   ring_buffer_.reset(new SharedMemory);
-  uint32 size_bytes = size * sizeof(CommandBufferEntry);
-  if (ring_buffer_->Create(std::wstring(), false, false, size_bytes)) {
-    if (ring_buffer_->Map(size_bytes))
+  if (ring_buffer_->Create(std::wstring(), false, false, size)) {
+    if (ring_buffer_->Map(size))
       return true;
   }
 
-  size_ = 0;
+  num_entries_ = 0;
   ring_buffer_.reset();
   return false;
 }
@@ -60,7 +59,7 @@ Buffer CommandBufferService::GetRingBuffer() {
 
 CommandBufferService::State CommandBufferService::GetState() {
   State state;
-  state.size = size_;
+  state.num_entries = num_entries_;
   state.get_offset = get_offset_;
   state.put_offset = put_offset_;
   state.token = token_;
@@ -70,7 +69,7 @@ CommandBufferService::State CommandBufferService::GetState() {
 }
 
 CommandBufferService::State CommandBufferService::Flush(int32 put_offset) {
-  if (put_offset < 0 || put_offset > size_) {
+  if (put_offset < 0 || put_offset > num_entries_) {
     error_ = gpu::error::kOutOfBounds;
     return GetState();
   }
@@ -85,7 +84,7 @@ CommandBufferService::State CommandBufferService::Flush(int32 put_offset) {
 }
 
 void CommandBufferService::SetGetOffset(int32 get_offset) {
-  DCHECK(get_offset >= 0 && get_offset < size_);
+  DCHECK(get_offset >= 0 && get_offset < num_entries_);
   get_offset_ = get_offset;
 }
 
diff --git a/gpu/command_buffer/service/command_buffer_service.h b/gpu/command_buffer/service/command_buffer_service.h
index ffd110f..d9be20a 100644
--- a/gpu/command_buffer/service/command_buffer_service.h
+++ b/gpu/command_buffer/service/command_buffer_service.h
@@ -50,7 +50,7 @@ class CommandBufferService : public CommandBuffer {
 
  private:
   scoped_ptr< base::SharedMemory> ring_buffer_;
-  int32 size_;
+  int32 num_entries_;
   int32 get_offset_;
   int32 put_offset_;
   scoped_ptr<Callback0::Type> put_offset_change_callback_;
diff --git a/gpu/command_buffer/service/gpu_processor_unittest.cc b/gpu/command_buffer/service/gpu_processor_unittest.cc
index cfc3662..6245a93 100644
--- a/gpu/command_buffer/service/gpu_processor_unittest.cc
+++ b/gpu/command_buffer/service/gpu_processor_unittest.cc
@@ -43,7 +43,7 @@ class GPUProcessorTest : public testing::Test {
       .WillByDefault(Return(shared_memory_buffer_));
 
     CommandBuffer::State default_state;
-    default_state.size = kRingBufferEntries;
+    default_state.num_entries = kRingBufferEntries;
     ON_CALL(*command_buffer_.get(), GetState())
       .WillByDefault(Return(default_state));
 
diff --git a/gpu/demos/framework/window.cc b/gpu/demos/framework/window.cc
index fce2a08..092dc1d 100644
--- a/gpu/demos/framework/window.cc
+++ b/gpu/demos/framework/window.cc
@@ -69,7 +69,7 @@ bool Window::CreateRenderContext(gfx::PluginWindowHandle hwnd) {
       NewCallback(gpu_processor, &GPUProcessor::ProcessCommands));
 
   GLES2CmdHelper* helper = new GLES2CmdHelper(command_buffer.get());
-  if (!helper->Initialize()) {
+  if (!helper->Initialize(kCommandBufferSize)) {
     // TODO(alokp): cleanup.
     return false;
   }
diff --git a/gpu/pgl/command_buffer_pepper.cc b/gpu/pgl/command_buffer_pepper.cc
index 6765f1e..53d67d2 100644
--- a/gpu/pgl/command_buffer_pepper.cc
+++ b/gpu/pgl/command_buffer_pepper.cc
@@ -63,7 +63,7 @@ CommandBuffer::State CommandBufferPepper::GetState() {
                               NULL);
 
   CommandBuffer::State state;
-  state.size = output_attribs[1];
+  state.num_entries = output_attribs[1];
   state.get_offset = output_attribs[3];
   state.put_offset = output_attribs[5];
   state.token = output_attribs[7];
@@ -106,7 +106,7 @@ CommandBuffer::State CommandBufferPepper::Flush(int32 put_offset) {
                               NULL);
 
   CommandBuffer::State state;
-  state.size = output_attribs[1];
+  state.num_entries = output_attribs[1];
   state.get_offset = output_attribs[3];
   state.put_offset = output_attribs[5];
   state.token = output_attribs[7];
@@ -181,7 +181,7 @@ gpu::error::Error CommandBufferPepper::GetCachedError() {
 
 CommandBuffer::State CommandBufferPepper::ConvertState() {
   CommandBuffer::State state;
-  state.size = context_->commandBufferSize;
+  state.num_entries = context_->commandBufferSize;
   state.get_offset = context_->getOffset;
   state.put_offset = context_->putOffset;
   state.token = context_->token;
diff --git a/gpu/pgl/pgl.cc b/gpu/pgl/pgl.cc
index b373478..c3b62f1 100644
--- a/gpu/pgl/pgl.cc
+++ b/gpu/pgl/pgl.cc
@@ -18,8 +18,8 @@ const int32 kTransferBufferSize = 512 * 1024;
 class PGLContextImpl {
  public:
   PGLContextImpl(NPP npp,
-              NPDevice* device,
-              NPDeviceContext3D* device_context);
+                 NPDevice* device,
+                 NPDeviceContext3D* device_context);
   ~PGLContextImpl();
 
   // Initlaize a PGL context with a transfer buffer of a particular size.
@@ -74,7 +74,8 @@ PGLBoolean PGLContextImpl::Initialize(int32 transfer_buffer_size) {
   command_buffer_ = new CommandBufferPepper(
       npp_, device_, device_context_);
   gles2_helper_ = new gpu::gles2::GLES2CmdHelper(command_buffer_);
-  if (gles2_helper_->Initialize()) {
+  gpu::Buffer buffer = command_buffer_->GetRingBuffer();
+  if (gles2_helper_->Initialize(buffer.size)) {
     transfer_buffer_id_ =
         command_buffer_->CreateTransferBuffer(kTransferBufferSize);
     gpu::Buffer transfer_buffer =
@@ -131,8 +132,7 @@ PGLBoolean PGLContextImpl::MakeCurrent(PGLContextImpl* pgl_context) {
     if (pgl_context->device_context_->error != NPDeviceContext3DError_NoError)
       return PGL_FALSE;
 #endif
-  }
-  else {
+  } else {
     gles2::SetGLContext(NULL);
   }
 
@@ -167,7 +167,6 @@ PGLInt PGLContextImpl::GetError() {
 }  // namespace anonymous
 
 extern "C" {
-
 PGLBoolean pglInitialize() {
   if (g_pgl_context_key_allocated)
     return PGL_TRUE;