Enable QuicConnectionId variable length flags

This change reenables the QuicConnectionId variable length flags
(which makes QuicConnectionId be represented in memory in network
byte order) and fixes chromium-specific behavior that used to rely
on the connection IDs being 64 bits.

Bug: b/123008920
Change-Id: I9497c516a8612be3c0aaddd152825f559a9086f3
Reviewed-on: https://chromium-review.googlesource.com/c/1418632
Commit-Queue: David Schinazi <dschinazi@chromium.org>
Auto-Submit: David Schinazi <dschinazi@chromium.org>
Reviewed-by: Steve Anton <steveanton@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625530}

net/http/http_proxy_client_socket_wrapper_unittest.cc

@@ -89,18 +89,20 @@ class HttpProxyClientSocketWrapperTest
             quic::QuicUtils::GetHeadersStreamId(quic_version_) +
             quic::QuicUtils::StreamIdDelta(quic_version_)),
         client_headers_include_h2_stream_dependency_(std::get<1>(GetParam())),
-        client_maker_(quic_version_,
-                      quic::EmptyQuicConnectionId(),
-                      &clock_,
-                      kProxyHost,
-                      quic::Perspective::IS_CLIENT,
-                      client_headers_include_h2_stream_dependency_),
-        server_maker_(quic_version_,
-                      quic::EmptyQuicConnectionId(),
-                      &clock_,
-                      kProxyHost,
-                      quic::Perspective::IS_SERVER,
-                      false),
+        client_maker_(
+            quic_version_,
+            quic::QuicUtils::CreateRandomConnectionId(&random_generator_),
+            &clock_,
+            kProxyHost,
+            quic::Perspective::IS_CLIENT,
+            client_headers_include_h2_stream_dependency_),
+        server_maker_(
+            quic_version_,
+            quic::QuicUtils::CreateRandomConnectionId(&random_generator_),
+            &clock_,
+            kProxyHost,
+            quic::Perspective::IS_SERVER,
+            false),
         header_stream_offset_(0),
         response_offset_(0),
         store_server_configs_in_properties_(false),

net/http/http_stream_factory_job_controller_unittest.cc

@@ -43,6 +43,7 @@
 #include "net/socket/socket_test_util.h"
 #include "net/spdy/spdy_test_util_common.h"
 #include "net/test/test_with_scoped_task_environment.h"
+#include "net/third_party/quic/core/quic_utils.h"
 #include "net/third_party/quic/test_tools/mock_random.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -349,7 +350,7 @@ class HttpStreamFactoryJobControllerTest
   quic::test::MockRandom random_generator_{0};
   QuicTestPacketMaker client_maker_{
       HttpNetworkSession::Params().quic_supported_versions[0],
-      quic::EmptyQuicConnectionId(),
+      quic::QuicUtils::CreateRandomConnectionId(&random_generator_),
       &clock_,
       kServerHostname,
       quic::Perspective::IS_CLIENT,

net/http/http_stream_factory_unittest.cc

@@ -62,6 +62,7 @@
 #include "net/test/test_data_directory.h"
 #include "net/test/test_with_scoped_task_environment.h"
 #include "net/third_party/quic/core/quic_server_id.h"
+#include "net/third_party/quic/core/quic_utils.h"
 #include "net/third_party/quic/test_tools/crypto_test_utils.h"
 #include "net/third_party/quic/test_tools/mock_random.h"
 #include "net/third_party/quic/test_tools/quic_test_utils.h"
@@ -2360,19 +2361,21 @@ class HttpStreamFactoryBidirectionalQuicTest
       : default_url_(kDefaultUrl),
         version_(std::get<0>(GetParam())),
         client_headers_include_h2_stream_dependency_(std::get<1>(GetParam())),
-        client_packet_maker_(version_,
-                             quic::EmptyQuicConnectionId(),
-                             &clock_,
-                             "www.example.org",
-                             quic::Perspective::IS_CLIENT,
-                             client_headers_include_h2_stream_dependency_),
-        server_packet_maker_(version_,
-                             quic::EmptyQuicConnectionId(),
-                             &clock_,
-                             "www.example.org",
-                             quic::Perspective::IS_SERVER,
-                             false),
         random_generator_(0),
+        client_packet_maker_(
+            version_,
+            quic::QuicUtils::CreateRandomConnectionId(&random_generator_),
+            &clock_,
+            "www.example.org",
+            quic::Perspective::IS_CLIENT,
+            client_headers_include_h2_stream_dependency_),
+        server_packet_maker_(
+            version_,
+            quic::QuicUtils::CreateRandomConnectionId(&random_generator_),
+            &clock_,
+            "www.example.org",
+            quic::Perspective::IS_SERVER,
+            false),
         proxy_resolution_service_(ProxyResolutionService::CreateDirect()),
         ssl_config_service_(new SSLConfigServiceDefaults) {
     clock_.AdvanceTime(quic::QuicTime::Delta::FromMilliseconds(20));
@@ -2451,11 +2454,11 @@ class HttpStreamFactoryBidirectionalQuicTest
   const quic::QuicTransportVersion version_;
   const bool client_headers_include_h2_stream_dependency_;
   quic::MockClock clock_;
+  quic::test::MockRandom random_generator_;
   test::QuicTestPacketMaker client_packet_maker_;
   test::QuicTestPacketMaker server_packet_maker_;
   MockTaggingClientSocketFactory socket_factory_;
   std::unique_ptr<HttpNetworkSession> session_;
-  quic::test::MockRandom random_generator_;
   MockCertVerifier cert_verifier_;
   ProofVerifyDetailsChromium verify_details_;
   MockCryptoClientStreamFactory crypto_client_stream_factory_;

net/quic/bidirectional_stream_quic_impl_unittest.cc

@@ -414,7 +414,7 @@ class BidirectionalStreamQuicImplTest
         crypto_config_(quic::test::crypto_test_utils::ProofVerifierForTesting(),
                        quic::TlsClientHandshaker::CreateSslCtx()),
         read_buffer_(base::MakeRefCounted<IOBufferWithSize>(4096)),
-        connection_id_(quic::QuicConnectionIdFromUInt64(2)),
+        connection_id_(quic::test::TestConnectionId(2)),
         stream_id_(GetNthClientInitiatedBidirectionalStreamId(0)),
         client_maker_(version_,
                       connection_id_,

net/quic/quic_chromium_client_session.cc

@@ -117,9 +117,7 @@ std::unique_ptr<base::Value> NetLogQuicConnectionMigrationFailureCallback(
     std::string reason,
     NetLogCaptureMode capture_mode) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
-  dict->SetString(
-      "connection_id",
-      base::NumberToString(quic::QuicConnectionIdToUInt64(connection_id)));
+  dict->SetString("connection_id", connection_id.ToString());
   dict->SetString("reason", reason);
   return std::move(dict);
 }
@@ -128,9 +126,7 @@ std::unique_ptr<base::Value> NetLogQuicConnectionMigrationSuccessCallback(
     quic::QuicConnectionId connection_id,
     NetLogCaptureMode capture_mode) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
-  dict->SetString(
-      "connection_id",
-      base::NumberToString(quic::QuicConnectionIdToUInt64(connection_id)));
+  dict->SetString("connection_id", connection_id.ToString());
   return std::move(dict);
 }
 
@@ -2707,9 +2703,7 @@ std::unique_ptr<base::Value> QuicChromiumClientSession::GetInfoAsValue(
 
   dict->SetInteger("total_streams", num_total_streams_);
   dict->SetString("peer_address", peer_address().ToString());
-  dict->SetKey(
-      "connection_id",
-      NetLogNumberValue(quic::QuicConnectionIdToUInt64(connection_id())));
+  dict->SetString("connection_id", connection_id().ToString());
   dict->SetBoolean("connected", connection()->connected());
   const quic::QuicConnectionStats& stats = connection()->GetStats();
   dict->SetInteger("packets_sent", stats.packets_sent);

net/quic/quic_chromium_client_session_test.cc

@@ -44,6 +44,7 @@
 #include "net/third_party/quic/core/http/quic_client_promised_info.h"
 #include "net/third_party/quic/core/quic_connection_id.h"
 #include "net/third_party/quic/core/quic_packet_writer.h"
+#include "net/third_party/quic/core/quic_utils.h"
 #include "net/third_party/quic/core/tls_client_handshaker.h"
 #include "net/third_party/quic/platform/api/quic_flags.h"
 #include "net/third_party/quic/platform/api/quic_test.h"
@@ -113,13 +114,13 @@ class QuicChromiumClientSessionTest
                      SocketTag()),
         destination_(kServerHostname, kServerPort),
         client_maker_(version_,
-                      quic::EmptyQuicConnectionId(),
+                      quic::QuicUtils::CreateRandomConnectionId(&random_),
                       &clock_,
                       kServerHostname,
                       quic::Perspective::IS_CLIENT,
                       client_headers_include_h2_stream_dependency_),
         server_maker_(version_,
-                      quic::EmptyQuicConnectionId(),
+                      quic::QuicUtils::CreateRandomConnectionId(&random_),
                       &clock_,
                       kServerHostname,
                       quic::Perspective::IS_SERVER,
@@ -147,7 +148,7 @@ class QuicChromiumClientSessionTest
     QuicChromiumPacketWriter* writer = new net::QuicChromiumPacketWriter(
         socket.get(), base::ThreadTaskRunnerHandle::Get().get());
     quic::QuicConnection* connection = new quic::QuicConnection(
-        quic::EmptyQuicConnectionId(),
+        quic::QuicUtils::CreateRandomConnectionId(&random_),
         quic::QuicSocketAddress(quic::QuicSocketAddressImpl(kIpEndPoint)),
         &helper_, &alarm_factory_, writer, true, quic::Perspective::IS_CLIENT,
         quic::test::SupportedVersions(
@@ -730,6 +731,9 @@ TEST_P(QuicChromiumClientSessionTest, ConnectionCloseBeforeStreamRequest) {
 }
 
 TEST_P(QuicChromiumClientSessionTest, ConnectionCloseBeforeHandshakeConfirmed) {
+  // Force the connection close packet to use long headers with connection ID.
+  server_maker_.SetEncryptionLevel(quic::ENCRYPTION_INITIAL);
+
   MockQuicData quic_data;
   quic_data.AddRead(ASYNC, ERR_IO_PENDING);
   quic_data.AddRead(

net/quic/quic_connection_logger.cc

@@ -99,9 +99,8 @@ std::unique_ptr<base::Value> NetLogQuicPacketHeaderCallback(
     const quic::QuicPacketHeader* header,
     NetLogCaptureMode /* capture_mode */) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
-  dict->SetKey("connection_id",
-               NetLogNumberValue(quic::QuicConnectionIdToUInt64(
-                   header->destination_connection_id)));
+  dict->SetString("connection_id",
+                  header->destination_connection_id.ToString());
   dict->SetInteger("reset_flag", header->reset_flag);
   dict->SetInteger("version_flag", header->version_flag);
   dict->SetKey("packet_number", NetLogNumberValue(header->packet_number));

net/quic/quic_flags_list.h

@@ -301,19 +301,19 @@ QUIC_FLAG(
 // byte order.
 QUIC_FLAG(bool,
           FLAGS_quic_restart_flag_quic_connection_ids_network_byte_order,
-          false)
+          true)
 
 // Allows use of new QUIC connection ID constructor designed for
 // variable-length connection IDs when acting as client.
 QUIC_FLAG(bool,
           FLAGS_quic_restart_flag_quic_variable_length_connection_ids_client,
-          false)
+          true)
 
 // Allows use of new QUIC connection ID constructor designed for
 // variable-length connection IDs when acting as server
 QUIC_FLAG(bool,
           FLAGS_quic_restart_flag_quic_variable_length_connection_ids_server,
-          false)
+          true)
 // If true, QuicPacketCreator::SetTransmissionType will set the transmission
 // type of the next successfully added frame.
 QUIC_FLAG(bool,

net/quic/quic_http_stream_test.cc

@@ -197,7 +197,7 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<
         read_buffer_(base::MakeRefCounted<IOBufferWithSize>(4096)),
         promise_id_(GetNthServerInitiatedUnidirectionalStreamId(0)),
         stream_id_(GetNthClientInitiatedBidirectionalStreamId(0)),
-        connection_id_(quic::QuicConnectionIdFromUInt64(2)),
+        connection_id_(quic::test::TestConnectionId(2)),
         client_maker_(version_,
                       connection_id_,
                       &clock_,