Getting rid of SpdyFramer::GetControlFrameStreamId(), which cleans up the code and also makes it easier to move forward with SPDY 4 work.

This lands server change 42008414.

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

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

3 files changed, 22 insertions, 75 deletions

diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index 82a063b..53070f2 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -1005,20 +1005,30 @@ 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_);
-  SpdyControlFrame control_frame(current_frame_buffer_.get(), false);
+  const SpdyControlFrame control_frame(current_frame_buffer_.get(), false);
 
   bool processed_successfully = true;
-  DCHECK(control_frame.type() == SYN_STREAM ||
-         control_frame.type() == SYN_REPLY ||
-         control_frame.type() == HEADERS);
+  SpdyStreamId stream_id = kInvalidStream;
+  if (control_frame.type() == SYN_STREAM) {
+    stream_id = reinterpret_cast<const SpdySynStreamControlFrame*>(
+        &control_frame)->stream_id();
+  } else if (control_frame.type() == SYN_REPLY) {
+    stream_id = reinterpret_cast<const SpdySynReplyControlFrame*>(
+        &control_frame)->stream_id();
+  } else if (control_frame.type() == HEADERS) {
+    stream_id = reinterpret_cast<const SpdyHeadersControlFrame*>(
+        &control_frame)->stream_id();
+  } else {
+    LOG(DFATAL) << "Unhandled frame type in ProcessControlFrameHeaderBlock.";
+  }
   size_t process_bytes = std::min(data_len, remaining_control_payload_);
   if (process_bytes > 0) {
     if (enable_compression_) {
       processed_successfully = IncrementallyDecompressControlFrameHeaderData(
-          &control_frame, data, process_bytes);
+          stream_id, data, process_bytes);
     } else {
       processed_successfully = IncrementallyDeliverControlFrameHeaderData(
-          &control_frame, data, process_bytes);
+          stream_id, data, process_bytes);
     }
 
     remaining_control_payload_ -= process_bytes;
@@ -1029,13 +1039,11 @@ size_t SpdyFramer::ProcessControlFrameHeaderBlock(const char* data,
   if (remaining_control_payload_ == 0 && processed_successfully) {
     // The complete header block has been delivered. We send a zero-length
     // OnControlFrameHeaderData() to indicate this.
-    visitor_->OnControlFrameHeaderData(
-        GetControlFrameStreamId(&control_frame), NULL, 0);
+    visitor_->OnControlFrameHeaderData(stream_id, NULL, 0);
 
     // If this is a FIN, tell the caller.
     if (control_frame.flags() & CONTROL_FLAG_FIN) {
-      visitor_->OnStreamFrameData(GetControlFrameStreamId(&control_frame),
-                                  NULL, 0, DATA_FLAG_FIN);
+      visitor_->OnStreamFrameData(stream_id, NULL, 0, DATA_FLAG_FIN);
     }
 
     CHANGE_STATE(SPDY_AUTO_RESET);
@@ -1912,7 +1920,7 @@ SpdyControlFrame* SpdyFramer::CompressControlFrame(
 // indicates that it cannot process any more data, or (more commonly) we
 // run out of data to deliver.
 bool SpdyFramer::IncrementallyDecompressControlFrameHeaderData(
-    const SpdyControlFrame* control_frame,
+    SpdyStreamId stream_id,
     const char* data,
     size_t len) {
   // Get a decompressor or set error.
@@ -1928,7 +1936,6 @@ bool SpdyFramer::IncrementallyDecompressControlFrameHeaderData(
 
   decomp->next_in = reinterpret_cast<Bytef*>(const_cast<char*>(data));
   decomp->avail_in = len;
-  const SpdyStreamId stream_id = GetControlFrameStreamId(control_frame);
   DCHECK_LT(0u, stream_id);
   while (decomp->avail_in > 0 && processed_successfully) {
     decomp->next_out = reinterpret_cast<Bytef*>(buffer);
@@ -1982,9 +1989,8 @@ bool SpdyFramer::IncrementallyDecompressControlFrameHeaderData(
 }
 
 bool SpdyFramer::IncrementallyDeliverControlFrameHeaderData(
-    const SpdyControlFrame* control_frame, const char* data, size_t len) {
+    SpdyStreamId stream_id, const char* data, size_t len) {
   bool read_successfully = true;
-  const SpdyStreamId stream_id = GetControlFrameStreamId(control_frame);
   while (read_successfully && len > 0) {
     size_t bytes_to_deliver = std::min(len, kHeaderDataChunkMaxSize);
     read_successfully = visitor_->OnControlFrameHeaderData(stream_id, data,
@@ -2023,48 +2029,6 @@ bool SpdyFramer::IsCompressible(const SpdyFrame& frame) const {
   return false;
 }
 
-/* static */
-SpdyStreamId SpdyFramer::GetControlFrameStreamId(
-    const SpdyControlFrame* control_frame) {
-  SpdyStreamId stream_id = kInvalidStream;
-  if (control_frame != NULL) {
-    switch (control_frame->type()) {
-      case SYN_STREAM:
-        stream_id = reinterpret_cast<const SpdySynStreamControlFrame*>(
-            control_frame)->stream_id();
-        break;
-      case SYN_REPLY:
-        stream_id = reinterpret_cast<const SpdySynReplyControlFrame*>(
-            control_frame)->stream_id();
-        break;
-      case HEADERS:
-        stream_id = reinterpret_cast<const SpdyHeadersControlFrame*>(
-            control_frame)->stream_id();
-        break;
-      case RST_STREAM:
-        stream_id = reinterpret_cast<const SpdyRstStreamControlFrame*>(
-            control_frame)->stream_id();
-        break;
-      case WINDOW_UPDATE:
-        stream_id = reinterpret_cast<const SpdyWindowUpdateControlFrame*>(
-            control_frame)->stream_id();
-        break;
-      // All of the following types are not part of a particular stream.
-      // They all fall through to the invalid control frame type case.
-      // (The default case isn't used so that the compile will break if a new
-      // control frame type is added but not included here.)
-      case SETTINGS:
-      case NOOP:
-      case PING:
-      case GOAWAY:
-      case CREDENTIAL:
-      case NUM_CONTROL_FRAME_TYPES:  // makes compiler happy
-        break;
-    }
-  }
-  return stream_id;
-}
-
 void SpdyFramer::SerializeNameValueBlock(
     SpdyFrameBuilder* builder,
     const SpdyFrameWithNameValueBlockIR& frame) const {
diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
index e7cd38e..9cef56c 100644
--- a/net/spdy/spdy_framer.h
+++ b/net/spdy/spdy_framer.h
@@ -475,12 +475,6 @@ class NET_EXPORT_PRIVATE SpdyFramer {
   // Returns true if a frame could be compressed.
   bool IsCompressible(const SpdyFrame& frame) const;
 
-  // Get the stream ID for the given control frame (SYN_STREAM, SYN_REPLY, and
-  // HEADERS). If the control frame is NULL or of another type, this
-  // function returns kInvalidStream.
-  static SpdyStreamId GetControlFrameStreamId(
-      const SpdyControlFrame* control_frame);
-
   // For ease of testing and experimentation we can tweak compression on/off.
   void set_enable_compression(bool value) {
     enable_compression_ = value;
@@ -567,14 +561,14 @@ class NET_EXPORT_PRIVATE SpdyFramer {
   // in decompressed form, in chunks. Returns true if the visitor has
   // accepted all of the chunks.
   bool IncrementallyDecompressControlFrameHeaderData(
-      const SpdyControlFrame* frame,
+      SpdyStreamId stream_id,
       const char* data,
       size_t len);
 
   // Deliver the given control frame's uncompressed headers block to the
   // visitor in chunks. Returns true if the visitor has accepted all of the
   // chunks.
-  bool IncrementallyDeliverControlFrameHeaderData(const SpdyControlFrame* frame,
+  bool IncrementallyDeliverControlFrameHeaderData(SpdyStreamId stream_id,
                                                   const char* data,
                                                   size_t len);
 
diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
index fe48f84..0aaf2bd1 100644
--- a/net/spdy/spdy_framer_test.cc
+++ b/net/spdy/spdy_framer_test.cc
@@ -1648,7 +1648,6 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) {
                  *frame,
                  IsSpdy2() ? kV2FrameData : kV3FrameData,
                  IsSpdy2() ? arraysize(kV2FrameData) : arraysize(kV3FrameData));
-    EXPECT_EQ(1u, SpdyFramer::GetControlFrameStreamId(frame.get()));
   }
 
   {
@@ -2015,7 +2014,6 @@ TEST_P(SpdyFramerTest, CreateRstStream) {
     scoped_ptr<SpdyRstStreamControlFrame> frame(
         framer.CreateRstStream(1, RST_STREAM_PROTOCOL_ERROR));
     CompareFrame(kDescription, *frame, kFrameData, arraysize(kFrameData));
-    EXPECT_EQ(1u, SpdyFramer::GetControlFrameStreamId(frame.get()));
   }
 
   {
@@ -2082,8 +2080,6 @@ TEST_P(SpdyFramerTest, CreateSettings) {
                  *frame,
                  IsSpdy2() ? kFrameDatav2 : kFrameDatav3,
                  arraysize(kFrameDatav3));  // Size is unchanged among versions.
-    EXPECT_EQ(SpdyFramer::kInvalidStream,
-              SpdyFramer::GetControlFrameStreamId(frame.get()));
 
     // Make sure that ParseSettings also works as advertised.
     SettingsMap parsed_settings;
@@ -2124,8 +2120,6 @@ TEST_P(SpdyFramerTest, CreateSettings) {
                  *frame,
                  kFrameData,
                  arraysize(kFrameData));
-    EXPECT_EQ(SpdyFramer::kInvalidStream,
-              SpdyFramer::GetControlFrameStreamId(frame.get()));
   }
 
   {
@@ -2155,8 +2149,6 @@ TEST_P(SpdyFramerTest, CreatePingFrame) {
     };
     scoped_ptr<SpdyPingControlFrame> frame(framer.CreatePingFrame(0x12345678u));
     CompareFrame(kDescription, *frame, kFrameData, arraysize(kFrameData));
-    EXPECT_EQ(SpdyFramer::kInvalidStream,
-              SpdyFramer::GetControlFrameStreamId(frame.get()));
   }
 }
 
@@ -2181,8 +2173,6 @@ TEST_P(SpdyFramerTest, CreateGoAway) {
                  *frame,
                  IsSpdy2() ? kV2FrameData : kV3FrameData,
                  IsSpdy2() ? arraysize(kV2FrameData) : arraysize(kV3FrameData));
-    EXPECT_EQ(SpdyFramer::kInvalidStream,
-              SpdyFramer::GetControlFrameStreamId(frame.get()));
   }
 
   {
@@ -2403,7 +2393,6 @@ TEST_P(SpdyFramerTest, CreateWindowUpdate) {
     scoped_ptr<SpdyWindowUpdateControlFrame> frame(
         framer.CreateWindowUpdate(1, 1));
     CompareFrame(kDescription, *frame, kFrameData, arraysize(kFrameData));
-    EXPECT_EQ(1u, SpdyFramer::GetControlFrameStreamId(frame.get()));
   }
 
   {