Issue metadata
Sign in to add a comment
|
Refactor how TCP fast open detection code is configured |
||||||||||||||||||||||||
Issue descriptionThe current code isn't threadsafe (Sets a bool asynchronously on the UI thread, reads it on the IO thread - which works, but can theoretically set off thread sanitizer), and enabling it via a global is just ugly. I propose we instead have a bool on the HttpNetworkSession if it's enabled, and use a LazyInstance/Singleton to track whether it's supported (We need an atomic to track the results of the probe, and can have the constructor of the LazyInstance trigger the probe - until the probe's run, we consider it not supported). This will make for a better API for the network service. We'll be setting up the service both in and out of process, and having multiple things fight over the global determining whether it's user-enabled or not just seems ugly.
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/53c597641f548317315a355a229db3e51441f90e commit 53c597641f548317315a355a229db3e51441f90e Author: Matt Menke <mmenke@chromium.org> Date: Thu Sep 14 16:38:12 2017 Rework TCP FastOpen configuration, and connect it to the network service TCP FastOpen used to be enabled by triggering a global probe and providing a global setting, which were then used to determine if requests should try to use TCP FastOpen, and if they actually could use it (One bool for each). This CL refactors the enabled boolean to be a property of HttpNetworkSession. The probe is still done globally, but it's now triggered lazily at the socket layer, rather than having to be explicitly run by the embedder. Since it's a non-blocking probe, that means that until the probe is complete, requests won't use FastOpen, even if enabled. Enabling the bool on HttpNetworkSession is now hooked up through the NetworkSessionConfigurator, which means that TCP FastOpen is will now work when the network service is enabled as well. Bug: 750417 Change-Id: Iec20e24b75ef166906d97660edd9f7b2d7d28a17 Reviewed-on: https://chromium-review.googlesource.com/663439 Commit-Queue: Matt Menke <mmenke@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Jana Iyengar <jri@chromium.org> Reviewed-by: Ryan Hamilton <rch@chromium.org> Cr-Commit-Position: refs/heads/master@{#501965} [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/chrome/browser/io_thread.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/components/network_session_configurator/browser/network_session_configurator.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/components/network_session_configurator/browser/network_session_configurator_unittest.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/components/network_session_configurator/common/network_switch_list.h [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/content/public/common/content_switches.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/content/public/common/content_switches.h [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/http/http_network_session.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/http/http_network_session.h [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/socket/client_socket_pool_manager.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/socket/tcp_socket.h [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/socket/tcp_socket_posix.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/socket/tcp_socket_win.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/socket/transport_client_socket_pool.cc [modify] https://crrev.com/53c597641f548317315a355a229db3e51441f90e/net/socket/transport_client_socket_pool.h
,
Sep 14 2017
,
Nov 7 2017
Apologies, applied the wrong component in bulk. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by mmenke@chromium.org
, Jul 31 2017