Remove SpdyControlFrame, and associated cleanup.

Useful for SPDY 4 development. Including the following refactor line items, among others:
 * Rename SpdyFramer::current_frame_len_ to current_frame_buffer_length_
 * Add fields, and centralize processing, for common frame header in SpdyFramer.
 * Add SpdyFrameReader::ReadUInt24(), useful for reading those pesky 24-bit length fields.

This lands server change 42289618.

Review URL: https://codereview.chromium.org/12212186

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

10 files changed, 198 insertions, 263 deletions

diff --git a/net/spdy/buffered_spdy_framer.h b/net/spdy/buffered_spdy_framer.h
index 1e1fa83..06247dd 100644
--- a/net/spdy/buffered_spdy_framer.h
+++ b/net/spdy/buffered_spdy_framer.h
@@ -180,6 +180,12 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
                                  SpdyDataFlags flags);
   SpdyPriority GetHighestPriority() const;
 
+  // Returns the (minimum) size of control frames (sans variable-length
+  // portions).
+  size_t GetControlFrameMinimumSize() const {
+    return spdy_framer_.GetControlFrameMinimumSize();
+  }
+
   int frames_received() const { return frames_received_; }
 
  private:
diff --git a/net/spdy/buffered_spdy_framer_spdy2_unittest.cc b/net/spdy/buffered_spdy_framer_spdy2_unittest.cc
index 192192b..98dde26 100644
--- a/net/spdy/buffered_spdy_framer_spdy2_unittest.cc
+++ b/net/spdy/buffered_spdy_framer_spdy2_unittest.cc
@@ -195,7 +195,7 @@ TEST_F(BufferedSpdyFramerSpdy2Test, OnSetting) {
 
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(2, visitor.setting_count_);
 }
@@ -218,7 +218,7 @@ TEST_F(BufferedSpdyFramerSpdy2Test, ReadSynStreamHeaderBlock) {
   TestBufferedSpdyVisitor visitor;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-      control_frame.get()->length() + SpdyControlFrame::kHeaderSize);
+      control_frame.get()->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(1, visitor.syn_frame_count_);
   EXPECT_EQ(0, visitor.syn_reply_frame_count_);
@@ -241,7 +241,7 @@ TEST_F(BufferedSpdyFramerSpdy2Test, ReadSynReplyHeaderBlock) {
   TestBufferedSpdyVisitor visitor;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-       control_frame.get()->length() + SpdyControlFrame::kHeaderSize);
+       control_frame.get()->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(0, visitor.syn_frame_count_);
   EXPECT_EQ(1, visitor.syn_reply_frame_count_);
@@ -264,7 +264,7 @@ TEST_F(BufferedSpdyFramerSpdy2Test, ReadHeadersHeaderBlock) {
   TestBufferedSpdyVisitor visitor;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-       control_frame.get()->length() + SpdyControlFrame::kHeaderSize);
+       control_frame.get()->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(0, visitor.syn_frame_count_);
   EXPECT_EQ(0, visitor.syn_reply_frame_count_);
diff --git a/net/spdy/buffered_spdy_framer_spdy3_unittest.cc b/net/spdy/buffered_spdy_framer_spdy3_unittest.cc
index 733b473..3645614 100644
--- a/net/spdy/buffered_spdy_framer_spdy3_unittest.cc
+++ b/net/spdy/buffered_spdy_framer_spdy3_unittest.cc
@@ -194,7 +194,7 @@ TEST_F(BufferedSpdyFramerSpdy3Test, OnSetting) {
 
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(2, visitor.setting_count_);
 }
@@ -217,7 +217,7 @@ TEST_F(BufferedSpdyFramerSpdy3Test, ReadSynStreamHeaderBlock) {
   TestBufferedSpdyVisitor visitor;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-      control_frame.get()->length() + SpdyControlFrame::kHeaderSize);
+      control_frame.get()->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(1, visitor.syn_frame_count_);
   EXPECT_EQ(0, visitor.syn_reply_frame_count_);
@@ -240,7 +240,7 @@ TEST_F(BufferedSpdyFramerSpdy3Test, ReadSynReplyHeaderBlock) {
   TestBufferedSpdyVisitor visitor;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-       control_frame.get()->length() + SpdyControlFrame::kHeaderSize);
+       control_frame.get()->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(0, visitor.syn_frame_count_);
   EXPECT_EQ(1, visitor.syn_reply_frame_count_);
@@ -263,7 +263,7 @@ TEST_F(BufferedSpdyFramerSpdy3Test, ReadHeadersHeaderBlock) {
   TestBufferedSpdyVisitor visitor;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-       control_frame.get()->length() + SpdyControlFrame::kHeaderSize);
+       control_frame.get()->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(0, visitor.syn_frame_count_);
   EXPECT_EQ(0, visitor.syn_reply_frame_count_);
diff --git a/net/spdy/spdy_frame_reader.cc b/net/spdy/spdy_frame_reader.cc
index a5919a6..0ab1f2f 100644
--- a/net/spdy/spdy_frame_reader.cc
+++ b/net/spdy/spdy_frame_reader.cc
@@ -75,6 +75,24 @@ bool SpdyFrameReader::ReadUInt31(uint32* result) {
   return success;
 }
 
+bool SpdyFrameReader::ReadUInt24(uint32* result) {
+  // Make sure that we have the whole uint24.
+  if (!CanRead(3)) {
+    OnFailure();
+    return false;
+  }
+
+  // Read into result.
+  *result = 0;
+  memcpy(reinterpret_cast<char*>(result) + 1, data_ + ofs_, 3);
+  *result = ntohl(*result);
+
+  // Iterate.
+  ofs_ += 3;
+
+  return true;
+}
+
 bool SpdyFrameReader::ReadStringPiece16(base::StringPiece* result) {
   // Read resultant length.
   uint16 result_len;
diff --git a/net/spdy/spdy_frame_reader.h b/net/spdy/spdy_frame_reader.h
index 9bf7833..8a658f1 100644
--- a/net/spdy/spdy_frame_reader.h
+++ b/net/spdy/spdy_frame_reader.h
@@ -55,6 +55,11 @@ class NET_EXPORT_PRIVATE SpdyFrameReader {
   // Returns true on success, false otherwise.
   bool ReadUInt31(uint32* result);
 
+  // Reads a 24-bit unsigned integer into the given output parameter.
+  // Forwards the internal iterater (by 3B) on success.
+  // Returns true on success, false otherwise.
+  bool ReadUInt24(uint32* result);
+
   // Reads a string prefixed with 16-bit length into the given output parameter.
   //
   // NOTE: Does not copy but rather references strings in the underlying buffer.
diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index f92a0a5..3139593 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -117,14 +117,7 @@ SpdyCredential::SpdyCredential() : slot(0) {}
 SpdyCredential::~SpdyCredential() {}
 
 SpdyFramer::SpdyFramer(int version)
-    : state_(SPDY_RESET),
-      previous_state_(SPDY_RESET),
-      error_code_(SPDY_NO_ERROR),
-      remaining_data_(0),
-      remaining_control_payload_(0),
-      remaining_control_header_(0),
-      current_frame_buffer_(new char[kControlFrameBufferSize]),
-      current_frame_len_(0),
+    : current_frame_buffer_(new char[kControlFrameBufferSize]),
       enable_compression_(true),
       visitor_(NULL),
       debug_visitor_(NULL),
@@ -134,6 +127,7 @@ SpdyFramer::SpdyFramer(int version)
       probable_http_response_(false) {
   DCHECK_GE(kMaxSpdyVersion, version);
   DCHECK_LE(kMinSpdyVersion, version);
+  Reset();
 }
 
 SpdyFramer::~SpdyFramer() {
@@ -152,7 +146,10 @@ void SpdyFramer::Reset() {
   remaining_data_ = 0;
   remaining_control_payload_ = 0;
   remaining_control_header_ = 0;
-  current_frame_len_ = 0;
+  current_frame_buffer_length_ = 0;
+  current_frame_type_ = NUM_CONTROL_FRAME_TYPES;
+  current_frame_flags_ = 0;
+  current_frame_length_ = 0;
   settings_scratch_.Reset();
 }
 
@@ -453,7 +450,7 @@ size_t SpdyFramer::ProcessInput(const char* data, size_t len) {
   } while (state_ != previous_state_);
  bottom:
   DCHECK(len == 0 || state_ == SPDY_ERROR);
-  if (current_frame_len_ == 0 &&
+  if (current_frame_buffer_length_ == 0 &&
       remaining_data_ == 0 &&
       remaining_control_payload_ == 0 &&
       remaining_control_header_ == 0) {
@@ -473,12 +470,13 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
   SpdyFrame current_frame(current_frame_buffer_.get(), false);
 
   // Update current frame buffer as needed.
-  if (current_frame_len_ < SpdyFrame::kHeaderSize) {
-    size_t bytes_desired = SpdyFrame::kHeaderSize - current_frame_len_;
+  if (current_frame_buffer_length_ < SpdyFrame::kHeaderSize) {
+    size_t bytes_desired =
+        GetControlFrameMinimumSize() - current_frame_buffer_length_;
     UpdateCurrentFrameBuffer(&data, &len, bytes_desired);
   }
 
-  if (current_frame_len_ < SpdyFrame::kHeaderSize) {
+  if (current_frame_buffer_length_ < SpdyFrame::kHeaderSize) {
     // TODO(rch): remove this empty block
     // Do nothing.
   } else {
@@ -522,120 +520,137 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
 
 void SpdyFramer::ProcessControlFrameHeader() {
   DCHECK_EQ(SPDY_NO_ERROR, error_code_);
-  DCHECK_LE(static_cast<size_t>(SpdyFrame::kHeaderSize), current_frame_len_);
-  SpdyControlFrame current_control_frame(current_frame_buffer_.get(), false);
+  DCHECK_LE(static_cast<size_t>(SpdyFrame::kHeaderSize),
+            current_frame_buffer_length_);
+  SpdyFrameReader reader(current_frame_buffer_.get(),
+                         current_frame_buffer_length_);
+
+  uint16 version = 0;
+  bool successful_read = reader.ReadUInt16(&version);
+  DCHECK(successful_read);
+  version &= ~kControlFlagMask;
+
+  successful_read =
+      reader.ReadUInt16(reinterpret_cast<uint16*>(&current_frame_type_));
+  DCHECK(successful_read);
+
+  successful_read = reader.ReadUInt8(&current_frame_flags_);
+  DCHECK(successful_read);
+
+  successful_read = reader.ReadUInt24(&current_frame_length_);
+  DCHECK(successful_read);
 
   // We check version before we check validity: version can never be 'invalid',
   // it can only be unsupported.
-  if (current_control_frame.version() != spdy_version_) {
-    DLOG(INFO) << "Unsupported SPDY version " << current_control_frame.version()
+  if (version != spdy_version_) {
+    DLOG(INFO) << "Unsupported SPDY version " << version
                << " (expected " << spdy_version_ << ")";
     set_error(SPDY_UNSUPPORTED_VERSION);
     return;
   }
 
-  // Next up, check to see if we have valid data.  This should be after version
-  // checking (otherwise if the the type were out of bounds due to a version
-  // upgrade we would misclassify the error) and before checking the type
-  // (type can definitely be out of bounds)
-  if (!current_control_frame.AppearsToBeAValidControlFrame()) {
+  if (current_frame_type_ < SYN_STREAM ||
+      current_frame_type_ >= NUM_CONTROL_FRAME_TYPES) {
     set_error(SPDY_INVALID_CONTROL_FRAME);
     return;
   }
 
-  if (current_control_frame.type() == NOOP) {
+  if (current_frame_type_ == NOOP) {
     DLOG(INFO) << "NOOP control frame found. Ignoring.";
     CHANGE_STATE(SPDY_AUTO_RESET);
     return;
   }
 
-  // Do some sanity checking on the control frame sizes.
-  switch (current_control_frame.type()) {
+  // Do some sanity checking on the control frame sizes and flags.
+  switch (current_frame_type_) {
     case SYN_STREAM:
-      if (current_control_frame.length() <
-          GetSynStreamMinimumSize() - SpdyControlFrame::kHeaderSize) {
+      if (current_frame_length_ <
+          GetSynStreamMinimumSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() &
+      } else if (current_frame_flags_ &
                  ~(CONTROL_FLAG_FIN | CONTROL_FLAG_UNIDIRECTIONAL)) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case SYN_REPLY:
-      if (current_control_frame.length() <
-          GetSynReplyMinimumSize() - SpdyControlFrame::kHeaderSize) {
+      if (current_frame_length_ <
+          GetSynReplyMinimumSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() & ~CONTROL_FLAG_FIN) {
+      } else if (current_frame_flags_ & ~CONTROL_FLAG_FIN) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case RST_STREAM:
-      if (current_control_frame.length() !=
-          GetRstStreamSize() - SpdyFrame::kHeaderSize) {
+      if (current_frame_length_ !=
+          GetRstStreamSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() != 0) {
+      } else if (current_frame_flags_ != 0) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case SETTINGS:
       // Make sure that we have an integral number of 8-byte key/value pairs,
       // plus a 4-byte length field.
-      if (current_control_frame.length() <
-          GetSettingsMinimumSize() - SpdyControlFrame::kHeaderSize ||
-          (current_control_frame.length() % 8 != 4)) {
+      if (current_frame_length_ <
+          GetSettingsMinimumSize() - GetControlFrameMinimumSize() ||
+          current_frame_length_ % 8 != 4) {
         DLOG(WARNING) << "Invalid length for SETTINGS frame: "
-                      << current_control_frame.length();
+                      << current_frame_length_;
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() &
+      } else if (current_frame_flags_ &
                  ~SETTINGS_FLAG_CLEAR_PREVIOUSLY_PERSISTED_SETTINGS) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case GOAWAY:
       {
-        if (current_control_frame.length() !=
-            GetGoAwaySize() - SpdyFrame::kHeaderSize) {
+        if (current_frame_length_ != GetGoAwaySize() - SpdyFrame::kHeaderSize) {
           set_error(SPDY_INVALID_CONTROL_FRAME);
-        } else if (current_control_frame.flags() != 0) {
+        } else if (current_frame_flags_ != 0) {
           set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
         }
         break;
       }
     case HEADERS:
-      if (current_control_frame.length() <
-          GetHeadersMinimumSize() - SpdyControlFrame::kHeaderSize) {
+      if (current_frame_length_ <
+          GetHeadersMinimumSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() & ~CONTROL_FLAG_FIN) {
+      } else if (current_frame_flags_ & ~CONTROL_FLAG_FIN) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case WINDOW_UPDATE:
-      if (current_control_frame.length() !=
-          GetWindowUpdateSize() - SpdyControlFrame::kHeaderSize) {
+      if (current_frame_length_ !=
+          GetWindowUpdateSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() != 0) {
+      } else if (current_frame_flags_ != 0) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case PING:
-      if (current_control_frame.length() !=
-          GetPingSize() - SpdyControlFrame::kHeaderSize) {
+      if (current_frame_length_ !=
+          GetPingSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() != 0) {
+      } else if (current_frame_flags_ != 0) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case CREDENTIAL:
-      if (current_control_frame.length() <
-          GetCredentialMinimumSize() - SpdyControlFrame::kHeaderSize) {
+      if (current_frame_length_ <
+          GetCredentialMinimumSize() - GetControlFrameMinimumSize()) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
-      } else if (current_control_frame.flags() != 0) {
+      } else if (current_frame_flags_ != 0) {
         set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     default:
       LOG(WARNING) << "Valid " << display_protocol_
                    << " control frame with unhandled type: "
-                   << current_control_frame.type();
+                   << current_frame_type_;
+      // This branch should be unreachable because of the frame type bounds
+      // check above. However, we DLOG(FATAL) here in an effort to painfully
+      // club the head of the developer who failed to keep this file in sync
+      // with spdy_protocol.h.
       DLOG(FATAL);
       set_error(SPDY_INVALID_CONTROL_FRAME);
       break;
@@ -645,7 +660,7 @@ void SpdyFramer::ProcessControlFrameHeader() {
     return;
   }
 
-  remaining_control_payload_ = current_control_frame.length();
+  remaining_control_payload_ = current_frame_length_;
   const size_t total_frame_size =
       remaining_control_payload_ + SpdyFrame::kHeaderSize;
   if (total_frame_size > GetControlFrameBufferMaxSize()) {
@@ -655,14 +670,14 @@ void SpdyFramer::ProcessControlFrameHeader() {
     return;
   }
 
-  if (current_control_frame.type() == CREDENTIAL) {
+  if (current_frame_type_ == CREDENTIAL) {
     CHANGE_STATE(SPDY_CREDENTIAL_FRAME_PAYLOAD);
     return;
   }
 
   // Determine the frame size without variable-length data.
   int32 frame_size_without_variable_data;
-  switch (current_control_frame.type()) {
+  switch (current_frame_type_) {
     case SYN_STREAM:
       syn_frame_processed_ = true;
       frame_size_without_variable_data = GetSynStreamMinimumSize();
@@ -698,9 +713,9 @@ void SpdyFramer::ProcessControlFrameHeader() {
     // remainder of the control frame's header before we can parse the header
     // block. The start of the header block varies with the control type.
     DCHECK_GE(frame_size_without_variable_data,
-              static_cast<int32>(current_frame_len_));
+              static_cast<int32>(current_frame_buffer_length_));
     remaining_control_header_ = frame_size_without_variable_data -
-        current_frame_len_;
+        current_frame_buffer_length_;
     remaining_control_payload_ += SpdyFrame::kHeaderSize -
         frame_size_without_variable_data;
     CHANGE_STATE(SPDY_CONTROL_FRAME_BEFORE_HEADER_BLOCK);
@@ -713,11 +728,12 @@ void SpdyFramer::ProcessControlFrameHeader() {
 size_t SpdyFramer::UpdateCurrentFrameBuffer(const char** data, size_t* len,
                                             size_t max_bytes) {
   size_t bytes_to_read = std::min(*len, max_bytes);
-  DCHECK_GE(kControlFrameBufferSize, current_frame_len_ + bytes_to_read);
-  memcpy(current_frame_buffer_.get() + current_frame_len_,
+  DCHECK_GE(kControlFrameBufferSize,
+            current_frame_buffer_length_ + bytes_to_read);
+  memcpy(current_frame_buffer_.get() + current_frame_buffer_length_,
          *data,
          bytes_to_read);
-  current_frame_len_ += bytes_to_read;
+  current_frame_buffer_length_ += bytes_to_read;
   *data += bytes_to_read;
   *len -= bytes_to_read;
   return bytes_to_read;
@@ -941,22 +957,15 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
   }
 
   if (remaining_control_header_ == 0) {
-    SpdyControlFrame control_frame(current_frame_buffer_.get(), false);
-    switch (control_frame.type()) {
+    SpdyFrameReader reader(current_frame_buffer_.get(),
+                           current_frame_buffer_length_);
+    reader.Seek(GetControlFrameMinimumSize());  // Seek past frame header.
+
+    switch (current_frame_type_) {
       case SYN_STREAM:
         {
-          SpdyFrameReader reader(current_frame_buffer_.get(),
-                                 current_frame_len_);
-          reader.Seek(4);  // Seek past control bit, type and version.
-
-          uint8 flags;
-          bool successful_read = reader.ReadUInt8(&flags);
-          DCHECK(successful_read);
-
-          reader.Seek(3);  // Seek past length.
-
           SpdyStreamId stream_id = kInvalidStream;
-          successful_read = reader.ReadUInt31(&stream_id);
+          bool successful_read = reader.ReadUInt31(&stream_id);
           DCHECK(successful_read);
 
           SpdyStreamId associated_to_stream_id = kInvalidStream;
@@ -987,8 +996,8 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
               associated_to_stream_id,
               priority,
               slot,
-              (flags & CONTROL_FLAG_FIN) != 0,
-              (flags & CONTROL_FLAG_UNIDIRECTIONAL) != 0);
+              (current_frame_flags_ & CONTROL_FLAG_FIN) != 0,
+              (current_frame_flags_ & CONTROL_FLAG_UNIDIRECTIONAL) != 0);
         }
         CHANGE_STATE(SPDY_CONTROL_FRAME_HEADER_BLOCK);
         break;
@@ -996,25 +1005,22 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
       case HEADERS:
         // SYN_REPLY and HEADERS are the same, save for the visitor call.
         {
-          SpdyFrameReader reader(current_frame_buffer_.get(),
-                                 current_frame_len_);
-          reader.Seek(4);  // Seek past control bit, type and version.
-          uint8 flags;
-          bool successful_read = reader.ReadUInt8(&flags);
-          DCHECK(successful_read);
-          reader.Seek(3);  // Seek past length.
           SpdyStreamId stream_id = kInvalidStream;
-          successful_read = reader.ReadUInt31(&stream_id);
+          bool successful_read = reader.ReadUInt31(&stream_id);
           DCHECK(successful_read);
           if (protocol_version() < 3) {
             // SPDY 2 had two unused bytes here. Seek past them.
             reader.Seek(2);
           }
           DCHECK(reader.IsDoneReading());
-          if (control_frame.type() == SYN_REPLY) {
-            visitor_->OnSynReply(stream_id, (flags & CONTROL_FLAG_FIN) != 0);
+          if (current_frame_type_ == SYN_REPLY) {
+            visitor_->OnSynReply(
+                stream_id,
+                (current_frame_flags_ & CONTROL_FLAG_FIN) != 0);
           } else {
-            visitor_->OnHeaders(stream_id, (flags & CONTROL_FLAG_FIN) != 0);
+            visitor_->OnHeaders(
+                stream_id,
+                (current_frame_flags_ & CONTROL_FLAG_FIN) != 0);
           }
         }
         CHANGE_STATE(SPDY_CONTROL_FRAME_HEADER_BLOCK);
@@ -1036,19 +1042,19 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
 size_t SpdyFramer::ProcessControlFrameHeaderBlock(const char* data,
                                                   size_t data_len) {
   DCHECK_EQ(SPDY_CONTROL_FRAME_HEADER_BLOCK, state_);
-  const SpdyControlFrame control_frame(current_frame_buffer_.get(), false);
 
   bool processed_successfully = true;
   SpdyStreamId stream_id = kInvalidStream;
-  if (control_frame.type() == SYN_STREAM ||
-      control_frame.type() == SYN_REPLY ||
-      control_frame.type() == HEADERS) {
+  if (current_frame_type_ == SYN_STREAM ||
+      current_frame_type_ == SYN_REPLY ||
+      current_frame_type_ == HEADERS) {
     // The logic for all three of the aforementioned frame types is identical,
     // since the stream id is the first field in the frame after the header.
-    SpdyFrameReader reader(current_frame_buffer_.get(), current_frame_len_);
+    SpdyFrameReader reader(current_frame_buffer_.get(),
+                           current_frame_buffer_length_);
     reader.Seek(GetControlFrameMinimumSize());  // Seek past frame header.
-    bool read_successful = reader.ReadUInt31(&stream_id);
-    DCHECK(read_successful);
+    bool successful_read = reader.ReadUInt31(&stream_id);
+    DCHECK(successful_read);
   } else {
     LOG(DFATAL) << "Unhandled frame type in ProcessControlFrameHeaderBlock.";
   }
@@ -1073,7 +1079,7 @@ size_t SpdyFramer::ProcessControlFrameHeaderBlock(const char* data,
     visitor_->OnControlFrameHeaderData(stream_id, NULL, 0);
 
     // If this is a FIN, tell the caller.
-    if (control_frame.flags() & CONTROL_FLAG_FIN) {
+    if (current_frame_flags_ & CONTROL_FLAG_FIN) {
       visitor_->OnStreamFrameData(stream_id, NULL, 0, DATA_FLAG_FIN);
     }
 
@@ -1092,8 +1098,7 @@ size_t SpdyFramer::ProcessControlFrameHeaderBlock(const char* data,
 size_t SpdyFramer::ProcessSettingsFramePayload(const char* data,
                                                size_t data_len) {
   DCHECK_EQ(SPDY_SETTINGS_FRAME_PAYLOAD, state_);
-  SpdyControlFrame control_frame(current_frame_buffer_.get(), false);
-  DCHECK_EQ(SETTINGS, control_frame.type());
+  DCHECK_EQ(SETTINGS, current_frame_type_);
   size_t unprocessed_bytes = std::min(data_len, remaining_control_payload_);
   size_t processed_bytes = 0;
 
@@ -1200,14 +1205,13 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
     remaining_control_payload_ -= bytes_read;
     remaining_data_ -= bytes_read;
     if (remaining_control_payload_ == 0) {
-      SpdyControlFrame control_frame(current_frame_buffer_.get(), false);
-      DCHECK(!control_frame.has_header_block());
+      SpdyFrameReader reader(current_frame_buffer_.get(),
+                             current_frame_buffer_length_);
+      reader.Seek(GetControlFrameMinimumSize());  // Skip frame header.
+
       // Use frame-specific handlers.
-      switch (control_frame.type()) {
+      switch (current_frame_type_) {
         case PING: {
-            SpdyFrameReader reader(current_frame_buffer_.get(),
-                                   current_frame_len_);
-            reader.Seek(GetControlFrameMinimumSize());  // Skip frame header.
             SpdyPingId id = 0;
             bool successful_read = reader.ReadUInt32(&id);
             DCHECK(successful_read);
@@ -1216,9 +1220,6 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
           }
           break;
         case WINDOW_UPDATE: {
-            SpdyFrameReader reader(current_frame_buffer_.get(),
-                                   current_frame_len_);
-            reader.Seek(SpdyFrame::kHeaderSize);  // Seek past frame header.
             SpdyStreamId stream_id = kInvalidStream;
             uint32 delta_window_size = 0;
             bool successful_read = reader.ReadUInt31(&stream_id);
@@ -1230,9 +1231,6 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
           }
           break;
         case RST_STREAM: {
-            SpdyFrameReader reader(current_frame_buffer_.get(),
-                                   current_frame_len_);
-            reader.Seek(GetControlFrameMinimumSize());  // Skip frame header.
             SpdyStreamId stream_id = kInvalidStream;
             bool successful_read = reader.ReadUInt32(&stream_id);
             DCHECK(successful_read);
@@ -1253,9 +1251,6 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
           }
           break;
         case GOAWAY: {
-            SpdyFrameReader reader(current_frame_buffer_.get(),
-                                   current_frame_len_);
-            reader.Seek(GetControlFrameMinimumSize());  // Skip frame header.
             SpdyStreamId last_accepted_stream_id = kInvalidStream;
             bool successful_read = reader.ReadUInt31(&last_accepted_stream_id);
             DCHECK(successful_read);
@@ -1279,7 +1274,7 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
           break;
         default:
           // Unreachable.
-          LOG(FATAL) << "Unhandled control frame " << control_frame.type();
+          LOG(FATAL) << "Unhandled control frame " << current_frame_type_;
       }
 
       CHANGE_STATE(SPDY_IGNORE_REMAINING_PAYLOAD);
diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
index 7264100..14dd478 100644
--- a/net/spdy/spdy_framer.h
+++ b/net/spdy/spdy_framer.h
@@ -621,7 +621,18 @@ class NET_EXPORT_PRIVATE SpdyFramer {
   size_t remaining_control_header_;
 
   scoped_array<char> current_frame_buffer_;
-  size_t current_frame_len_;  // Number of bytes read into the current_frame_.
+  // Number of bytes read into the current_frame_buffer_.
+  size_t current_frame_buffer_length_;
+
+  // The type of the frame currently being read. Set to NUM_CONTROL_FRAME_TYPES
+  // if currently processing a DATA frame.
+  SpdyControlType current_frame_type_;
+
+  // The flags field of the frame currently being read.
+  uint8 current_frame_flags_;
+
+  // The length field of the frame currently being read.
+  uint32 current_frame_length_;
 
   // Scratch space for handling SETTINGS frames.
   // TODO(hkhalil): Unify memory for this scratch space with
diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
index de0b7e7..adcc7fb 100644
--- a/net/spdy/spdy_framer_test.cc
+++ b/net/spdy/spdy_framer_test.cc
@@ -1318,14 +1318,10 @@ TEST_P(SpdyFramerTest, HeaderCompression) {
   block[kHeader1] = kValue1;
   block[kHeader2] = kValue2;
   SpdyControlFlags flags(CONTROL_FLAG_NONE);
-  scoped_ptr<SpdyFrame> syn_frame_1(
-      send_framer.CreateSynStream(1,  // stream id
-                                  0,  // associated stream id
-                                  0,  // priority
-                                  0,  // credential slot
-                                  flags,
-                                  true,  // compress
-                                  &block));
+  SpdySynStreamIR syn_ir_1(1);
+  syn_ir_1.SetHeader(kHeader1, kValue1);
+  syn_ir_1.SetHeader(kHeader2, kValue2);
+  scoped_ptr<SpdyFrame> syn_frame_1(send_framer.SerializeSynStream(syn_ir_1));
   EXPECT_TRUE(syn_frame_1.get() != NULL);
 
   // SYN_STREAM #2
@@ -1342,7 +1338,7 @@ TEST_P(SpdyFramerTest, HeaderCompression) {
 
   // Now start decompressing
   scoped_ptr<SpdyFrame> decompressed;
-  scoped_ptr<SpdyFrame> syn_frame;
+  scoped_ptr<SpdyFrame> uncompressed;
   base::StringPiece serialized_headers;
   SpdyHeaderBlock decompressed_headers;
 
@@ -1351,8 +1347,6 @@ TEST_P(SpdyFramerTest, HeaderCompression) {
       &recv_framer, *syn_frame_1.get()));
   EXPECT_TRUE(decompressed.get() != NULL);
   EXPECT_TRUE(decompressed->is_control_frame());
-  EXPECT_EQ(SYN_STREAM,
-            reinterpret_cast<SpdyControlFrame*>(decompressed.get())->type());
   serialized_headers = GetSerializedHeaders(decompressed.get(), send_framer);
   EXPECT_TRUE(recv_framer.ParseHeaderBlockInBuffer(serialized_headers.data(),
                                                    serialized_headers.size(),
@@ -1366,8 +1360,6 @@ TEST_P(SpdyFramerTest, HeaderCompression) {
       &recv_framer, *syn_frame_2.get()));
   EXPECT_TRUE(decompressed.get() != NULL);
   EXPECT_TRUE(decompressed->is_control_frame());
-  EXPECT_EQ(SYN_STREAM,
-            reinterpret_cast<SpdyControlFrame*>(decompressed.get())->type());
   serialized_headers = GetSerializedHeaders(decompressed.get(), send_framer);
   decompressed_headers.clear();
   EXPECT_TRUE(recv_framer.ParseHeaderBlockInBuffer(serialized_headers.data(),
@@ -2455,7 +2447,7 @@ TEST_P(SpdyFramerTest, ReadCompressedSynStreamHeaderBlock) {
   visitor.use_compression_ = true;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.syn_frame_count_);
   EXPECT_TRUE(CompareHeaderBlocks(&headers, &visitor.headers_));
 }
@@ -2475,7 +2467,7 @@ TEST_P(SpdyFramerTest, ReadCompressedSynReplyHeaderBlock) {
   visitor.use_compression_ = true;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.syn_reply_frame_count_);
   EXPECT_TRUE(CompareHeaderBlocks(&headers, &visitor.headers_));
 }
@@ -2495,7 +2487,7 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlock) {
   visitor.use_compression_ = true;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.headers_frame_count_);
   // control_frame_header_data_count_ depends on the random sequence
   // produced by rand(), so adding, removing or running single tests
@@ -2522,7 +2514,7 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlockWithHalfClose) {
   visitor.use_compression_ = true;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.headers_frame_count_);
   // control_frame_header_data_count_ depends on the random sequence
   // produced by rand(), so adding, removing or running single tests
@@ -2565,7 +2557,7 @@ TEST_P(SpdyFramerTest, ControlFrameAtMaxSizeLimit) {
   EXPECT_TRUE(control_frame.get() != NULL);
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_TRUE(visitor.header_buffer_valid_);
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(1, visitor.syn_frame_count_);
@@ -2597,7 +2589,7 @@ TEST_P(SpdyFramerTest, ControlFrameTooLarge) {
   EXPECT_TRUE(control_frame.get() != NULL);
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_FALSE(visitor.header_buffer_valid_);
   EXPECT_EQ(1, visitor.error_count_);
   EXPECT_EQ(SpdyFramer::SPDY_CONTROL_PAYLOAD_TOO_LARGE,
@@ -2632,7 +2624,7 @@ TEST_P(SpdyFramerTest, ControlFrameMuchTooLarge) {
   visitor.use_compression_ = true;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_FALSE(visitor.header_buffer_valid_);
   EXPECT_EQ(1, visitor.error_count_);
   EXPECT_EQ(SpdyFramer::SPDY_CONTROL_PAYLOAD_TOO_LARGE,
@@ -2673,7 +2665,7 @@ TEST_P(SpdyFramerTest, DecompressCorruptHeaderBlock) {
   visitor.use_compression_ = true;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.error_count_);
   EXPECT_EQ(SpdyFramer::SPDY_DECOMPRESS_FAILURE, visitor.framer_.error_code());
   EXPECT_EQ(0u, visitor.header_buffer_length_);
@@ -2725,7 +2717,7 @@ TEST_P(SpdyFramerTest, ReadZeroLenSettingsFrame) {
   visitor.use_compression_ = false;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   // Should generate an error, since zero-len settings frames are unsupported.
   EXPECT_EQ(1, visitor.error_count_);
 }
@@ -2744,7 +2736,7 @@ TEST_P(SpdyFramerTest, ReadBogusLenSettingsFrame) {
   visitor.use_compression_ = false;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   // Should generate an error, since zero-len settings frames are unsupported.
   EXPECT_EQ(1, visitor.error_count_);
 }
@@ -2761,21 +2753,21 @@ TEST_P(SpdyFramerTest, ReadLargeSettingsFrame) {
   settings[SETTINGS_ROUND_TRIP_TIME] = SettingsFlagsAndValue(flags, 0x00000004);
   scoped_ptr<SpdyFrame> control_frame(framer.CreateSettings(settings));
   EXPECT_LT(SpdyFramer::kControlFrameBufferSize,
-            control_frame->length() + SpdyControlFrame::kHeaderSize);
+            control_frame->length() + framer.GetControlFrameMinimumSize());
   TestSpdyVisitor visitor(spdy_version_);
   visitor.use_compression_ = false;
 
   // Read all at once.
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(settings.size(), static_cast<unsigned>(visitor.setting_count_));
 
   // Read data in small chunks.
   size_t framed_data = 0;
   size_t unframed_data = control_frame->length() +
-                         SpdyControlFrame::kHeaderSize;
+                         framer.GetControlFrameMinimumSize();
   size_t kReadChunkSize = 5;  // Read five bytes at a time.
   while (unframed_data > 0) {
     size_t to_read = min(kReadChunkSize, unframed_data);
@@ -2874,7 +2866,7 @@ TEST_P(SpdyFramerTest, ReadWindowUpdate) {
   TestSpdyVisitor visitor(spdy_version_);
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.last_window_update_stream_);
   EXPECT_EQ(2, visitor.last_window_update_delta_);
 }
@@ -2894,7 +2886,7 @@ TEST_P(SpdyFramerTest, ReadCredentialFrame) {
   visitor.use_compression_ = false;
   visitor.SimulateInFramer(
       reinterpret_cast<unsigned char*>(control_frame->data()),
-      control_frame->length() + SpdyControlFrame::kHeaderSize);
+      control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(control_frame->length(), visitor.credential_buffer_length_);
   EXPECT_EQ(credential.slot, visitor.credential_.slot);
@@ -2953,7 +2945,7 @@ TEST_P(SpdyFramerTest, ReadCredentialFrameWithNoPayload) {
   control_frame->set_length(0);
   unsigned char* data =
       reinterpret_cast<unsigned char*>(control_frame->data());
-  visitor.SimulateInFramer(data, SpdyControlFrame::kHeaderSize);
+  visitor.SimulateInFramer(data, framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.error_count_);
 }
 
@@ -2972,10 +2964,10 @@ TEST_P(SpdyFramerTest, ReadCredentialFrameWithCorruptProof) {
   visitor.use_compression_ = false;
   unsigned char* data =
       reinterpret_cast<unsigned char*>(control_frame->data());
-  size_t offset = SpdyControlFrame::kHeaderSize + 4;
+  size_t offset = framer.GetControlFrameMinimumSize() + 4;
   data[offset] = 0xFF;  // Proof length is past the end of the frame
   visitor.SimulateInFramer(
-      data, control_frame->length() + SpdyControlFrame::kHeaderSize);
+      data, control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.error_count_);
 }
 
@@ -2994,10 +2986,11 @@ TEST_P(SpdyFramerTest, ReadCredentialFrameWithCorruptCertificate) {
   visitor.use_compression_ = false;
   unsigned char* data =
       reinterpret_cast<unsigned char*>(control_frame->data());
-  size_t offset = SpdyControlFrame::kHeaderSize + credential.proof.length();
+  size_t offset =
+      framer.GetControlFrameMinimumSize() + credential.proof.length();
   data[offset] = 0xFF;  // Certificate length is past the end of the frame
   visitor.SimulateInFramer(
-      data, control_frame->length() + SpdyControlFrame::kHeaderSize);
+      data, control_frame->length() + framer.GetControlFrameMinimumSize());
   EXPECT_EQ(1, visitor.error_count_);
 }
 
@@ -3013,15 +3006,13 @@ TEST_P(SpdyFramerTest, ReadGarbage) {
 
 TEST_P(SpdyFramerTest, ReadGarbageWithValidVersion) {
   SpdyFramer framer(spdy_version_);
-  char garbage_frame[256];
-  memset(garbage_frame, ~0, sizeof(garbage_frame));
-  SpdyControlFrame control_frame(&garbage_frame[0], false);
-  control_frame.set_version(spdy_version_);
+  const unsigned char kFrameData[] = {
+    0x80, spdy_version_, 0xff, 0xff,
+    0xff, 0xff, 0xff, 0xff,
+  };
   TestSpdyVisitor visitor(spdy_version_);
   visitor.use_compression_ = false;
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(control_frame.data()),
-      sizeof(garbage_frame));
+  visitor.SimulateInFramer(kFrameData, arraysize(kFrameData));
   EXPECT_EQ(1, visitor.error_count_);
 }
 
diff --git a/net/spdy/spdy_protocol.h b/net/spdy/spdy_protocol.h
index 6b698c0..54b1d01 100644
--- a/net/spdy/spdy_protocol.h
+++ b/net/spdy/spdy_protocol.h
@@ -15,6 +15,7 @@
 #include "base/logging.h"
 #include "base/string_piece.h"
 #include "base/sys_byteorder.h"
+#include "net/base/net_export.h"
 #include "net/spdy/spdy_bitmasks.h"
 
 //  Data Frame Format
@@ -549,12 +550,17 @@ class SpdyFrameWithFinIR : public SpdyFrameWithStreamIdIR {
 
 // Abstract class intended to be inherited by IRs that contain a name-value
 // block. Implies SpdyFrameWithFinIR.
-class SpdyFrameWithNameValueBlockIR : public SpdyFrameWithFinIR {
+class NET_EXPORT_PRIVATE SpdyFrameWithNameValueBlockIR
+    : public SpdyFrameWithFinIR {
  public:
   const SpdyNameValueBlock& name_value_block() const {
     return name_value_block_;
   }
   SpdyNameValueBlock* GetMutableNameValueBlock() { return &name_value_block_; }
+  void SetHeader(const base::StringPiece& name,
+                 const base::StringPiece& value) {
+    name_value_block_[name.as_string()] = value.as_string();
+  }
 
  protected:
   explicit SpdyFrameWithNameValueBlockIR(SpdyStreamId stream_id);
@@ -879,63 +885,6 @@ class SpdyDataFrame : public SpdyFrame {
   DISALLOW_COPY_AND_ASSIGN(SpdyDataFrame);
 };
 
-// A Control Frame.
-class SpdyControlFrame : public SpdyFrame {
- public:
-  explicit SpdyControlFrame(size_t size) : SpdyFrame(size) {}
-  SpdyControlFrame(char* data, bool owns_buffer)
-      : SpdyFrame(data, owns_buffer) {}
-
-  // Callers can use this method to check if the frame appears to be a valid
-  // frame.  Does not guarantee that there are no errors.
-  bool AppearsToBeAValidControlFrame() const {
-    // Right now we only check if the frame has an out-of-bounds type.
-    uint16 type = ntohs(block()->control_.type_);
-    // NOOP is not a 'valid' control frame in SPDY/3 and beyond.
-    return type >= SYN_STREAM &&
-        type < NUM_CONTROL_FRAME_TYPES &&
-        (version() == 2 || type != NOOP);
-  }
-
-  uint16 version() const {
-    const int kVersionMask = 0x7fff;
-    return ntohs(block()->control_.version_) & kVersionMask;
-  }
-
-  void set_version(uint16 version) {
-    const uint16 kControlBit = 0x80;
-    DCHECK_EQ(0, version & kControlBit);
-    mutable_block()->control_.version_ = kControlBit | htons(version);
-  }
-
-  SpdyControlType type() const {
-    uint16 type = ntohs(block()->control_.type_);
-    LOG_IF(DFATAL, type < SYN_STREAM || type >= NUM_CONTROL_FRAME_TYPES)
-        << "Invalid control frame type " << type;
-    return static_cast<SpdyControlType>(type);
-  }
-
-  void set_type(SpdyControlType type) {
-    DCHECK(type >= SYN_STREAM && type < NUM_CONTROL_FRAME_TYPES);
-    mutable_block()->control_.type_ = htons(type);
-  }
-
-  // Returns true if this control frame is of a type that has a header block,
-  // otherwise it returns false.
-  bool has_header_block() const {
-    return type() == SYN_STREAM || type() == SYN_REPLY || type() == HEADERS;
-  }
-
- private:
-  const struct SpdyFrameBlock* block() const {
-    return frame_;
-  }
-  struct SpdyFrameBlock* mutable_block() {
-    return frame_;
-  }
-  DISALLOW_COPY_AND_ASSIGN(SpdyControlFrame);
-};
-
 }  // namespace net
 
 #endif  // NET_SPDY_SPDY_PROTOCOL_H_
diff --git a/net/spdy/spdy_protocol_test.cc b/net/spdy/spdy_protocol_test.cc
index db56d3a..cd4c36c 100644
--- a/net/spdy/spdy_protocol_test.cc
+++ b/net/spdy/spdy_protocol_test.cc
@@ -16,7 +16,7 @@ enum SpdyProtocolTestTypes {
   SPDY3 = 3,
 };
 
-} // namespace
+}  // namespace
 
 namespace net {
 
@@ -42,7 +42,6 @@ INSTANTIATE_TEST_CASE_P(SpdyProtocolTests,
 TEST_P(SpdyProtocolTest, ProtocolConstants) {
   EXPECT_EQ(8u, SpdyFrame::kHeaderSize);
   EXPECT_EQ(8u, SpdyDataFrame::size());
-  EXPECT_EQ(8u, SpdyControlFrame::kHeaderSize);
   EXPECT_EQ(4u, sizeof(FlagsAndLength));
   EXPECT_EQ(1, SYN_STREAM);
   EXPECT_EQ(2, SYN_REPLY);
@@ -119,20 +118,6 @@ TEST_P(SpdyProtocolTest, TestDataFrame) {
   EXPECT_EQ(length, frame.length());
 }
 
-TEST_P(SpdyProtocolTest, HasHeaderBlock) {
-  SpdyControlFrame frame(SpdyControlFrame::kHeaderSize);
-  for (SpdyControlType type = SYN_STREAM;
-      type < NUM_CONTROL_FRAME_TYPES;
-      type = static_cast<SpdyControlType>(type + 1)) {
-    frame.set_type(type);
-    if (type == SYN_STREAM || type == SYN_REPLY || type == HEADERS) {
-      EXPECT_TRUE(frame.has_header_block());
-    } else {
-      EXPECT_FALSE(frame.has_header_block());
-    }
-  }
-}
-
 class SpdyProtocolDeathTest : public SpdyProtocolTest {};
 
 // All tests are run with two different SPDY versions: SPDY/2 and SPDY/3.
@@ -168,29 +153,4 @@ TEST_P(SpdyProtocolDeathTest, TestDataFrame) {
   EXPECT_EQ(0, frame.flags());
 }
 
-TEST_P(SpdyProtocolDeathTest, TestSpdyControlFrameType) {
-  SpdyControlFrame frame(SpdyControlFrame::kHeaderSize);
-  memset(frame.data(), 255, SpdyControlFrame::kHeaderSize);
-
-  // type() should be out of bounds.
-  EXPECT_FALSE(frame.AppearsToBeAValidControlFrame());
-
-  frame.set_version(spdy_version_);
-  uint16 version = frame.version();
-
-  for (int i = SYN_STREAM; i <= WINDOW_UPDATE; ++i) {
-    frame.set_type(static_cast<SpdyControlType>(i));
-    EXPECT_EQ(i, static_cast<int>(frame.type()));
-    if (!IsSpdy2() && i == NOOP) {
-      // NOOP frames aren't 'valid'.
-      EXPECT_FALSE(frame.AppearsToBeAValidControlFrame());
-    } else {
-      EXPECT_TRUE(frame.AppearsToBeAValidControlFrame());
-    }
-    // Make sure setting type does not alter the version block.
-    EXPECT_EQ(version, frame.version());
-    EXPECT_TRUE(frame.is_control_frame());
-  }
-}
-
 }  // namespace net