Enable kAsyncQuicSession on non-mobile platforms. [chromium/src : main]

Liza Burakova (Gerrit)

unread,

May 25, 2023, 11:43:14 AM5/25/23

to David Benjamin, bnc+...@chromium.org, net-r...@chromium.org, Will Harris

Attention is currently required from: David Benjamin.

Liza Burakova would like David Benjamin to review this change.

Enable kAsyncQuicSession on non-mobile platforms.

This change enables kAsyncQuicSession by default on platforms that
cronet does not run on. This flag was originally set up to ensure
cronet didn't break in subtle ways from a complex change, but it
needs to be on for the Windows sandbox.

Bug: 1448725
Change-Id: I3bc23d93083ec094b006798cc539b21e05633eb5
---
M net/base/features.cc
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/base/features.cc b/net/base/features.cc
index c1f7597..a7b8192 100644
--- a/net/base/features.cc
+++ b/net/base/features.cc
@@ -316,10 +316,17 @@
              base::FEATURE_DISABLED_BY_DEFAULT);
 #endif
 
-// A flag to use asynchronous session creation for new QUIC sessions.
+// A flag to use asynchronous session creation for new QUIC sessions. It is
+// disabled by default on iOS and Android builds to ensure it doesn't ship on
+// cronet.
+#if BUILDFLAG(IS_ANDROID) || BUILD(IS_IOS)
 BASE_FEATURE(kAsyncQuicSession,
              "AsyncQuicSession",
              base::FEATURE_DISABLED_BY_DEFAULT);
+#endif
+BASE_FEATURE(kAsyncQuicSession,
+             "AsyncQuicSession",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 
 // IP protection experiment configuration settings
 BASE_FEATURE(kEnableIpProtectionProxy,

To view, visit change 4567705. To unsubscribe, or for help writing mail filters, visit settings.

Liza Burakova (Gerrit)

unread,

May 25, 2023, 11:43:19 AM5/25/23

to bnc+...@chromium.org, net-r...@chromium.org, David Benjamin, Will Harris, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: David Benjamin.

View Change

To view, visit change 4567705. To unsubscribe, or for help writing mail filters, visit settings.

Will Harris (Gerrit)

unread,

May 25, 2023, 12:15:15 PM5/25/23

to Liza Burakova, bnc+...@chromium.org, net-r...@chromium.org, David Benjamin, Will Harris, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: David Benjamin, Liza Burakova.

View Change

1 comment:

File net/base/features.cc:
- Patch Set #2, Line 325: base::FEATURE_DISABLED_BY_DEFAULT);
  nit: I think convention here is normally to wrap just the FEATURE_DISABLED_BY_DEFAULT or FEATURE_ENABLED_BY_DEFAULT in the platform ifdefs
  e.g. https://source.chromium.org/chromium/chromium/src/+/main:cc/base/features.cc;l=33 (and many others).

To view, visit change 4567705. To unsubscribe, or for help writing mail filters, visit settings.

David Benjamin (Gerrit)

unread,

May 25, 2023, 12:30:12 PM5/25/23

to Liza Burakova, bnc+...@chromium.org, net-r...@chromium.org, Will Harris, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Liza Burakova.

View Change

2 comments:

File net/base/features.cc:
- Patch Set #2, Line 322: #if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
  FWIW, we also have a `CRONET_BUILD` define if we specifically want to exclude Cronet. (Your call. I don't have context here.)
- Patch Set #2, Line 326: #endif
  Think you need a `#else` here. Android's getting the feature defined twice right now. :-)

To view, visit change 4567705. To unsubscribe, or for help writing mail filters, visit settings.

Liza Burakova (Gerrit)

unread,

May 25, 2023, 12:34:04 PM5/25/23

to bnc+...@chromium.org, net-r...@chromium.org, David Benjamin, Will Harris, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: David Benjamin.

View Change

2 comments:

File net/base/features.cc:

- Patch Set #2, Line 322: #if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)

- FWIW, we also have a `CRONET_BUILD` define if we specifically want to exclude Cronet. (Your call. […]
  Oh CRONET_BUILD works perfectly for this, actually. Thanks I'll do that.

- Patch Set #2, Line 326: #endif

- Think you need a `#else` here. Android's getting the feature defined twice right now. […]
  Yep, I realized that, I was over eager in getting this out from a WIP state oops.

To view, visit change 4567705. To unsubscribe, or for help writing mail filters, visit settings.

Liza Burakova (Gerrit)

unread,

Oct 2, 2023, 4:31:40 PM10/2/23

to bnc+...@chromium.org, net-r...@chromium.org, David Benjamin, Will Harris, Chromium LUCI CQ, chromium...@chromium.org

Liza Burakova abandoned this change.

View Change

Abandoned

To view, visit change 4567705. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward