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

14 files changed, 46 insertions, 41 deletions

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;