Add a SpdyFramer extension API implementation.

net/spdy/spdy_frame_builder.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_SPDY_SPDY_FRAME_BUILDER_H_
 #define NET_SPDY_SPDY_FRAME_BUILDER_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 44 matching lines @@
                      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);
 
+  // Populates this frame with a HTTP2 frame prefix with type and length
+  // information.
+  bool BeginNewFrame(const SpdyFramer& framer,
+                     uint8_t type,

[xunjieli, 2017/04/04 14:36:36]

Isn't a SpdyFrameType a uint8_t?
What's the diff between this method and the one above?

[Bence, 2017/04/04 15:19:20]

SpdyFrameType is an enum class, and implicit cast is not allowed for enum
classes.  For extension frame types that are not included in the SpdyFrameType
enum it feels wrong to cast them into a SpdyFrameType (an enum with an invalid
value?), and for frame types that are included, it feels wrong having to cast
them into the underlying uint8_t type at the call site.  That's why there are
two methods.

[xunjieli, 2017/04/04 15:44:41]

I see. Function overloads is discouraged per
https://google.github.io/styleguide/cppguide.html#Function_Overloading

"If a function is overloaded by the argument types alone, a reader may have to
understand C++'s complex matching rules in order to tell what's going on. Also
many people are confused by the semantics of inheritance if a derived class
overrides only some of the variants of a function.

If you want to overload a function, consider qualifying the name with some
information about the arguments, e.g., AppendString(), AppendInt() rather than
just Append(). If you are overloading a function to support variable number of
arguments of the same type, consider making it take a std::vector so that the
user can use an initializer list to specify the arguments. "

We could have a different name to this variant, something like
BeginNewFrameWithRawType(). Even with a different name, two uint8_t arguments
are next to each other. It's likely that a user can switch |flags| and |type|
without knowing. I prefer explicit cast at call places in this case.

[Bence, 2017/04/04 16:58:47]

Sorry, I was not aware of this recommendation from the style guide.

I'm removing one of the two method.  This also means that the
DCHECK(IsDefinedFrameType) must be removed, because this method can be called
with an undefined (extension) frame type.

For the same reason, I decided to keep the method with the uint8_t signature,
and remove the one with SpdyFrameType, because SpdyFrameType implies that the
frame type is defined.  Also, for extension frames, calling a method that takes
a SpdyFrameType would involve casting an uint8_t to SpdyFrameType only so that
it would be cast back the first thing in BeginNewFrame, which is silly.  Also,
ParseFrameType() explicitly is undefined for undefined frames.

As you said, the disadvantage of this is that |type| and |flags| are
side-by-side and of identical types, so a caller might pass them in the wrong
order.  I still think this is the best option.

[xunjieli, 2017/04/04 17:07:35]

Acknowledged. That makes sense. 

+                     uint8_t flags,
+                     SpdyStreamId stream_id,
+                     size_t length);
+
   // Takes the buffer from the SpdyFrameBuilder.
   SpdySerializedFrame take() {
     SPDY_BUG_IF(output_ != nullptr) << "ZeroCopyOutputBuffer is used to build "
                                     << "frames. take() shouldn't be called";
     SPDY_BUG_IF(kMaxFrameSizeLimit < length_)
         << "Frame length " << length_
         << " is longer than the maximum possible allowed length.";
     SpdySerializedFrame rv(buffer_.release(), length(), true);
     capacity_ = 0;
     length_ = 0;
@@ skipping 69 matching lines @@
   const size_t kLengthFieldLength = 3;
   char* start_of_current_frame_ = nullptr;
   size_t bytes_of_length_written_in_first_block_ = kLengthFieldLength;
   // In case length of a new frame is cross blocks.
   char* start_of_current_frame_in_next_block_ = nullptr;
 };
 
 }  // namespace net
 
 #endif  // NET_SPDY_SPDY_FRAME_BUILDER_H_