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

net/quic/quic_config.h

 // Copyright (c) 2013 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_QUIC_QUIC_CONFIG_H_
 #define NET_QUIC_QUIC_CONFIG_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 197 matching lines @@
       HelloType hello_type,
       std::string* error_details) OVERRIDE;
 
  private:
   QuicTag send_value_;
   bool has_send_value_;
   QuicTag receive_value_;
   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 {
  public:
   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_control() const;
+  QuicTag congestion_feedback() const;
+
+  void SetCongestionOptionsToSend(const QuicTagVector& congestion_options);
+
+  bool HasReceivedCongestionOptions() const;
+
+  QuicTagVector ReceivedCongestionOptions() const;
 
   void SetLossDetectionToSend(QuicTag loss_detection);
 
   bool HasReceivedLossDetection() const;
 
   QuicTag ReceivedLossDetection() const;
 
   void set_idle_connection_state_lifetime(
       QuicTime::Delta max_idle_connection_state_lifetime,
       QuicTime::Delta default_idle_conection_state_lifetime);
@@ skipping 48 matching lines @@
   // Calls ProcessPeerHello on each negotiable parameter. On failure returns
   // the corresponding QuicErrorCode and sets detailed error in |error_details|.
   QuicErrorCode ProcessPeerHello(const CryptoHandshakeMessage& peer_hello,
                                  HelloType hello_type,
                                  std::string* error_details);
 
  private:
   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
   QuicNegotiableUint32 idle_connection_state_lifetime_seconds_;
   // Keepalive timeout, or 0 to turn off keepalive probes
   QuicNegotiableUint32 keepalive_timeout_seconds_;
   // Maximum number of streams that the connection can support.
   QuicNegotiableUint32 max_streams_per_connection_;
   // Maximum time till the session can be alive before crypto handshake is
   // finished. (Not negotiated).
   QuicTime::Delta max_time_before_crypto_handshake_;
   // Initial congestion window in packets.
   QuicFixedUint32 initial_congestion_window_;
   // Initial round trip time estimate in microseconds.
   QuicFixedUint32 initial_round_trip_time_us_;
   // Initial flow control receive window in bytes.
   QuicFixedUint32 initial_flow_control_window_bytes_;
 };
 
 }  // namespace net
 
 #endif  // NET_QUIC_QUIC_CONFIG_H_