Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(580)

Issue 208663003: Revert of Fix SPDY error-handling if the connection gets closed just after use. (Closed)

Created:
6 years, 9 months ago by vandebo (ex-Chrome)
Modified:
6 years, 9 months ago
Reviewers:
Ryan Hamilton, davidben
CC:
chromium-reviews, cbentzel+watch_chromium.org, mmenke, Johnny
Visibility:
Public.

Description

Revert of Fix SPDY error-handling if the connection gets closed just after use. (https://codereview.chromium.org/200723004/) Reason for revert: Linux LSan isn't happy: AddressSanitizer: stack-buffer-overflow http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Tests%20%281%29/builds/650/steps/net_unittests/logs/GetUploadProgressBeforeInitialization_0 Original issue's description: > Fix SPDY error-handling if the connection gets closed just after use. > > - Make SpdySession::IsReused() return true if the underlying socket was > UNUSED_IDLE. This makes the HttpNetworkTransaction-level retry try a fresh > socket in case a preconnected socket was stale. > > - If the SpdySession closes in an event loop iteration between > HttpStreamFactoryImplJob::DoCreateStream and OnNewSpdySessionReadyCallback, > propogate the error to the request to prevent it from hanging. Do so by > creating the originating request's SpdyHttpStream as soon as the SpdySession > is created so it can sample SpdySession::IsReused() and advise > HttpNetworkTransaction on error. > > - Delay pumping the SpdySession read loop by an event loop iteration. This > simplifies some logic and ensures that HttpNetworkTransaction receives a > SpdyHttpStream to advise retry logic. This does mean we lose the error code; > it now follows the asynchronous case and turns into ERR_CONNECTION_CLOSED in > SpdyHttpStream::InitializeStream. > > BUG=352156 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258647 TBR=rch@chromium.org,davidben@chromium.org NOTREECHECKS=true NOTRY=true BUG=352156 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258657

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+148 lines, -217 lines) Patch
M net/http/http_network_transaction_unittest.cc View 6 chunks +31 lines, -95 lines 0 comments Download
M net/http/http_proxy_client_socket_pool.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M net/http/http_stream_factory_impl_job.cc View 2 chunks +11 lines, -20 lines 0 comments Download
M net/http/http_stream_factory_impl_request.h View 2 chunks +0 lines, -2 lines 0 comments Download
M net/http/http_stream_factory_impl_request.cc View 2 chunks +13 lines, -17 lines 0 comments Download
M net/spdy/spdy_session.h View 3 chunks +10 lines, -10 lines 0 comments Download
M net/spdy/spdy_session.cc View 5 chunks +22 lines, -18 lines 0 comments Download
M net/spdy/spdy_session_pool.h View 1 chunk +5 lines, -4 lines 0 comments Download
M net/spdy/spdy_session_pool.cc View 3 chunks +15 lines, -8 lines 0 comments Download
M net/spdy/spdy_session_unittest.cc View 4 chunks +8 lines, -11 lines 0 comments Download
M net/spdy/spdy_test_util_common.h View 2 chunks +6 lines, -9 lines 0 comments Download
M net/spdy/spdy_test_util_common.cc View 4 chunks +22 lines, -18 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
vandebo (ex-Chrome)
Created Revert of Fix SPDY error-handling if the connection gets closed just after use.
6 years, 9 months ago (2014-03-21 20:15:46 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vandebo@chromium.org/208663003/1
6 years, 9 months ago (2014-03-21 20:16:00 UTC) #2
commit-bot: I haz the power
6 years, 9 months ago (2014-03-21 20:17:09 UTC) #3
Message was sent while issue was closed.
Change committed as 258657

Powered by Google App Engine
This is Rietveld 408576698