Optimize GetBucketContents

TEST=unit tests
BUG=122642


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

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

6 files changed, 285 insertions, 109 deletions

diff --git a/gpu/command_buffer/client/cmd_buffer_helper.h b/gpu/command_buffer/client/cmd_buffer_helper.h
index 8a7e122..95f3a49 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper.h
+++ b/gpu/command_buffer/client/cmd_buffer_helper.h
@@ -209,14 +209,20 @@ class GPU_EXPORT CommandBufferHelper {
     }
   }
 
-  void GetBucketSize(uint32 bucket_id,
-                     uint32 shared_memory_id,
-                     uint32 shared_memory_offset) {
-    cmd::GetBucketSize* cmd = GetCmdSpace<cmd::GetBucketSize>();
+  void GetBucketStart(uint32 bucket_id,
+                      uint32 result_memory_id,
+                      uint32 result_memory_offset,
+                      uint32 data_memory_size,
+                      uint32 data_memory_id,
+                      uint32 data_memory_offset) {
+    cmd::GetBucketStart* cmd = GetCmdSpace<cmd::GetBucketStart>();
     if (cmd) {
       cmd->Init(bucket_id,
-                shared_memory_id,
-                shared_memory_offset);
+                result_memory_id,
+                result_memory_offset,
+                data_memory_size,
+                data_memory_id,
+                data_memory_offset);
     }
   }
 
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 2519ba0..0230f25 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -633,30 +633,41 @@ bool GLES2Implementation::GetBucketContents(uint32 bucket_id,
                                             std::vector<int8>* data) {
   TRACE_EVENT0("gpu", "GLES2::GetBucketContents");
   GPU_DCHECK(data);
-  typedef cmd::GetBucketSize::Result Result;
+  const uint32 kStartSize = 32 * 1024;
+  ScopedTransferBufferPtr buffer(kStartSize, helper_, transfer_buffer_);
+  if (!buffer.valid()) {
+    return false;
+  }
+  typedef cmd::GetBucketStart::Result Result;
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return false;
   }
   *result = 0;
-  helper_->GetBucketSize(bucket_id, GetResultShmId(), GetResultShmOffset());
+  helper_->GetBucketStart(
+      bucket_id, GetResultShmId(), GetResultShmOffset(),
+      buffer.size(), buffer.shm_id(), buffer.offset());
   WaitForCmd();
   uint32 size = *result;
   data->resize(size);
   if (size > 0u) {
     uint32 offset = 0;
     while (size) {
-      ScopedTransferBufferPtr buffer(size, helper_, transfer_buffer_);
       if (!buffer.valid()) {
-        return false;
+        buffer.Reset(size);
+        if (!buffer.valid()) {
+          return false;
+        }
+        helper_->GetBucketData(
+            bucket_id, offset, buffer.size(), buffer.shm_id(), buffer.offset());
+        WaitForCmd();
       }
-      helper_->GetBucketData(
-          bucket_id, offset, buffer.size(), buffer.shm_id(), buffer.offset());
-      WaitForCmd();
-      memcpy(&(*data)[offset], buffer.address(), buffer.size());
-      offset += buffer.size();
-      size -= buffer.size();
-    }
+      uint32 size_to_copy = std::min(size, buffer.size());
+      memcpy(&(*data)[offset], buffer.address(), size_to_copy);
+      offset += size_to_copy;
+      size -= size_to_copy;
+      buffer.Release();
+    };
     // Free the bucket. This is not required but it does free up the memory.
     // and we don't have to wait for the result so from the client's perspective
     // it's cheap.
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index 7c4851a..e4a1ebe 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -446,6 +446,10 @@ class GLES2ImplementationTest : public testing::Test {
     return gl_->GetError();
   }
 
+  bool GetBucketContents(uint32 bucket_id, std::vector<int8>* data) {
+    return gl_->GetBucketContents(bucket_id, data);
+  }
+
   Sequence sequence_;
   scoped_ptr<MockClientCommandBuffer> command_buffer_;
   scoped_ptr<GLES2CmdHelper> helper_;
@@ -505,6 +509,57 @@ TEST_F(GLES2ImplementationTest, Basic) {
   EXPECT_TRUE(gl_->share_group() != NULL);
 }
 
+TEST_F(GLES2ImplementationTest, GetBucketContents) {
+  const uint32 kBucketId = GLES2Implementation::kResultBucketId;
+  const uint32 kTestSize = MaxTransferBufferSize() + 32;
+
+  scoped_array<uint8> buf(new uint8 [kTestSize]);
+  uint8* expected_data = buf.get();
+  for (uint32 ii = 0; ii < kTestSize; ++ii) {
+    expected_data[ii] = ii * 3;
+  }
+
+  struct Cmds {
+    cmd::GetBucketStart get_bucket_start;
+    cmd::SetToken set_token1;
+    cmd::GetBucketData get_bucket_data;
+    cmd::SetToken set_token2;
+    cmd::SetBucketSize set_bucket_size2;
+  };
+
+  ExpectedMemoryInfo mem1 = GetExpectedMemory(MaxTransferBufferSize());
+  ExpectedMemoryInfo result1 = GetExpectedResultMemory(sizeof(uint32));
+  ExpectedMemoryInfo mem2 = GetExpectedMemory(
+      kTestSize - MaxTransferBufferSize());
+
+  Cmds expected;
+  expected.get_bucket_start.Init(
+      kBucketId, result1.id, result1.offset,
+      MaxTransferBufferSize(), mem1.id, mem1.offset);
+  expected.set_token1.Init(GetNextToken());
+  expected.get_bucket_data.Init(
+      kBucketId, MaxTransferBufferSize(),
+      kTestSize - MaxTransferBufferSize(), mem2.id, mem2.offset);
+  expected.set_bucket_size2.Init(kBucketId, 0);
+  expected.set_token2.Init(GetNextToken());
+
+  EXPECT_CALL(*command_buffer(), OnFlush())
+      .WillOnce(DoAll(
+          SetMemory(result1.ptr, kTestSize),
+          SetMemoryFromArray(
+              mem1.ptr, expected_data, MaxTransferBufferSize())))
+      .WillOnce(SetMemoryFromArray(
+          mem2.ptr, expected_data + MaxTransferBufferSize(),
+          kTestSize - MaxTransferBufferSize()))
+      .RetiresOnSaturation();
+
+  std::vector<int8> data;
+  GetBucketContents(kBucketId, &data);
+  EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
+  ASSERT_EQ(kTestSize, data.size());
+  EXPECT_EQ(0, memcmp(expected_data, &data[0], data.size()));
+}
+
 TEST_F(GLES2ImplementationTest, ShaderSource) {
   const uint32 kBucketId = GLES2Implementation::kResultBucketId;
   const GLuint kShaderId = 456;
@@ -566,29 +621,28 @@ TEST_F(GLES2ImplementationTest, GetShaderSource) {
   struct Cmds {
     cmd::SetBucketSize set_bucket_size1;
     GetShaderSource get_shader_source;
-    cmd::GetBucketSize get_bucket_size;
-    cmd::GetBucketData get_bucket_data;
+    cmd::GetBucketStart get_bucket_start;
     cmd::SetToken set_token1;
     cmd::SetBucketSize set_bucket_size2;
   };
 
+  ExpectedMemoryInfo mem1 = GetExpectedMemory(MaxTransferBufferSize());
   ExpectedMemoryInfo result1 = GetExpectedResultMemory(sizeof(uint32));
-  ExpectedMemoryInfo mem1 = GetExpectedMemory(sizeof(kString));
 
   Cmds expected;
   expected.set_bucket_size1.Init(kBucketId, 0);
   expected.get_shader_source.Init(kShaderId, kBucketId);
-  expected.get_bucket_size.Init(kBucketId, result1.id, result1.offset);
-  expected.get_bucket_data.Init(
-      kBucketId, 0, sizeof(kString), mem1.id, mem1.offset);
+  expected.get_bucket_start.Init(
+      kBucketId, result1.id, result1.offset,
+      MaxTransferBufferSize(), mem1.id, mem1.offset);
   expected.set_token1.Init(GetNextToken());
   expected.set_bucket_size2.Init(kBucketId, 0);
   char buf[sizeof(kString) + 1];
   memset(buf, kBad, sizeof(buf));
 
   EXPECT_CALL(*command_buffer(), OnFlush())
-      .WillOnce(SetMemory(result1.ptr, uint32(sizeof(kString))))
-      .WillOnce(SetMemory(mem1.ptr, kString))
+      .WillOnce(DoAll(SetMemory(result1.ptr, uint32(sizeof(kString))),
+                      SetMemory(mem1.ptr, kString)))
       .RetiresOnSaturation();
 
   GLsizei length = 0;
@@ -1604,34 +1658,33 @@ TEST_F(GLES2ImplementationTest, GetProgramInfoCHROMIUMGoodArgs) {
   const Str7 kString = {"foobar"};
   char buf[20];
 
-  ExpectedMemoryInfo result1 =
-      GetExpectedResultMemory(sizeof(cmd::GetBucketSize::Result));
   ExpectedMemoryInfo mem1 =
-      GetExpectedMemory(sizeof(kString));
+      GetExpectedMemory(MaxTransferBufferSize());
+  ExpectedMemoryInfo result1 =
+      GetExpectedResultMemory(sizeof(cmd::GetBucketStart::Result));
   ExpectedMemoryInfo result2 =
       GetExpectedResultMemory(sizeof(GetError::Result));
 
   memset(buf, kBad, sizeof(buf));
   EXPECT_CALL(*command_buffer(), OnFlush())
-      .WillOnce(SetMemory(result1.ptr, uint32(sizeof(kString))))
-      .WillOnce(SetMemory(mem1.ptr, kString))
+      .WillOnce(DoAll(SetMemory(result1.ptr, uint32(sizeof(kString))),
+                      SetMemory(mem1.ptr, kString)))
       .WillOnce(SetMemory(result2.ptr, GLuint(GL_NO_ERROR)))
       .RetiresOnSaturation();
 
   struct Cmds {
     cmd::SetBucketSize set_bucket_size1;
     GetProgramInfoCHROMIUM get_program_info;
-    cmd::GetBucketSize get_bucket_size;
-    cmd::GetBucketData get_bucket_data;
+    cmd::GetBucketStart get_bucket_start;
     cmd::SetToken set_token1;
     cmd::SetBucketSize set_bucket_size2;
   };
   Cmds expected;
   expected.set_bucket_size1.Init(kBucketId, 0);
   expected.get_program_info.Init(kProgramId, kBucketId);
-  expected.get_bucket_size.Init(kBucketId, result1.id, result1.offset);
-  expected.get_bucket_data.Init(
-      kBucketId, 0, sizeof(kString), mem1.id, mem1.offset);
+  expected.get_bucket_start.Init(
+      kBucketId, result1.id, result1.offset,
+      MaxTransferBufferSize(), mem1.id, mem1.offset);
   expected.set_token1.Init(GetNextToken());
   expected.set_bucket_size2.Init(kBucketId, 0);
   gl_->GetProgramInfoCHROMIUM(kProgramId, sizeof(buf), &size, &buf);
@@ -1649,9 +1702,9 @@ TEST_F(GLES2ImplementationTest, GetProgramInfoCHROMIUMBadArgs) {
   const Str7 kString = {"foobar"};
   char buf[20];
 
+  ExpectedMemoryInfo mem1 = GetExpectedMemory(MaxTransferBufferSize());
   ExpectedMemoryInfo result1 =
-      GetExpectedResultMemory(sizeof(cmd::GetBucketSize::Result));
-  ExpectedMemoryInfo mem1 = GetExpectedMemory(sizeof(kString));
+      GetExpectedResultMemory(sizeof(cmd::GetBucketStart::Result));
   ExpectedMemoryInfo result2 =
       GetExpectedResultMemory(sizeof(GetError::Result));
   ExpectedMemoryInfo result3 =
@@ -1660,8 +1713,8 @@ TEST_F(GLES2ImplementationTest, GetProgramInfoCHROMIUMBadArgs) {
       GetExpectedResultMemory(sizeof(GetError::Result));
 
   EXPECT_CALL(*command_buffer(), OnFlush())
-      .WillOnce(SetMemory(result1.ptr, uint32(sizeof(kString))))
-      .WillOnce(SetMemory(mem1.ptr,  kString))
+      .WillOnce(DoAll(SetMemory(result1.ptr, uint32(sizeof(kString))),
+                      SetMemory(mem1.ptr,  kString)))
       .WillOnce(SetMemory(result2.ptr, GLuint(GL_NO_ERROR)))
       .WillOnce(SetMemory(result3.ptr, GLuint(GL_NO_ERROR)))
       .WillOnce(SetMemory(result4.ptr, GLuint(GL_NO_ERROR)))
@@ -1671,17 +1724,16 @@ TEST_F(GLES2ImplementationTest, GetProgramInfoCHROMIUMBadArgs) {
   struct Cmds {
     cmd::SetBucketSize set_bucket_size1;
     GetProgramInfoCHROMIUM get_program_info;
-    cmd::GetBucketSize get_bucket_size;
-    cmd::GetBucketData get_bucket_data;
+    cmd::GetBucketStart get_bucket_start;
     cmd::SetToken set_token1;
     cmd::SetBucketSize set_bucket_size2;
   };
   Cmds expected;
   expected.set_bucket_size1.Init(kBucketId, 0);
   expected.get_program_info.Init(kProgramId, kBucketId);
-  expected.get_bucket_size.Init(kBucketId, result1.id, result1.offset);
-  expected.get_bucket_data.Init(
-      kBucketId, 0, sizeof(kString), mem1.id, mem1.offset);
+  expected.get_bucket_start.Init(
+      kBucketId, result1.id, result1.offset,
+      MaxTransferBufferSize(), mem1.id, mem1.offset);
   expected.set_token1.Init(GetNextToken());
   expected.set_bucket_size2.Init(kBucketId, 0);
   gl_->GetProgramInfoCHROMIUM(kProgramId, 6, &size, &buf);
@@ -2134,28 +2186,27 @@ TEST_F(GLES2ImplementationTest, GetString) {
   struct Cmds {
     cmd::SetBucketSize set_bucket_size1;
     GetString get_string;
-    cmd::GetBucketSize get_bucket_size;
-    cmd::GetBucketData get_bucket_data;
+    cmd::GetBucketStart get_bucket_start;
     cmd::SetToken set_token1;
     cmd::SetBucketSize set_bucket_size2;
   };
+  ExpectedMemoryInfo mem1 = GetExpectedMemory(MaxTransferBufferSize());
   ExpectedMemoryInfo result1 =
-      GetExpectedResultMemory(sizeof(cmd::GetBucketSize::Result));
-  ExpectedMemoryInfo mem1 = GetExpectedMemory(sizeof(kString));
+      GetExpectedResultMemory(sizeof(cmd::GetBucketStart::Result));
   Cmds expected;
   expected.set_bucket_size1.Init(kBucketId, 0);
   expected.get_string.Init(GL_EXTENSIONS, kBucketId);
-  expected.get_bucket_size.Init(kBucketId, result1.id, result1.offset);
-  expected.get_bucket_data.Init(
-      kBucketId, 0, sizeof(kString), mem1.id, mem1.offset);
+  expected.get_bucket_start.Init(
+      kBucketId, result1.id, result1.offset,
+      MaxTransferBufferSize(), mem1.id, mem1.offset);
   expected.set_token1.Init(GetNextToken());
   expected.set_bucket_size2.Init(kBucketId, 0);
   char buf[sizeof(kString) + 1];
   memset(buf, kBad, sizeof(buf));
 
   EXPECT_CALL(*command_buffer(), OnFlush())
-      .WillOnce(SetMemory(result1.ptr, uint32(sizeof(kString))))
-      .WillOnce(SetMemory(mem1.ptr, kString))
+      .WillOnce(DoAll(SetMemory(result1.ptr, uint32(sizeof(kString))),
+                      SetMemory(mem1.ptr, kString)))
       .RetiresOnSaturation();
 
   const GLubyte* result = gl_->GetString(GL_EXTENSIONS);
@@ -2169,30 +2220,29 @@ TEST_F(GLES2ImplementationTest, PixelStoreiGLPackReverseRowOrderANGLE) {
   struct Cmds {
     cmd::SetBucketSize set_bucket_size1;
     GetString get_string;
-    cmd::GetBucketSize get_bucket_size;
-    cmd::GetBucketData get_bucket_data;
+    cmd::GetBucketStart get_bucket_start;
     cmd::SetToken set_token1;
     cmd::SetBucketSize set_bucket_size2;
     PixelStorei pixel_store;
   };
 
+  ExpectedMemoryInfo mem1 = GetExpectedMemory(MaxTransferBufferSize());
   ExpectedMemoryInfo result1 =
-      GetExpectedResultMemory(sizeof(cmd::GetBucketSize::Result));
-  ExpectedMemoryInfo mem1 = GetExpectedMemory(sizeof(kString));
+      GetExpectedResultMemory(sizeof(cmd::GetBucketStart::Result));
 
   Cmds expected;
   expected.set_bucket_size1.Init(kBucketId, 0);
   expected.get_string.Init(GL_EXTENSIONS, kBucketId);
-  expected.get_bucket_size.Init(kBucketId, result1.id, result1.offset);
-  expected.get_bucket_data.Init(
-      kBucketId, 0, sizeof(kString), mem1.id, mem1.offset);
+  expected.get_bucket_start.Init(
+      kBucketId, result1.id, result1.offset,
+      MaxTransferBufferSize(), mem1.id, mem1.offset);
   expected.set_token1.Init(GetNextToken());
   expected.set_bucket_size2.Init(kBucketId, 0);
   expected.pixel_store.Init(GL_PACK_REVERSE_ROW_ORDER_ANGLE, 1);
 
   EXPECT_CALL(*command_buffer(), OnFlush())
-      .WillOnce(SetMemory(result1.ptr, uint32(sizeof(kString))))
-      .WillOnce(SetMemory(mem1.ptr, kString))
+      .WillOnce(DoAll(SetMemory(result1.ptr, uint32(sizeof(kString))),
+                      SetMemory(mem1.ptr, kString)))
       .RetiresOnSaturation();
 
   gl_->PixelStorei(GL_PACK_REVERSE_ROW_ORDER_ANGLE, 1);
diff --git a/gpu/command_buffer/common/cmd_buffer_common.h b/gpu/command_buffer/common/cmd_buffer_common.h
index 97b4495..2e0073a 100644
--- a/gpu/command_buffer/common/cmd_buffer_common.h
+++ b/gpu/command_buffer/common/cmd_buffer_common.h
@@ -158,7 +158,7 @@ namespace cmd {
   OP(SetBucketSize)                 /*  7 */ \
   OP(SetBucketData)                 /*  8 */ \
   OP(SetBucketDataImmediate)        /*  9 */ \
-  OP(GetBucketSize)                 /* 10 */ \
+  OP(GetBucketStart)                /* 10 */ \
   OP(GetBucketData)                 /* 11 */ \
 
 // Common commands.
@@ -533,14 +533,19 @@ COMPILE_ASSERT(offsetof(SetBucketDataImmediate, offset) == 8,
 COMPILE_ASSERT(offsetof(SetBucketDataImmediate, size) == 12,
                Offsetof_SetBucketDataImmediate_size_not_12);
 
-// Gets the size of a bucket the service has available. Sending a variable size
-// result back to the client, for example any API that returns a string, is
-// problematic since the largest thing you can send back is the size of your
-// shared memory. This command along with GetBucketData implements a way to get
-// a result a piece at a time to help solve that problem in a generic way.
-struct GetBucketSize {
-  typedef GetBucketSize ValueType;
-  static const CommandId kCmdId = kGetBucketSize;
+// Gets the start of a bucket the service has available. Sending a variable size
+// result back to the client and the portion of that result that fits in the
+// supplied shared memory. If the size of the result is larger than the supplied
+// shared memory the rest of the bucket's contents can be retrieved with
+// GetBucketData.
+//
+// This is used for example for any API that returns a string. The problem is
+// the largest thing you can send back in 1 command is the size of your shared
+// memory. This command along with GetBucketData implements a way to get a
+// result a piece at a time to help solve that problem in a generic way.
+struct GetBucketStart {
+  typedef GetBucketStart ValueType;
+  static const CommandId kCmdId = kGetBucketStart;
   static const cmd::ArgFlags kArgFlags = cmd::kFixed;
 
   typedef uint32 Result;
@@ -550,39 +555,60 @@ struct GetBucketSize {
   }
 
   void Init(uint32 _bucket_id,
-            uint32 _shared_memory_id,
-            uint32 _shared_memory_offset) {
+            uint32 _result_memory_id,
+            uint32 _result_memory_offset,
+            uint32 _data_memory_size,
+            uint32 _data_memory_id,
+            uint32 _data_memory_offset) {
     SetHeader();
     bucket_id = _bucket_id;
-    shared_memory_id = _shared_memory_id;
-    shared_memory_offset = _shared_memory_offset;
+    result_memory_id = _result_memory_id;
+    result_memory_offset = _result_memory_offset;
+    data_memory_size = _data_memory_size;
+    data_memory_id = _data_memory_id;
+    data_memory_offset = _data_memory_offset;
   }
   static void* Set(void* cmd,
                    uint32 _bucket_id,
-                   uint32 _shared_memory_id,
-                   uint32 _shared_memory_offset) {
+                   uint32 _result_memory_id,
+                   uint32 _result_memory_offset,
+                   uint32 _data_memory_size,
+                   uint32 _data_memory_id,
+                   uint32 _data_memory_offset) {
     static_cast<ValueType*>(cmd)->Init(
         _bucket_id,
-        _shared_memory_id,
-        _shared_memory_offset);
+        _result_memory_id,
+        _result_memory_offset,
+        _data_memory_size,
+        _data_memory_id,
+        _data_memory_offset);
     return NextCmdAddress<ValueType>(cmd);
   }
 
   CommandHeader header;
   uint32 bucket_id;
-  uint32 shared_memory_id;
-  uint32 shared_memory_offset;
+  uint32 result_memory_id;
+  uint32 result_memory_offset;
+  uint32 data_memory_size;
+  uint32 data_memory_id;
+  uint32 data_memory_offset;
 };
 
-COMPILE_ASSERT(sizeof(GetBucketSize) == 16, Sizeof_GetBucketSize_is_not_16);
-COMPILE_ASSERT(offsetof(GetBucketSize, header) == 0,
-               Offsetof_GetBucketSize_header_not_0);
-COMPILE_ASSERT(offsetof(GetBucketSize, bucket_id) == 4,
-               Offsetof_GetBucketSize_bucket_id_not_4);
-COMPILE_ASSERT(offsetof(GetBucketSize, shared_memory_id) == 8,
-               Offsetof_GetBucketSize_shared_memory_id_not_8);
-COMPILE_ASSERT(offsetof(GetBucketSize, shared_memory_offset) == 12,
-               Offsetof_GetBucketSize_shared_memory_offset_not_12);
+COMPILE_ASSERT(sizeof(GetBucketStart) == 28, Sizeof_GetBucketStart_is_not_28);
+COMPILE_ASSERT(offsetof(GetBucketStart, header) == 0,
+               Offsetof_GetBucketStart_header_not_0);
+COMPILE_ASSERT(offsetof(GetBucketStart, bucket_id) == 4,
+               Offsetof_GetBucketStart_bucket_id_not_4);
+COMPILE_ASSERT(offsetof(GetBucketStart, result_memory_id) == 8,
+               Offsetof_GetBucketStart_result_memory_id_not_8);
+COMPILE_ASSERT(offsetof(GetBucketStart, result_memory_offset) == 12,
+               Offsetof_GetBucketStart_result_memory_offset_not_12);
+COMPILE_ASSERT(offsetof(GetBucketStart, data_memory_size) == 16,
+               Offsetof_GetBucketStart_data_memory_size_not_16);
+COMPILE_ASSERT(offsetof(GetBucketStart, data_memory_id) == 20,
+               Offsetof_GetBucketStart_data_memory_id_not_20);
+COMPILE_ASSERT(offsetof(GetBucketStart, data_memory_offset) == 24,
+               Offsetof_GetBucketStart_data_memory_offset_not_24);
 
 // Gets a piece of a result the service as available.
 // See GetBucketSize.
diff --git a/gpu/command_buffer/service/common_decoder.cc b/gpu/command_buffer/service/common_decoder.cc
index 7b28603..b9886ce 100644
--- a/gpu/command_buffer/service/common_decoder.cc
+++ b/gpu/command_buffer/service/common_decoder.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -286,24 +286,40 @@ error::Error CommonDecoder::HandleSetBucketDataImmediate(
   return error::kNoError;
 }
 
-error::Error CommonDecoder::HandleGetBucketSize(
+error::Error CommonDecoder::HandleGetBucketStart(
     uint32 immediate_data_size,
-    const cmd::GetBucketSize& args) {
+    const cmd::GetBucketStart& args) {
   uint32 bucket_id = args.bucket_id;
-  uint32* data = GetSharedMemoryAs<uint32*>(
-      args.shared_memory_id, args.shared_memory_offset, sizeof(*data));
-  if (!data) {
+  uint32* result = GetSharedMemoryAs<uint32*>(
+      args.result_memory_id, args.result_memory_offset, sizeof(*result));
+  int32 data_memory_id = args.data_memory_id;
+  uint32 data_memory_offset = args.data_memory_offset;
+  uint32 data_memory_size = args.data_memory_size;
+  uint8* data = NULL;
+  if (data_memory_size != 0 || data_memory_id != 0 || data_memory_offset != 0) {
+    data = GetSharedMemoryAs<uint8*>(
+        args.data_memory_id, args.data_memory_offset, args.data_memory_size);
+    if (!data) {
+      return error::kInvalidArguments;
+    }
+  }
+  if (!result) {
     return error::kInvalidArguments;
   }
   // Check that the client initialized the result.
-  if (*data != 0) {
+  if (*result != 0) {
     return error::kInvalidArguments;
   }
   Bucket* bucket = GetBucket(bucket_id);
   if (!bucket) {
     return error::kInvalidArguments;
   }
-  *data = bucket->size();
+  uint32 bucket_size = bucket->size();
+  *result = bucket_size;
+  if (data) {
+    uint32 size = std::min(data_memory_size, bucket_size);
+    memcpy(data, bucket->GetData(0, size), size);
+  }
   return error::kNoError;
 }
 
diff --git a/gpu/command_buffer/service/common_decoder_unittest.cc b/gpu/command_buffer/service/common_decoder_unittest.cc
index 571ffa0..53f6235 100644
--- a/gpu/command_buffer/service/common_decoder_unittest.cc
+++ b/gpu/command_buffer/service/common_decoder_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -471,36 +471,103 @@ TEST_F(CommonDecoderTest, SetBucketDataImmediate) {
             ExecuteImmediateCmd(cmd, sizeof(kData2)));
 }
 
-TEST_F(CommonDecoderTest, GetBucketSize) {
+TEST_F(CommonDecoderTest, GetBucketStart) {
   cmd::SetBucketSize size_cmd;
-  cmd::GetBucketSize cmd;
+  cmd::SetBucketData set_cmd;
+  cmd::GetBucketStart cmd;
 
-  const uint32 kBucketSize = 456;
+  static const char kData[] = "1234567890123456789";
+  static const char zero[sizeof(kData)] = { 0, };
+
+  const uint32 kBucketSize = sizeof(kData);
   const uint32 kBucketId = 123;
   const uint32 kInvalidBucketId = 124;
 
-  size_cmd.Init(kBucketId, kBucketSize);
+  // Put data in the bucket.
+  size_cmd.Init(kBucketId, sizeof(kData));
   EXPECT_EQ(error::kNoError, ExecuteCmd(size_cmd));
-
-  // Check that the size is correct.
   const uint32 kSomeOffsetInSharedMemory = 50;
+  uint8* start = engine_.GetSharedMemoryAs<uint8*>(kSomeOffsetInSharedMemory);
+  memcpy(start, kData, sizeof(kData));
+  set_cmd.Init(kBucketId, 0, sizeof(kData),
+               MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(set_cmd));
+
+  // Check that the size is correct with no data buffer.
   uint32* memory =
       engine_.GetSharedMemoryAs<uint32*>(kSomeOffsetInSharedMemory);
   *memory = 0x0;
   cmd.Init(kBucketId,
-           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory);
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           0, 0, 0);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(kBucketSize, *memory);
 
-  // Check that it fails if the bucket_id is invalid
+  // Check that the data is copied with data buffer.
+  const uint32 kDataOffsetInSharedMemory = 54;
+  uint8* data = engine_.GetSharedMemoryAs<uint8*>(kDataOffsetInSharedMemory);
+  *memory = 0x0;
+  memset(data, 0, sizeof(kData));
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           kBucketSize, MockCommandBufferEngine::kValidShmId,
+           kDataOffsetInSharedMemory);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(kBucketSize, *memory);
+  EXPECT_EQ(0, memcmp(data, kData, kBucketSize));
+
+  // Check that we can get a piece.
+  *memory = 0x0;
+  memset(data, 0, sizeof(kData));
+  const uint32 kPieceSize = kBucketSize / 2;
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           kPieceSize, MockCommandBufferEngine::kValidShmId,
+           kDataOffsetInSharedMemory);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(kBucketSize, *memory);
+  EXPECT_EQ(0, memcmp(data, kData, kPieceSize));
+  EXPECT_EQ(0, memcmp(data + kPieceSize, zero, sizeof(kData) - kPieceSize));
+
+  // Check that it fails if the result_id is invalid
   cmd.Init(kInvalidBucketId,
-           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory);
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           0, 0, 0);
+  EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
+
+  // Check that it fails if the data_id is invalid
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           1, MockCommandBufferEngine::kInvalidShmId, 0);
+  EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
+
+  // Check that it fails if the data_size is invalid
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           1, 0, 0);
+  EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           MockCommandBufferEngine::kBufferSize + 1,
+           MockCommandBufferEngine::kValidShmId, 0);
+  EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
+
+  // Check that it fails if the data_offset is invalid
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           0, 0, 1);
+  EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
+  cmd.Init(kBucketId,
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           MockCommandBufferEngine::kBufferSize,
+           MockCommandBufferEngine::kValidShmId, 1);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
 
   // Check that it fails if the result size is not set to zero
   *memory = 0x1;
   cmd.Init(kBucketId,
-           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory);
+           MockCommandBufferEngine::kValidShmId, kSomeOffsetInSharedMemory,
+           0, 0, 0);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
 }