Update spdy_protocol to support the GOAWAY message.

Update SpdyControlFrame to remove the stream_id.
Update associated tests.

BUG=none
TEST=spdy_protocol_test.cc

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

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

6 files changed, 235 insertions, 68 deletions

diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index e001d86..2d9ea98 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -237,7 +237,7 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
 
     // This is just a sanity check for help debugging early frame errors.
     if (remaining_payload_ > 1000000u) {
-      LOG(ERROR) <<
+      LOG(WARNING) <<
           "Unexpectedly large frame.  Spdy session is likely corrupt.";
     }
 
@@ -293,10 +293,14 @@ void SpdyFramer::ProcessControlFrameHeader() {
       // NOOP.  Swallow it.
       CHANGE_STATE(SPDY_AUTO_RESET);
       return;
+    case GOAWAY:
+      if (current_control_frame.length() !=
+          SpdyGoAwayControlFrame::size() - SpdyFrame::size())
+        set_error(SPDY_INVALID_CONTROL_FRAME);
+      break;
     default:
       LOG(WARNING) << "Valid spdy control frame with unknown type: "
-                   << current_control_frame.type()
-                   << ".  This should never happen";
+                   << current_control_frame.type();
       DCHECK(false);
       set_error(SPDY_INVALID_CONTROL_FRAME);
       break;
@@ -338,8 +342,11 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
 
     // If this is a FIN, tell the caller.
     if (control_frame.type() == SYN_REPLY &&
-        control_frame.flags() & CONTROL_FLAG_FIN)
-      visitor_->OnStreamFrameData(control_frame.stream_id(), NULL, 0);
+        control_frame.flags() & CONTROL_FLAG_FIN) {
+      visitor_->OnStreamFrameData(reinterpret_cast<SpdySynReplyControlFrame*>(
+                                      &control_frame)->stream_id(),
+                                  NULL, 0);
+    }
 
     CHANGE_STATE(SPDY_IGNORE_REMAINING_PAYLOAD);
   } while (false);
@@ -518,6 +525,18 @@ SpdyRstStreamControlFrame* SpdyFramer::CreateRstStream(SpdyStreamId stream_id,
   return reinterpret_cast<SpdyRstStreamControlFrame*>(frame.take());
 }
 
+/* static */
+SpdyGoAwayControlFrame* SpdyFramer::CreateGoAway(
+    SpdyStreamId last_accepted_stream_id) {
+  SpdyFrameBuilder frame;
+  frame.WriteUInt16(kControlFlagMask | kSpdyProtocolVersion);
+  frame.WriteUInt16(GOAWAY);
+  size_t go_away_size = SpdyGoAwayControlFrame::size() - SpdyFrame::size();
+  frame.WriteUInt32(go_away_size);
+  frame.WriteUInt32(last_accepted_stream_id);
+  return reinterpret_cast<SpdyGoAwayControlFrame*>(frame.take());
+}
+
 SpdySynReplyControlFrame* SpdyFramer::CreateSynReply(SpdyStreamId stream_id,
     SpdyControlFlags flags, bool compressed, SpdyHeaderBlock* headers) {
 
@@ -582,7 +601,13 @@ SpdyControlFrame* SpdyFramer::CreateNopFrame() {
   return reinterpret_cast<SpdyControlFrame*>(frame.take());
 }
 
-static const int kCompressorLevel = Z_DEFAULT_COMPRESSION;
+// The following compression setting are based on Brian Olson's analysis. See
+// https://groups.google.com/group/spdy-dev/browse_thread/thread/dfaf498542fac792
+// for more details.
+static const int kCompressorLevel = 9;
+static const int kCompressorWindowSizeInBits = 11;
+static const int kCompressorMemLevel = 1;
+
 // This is just a hacked dictionary to use for shrinking HTTP-like headers.
 // TODO(mbelshe): Use a scientific methodology for computing the dictionary.
 const char SpdyFramer::kDictionary[] =
@@ -610,7 +635,12 @@ bool SpdyFramer::InitializeCompressor() {
   compressor_.reset(new z_stream);
   memset(compressor_.get(), 0, sizeof(z_stream));
 
-  int success = deflateInit(compressor_.get(), kCompressorLevel);
+  int success = deflateInit2(compressor_.get(),
+                             kCompressorLevel,
+                             Z_DEFLATED,
+                             kCompressorWindowSizeInBits,
+                             kCompressorMemLevel,
+                             Z_DEFAULT_STRATEGY);
   if (success == Z_OK)
     success = deflateSetDictionary(compressor_.get(),
                                    reinterpret_cast<const Bytef*>(kDictionary),
diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
index fedb6e3..c97ba682 100644
--- a/net/spdy/spdy_framer.h
+++ b/net/spdy/spdy_framer.h
@@ -148,6 +148,13 @@ class SpdyFramer {
   static SpdyRstStreamControlFrame* CreateRstStream(SpdyStreamId stream_id,
                                                     int status);
 
+  // Creates an instance of SpdyGoAwayControlFrame. The GOAWAY frame is used
+  // prior to the shutting down of the TCP connection, and includes the
+  // stream_id of the last stream the sender of the frame is willing to process
+  // to completion.
+  static SpdyGoAwayControlFrame* CreateGoAway(
+      SpdyStreamId last_accepted_stream_id);
+
   // Create a SpdySynReplyControlFrame.
   // |stream_id| is the stream for this frame.
   // |flags| is the flags to use with the data.
diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 2d612f6..3d8ccd7 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -109,7 +109,7 @@ static const unsigned char kGetSyn[] = {
 static const unsigned char kGetSynCompressed[] = {
   0x80, 0x01, 0x00, 0x01, 0x01, 0x00, 0x00, 0x47,
   0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
-  0xc0, 0x00, 0x78, 0xbb, 0xdf, 0xa2, 0x51, 0xb2,
+  0xc0, 0x00, 0x38, 0xea, 0xdf, 0xa2, 0x51, 0xb2,
   0x62, 0x60, 0x66, 0x60, 0xcb, 0x05, 0xe6, 0xc3,
   0xfc, 0x14, 0x06, 0x66, 0x77, 0xd7, 0x10, 0x06,
   0x66, 0x90, 0xa0, 0x58, 0x46, 0x49, 0x49, 0x81,
diff --git a/net/spdy/spdy_protocol.h b/net/spdy/spdy_protocol.h
index c40b555..5abb202 100644
--- a/net/spdy/spdy_protocol.h
+++ b/net/spdy/spdy_protocol.h
@@ -70,6 +70,39 @@
 //  |        Status code (32 bits)     |
 //  +----------------------------------+
 //
+//  Control Frame: HELLO
+//  +----------------------------------+
+//  |1|000000000000001|0000000000000100|
+//  +----------------------------------+
+//  | flags (8)  |  Length (24 bits)   |
+//  +----------------------------------+
+//  |     Unused     |# of entries (16)|
+//  +----------------------------------+
+//
+//  Control Frame: NOOP
+//  +----------------------------------+
+//  |1|000000000000001|0000000000000101|
+//  +----------------------------------+
+//  | flags (8)  |  Length (24 bits)   | = 0
+//  +----------------------------------+
+//
+//  Control Frame: PING
+//  +----------------------------------+
+//  |1|000000000000001|0000000000000110|
+//  +----------------------------------+
+//  | flags (8)  |  Length (24 bits)   | = 4
+//  +----------------------------------+
+//  |        Unique id (32 bits)       |
+//  +----------------------------------+
+//
+//  Control Frame: GOAWAY
+//  +----------------------------------+
+//  |1|000000000000001|0000000000000111|
+//  +----------------------------------+
+//  | flags (8)  |  Length (24 bits)   | = 4
+//  +----------------------------------+
+//  |X|  Last-accepted-stream-id       |
+//  +----------------------------------+
 
 namespace spdy {
 
@@ -156,31 +189,31 @@ struct SpdyFrameBlock {
   FlagsAndLength flags_length_;
 };
 
-// A Control Frame structure.
-struct SpdyControlFrameBlock : SpdyFrameBlock {
-  // Note: technically, this probably should be moved to the
-  //       various control frame subclasses.  Not all control
-  //       frames will always have stream_ids.
-  SpdyStreamId stream_id_;
-};
-
 // A SYN_STREAM Control Frame structure.
-struct SpdySynStreamControlFrameBlock : SpdyControlFrameBlock {
+struct SpdySynStreamControlFrameBlock : SpdyFrameBlock {
+  SpdyStreamId stream_id_;
   SpdyStreamId associated_stream_id_;
   SpdyPriority priority_;
   uint8 unused_;
 };
 
 // A SYN_REPLY Control Frame structure.
-struct SpdySynReplyControlFrameBlock : SpdyControlFrameBlock {
+struct SpdySynReplyControlFrameBlock : SpdyFrameBlock {
+  SpdyStreamId stream_id_;
   uint16 unused_;
 };
 
-// A FNI_STREAM Control Frame structure.
-struct SpdyRstStreamControlFrameBlock : SpdyControlFrameBlock {
+// A RST_STREAM Control Frame structure.
+struct SpdyRstStreamControlFrameBlock : SpdyFrameBlock {
+  SpdyStreamId stream_id_;
   uint32 status_;
 };
 
+// A GOAWAY Control Frame structure
+struct SpdyGoAwayControlFrameBlock : SpdyFrameBlock {
+  SpdyStreamId last_accepted_stream_id_;
+};
+
 #pragma pack(pop)
 
 // -------------------------------------------------------------------------
@@ -210,7 +243,7 @@ class SpdyFrame {
     DCHECK(frame_);
   }
 
-  virtual ~SpdyFrame() {
+  ~SpdyFrame() {
     if (owns_buffer_) {
       char* buffer = reinterpret_cast<char*>(frame_);
       delete [] buffer;
@@ -260,7 +293,6 @@ class SpdyDataFrame : public SpdyFrame {
   SpdyDataFrame() : SpdyFrame(size()) {}
   SpdyDataFrame(char* data, bool owns_buffer)
       : SpdyFrame(data, owns_buffer) {}
-  virtual ~SpdyDataFrame() {}
 
   SpdyStreamId stream_id() const {
     return ntohl(frame_->data_.stream_id_) & kStreamIdMask;
@@ -288,7 +320,6 @@ class SpdyControlFrame : public SpdyFrame {
   explicit SpdyControlFrame(size_t size) : SpdyFrame(size) {}
   SpdyControlFrame(char* data, bool owns_buffer)
       : SpdyFrame(data, owns_buffer) {}
-  virtual ~SpdyControlFrame() {}
 
   // Callers can use this method to check if the frame appears to be a valid
   // frame.  Does not guarantee that there are no errors.
@@ -320,24 +351,16 @@ class SpdyControlFrame : public SpdyFrame {
     mutable_block()->control_.type_ = htons(type);
   }
 
-  SpdyStreamId stream_id() const {
-    return ntohl(block()->stream_id_) & kStreamIdMask;
-  }
-
-  void set_stream_id(SpdyStreamId id) {
-    mutable_block()->stream_id_ = htonl(id & kStreamIdMask);
-  }
-
-  // Returns the size of the SpdyControlFrameBlock structure.
+  // Returns the size of the SpdyFrameBlock structure.
   // Note: this is not the size of the SpdyControlFrame class.
-  static size_t size() { return sizeof(SpdyControlFrameBlock); }
+  static size_t size() { return sizeof(SpdyFrameBlock); }
 
  private:
-  const struct SpdyControlFrameBlock* block() const {
-    return static_cast<SpdyControlFrameBlock*>(frame_);
+  const struct SpdyFrameBlock* block() const {
+    return frame_;
   }
-  struct SpdyControlFrameBlock* mutable_block() {
-    return static_cast<SpdyControlFrameBlock*>(frame_);
+  struct SpdyFrameBlock* mutable_block() {
+    return frame_;
   }
   DISALLOW_COPY_AND_ASSIGN(SpdyControlFrame);
 };
@@ -348,7 +371,14 @@ class SpdySynStreamControlFrame : public SpdyControlFrame {
   SpdySynStreamControlFrame() : SpdyControlFrame(size()) {}
   SpdySynStreamControlFrame(char* data, bool owns_buffer)
       : SpdyControlFrame(data, owns_buffer) {}
-  virtual ~SpdySynStreamControlFrame() {}
+
+  SpdyStreamId stream_id() const {
+    return ntohl(block()->stream_id_) & kStreamIdMask;
+  }
+
+  void set_stream_id(SpdyStreamId id) {
+    mutable_block()->stream_id_ = htonl(id & kStreamIdMask);
+  }
 
   SpdyStreamId associated_stream_id() const {
     return ntohl(block()->associated_stream_id_) & kStreamIdMask;
@@ -391,7 +421,14 @@ class SpdySynReplyControlFrame : public SpdyControlFrame {
   SpdySynReplyControlFrame() : SpdyControlFrame(size()) {}
   SpdySynReplyControlFrame(char* data, bool owns_buffer)
       : SpdyControlFrame(data, owns_buffer) {}
-  virtual ~SpdySynReplyControlFrame() {}
+
+  SpdyStreamId stream_id() const {
+    return ntohl(block()->stream_id_) & kStreamIdMask;
+  }
+
+  void set_stream_id(SpdyStreamId id) {
+    mutable_block()->stream_id_ = htonl(id & kStreamIdMask);
+  }
 
   int header_block_len() const {
     return length() - (size() - SpdyFrame::size());
@@ -421,7 +458,14 @@ class SpdyRstStreamControlFrame : public SpdyControlFrame {
   SpdyRstStreamControlFrame() : SpdyControlFrame(size()) {}
   SpdyRstStreamControlFrame(char* data, bool owns_buffer)
       : SpdyControlFrame(data, owns_buffer) {}
-  virtual ~SpdyRstStreamControlFrame() {}
+
+  SpdyStreamId stream_id() const {
+    return ntohl(block()->stream_id_) & kStreamIdMask;
+  }
+
+  void set_stream_id(SpdyStreamId id) {
+    mutable_block()->stream_id_ = htonl(id & kStreamIdMask);
+  }
 
   uint32 status() const { return ntohl(block()->status_); }
   void set_status(uint32 status) { mutable_block()->status_ = htonl(status); }
@@ -440,6 +484,32 @@ class SpdyRstStreamControlFrame : public SpdyControlFrame {
   DISALLOW_COPY_AND_ASSIGN(SpdyRstStreamControlFrame);
 };
 
+class SpdyGoAwayControlFrame : public SpdyControlFrame {
+ public:
+  SpdyGoAwayControlFrame() : SpdyControlFrame(size()) {}
+  SpdyGoAwayControlFrame(char* data, bool owns_buffer)
+      : SpdyControlFrame(data, owns_buffer) {}
+
+  SpdyStreamId last_accepted_stream_id() const {
+    return ntohl(block()->last_accepted_stream_id_) & kStreamIdMask;
+  }
+
+  void set_last_accepted_stream_id(SpdyStreamId id) {
+    mutable_block()->last_accepted_stream_id_ = htonl(id & kStreamIdMask);
+  }
+
+  static size_t size() { return sizeof(SpdyGoAwayControlFrameBlock); }
+
+ private:
+  const struct SpdyGoAwayControlFrameBlock* block() const {
+    return static_cast<SpdyGoAwayControlFrameBlock*>(frame_);
+  }
+  struct SpdyGoAwayControlFrameBlock* mutable_block() {
+    return static_cast<SpdyGoAwayControlFrameBlock*>(frame_);
+  }
+  DISALLOW_COPY_AND_ASSIGN(SpdyGoAwayControlFrame);
+};
+
 }  // namespace spdy
 
 #endif  // NET_SPDY_SPDY_PROTOCOL_H_
diff --git a/net/spdy/spdy_protocol_test.cc b/net/spdy/spdy_protocol_test.cc
index 7c171bf..128ff47 100644
--- a/net/spdy/spdy_protocol_test.cc
+++ b/net/spdy/spdy_protocol_test.cc
@@ -12,6 +12,8 @@
 using spdy::SpdyDataFrame;
 using spdy::SpdyFrame;
 using spdy::SpdyControlFrame;
+using spdy::SpdyControlType;
+using spdy::SpdyGoAwayControlFrame;
 using spdy::SpdySynStreamControlFrame;
 using spdy::SpdySynReplyControlFrame;
 using spdy::SpdyRstStreamControlFrame;
@@ -21,6 +23,7 @@ using spdy::FlagsAndLength;
 using spdy::kLengthMask;
 using spdy::kStreamIdMask;
 using spdy::kSpdyProtocolVersion;
+using spdy::GOAWAY;
 using spdy::SYN_STREAM;
 using spdy::SYN_REPLY;
 using spdy::RST_STREAM;
@@ -33,14 +36,16 @@ namespace {
 TEST(SpdyProtocolTest, ProtocolConstants) {
   EXPECT_EQ(8u, SpdyFrame::size());
   EXPECT_EQ(8u, SpdyDataFrame::size());
-  EXPECT_EQ(12u, SpdyControlFrame::size());
+  EXPECT_EQ(8u, SpdyControlFrame::size());
   EXPECT_EQ(18u, SpdySynStreamControlFrame::size());
   EXPECT_EQ(14u, SpdySynReplyControlFrame::size());
   EXPECT_EQ(16u, SpdyRstStreamControlFrame::size());
+  EXPECT_EQ(12u, SpdyGoAwayControlFrame::size());
   EXPECT_EQ(4u, sizeof(FlagsAndLength));
   EXPECT_EQ(1, SYN_STREAM);
   EXPECT_EQ(2, SYN_REPLY);
   EXPECT_EQ(3, RST_STREAM);
+  EXPECT_EQ(7, GOAWAY);
 }
 
 // Test some of the protocol helper functions
@@ -72,7 +77,7 @@ TEST(SpdyProtocolTest, ControlFrameStructs) {
   scoped_ptr<SpdySynStreamControlFrame> syn_frame(
       framer.CreateSynStream(123, 456, 2, CONTROL_FLAG_FIN, false, &headers));
   EXPECT_EQ(kSpdyProtocolVersion, syn_frame->version());
-  EXPECT_EQ(true, syn_frame->is_control_frame());
+  EXPECT_TRUE(syn_frame->is_control_frame());
   EXPECT_EQ(SYN_STREAM, syn_frame->type());
   EXPECT_EQ(123u, syn_frame->stream_id());
   EXPECT_EQ(456u, syn_frame->associated_stream_id());
@@ -86,22 +91,29 @@ TEST(SpdyProtocolTest, ControlFrameStructs) {
   scoped_ptr<SpdySynReplyControlFrame> syn_reply(
       framer.CreateSynReply(123, CONTROL_FLAG_NONE, false, &headers));
   EXPECT_EQ(kSpdyProtocolVersion, syn_reply->version());
-  EXPECT_EQ(true, syn_reply->is_control_frame());
+  EXPECT_TRUE(syn_reply->is_control_frame());
   EXPECT_EQ(SYN_REPLY, syn_reply->type());
   EXPECT_EQ(123u, syn_reply->stream_id());
   EXPECT_EQ(2, syn_reply->header_block_len());
   EXPECT_EQ(0, syn_reply->flags());
 
-  scoped_ptr<SpdyRstStreamControlFrame> fin_frame(
+  scoped_ptr<SpdyRstStreamControlFrame> rst_frame(
       framer.CreateRstStream(123, 444));
-  EXPECT_EQ(kSpdyProtocolVersion, fin_frame->version());
-  EXPECT_EQ(true, fin_frame->is_control_frame());
-  EXPECT_EQ(RST_STREAM, fin_frame->type());
-  EXPECT_EQ(123u, fin_frame->stream_id());
-  EXPECT_EQ(444u, fin_frame->status());
-  fin_frame->set_status(555);
-  EXPECT_EQ(555u, fin_frame->status());
-  EXPECT_EQ(0, fin_frame->flags());
+  EXPECT_EQ(kSpdyProtocolVersion, rst_frame->version());
+  EXPECT_TRUE(rst_frame->is_control_frame());
+  EXPECT_EQ(RST_STREAM, rst_frame->type());
+  EXPECT_EQ(123u, rst_frame->stream_id());
+  EXPECT_EQ(444u, rst_frame->status());
+  rst_frame->set_status(555);
+  EXPECT_EQ(555u, rst_frame->status());
+  EXPECT_EQ(0, rst_frame->flags());
+
+  scoped_ptr<SpdyGoAwayControlFrame> goaway_frame(
+      framer.CreateGoAway(123));
+  EXPECT_EQ(kSpdyProtocolVersion, goaway_frame->version());
+  EXPECT_TRUE(goaway_frame->is_control_frame());
+  EXPECT_EQ(GOAWAY, goaway_frame->type());
+  EXPECT_EQ(123u, goaway_frame->last_accepted_stream_id());
 }
 
 TEST(SpdyProtocolTest, TestDataFrame) {
@@ -133,7 +145,7 @@ TEST(SpdyProtocolTest, TestDataFrame) {
   // flags() == x.  Also make sure the length is unaltered.
   memset(frame.data(), '1', SpdyDataFrame::size());
   uint32 length = frame.length();
-  frame.set_flags(0);
+  frame.set_flags(0u);
   EXPECT_EQ(0u, frame.flags());
   EXPECT_EQ(length, frame.length());
   int8 all_flags = ~0;
@@ -147,12 +159,13 @@ TEST(SpdyProtocolTest, TestDataFrame) {
 }
 
 // Make sure that overflows both die in debug mode, and do not cause problems
-// in opt mode.  Note:  Chrome doesn't die on DCHECK failures, so the
-// EXPECT_DEBUG_DEATH doesn't work.
+// in opt mode.  Note:  The EXPECT_DEBUG_DEATH call does not work on Win32 yet,
+// so we comment it out.
 TEST(SpdyProtocolDeathTest, TestDataFrame) {
   SpdyDataFrame frame;
 
   frame.set_stream_id(0);
+  // TODO(mbelshe):  implement EXPECT_DEBUG_DEATH on windows.
 #ifndef WIN32
   EXPECT_DEBUG_DEATH(frame.set_stream_id(~0), "");
 #endif
@@ -165,18 +178,62 @@ TEST(SpdyProtocolDeathTest, TestDataFrame) {
   EXPECT_EQ(0, frame.flags());
 }
 
-TEST(SpdyProtocolDeathTest, TestSpdyControlFrame) {
-  SpdyControlFrame frame(SpdyControlFrame::size());
-  memset(frame.data(), '1', SpdyControlFrame::size());
+TEST(SpdyProtocolDeathTest, TestSpdyControlFrameStreamId) {
+  SpdyControlFrame frame_store(SpdySynStreamControlFrame::size());
+  memset(frame_store.data(), '1', SpdyControlFrame::size());
+  SpdySynStreamControlFrame* frame =
+      reinterpret_cast<SpdySynStreamControlFrame*>(&frame_store);
 
   // Set the stream ID to various values.
-  frame.set_stream_id(0);
-  EXPECT_EQ(0u, frame.stream_id());
-  EXPECT_FALSE(frame.is_control_frame());
-  frame.set_stream_id(~0 & kStreamIdMask);
-  EXPECT_EQ(~0 & kStreamIdMask, frame.stream_id());
-  EXPECT_FALSE(frame.is_control_frame());
+  frame->set_stream_id(0);
+  EXPECT_EQ(0u, frame->stream_id());
+  EXPECT_FALSE(frame->is_control_frame());
+  frame->set_stream_id(kStreamIdMask);
+  EXPECT_EQ(kStreamIdMask, frame->stream_id());
+  EXPECT_FALSE(frame->is_control_frame());
 }
 
-}  // namespace
+TEST(SpdyProtocolDeathTest, TestSpdyControlFrameVersion) {
+  const unsigned int kVersionMask = 0x7fff;
+  SpdyControlFrame frame(SpdySynStreamControlFrame::size());
+  memset(frame.data(), '1', SpdyControlFrame::size());
 
+  // Set the version to various values, and make sure it does not affect the
+  // type.
+  frame.set_type(SYN_STREAM);
+  frame.set_version(0);
+  EXPECT_EQ(0, frame.version());
+  EXPECT_TRUE(frame.is_control_frame());
+  EXPECT_EQ(SYN_STREAM, frame.type());
+
+  SpdySynStreamControlFrame* syn_stream =
+      reinterpret_cast<SpdySynStreamControlFrame*>(&frame);
+  syn_stream->set_stream_id(~0 & kVersionMask);
+  EXPECT_EQ(~0 & kVersionMask, syn_stream->stream_id());
+  EXPECT_TRUE(frame.is_control_frame());
+  EXPECT_EQ(SYN_STREAM, frame.type());
+}
+
+TEST(SpdyProtocolDeathTest, TestSpdyControlFrameType) {
+  SpdyControlFrame frame(SpdyControlFrame::size());
+  memset(frame.data(), 255, SpdyControlFrame::size());
+
+  // type() should be out of bounds.
+  EXPECT_FALSE(frame.AppearsToBeAValidControlFrame());
+
+  uint16 version = frame.version();
+
+  for (int i = SYN_STREAM; i <= spdy::NOOP; ++i) {
+    frame.set_type(static_cast<SpdyControlType>(i));
+    EXPECT_EQ(i, static_cast<int>(frame.type()));
+    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
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index aee80504..b81661c 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -848,6 +848,8 @@ void SpdySession::OnSyn(const spdy::SpdySynStreamControlFrame* frame,
                         const spdy::SpdyHeaderBlock* headers) {
   spdy::SpdyStreamId stream_id = frame->stream_id();
 
+  LOG(INFO) << "Spdy SynStream for stream " << stream_id;
+
   // Server-initiated streams should have even sequence numbers.
   if ((stream_id & 0x1) != 0) {
     LOG(ERROR) << "Received invalid OnSyn stream id " << stream_id;
@@ -937,6 +939,8 @@ void SpdySession::OnSynReply(const spdy::SpdySynReplyControlFrame* frame,
                              const spdy::SpdyHeaderBlock* headers) {
   DCHECK(headers);
   spdy::SpdyStreamId stream_id = frame->stream_id();
+  LOG(INFO) << "Spdy SynReply for stream " << stream_id;
+
   bool valid_stream = IsStreamActive(stream_id);
   if (!valid_stream) {
     // NOTE:  it may just be that the stream was cancelled.
@@ -1001,18 +1005,15 @@ void SpdySession::OnControl(const spdy::SpdyControlFrame* frame) {
 
   switch (type) {
     case spdy::SYN_STREAM:
-      LOG(INFO) << "Spdy SynStream for stream " << frame->stream_id();
       OnSyn(reinterpret_cast<const spdy::SpdySynStreamControlFrame*>(frame),
             &headers);
       break;
     case spdy::SYN_REPLY:
-      LOG(INFO) << "Spdy SynReply for stream " << frame->stream_id();
       OnSynReply(
           reinterpret_cast<const spdy::SpdySynReplyControlFrame*>(frame),
           &headers);
       break;
     case spdy::RST_STREAM:
-      LOG(INFO) << "Spdy Fin for stream " << frame->stream_id();
       OnFin(reinterpret_cast<const spdy::SpdyRstStreamControlFrame*>(frame));
       break;
     default:
@@ -1022,6 +1023,8 @@ void SpdySession::OnControl(const spdy::SpdyControlFrame* frame) {
 
 void SpdySession::OnFin(const spdy::SpdyRstStreamControlFrame* frame) {
   spdy::SpdyStreamId stream_id = frame->stream_id();
+  LOG(INFO) << "Spdy Fin for stream " << stream_id;
+
   bool valid_stream = IsStreamActive(stream_id);
   if (!valid_stream) {
     // NOTE:  it may just be that the stream was cancelled.