Do not use SpdyFrameBuilder::OverwriteLength().

Change SpdyFrameBuilder::BeginNewFrame() not to use OverwriteLength().
Proteced by FLAGS_chromium_http2_flag_remove_rewritelength.

This CL lands server change 143445916 by yasong.

BUG=488484

Review-Url: https://codereview.chromium.org/2611293002
Cr-Commit-Position: refs/heads/master@{#442931}

net/spdy/spdy_flags.cc

@@ -9,6 +9,9 @@ namespace net {
 // Log compressed size of HTTP/2 requests.
 bool FLAGS_chromium_http2_flag_log_compressed_size = true;
 
+// If true, remove use of SpdyFrameBuilder::OverwriteLength().
+bool FLAGS_chromium_http2_flag_remove_rewritelength = true;
+
 // Use //net/http2/hpack/decoder as HPACK decoder.
 bool FLAGS_chromium_http2_flag_spdy_use_hpack_decoder2 = false;
 

net/spdy/spdy_flags.h

@@ -10,6 +10,7 @@
 namespace net {
 
 NET_EXPORT_PRIVATE extern bool FLAGS_chromium_http2_flag_log_compressed_size;
+NET_EXPORT_PRIVATE extern bool FLAGS_chromium_http2_flag_remove_rewritelength;
 NET_EXPORT_PRIVATE extern bool
     FLAGS_chromium_http2_flag_spdy_use_hpack_decoder2;
 NET_EXPORT_PRIVATE extern bool

net/spdy/spdy_frame_builder.cc

@@ -65,6 +65,29 @@ bool SpdyFrameBuilder::BeginNewFrame(const SpdyFramer& framer,
   return success;
 }
 
+bool SpdyFrameBuilder::BeginNewFrame(const SpdyFramer& framer,
+                                     SpdyFrameType type,
+                                     uint8_t flags,
+                                     SpdyStreamId stream_id,
+                                     size_t length) {
+  DCHECK(IsValidFrameType(SerializeFrameType(type)));
+  DCHECK_EQ(0u, stream_id & ~kStreamIdMask);
+  bool success = true;
+  SPDY_BUG_IF(framer.GetFrameMaximumSize() < length_)
+      << "Frame length  " << length_
+      << " is longer than the maximum allowed length.";
+
+  offset_ += length_;
+  length_ = 0;
+
+  success &= WriteUInt24(length);
+  success &= WriteUInt8(SerializeFrameType(type));
+  success &= WriteUInt8(flags);
+  success &= WriteUInt32(stream_id);
+  DCHECK_EQ(framer.GetDataFrameMinimumSize(), length_);
+  return success;
+}
+
 bool SpdyFrameBuilder::WriteStringPiece16(const base::StringPiece& value) {
   if (value.size() > 0xffff) {
     DCHECK(false) << "Tried to write string with length > 16bit.";

net/spdy/spdy_frame_builder.h

@@ -58,6 +58,14 @@ class NET_EXPORT_PRIVATE SpdyFrameBuilder {
                      uint8_t flags,
                      SpdyStreamId stream_id);
 
+  // Populates this frame with a HTTP2 frame prefix with length information.
+  // The given type must be a control frame type.
+  bool BeginNewFrame(const SpdyFramer& framer,
+                     SpdyFrameType type,
+                     uint8_t flags,
+                     SpdyStreamId stream_id,
+                     size_t length);
+
   // Takes the buffer from the SpdyFrameBuilder.
   SpdySerializedFrame take() {
     SPDY_BUG_IF(kMaxFrameSizeLimit < length_)

net/spdy/spdy_framer.cc

@@ -159,6 +159,7 @@ SpdyFramer::SpdyFramer(SpdyFramer::DecoderAdapterFactoryFn adapter_factory,
   if (adapter_factory != nullptr) {
     decoder_adapter_ = adapter_factory(this);
   }
+  skip_rewritelength_ = FLAGS_chromium_http2_flag_remove_rewritelength;
 }
 
 SpdyFramer::SpdyFramer(CompressionOption option)
@@ -1860,12 +1861,21 @@ SpdySerializedFrame SpdyFramer::SerializeDataFrameHeaderWithPaddingLengthField(
   }
 
   SpdyFrameBuilder builder(frame_size);
-  builder.BeginNewFrame(*this, DATA, flags, data_ir.stream_id());
-  if (data_ir.padded()) {
-    builder.WriteUInt8(data_ir.padding_payload_len() & 0xff);
+  if (!skip_rewritelength_) {
+    builder.BeginNewFrame(*this, DATA, flags, data_ir.stream_id());
+    if (data_ir.padded()) {
+      builder.WriteUInt8(data_ir.padding_payload_len() & 0xff);
+    }
+    builder.OverwriteLength(*this, num_padding_fields + data_ir.data_len() +
+                                       data_ir.padding_payload_len());
+  } else {
+    builder.BeginNewFrame(*this, DATA, flags, data_ir.stream_id(),
+                          num_padding_fields + data_ir.data_len() +
+                              data_ir.padding_payload_len());
+    if (data_ir.padded()) {
+      builder.WriteUInt8(data_ir.padding_payload_len() & 0xff);
+    }
   }
-  builder.OverwriteLength(*this, num_padding_fields + data_ir.data_len() +
-                                     data_ir.padding_payload_len());
   DCHECK_EQ(frame_size, builder.length());
   return builder.take();
 }
@@ -1993,7 +2003,29 @@ SpdySerializedFrame SpdyFramer::SerializeHeaders(const SpdyHeadersIR& headers) {
   }
 
   SpdyFrameBuilder builder(size);
-  builder.BeginNewFrame(*this, HEADERS, flags, headers.stream_id());
+
+  if (!skip_rewritelength_) {
+    builder.BeginNewFrame(*this, HEADERS, flags, headers.stream_id());
+  } else {
+    // Compute frame length field.
+    size_t length_field = 0;
+    if (headers.padded()) {
+      length_field += 1;  // Padding length field.
+    }
+    if (headers.has_priority()) {
+      length_field += 4;  // Dependency field.
+      length_field += 1;  // Weight field.
+    }
+    length_field += headers.padding_payload_len();
+    length_field += hpack_encoding.size();
+    // If the HEADERS frame with payload would exceed the max frame size, then
+    // WritePayloadWithContinuation() will serialize CONTINUATION frames as
+    // necessary.
+    length_field =
+        std::min(length_field, kMaxControlFrameSize - GetFrameHeaderSize());
+    builder.BeginNewFrame(*this, HEADERS, flags, headers.stream_id(),
+                          length_field);
+  }
   DCHECK_EQ(GetHeadersMinimumSize(), builder.length());
 
   int padding_payload_len = 0;
@@ -2066,10 +2098,13 @@ SpdySerializedFrame SpdyFramer::SerializePushPromise(
   }
 
   SpdyFrameBuilder builder(size);
-  builder.BeginNewFrame(*this,
-                        PUSH_PROMISE,
-                        flags,
-                        push_promise.stream_id());
+  if (!skip_rewritelength_) {
+    builder.BeginNewFrame(*this, PUSH_PROMISE, flags, push_promise.stream_id());
+  } else {
+    size_t length = std::min(size, kMaxControlFrameSize) - GetFrameHeaderSize();
+    builder.BeginNewFrame(*this, PUSH_PROMISE, flags, push_promise.stream_id(),
+                          length);
+  }
   int padding_payload_len = 0;
   if (push_promise.padded()) {
     builder.WriteUInt8(push_promise.padding_payload_len());
@@ -2399,7 +2434,7 @@ void SpdyFramer::WritePayloadWithContinuation(SpdyFrameBuilder* builder,
     string padding = string(padding_payload_len, 0);
     builder->WriteBytes(padding.data(), padding.length());
   }
-  if (bytes_remaining > 0) {
+  if (bytes_remaining > 0 && !skip_rewritelength_) {
     builder->OverwriteLength(*this,
                              kMaxControlFrameSize - GetFrameHeaderSize());
   }
@@ -2412,7 +2447,12 @@ void SpdyFramer::WritePayloadWithContinuation(SpdyFrameBuilder* builder,
     if (bytes_remaining == bytes_to_write) {
       flags |= end_flag;
     }
-    builder->BeginNewFrame(*this, CONTINUATION, flags, stream_id);
+    if (!skip_rewritelength_) {
+      builder->BeginNewFrame(*this, CONTINUATION, flags, stream_id);
+    } else {
+      builder->BeginNewFrame(*this, CONTINUATION, flags, stream_id,
+                             bytes_to_write);
+    }
     // Write payload fragment.
     builder->WriteBytes(
         &hpack_encoding[hpack_encoding.size() - bytes_remaining],

net/spdy/spdy_framer.h

@@ -765,6 +765,9 @@ class NET_EXPORT_PRIVATE SpdyFramer {
   // If true, then ProcessInput returns after processing a full frame,
   // rather than reading all available input.
   bool process_single_input_frame_ = false;
+
+  // Latched value of FLAGS_chromium_http2_flag_remove_rewritelength.
+  bool skip_rewritelength_ = false;
 };
 
 }  // namespace net