Populate granular quic connection error code when a connection is closed during connection migration. |
|||
Issue descriptionWhen https://chromium-review.googlesource.com/c/chromium/src/+/1016009 is landed, we expect granular quic error code will be populated for reporting when net error is ERR_NETWORK_CHANGED. However, it looks like the error code is not correctly generated, as data collected by cronet apps indicates this field is always "QUIC_NO_ERROR".
,
Jan 17
(6 days ago)
,
Jan 17
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59aaf078eaa908431e93a31c871f8d93e7821218 commit 59aaf078eaa908431e93a31c871f8d93e7821218 Author: Zhongyi Shi <zhongyi@chromium.org> Date: Thu Jan 17 03:32:13 2019 Close all handles after connection close when session decides to close connection on error. This change also conditionally override quic connection error only when the QuicChromiumClientStream contains a valid connection error code. Bug: 922739 Change-Id: Ib83d65cd5c03f89842a344fa599d3f874107c5b7 Reviewed-on: https://chromium-review.googlesource.com/c/1415304 Commit-Queue: Ryan Hamilton <rch@chromium.org> Reviewed-by: Ryan Hamilton <rch@chromium.org> Cr-Commit-Position: refs/heads/master@{#623555} [modify] https://crrev.com/59aaf078eaa908431e93a31c871f8d93e7821218/net/quic/quic_chromium_client_session.cc [modify] https://crrev.com/59aaf078eaa908431e93a31c871f8d93e7821218/net/quic/quic_http_stream.cc [modify] https://crrev.com/59aaf078eaa908431e93a31c871f8d93e7821218/net/quic/quic_stream_factory_test.cc
,
Jan 17
(5 days ago)
|
|||
►
Sign in to add a comment |
|||
Comment 1 by zhongyi@chromium.org
, Jan 16 (6 days ago)The root cause is described as follows: When a QuicConnection is closed, the granular quic connection error code leading to the connection close will be populated. And when QuicSession::OnConnectionClosed is invoked, the error code will be passed to QuicSession. QuicSession will pass that down to QuicChromiumClientSession::Handle when QuicChromiumClientSession::CloseAllHandles. And when the upper layer (URLRequest) decides to fetch the error codes, it will eventually fetch the handle's cached error code. This works most of the time, however, if a connection is closed by session via QuicChromiumClientSession::CloseSessionOnError, the code path is a little bit different. Unfortunately we closed connection after closing all the handles. By the time handles are closed, the connection is still alive and there's no connection error. void QuicChromiumClientSession::CloseSessionOnError( int net_error, quic::QuicErrorCode quic_error, quic::ConnectionCloseBehavior behavior) { base::UmaHistogramSparse("Net.QuicSession.CloseSessionOnError", -net_error); if (!callback_.is_null()) { base::ResetAndReturn(&callback_).Run(net_error); } CloseAllStreams(net_error); CloseAllHandles(net_error); <-------------------------- disconnect session with handle. net_log_.AddEvent(NetLogEventType::QUIC_SESSION_CLOSE_ON_ERROR, NetLog::IntCallback("net_error", net_error)); if (connection()->connected()) connection()->CloseConnection(quic_error, "net error", behavior); <--------------------- generate quic error code. DCHECK(!connection()->connected()); NotifyFactoryOfSessionClosed(); }