Add a SpdyFramer extension API implementation.

net/spdy/spdy_frame_builder.h

Index: net/spdy/spdy_frame_builder.h
diff --git a/net/spdy/spdy_frame_builder.h b/net/spdy/spdy_frame_builder.h
index 2715452d90b702b9c371fb32f5f2027562bca9de..cd13f5bef4f0f2cc32ace98d8fdd3b565adfd8b9 100644
--- a/net/spdy/spdy_frame_builder.h
+++ b/net/spdy/spdy_frame_builder.h
@@ -62,6 +62,14 @@ class NET_EXPORT_PRIVATE SpdyFrameBuilder {
                      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 "