Factor out the code to write control and frame headers into a new constructors of SpdyFrameBuilder.

Remove unused empty SpdyFrameBuilder constructor.


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

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

net/spdy/spdy_frame_builder.cc

@@ -9,10 +9,49 @@
 
 namespace net {
 
-SpdyFrameBuilder::SpdyFrameBuilder(size_t size)
+namespace {
+
+// Creates a FlagsAndLength.
+FlagsAndLength CreateFlagsAndLength(SpdyControlFlags flags, size_t length) {
+  DCHECK_EQ(0u, length & ~static_cast<size_t>(kLengthMask));
+  FlagsAndLength flags_length;
+  flags_length.length_ = htonl(static_cast<uint32>(length));
+  DCHECK_EQ(0, flags & ~kControlFlagsMask);
+  flags_length.flags_[0] = flags;
+  return flags_length;
+}
+
+}  // namespace
+
+SpdyFrameBuilder::SpdyFrameBuilder(SpdyControlType type,
+                                   SpdyControlFlags flags,
+                                   int spdy_version,
+                                   size_t size)
+    : buffer_(new char[size]),
+      capacity_(size),
+      length_(0) {
+  FlagsAndLength flags_length = CreateFlagsAndLength(
+      flags, size - SpdyFrame::kHeaderSize);
+  WriteUInt16(kControlFlagMask | spdy_version);
+  WriteUInt16(type);
+  WriteBytes(&flags_length, sizeof(flags_length));
+}
+
+SpdyFrameBuilder::SpdyFrameBuilder(SpdyStreamId stream_id,
+                                   SpdyDataFlags flags,
+                                   size_t size)
     : buffer_(new char[size]),
       capacity_(size),
       length_(0) {
+  DCHECK_EQ(0u, stream_id & ~kStreamIdMask);
+  WriteUInt32(stream_id);
+  size_t length = size - SpdyFrame::kHeaderSize;
+  DCHECK_EQ(0u, length & ~static_cast<size_t>(kLengthMask));
+  FlagsAndLength flags_length;
+  flags_length.length_ = htonl(length);
+  DCHECK_EQ(0, flags & ~kDataFlagsMask);
+  flags_length.flags_[0] = flags;
+  WriteBytes(&flags_length, sizeof(flags_length));
 }
 
 SpdyFrameBuilder::~SpdyFrameBuilder() {

net/spdy/spdy_frame_builder.h

@@ -27,10 +27,16 @@ class NET_EXPORT_PRIVATE SpdyFrameBuilder {
  public:
   ~SpdyFrameBuilder();
 
-  SpdyFrameBuilder();
-
-  // Initiailizes a SpdyFrameBuilder with a buffer of given size.
-  explicit SpdyFrameBuilder(size_t size);
+  // Initializes a SpdyFrameBuilder with a buffer of given size,
+  // populate with a SPDY control frame header based
+  // on |type|, |flags|, and |spdy_version|.
+  SpdyFrameBuilder(SpdyControlType type, SpdyControlFlags flags,
+                   int spdy_version, size_t size);
+
+  // Initiailizes a SpdyFrameBuilder with a buffer of given size,
+  // populated with a SPDY data frame header based on
+  // |stream_id| and |flags|.
+  SpdyFrameBuilder(SpdyStreamId stream_id, SpdyDataFlags flags,  size_t size);
 
   // Returns the size of the SpdyFrameBuilder's data.
   int length() const { return length_; }

net/spdy/spdy_framer_test.cc

@@ -438,18 +438,14 @@ using test::SpdyFramerTestUtil;
 using test::TestSpdyVisitor;
 
 TEST(SpdyFrameBuilderTest, WriteLimits) {
-  SpdyFrameBuilder builder(kLengthMask + 4);
-  // length field should fail.
-  EXPECT_FALSE(builder.WriteBytes(reinterpret_cast<const void*>(0x1),
-                                  kLengthMask + 1));
-  EXPECT_EQ(0, builder.length());
-
-  // Writing a block of the maximum allowed size should succeed.
+  SpdyFrameBuilder builder(1, DATA_FLAG_NONE, kLengthMask + 8);
+  // Data frame header is 8 bytes
+  EXPECT_EQ(8, builder.length());
   const std::string kLargeData(kLengthMask, 'A');
   builder.WriteUInt32(kLengthMask);
-  EXPECT_EQ(4, builder.length());
-  EXPECT_TRUE(builder.WriteBytes(kLargeData.data(), kLengthMask));
-  EXPECT_EQ(4 + kLengthMask, static_cast<unsigned>(builder.length()));
+  EXPECT_EQ(12, builder.length());
+  EXPECT_TRUE(builder.WriteBytes(kLargeData.data(), kLengthMask - 4));
+  EXPECT_EQ(kLengthMask + 8, static_cast<unsigned>(builder.length()));
 }
 
 enum SpdyFramerTestTypes {
@@ -581,11 +577,8 @@ TEST_P(SpdyFramerTest, UndersizedHeaderBlockInBuffer) {
 
 TEST_P(SpdyFramerTest, OutOfOrderHeaders) {
   // Frame builder with plentiful buffer size.
-  SpdyFrameBuilder frame(1024);
+  SpdyFrameBuilder frame(SYN_STREAM, CONTROL_FLAG_NONE, 1, 1024);
 
-  frame.WriteUInt16(kControlFlagMask | 1);
-  frame.WriteUInt16(SYN_STREAM);
-  frame.WriteUInt32(0);  // Placeholder for the length.
   frame.WriteUInt32(3);  // stream_id
   frame.WriteUInt32(0);  // Associated stream id
   frame.WriteUInt16(0);  // Priority.
@@ -691,11 +684,8 @@ TEST_P(SpdyFramerTest, ParseCredentialFrameData) {
 
 TEST_P(SpdyFramerTest, DuplicateHeader) {
   // Frame builder with plentiful buffer size.
-  SpdyFrameBuilder frame(1024);
+  SpdyFrameBuilder frame(SYN_STREAM, CONTROL_FLAG_NONE, 1, 1024);
 
-  frame.WriteUInt16(kControlFlagMask | 1);
-  frame.WriteUInt16(SYN_STREAM);
-  frame.WriteUInt32(0);  // Placeholder for the length.
   frame.WriteUInt32(3);  // stream_id
   frame.WriteUInt32(0);  // associated stream id
   frame.WriteUInt16(0);  // Priority.
@@ -731,11 +721,8 @@ TEST_P(SpdyFramerTest, DuplicateHeader) {
 
 TEST_P(SpdyFramerTest, MultiValueHeader) {
   // Frame builder with plentiful buffer size.
-  SpdyFrameBuilder frame(1024);
+  SpdyFrameBuilder frame(SYN_STREAM, CONTROL_FLAG_NONE, 1, 1024);
 
-  frame.WriteUInt16(kControlFlagMask | 1);
-  frame.WriteUInt16(SYN_STREAM);
-  frame.WriteUInt32(0);  // Placeholder for the length.
   frame.WriteUInt32(3);  // stream_id
   frame.WriteUInt32(0);  // associated stream id
   frame.WriteUInt16(0);  // Priority.