Cleanup control_frame_fields_ if coalescer rejected the headers.

net/spdy/buffered_spdy_framer_unittest.cc

Index: net/spdy/buffered_spdy_framer_unittest.cc
diff --git a/net/spdy/buffered_spdy_framer_unittest.cc b/net/spdy/buffered_spdy_framer_unittest.cc
index aba9a5c94caf75ca9b73c644da2e5033a7553a76..435bb05ebdc3fc68c66e5d7d603df77ac7f0f944 100644
--- a/net/spdy/buffered_spdy_framer_unittest.cc
+++ b/net/spdy/buffered_spdy_framer_unittest.cc
@@ -127,10 +127,11 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface {
   }
 
   // Convenience function which runs a framer simulation with particular input.
-  void SimulateInFramer(const unsigned char* input, size_t size) {
+  void SimulateInFramer(const SpdySerializedFrame& frame) {

[Maks Orlovich, 2017/02/07 18:59:48]

This could/should arguably be a separate cleanup CL, please let me know if you
would prefer it that way. 

Speaking of other CLs, header_buffer_, header_buffer_valid_, header_stream_id_
in BufferedSpdyFramer all seem dead to me --- worth a cleanup CL?

[Bence, 2017/02/07 20:01:17]

I really do not mind doing multiple things in the same CL as long as they are
not too complicated.  So this is okay with me.
 
Totally, yes please!

+    const char* input_ptr = frame.data();
+    size_t size = frame.size();

[Bence, 2017/02/07 20:01:17]

No need for named variable |size| which is only used once two lines below. 
Instead write:
size_t input_remaining = frame.size();

[Maks Orlovich, 2017/02/07 20:40:38]

Done.

     buffered_spdy_framer_.set_visitor(this);
     size_t input_remaining = size;
-    const char* input_ptr = reinterpret_cast<const char*>(input);
     while (input_remaining > 0 &&
            buffered_spdy_framer_.spdy_framer_error() ==
                SpdyFramer::SPDY_NO_ERROR) {
@@ -187,9 +188,7 @@ TEST_F(BufferedSpdyFramerTest, OnSetting) {
   SpdySerializedFrame control_frame(framer.SerializeSettings(settings_ir));
   TestBufferedSpdyVisitor visitor;
 
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(control_frame.data()),
-      control_frame.size());
+  visitor.SimulateInFramer(control_frame);
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(2, visitor.setting_count_);
 }
@@ -207,9 +206,7 @@ TEST_F(BufferedSpdyFramerTest, HeaderListTooLarge) {
   EXPECT_TRUE(control_frame);
 
   TestBufferedSpdyVisitor visitor;
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-      control_frame.get()->size());
+  visitor.SimulateInFramer(*control_frame);
 
   EXPECT_EQ(1, visitor.error_count_);
   EXPECT_EQ(0, visitor.headers_frame_count_);
@@ -217,6 +214,29 @@ TEST_F(BufferedSpdyFramerTest, HeaderListTooLarge) {
   EXPECT_EQ(SpdyHeaderBlock(), visitor.headers_);
 }
 
+TEST_F(BufferedSpdyFramerTest, ValidHeadersAfterInvalidHeaders) {
+  SpdyHeaderBlock headers;
+  headers["invalid"] = "\r\n\r\n";
+
+  SpdyHeaderBlock headers2;
+  headers["alpha"] = "beta";
+
+  SpdyTestUtil spdy_test_util;
+  SpdySerializedFrame headers_frame(
+      spdy_test_util.ConstructSpdyReply(1, std::move(headers)));
+  SpdySerializedFrame headers_frame2(
+      spdy_test_util.ConstructSpdyReply(2, std::move(headers2)));
+
+  TestBufferedSpdyVisitor visitor;
+  visitor.SimulateInFramer(headers_frame);
+  EXPECT_EQ(1, visitor.error_count_);
+  EXPECT_EQ(0, visitor.headers_frame_count_);
+
+  visitor.SimulateInFramer(headers_frame2);
+  EXPECT_EQ(1, visitor.error_count_);
+  EXPECT_EQ(1, visitor.headers_frame_count_);
+}
+
 TEST_F(BufferedSpdyFramerTest, ReadHeadersHeaderBlock) {
   SpdyHeaderBlock headers;
   headers["alpha"] = "beta";
@@ -230,9 +250,7 @@ TEST_F(BufferedSpdyFramerTest, ReadHeadersHeaderBlock) {
   EXPECT_TRUE(control_frame.get() != NULL);
 
   TestBufferedSpdyVisitor visitor;
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-      control_frame.get()->size());
+  visitor.SimulateInFramer(*control_frame);
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(1, visitor.headers_frame_count_);
   EXPECT_EQ(0, visitor.push_promise_frame_count_);
@@ -249,9 +267,7 @@ TEST_F(BufferedSpdyFramerTest, ReadPushPromiseHeaderBlock) {
   EXPECT_TRUE(control_frame.get() != NULL);
 
   TestBufferedSpdyVisitor visitor;
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(control_frame.get()->data()),
-      control_frame.get()->size());
+  visitor.SimulateInFramer(*control_frame);
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(0, visitor.headers_frame_count_);
   EXPECT_EQ(1, visitor.push_promise_frame_count_);
@@ -266,9 +282,7 @@ TEST_F(BufferedSpdyFramerTest, GoAwayDebugData) {
       framer.CreateGoAway(2u, ERROR_CODE_FRAME_SIZE_ERROR, "foo"));
 
   TestBufferedSpdyVisitor visitor;
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(goaway_frame.get()->data()),
-      goaway_frame.get()->size());
+  visitor.SimulateInFramer(*goaway_frame);
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(1, visitor.goaway_count_);
   EXPECT_EQ(2u, visitor.goaway_last_accepted_stream_id_);
@@ -289,9 +303,7 @@ TEST_F(BufferedSpdyFramerTest, OnAltSvc) {
   SpdySerializedFrame altsvc_frame(framer.SerializeFrame(altsvc_ir));
 
   TestBufferedSpdyVisitor visitor;
-  visitor.SimulateInFramer(
-      reinterpret_cast<unsigned char*>(altsvc_frame.data()),
-      altsvc_frame.size());
+  visitor.SimulateInFramer(altsvc_frame);
   EXPECT_EQ(0, visitor.error_count_);
   EXPECT_EQ(1, visitor.altsvc_count_);
   EXPECT_EQ(altsvc_stream_id, visitor.altsvc_stream_id_);