Default turning on goaway sessions on ip address change for iOS.

Bug: 846079
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I384116a58acb044453a42dbedc1c9f7d23290640
Reviewed-on: https://chromium-review.googlesource.com/1093311Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567549}

components/cronet/url_request_context_config.cc

@@ -13,6 +13,7 @@
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "components/cronet/stale_host_resolver.h"
 #include "net/base/address_family.h"
 #include "net/cert/caching_cert_verifier.h"
@@ -124,6 +125,14 @@ const char kDisableIPv6OnWifi[] = "disable_ipv6_on_wifi";
 
 const char kSSLKeyLogFile[] = "ssl_key_log_file";
 
+// "goaway_sessions_on_ip_change" is default on for iOS unless overrided via
+// experimental options explicitly.
+#if defined(OS_IOS)
+const bool kDefaultQuicGoAwaySessionsOnIpChange = true;
+#else
+const bool kDefaultQuicGoAwaySessionsOnIpChange = false;
+#endif
+
 }  // namespace
 
 URLRequestContextConfig::QuicHint::QuicHint(const std::string& host,
@@ -283,13 +292,21 @@ void URLRequestContextConfig::ParseAndSetExperimentalOptions(
                                 &quic_close_sessions_on_ip_change)) {
         session_params->quic_close_sessions_on_ip_change =
             quic_close_sessions_on_ip_change;
+        if (quic_close_sessions_on_ip_change &&
+            kDefaultQuicGoAwaySessionsOnIpChange) {
+          // "close_sessions_on_ip_change" and "goaway_sessions_on_ip_change"
+          // are mutually exclusive. Turn off the goaway option which is
+          // default on for iOS if "close_sessions_on_ip_change" is set via
+          // experimental options.
+          session_params->quic_goaway_sessions_on_ip_change = false;
+        }
       }
 
-      bool quic_goaway_sessions_on_ip_change = false;
+      bool goaway_sessions_on_ip_change;
       if (quic_args->GetBoolean(kQuicGoAwaySessionsOnIpChange,
-                                &quic_goaway_sessions_on_ip_change)) {
+                                &goaway_sessions_on_ip_change)) {
         session_params->quic_goaway_sessions_on_ip_change =
-            quic_goaway_sessions_on_ip_change;
+            goaway_sessions_on_ip_change;
       }
 
       bool quic_allow_server_migration = false;
@@ -545,8 +562,13 @@ void URLRequestContextConfig::ConfigureURLRequestContextBuilder(
   net::HttpNetworkSession::Params session_params;
   session_params.enable_http2 = enable_spdy;
   session_params.enable_quic = enable_quic;
-  if (enable_quic)
+  if (enable_quic) {
     session_params.quic_user_agent_id = quic_user_agent_id;
+    // Note goaway sessions on ip change will be turned on by default
+    // for iOS unless overrided via experiemental options.
+    session_params.quic_goaway_sessions_on_ip_change =
+        kDefaultQuicGoAwaySessionsOnIpChange;
+  }
 
   ParseAndSetExperimentalOptions(context_builder, &session_params, net_log);
   context_builder->set_http_network_session_params(session_params);

components/cronet/url_request_context_config_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/json/json_writer.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "net/cert/cert_verifier.h"
 #include "net/http/http_network_session.h"
 #include "net/log/net_log.h"
@@ -189,7 +190,78 @@ TEST(URLRequestContextConfigTest, SetQuicServerMigrationOptions) {
   EXPECT_TRUE(params->quic_allow_server_migration);
 }
 
-TEST(URLRequestContextConfigTest, SetQuicGoAwaySessionsOnIPChange) {
+// Test that goaway_sessions_on_ip_change is set on by default for iOS.
+#if defined(OS_IOS)
+#define MAYBE_SetQuicGoAwaySessionsOnIPChangeByDefault \
+  SetQuicGoAwaySessionsOnIPChangeByDefault
+#else
+#define MAYBE_SetQuicGoAwaySessionsOnIPChangeByDefault \
+  DISABLED_SetQuicGoAwaySessionsOnIPChangeByDefault
+#endif
+TEST(URLRequestContextConfigTest,
+     MAYBE_SetQuicGoAwaySessionsOnIPChangeByDefault) {
+  base::test::ScopedTaskEnvironment scoped_task_environment_(
+      base::test::ScopedTaskEnvironment::MainThreadType::IO);
+
+  URLRequestContextConfig config(
+      // Enable QUIC.
+      true,
+      // QUIC User Agent ID.
+      "Default QUIC User Agent ID",
+      // Enable SPDY.
+      true,
+      // Enable Brotli.
+      false,
+      // Type of http cache.
+      URLRequestContextConfig::HttpCacheType::DISK,
+      // Max size of http cache in bytes.
+      1024000,
+      // Disable caching for HTTP responses. Other information may be stored in
+      // the cache.
+      false,
+      // Storage path for http cache and cookie storage.
+      "/data/data/org.chromium.net/app_cronet_test/test_storage",
+      // Accept-Language request header field.
+      "foreign-language",
+      // User-Agent request header field.
+      "fake agent",
+      // JSON encoded experimental options.
+      "{\"QUIC\":{}}",
+      // MockCertVerifier to use for testing purposes.
+      std::unique_ptr<net::CertVerifier>(),
+      // Enable network quality estimator.
+      false,
+      // Enable Public Key Pinning bypass for local trust anchors.
+      true,
+      // Certificate verifier cache data.
+      "");
+
+  net::URLRequestContextBuilder builder;
+  net::NetLog net_log;
+  config.ConfigureURLRequestContextBuilder(&builder, &net_log);
+  // Set a ProxyConfigService to avoid DCHECK failure when building.
+  builder.set_proxy_config_service(
+      std::make_unique<net::ProxyConfigServiceFixed>(
+          net::ProxyConfigWithAnnotation::CreateDirect()));
+  std::unique_ptr<net::URLRequestContext> context(builder.Build());
+  const net::HttpNetworkSession::Params* params =
+      context->GetNetworkSessionParams();
+
+  EXPECT_FALSE(params->quic_close_sessions_on_ip_change);
+  EXPECT_TRUE(params->quic_goaway_sessions_on_ip_change);
+}
+
+// Tests that goaway_sessions_on_ip_changes can be set on via
+// experimental options on non-iOS.
+#if !defined(OS_IOS)
+#define MAYBE_SetQuicGoAwaySessionsOnIPChangeViaExperimentOptions \
+  SetQuicGoAwaySessionsOnIPChangeViaExperimentOptions
+#else
+#define MAYBE_SetQuicGoAwaySessionsOnIPChangeViaExperimentOptions \
+  DISABLED_SetQuicGoAwaySessionsOnIPChangeViaExperimentOptions
+#endif
+TEST(URLRequestContextConfigTest,
+     MAYBE_SetQuicGoAwaySessionsOnIPChangeViaExperimentOptions) {
   base::test::ScopedTaskEnvironment scoped_task_environment_(
       base::test::ScopedTaskEnvironment::MainThreadType::IO);
 
@@ -241,6 +313,68 @@ TEST(URLRequestContextConfigTest, SetQuicGoAwaySessionsOnIPChange) {
   EXPECT_TRUE(params->quic_goaway_sessions_on_ip_change);
 }
 
+// Test that goaway_sessions_on_ip_change can be set to false via
+// exprimental options on iOS.
+#if defined(OS_IOS)
+#define MAYBE_DisableQuicGoAwaySessionsOnIPChangeViaExperimentOptions \
+  DisableQuicGoAwaySessionsOnIPChangeViaExperimentOptions
+#else
+#define MAYBE_DisableQuicGoAwaySessionsOnIPChangeViaExperimentOptions \
+  DISABLED_DisableQuicGoAwaySessionsOnIPChangeViaExperimentOptions
+#endif
+TEST(URLRequestContextConfigTest,
+     MAYBE_DisableQuicGoAwaySessionsOnIPChangeViaExperimentOptions) {
+  base::test::ScopedTaskEnvironment scoped_task_environment_(
+      base::test::ScopedTaskEnvironment::MainThreadType::IO);
+
+  URLRequestContextConfig config(
+      // Enable QUIC.
+      true,
+      // QUIC User Agent ID.
+      "Default QUIC User Agent ID",
+      // Enable SPDY.
+      true,
+      // Enable Brotli.
+      false,
+      // Type of http cache.
+      URLRequestContextConfig::HttpCacheType::DISK,
+      // Max size of http cache in bytes.
+      1024000,
+      // Disable caching for HTTP responses. Other information may be stored in
+      // the cache.
+      false,
+      // Storage path for http cache and cookie storage.
+      "/data/data/org.chromium.net/app_cronet_test/test_storage",
+      // Accept-Language request header field.
+      "foreign-language",
+      // User-Agent request header field.
+      "fake agent",
+      // JSON encoded experimental options.
+      "{\"QUIC\":{\"goaway_sessions_on_ip_change\":false}}",
+      // MockCertVerifier to use for testing purposes.
+      std::unique_ptr<net::CertVerifier>(),
+      // Enable network quality estimator.
+      false,
+      // Enable Public Key Pinning bypass for local trust anchors.
+      true,
+      // Certificate verifier cache data.
+      "");
+
+  net::URLRequestContextBuilder builder;
+  net::NetLog net_log;
+  config.ConfigureURLRequestContextBuilder(&builder, &net_log);
+  // Set a ProxyConfigService to avoid DCHECK failure when building.
+  builder.set_proxy_config_service(
+      std::make_unique<net::ProxyConfigServiceFixed>(
+          net::ProxyConfigWithAnnotation::CreateDirect()));
+  std::unique_ptr<net::URLRequestContext> context(builder.Build());
+  const net::HttpNetworkSession::Params* params =
+      context->GetNetworkSessionParams();
+
+  EXPECT_FALSE(params->quic_close_sessions_on_ip_change);
+  EXPECT_FALSE(params->quic_goaway_sessions_on_ip_change);
+}
+
 TEST(URLRequestContextConfigTest, SetQuicConnectionMigrationV2Options) {
   base::test::ScopedTaskEnvironment scoped_task_environment_(
       base::test::ScopedTaskEnvironment::MainThreadType::IO);