Allow QUIC's BBR congestion control to be negotiated in the crypto

net/quic/quic_config.h

Index: net/quic/quic_config.h
diff --git a/net/quic/quic_config.h b/net/quic/quic_config.h
index 88ad8b6a91c602d7e5a4e6058d74269a2c4bb5c9..4d65097bc85aadfae93c28bf541a7d97aae23a80 100644
--- a/net/quic/quic_config.h
+++ b/net/quic/quic_config.h
@@ -215,6 +215,42 @@ class NET_EXPORT_PRIVATE QuicFixedTag : public QuicConfigValue {
   bool has_receive_value_;
 };
 
+// Stores tag from CHLO or SHLO messages that are not negotiated.

[wtc, 2014/06/03 02:00:53]

1. If I only read this class declaration, I have no idea how to use it. Below I
suggest some potential improvements.

2. Nit: "tag" is confusing because the value of this tag is a vector of tags.
It's not clear whether "tag" refers to the name or the value.

Also, this comment should point out that the config value is a tag vector, in
addition to the fixed, non-negotiated aspect.

[Ian Swett, 2014/06/03 13:31:29]

Agreed on documentation.  This class follows the pattern of classes in
QuicConfig in general.  I'm not sure if we want to document every one separately
or not?  In practice, they're pretty straightforward in context, but more
comments are fine by me.

+class NET_EXPORT_PRIVATE QuicFixedTagVector : public QuicConfigValue {
+ public:
+  QuicFixedTagVector(QuicTag name, QuicConfigPresence presence);
+  virtual ~QuicFixedTagVector();
+
+  bool HasSendValues() const;
+
+  QuicTagVector GetSendValues() const;
+
+  void SetSendValues(const QuicTagVector& values);
+
+  bool HasReceivedValues() const;
+
+  QuicTagVector GetReceivedValues() const;
+
+  void SetReceivedValues(const QuicTagVector& values);

[wtc, 2014/06/03 02:00:53]

Nit: document "send values" and "receive values"

+
+  // If has_send_value is true, serialises |tag_vector_| and |send_value_| to

[wtc, 2014/06/03 02:00:53]

Nit: this comment seems stale.

"has_send_value" is missing the trailing '_'.

There is no member by the name "tag_vector_".

send_value_ => send_values_

+  // |out|.
+  virtual void ToHandshakeMessage(CryptoHandshakeMessage* out) const OVERRIDE;
+
+  // Sets |receive_values_| to the corresponding value from |client_hello_| if

[wtc, 2014/06/03 02:00:53]

1. Typo: client_hello_ => peer_hello

2. "if it exists" probably should be deleted.

+  // it exists.
+  virtual QuicErrorCode ProcessPeerHello(
+      const CryptoHandshakeMessage& peer_hello,
+      HelloType hello_type,

[wtc, 2014/06/03 02:00:53]

Nit: I believe |hello_type| can be inferred from peer_hello.tag(). So this
parameter doesn't seem necessary.

+      std::string* error_details) OVERRIDE;
+
+ private:
+  QuicTagVector send_values_;
+  bool has_send_values_;
+  QuicTagVector receive_values_;
+  bool has_receive_values_;
+};
+
 // QuicConfig contains non-crypto configuration options that are negotiated in
 // the crypto handshake.
 class NET_EXPORT_PRIVATE QuicConfig {
@@ -222,10 +258,16 @@ class NET_EXPORT_PRIVATE QuicConfig {
   QuicConfig();
   ~QuicConfig();
 
-  void set_congestion_control(const QuicTagVector& congestion_control,
-                              QuicTag default_congestion_control);
+  void set_congestion_feedback(const QuicTagVector& congestion_feedback,
+                               QuicTag default_congestion_feedback);
+
+  QuicTag congestion_feedback() const;
+
+  void SetCongestionOptionsToSend(const QuicTagVector& congestion_options);
+
+  bool HasReceivedCongestionOptions() const;
 
-  QuicTag congestion_control() const;
+  QuicTagVector ReceivedCongestionOptions() const;
 
   void SetLossDetectionToSend(QuicTag loss_detection);
 
@@ -294,7 +336,9 @@ class NET_EXPORT_PRIVATE QuicConfig {
   friend class test::QuicConfigPeer;
 
   // Congestion control feedback type.
-  QuicNegotiableTag congestion_control_;
+  QuicNegotiableTag congestion_feedback_;
+  // Congestion control option.

[wtc, 2014/06/03 02:00:53]

Nit: option => options

+  QuicFixedTagVector congestion_options_;
   // Loss detection feedback type.
   QuicFixedTag loss_detection_;
   // Idle connection state lifetime