Use CompoundBuffer in MessageDecoder.

TEST=Unittests
BUG=None

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

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

5 files changed, 106 insertions, 79 deletions

diff --git a/remoting/base/compound_buffer.cc b/remoting/base/compound_buffer.cc
index 9e8b8c3..2808f6d 100644
--- a/remoting/base/compound_buffer.cc
+++ b/remoting/base/compound_buffer.cc
@@ -88,6 +88,49 @@ void CompoundBuffer::PrependCopyOf(const char* data, int size) {
   Prepend(buffer, size);
 }
 
+void CompoundBuffer::CropFront(int bytes) {
+  CHECK(!locked_);
+
+  if (total_bytes_ <= bytes) {
+    Clear();
+    return;
+  }
+
+  total_bytes_ -= bytes;
+  while (chunks_.size() > 0 && chunks_.front().size <= bytes) {
+    bytes -= chunks_.front().size;
+    chunks_.pop_front();
+  }
+  if (chunks_.size() > 0 && bytes > 0) {
+    chunks_.front().start += bytes;
+    chunks_.front().size -= bytes;
+    DCHECK_GT(chunks_.front().size, 0);
+    bytes = 0;
+  }
+  DCHECK_EQ(bytes, 0);
+}
+
+void CompoundBuffer::CropBack(int bytes) {
+  CHECK(!locked_);
+
+  if (total_bytes_ <= bytes) {
+    Clear();
+    return;
+  }
+
+  total_bytes_ -= bytes;
+  while (chunks_.size() > 0 && chunks_.back().size <= bytes) {
+    bytes -= chunks_.back().size;
+    chunks_.pop_back();
+  }
+  if (chunks_.size() > 0 && bytes > 0) {
+    chunks_.back().size -= bytes;
+    DCHECK_GT(chunks_.back().size, 0);
+    bytes = 0;
+  }
+  DCHECK_EQ(bytes, 0);
+}
+
 void CompoundBuffer::Lock() {
   locked_ = true;
 }
diff --git a/remoting/base/compound_buffer.h b/remoting/base/compound_buffer.h
index 050182a..1b0a546 100644
--- a/remoting/base/compound_buffer.h
+++ b/remoting/base/compound_buffer.h
@@ -52,6 +52,10 @@ class CompoundBuffer {
   void AppendCopyOf(const char* data, int data_size);
   void PrependCopyOf(const char* data, int data_size);
 
+  // Drop |bytes| bytes from the beginning or the end of the buffer.
+  void CropFront(int bytes);
+  void CropBack(int bytes);
+
   // Current size of the buffer.
   int total_bytes() const { return total_bytes_; }
 
diff --git a/remoting/base/compound_buffer_unittest.cc b/remoting/base/compound_buffer_unittest.cc
index 63ede5a..2b90bfe 100644
--- a/remoting/base/compound_buffer_unittest.cc
+++ b/remoting/base/compound_buffer_unittest.cc
@@ -24,6 +24,9 @@ const int kChunkSizes1[] = {1, 10, 20, -1};
 // Chunk sizes used to test CopyFrom().
 const int kCopySizes0[] = {10, 3, -1};
 const int kCopySizes1[] = {20, -1};
+
+const int kCropSizes[] = {1, -1};
+
 }  // namespace
 
 class CompoundBufferTest : public testing::Test {
@@ -52,6 +55,22 @@ class CompoundBufferTest : public testing::Test {
     EXPECT_TRUE(CompareData(copy, data_->data() + pos, size));
   }
 
+  void TestCropFront(int pos, int size) {
+    CompoundBuffer cropped;
+    cropped.CopyFrom(target_, 0, target_.total_bytes());
+    cropped.CropFront(pos);
+    EXPECT_TRUE(CompareData(cropped, data_->data() + pos,
+                            target_.total_bytes() - pos));
+  }
+
+  void TestCropBack(int pos, int size) {
+    CompoundBuffer cropped;
+    cropped.CopyFrom(target_, 0, target_.total_bytes());
+    cropped.CropBack(pos);
+    EXPECT_TRUE(CompareData(cropped, data_->data(),
+                            target_.total_bytes() - pos));
+  }
+
  protected:
   virtual void SetUp() {
     data_ = new IOBuffer(kDataSize);
@@ -210,6 +229,24 @@ TEST_F(CompoundBufferTest, PrependCopyOf) {
   EXPECT_TRUE(CompareData(target_, data_->data(), kDataSize));
 }
 
+TEST_F(CompoundBufferTest, CropFront) {
+  target_.Clear();
+  IterateOverPieces(kChunkSizes1, NewCallback(
+      static_cast<CompoundBufferTest*>(this), &CompoundBufferTest::Append));
+  IterateOverPieces(kCropSizes, NewCallback(
+        static_cast<CompoundBufferTest*>(this),
+      &CompoundBufferTest::TestCropFront));
+}
+
+TEST_F(CompoundBufferTest, CropBack) {
+  target_.Clear();
+  IterateOverPieces(kChunkSizes1, NewCallback(
+      static_cast<CompoundBufferTest*>(this), &CompoundBufferTest::Append));
+  IterateOverPieces(kCropSizes, NewCallback(
+      static_cast<CompoundBufferTest*>(this),
+      &CompoundBufferTest::TestCropBack));
+}
+
 TEST_F(CompoundBufferTest, CopyFrom) {
   target_.Clear();
   IterateOverPieces(kChunkSizes1, NewCallback(
diff --git a/remoting/protocol/message_decoder.cc b/remoting/protocol/message_decoder.cc
index 2e31bbc..cf44d45 100644
--- a/remoting/protocol/message_decoder.cc
+++ b/remoting/protocol/message_decoder.cc
@@ -13,8 +13,7 @@
 namespace remoting {
 
 MessageDecoder::MessageDecoder()
-    : available_bytes_(0),
-      next_payload_(0),
+    : next_payload_(0),
       next_payload_known_(false) {
 }
 
@@ -22,12 +21,10 @@ MessageDecoder::~MessageDecoder() {}
 
 void MessageDecoder::AddBuffer(scoped_refptr<net::IOBuffer> data,
                                int data_size) {
-  buffer_list_.push_back(make_scoped_refptr(
-      new net::DrainableIOBuffer(data, data_size)));
-  available_bytes_ += data_size;
+  buffer_.Append(data, data_size);
 }
 
-CompoundBuffer* MessageDecoder::CreateCompoundBufferFromData() {
+bool MessageDecoder::GetNextMessageData(CompoundBuffer* message_buffer) {
   // Determine the payload size. If we already know it then skip this part.
   // We may not have enough data to determine the payload size so use a
   // utility function to find out.
@@ -40,72 +37,30 @@ CompoundBuffer* MessageDecoder::CreateCompoundBufferFromData() {
 
   // If the next payload size is still not known or we don't have enough
   // data for parsing then exit.
-  if (!next_payload_known_ || available_bytes_ < next_payload_)
-    return NULL;
-  next_payload_known_ = false;
-
-  // The following loop gather buffers in |buffer_list_| that sum up to
-  // |next_payload_| bytes. These buffers are added to |stream|.
-
-  // Create a CompoundBuffer for parsing.
-  CompoundBuffer* result = new CompoundBuffer();
-  while (next_payload_ > 0 && !buffer_list_.empty()) {
-    scoped_refptr<net::DrainableIOBuffer> buffer = buffer_list_.front();
-    int read_bytes = std::min(buffer->BytesRemaining(), next_payload_);
-
-    // This will reference the same base pointer but maintain it's own
-    // version of data pointer.
-    result->Append(buffer, read_bytes);
-
-    // Adjust counters.
-    buffer->DidConsume(read_bytes);
-    next_payload_ -= read_bytes;
-    available_bytes_ -= read_bytes;
+  if (!next_payload_known_ || buffer_.total_bytes() < next_payload_)
+    return false;
 
-    // If the front buffer is fully read then remove it from the queue.
-    if (!buffer->BytesRemaining())
-      buffer_list_.pop_front();
-  }
-  DCHECK_EQ(0, next_payload_);
-  DCHECK_LE(0, available_bytes_);
-  result->Lock();
-  return result;
-}
+  message_buffer->CopyFrom(buffer_, 0, next_payload_);
+  message_buffer->Lock();
+  buffer_.CropFront(next_payload_);
+  next_payload_known_ = false;
 
-static int GetHeaderSize(const std::string& header) {
-  return header.length();
+  return true;
 }
 
 bool MessageDecoder::GetPayloadSize(int* size) {
   // The header has a size of 4 bytes.
   const int kHeaderSize = sizeof(int32);
 
-  if (available_bytes_ < kHeaderSize)
+  if (buffer_.total_bytes() < kHeaderSize)
     return false;
 
-  std::string header;
-  while (GetHeaderSize(header) < kHeaderSize && !buffer_list_.empty()) {
-    scoped_refptr<net::DrainableIOBuffer> buffer = buffer_list_.front();
-
-    // Find out how many bytes we need and how many bytes are available in this
-    // buffer.
-    int needed_bytes = kHeaderSize - GetHeaderSize(header);
-    int available_bytes = buffer->BytesRemaining();
-
-    // Then append the required bytes into the header and advance the last
-    // read position.
-    int read_bytes = std::min(needed_bytes, available_bytes);
-    header.append(buffer->data(), read_bytes);
-    buffer->DidConsume(read_bytes);
-    available_bytes_ -= read_bytes;
-
-    // If the buffer is depleted then remove it from the queue.
-    if (!buffer->BytesRemaining()) {
-      buffer_list_.pop_front();
-    }
-  }
-
-  *size = talk_base::GetBE32(header.c_str());
+  CompoundBuffer header_buffer;
+  char header[kHeaderSize];
+  header_buffer.CopyFrom(buffer_, 0, kHeaderSize);
+  header_buffer.CopyTo(header, kHeaderSize);
+  *size = talk_base::GetBE32(header);
+  buffer_.CropFront(kHeaderSize);
   return true;
 }
 
diff --git a/remoting/protocol/message_decoder.h b/remoting/protocol/message_decoder.h
index ea7acee..1978dc2 100644
--- a/remoting/protocol/message_decoder.h
+++ b/remoting/protocol/message_decoder.h
@@ -14,11 +14,6 @@
 #include "remoting/base/compound_buffer.h"
 #include "third_party/protobuf/src/google/protobuf/message_lite.h"
 
-namespace net {
-class DrainableIOBuffer;
-class IOBuffer;
-}  // namespace net
-
 namespace remoting {
 
 class ChromotingClientMessage;
@@ -66,18 +61,14 @@ class MessageDecoder {
   }
 
   private:
-  // TODO(sergeyu): It might be more efficient to memcopy data to one big buffer
-  // instead of storing chunks in dqueue.
-  typedef std::deque<scoped_refptr<net::DrainableIOBuffer> > BufferList;
-
   // Parse one message from |buffer_list_|. Return true if sucessful.
   template <class MessageType>
   bool ParseOneMessage(MessageType** message) {
-    scoped_ptr<CompoundBuffer> buffer(CreateCompoundBufferFromData());
-    if (!buffer.get())
+    CompoundBuffer buffer;
+    if (!GetNextMessageData(&buffer))
       return false;
 
-    CompoundBufferInputStream stream(buffer.get());
+    CompoundBufferInputStream stream(&buffer);
     *message = new MessageType();
     bool ret = (*message)->ParseFromZeroCopyStream(&stream);
     if (!ret)
@@ -87,17 +78,14 @@ class MessageDecoder {
 
   void AddBuffer(scoped_refptr<net::IOBuffer> data, int data_size);
 
-  CompoundBuffer* CreateCompoundBufferFromData();
+  bool GetNextMessageData(CompoundBuffer* message_buffer);
 
   // Retrieves the read payload size of the current protocol buffer via |size|.
   // Returns false and leaves |size| unmodified, if we do not have enough data
   // to retrieve the current size.
   bool GetPayloadSize(int* size);
 
-  BufferList buffer_list_;
-
-  // The number of bytes in |buffer_list_| not consumed.
-  int available_bytes_;
+  CompoundBuffer buffer_;
 
   // |next_payload_| stores the size of the next payload if known.
   // |next_payload_known_| is true if the size of the next payload is known.