Issue 12545035: Refactor QuicClientSession so that it owns the underlying socket

Issue 12545035: Refactor QuicClientSession so that it owns the underlying socket (Closed)

Created:
7 years, 9 months ago by Ryan Hamilton

Modified:
7 years, 9 months ago

Reviewers:
ramant (doing other things)

CC:
chromium-reviews, cbentzel+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Refactor QuicClientSession so that it owns the underlying socket which is also stored in the QuicConnectionHelper. Make the Helper owned by the Connection, which is in turn owned by the Session. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188284

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rebase #

Patch Set 3 : Fix test leak #

Created: 7 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+26 lines, -45 lines)			Patch
M	net/quic/quic_client_session.h	View		3 chunks	+3 lines, -2 lines	0 comments	Download
M	net/quic/quic_client_session.cc	View		3 chunks	+5 lines, -5 lines	0 comments	Download
M	net/quic/quic_connection.h	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	net/quic/quic_connection_helper.h	View		2 chunks	+1 line, -6 lines	0 comments	Download
M	net/quic/quic_connection_helper.cc	View		1 chunk	+0 lines, -13 lines	0 comments	Download
M	net/quic/quic_connection_helper_test.cc	View	1 2	2 chunks	+8 lines, -7 lines	0 comments	Download
M	net/quic/quic_connection_test.cc	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	net/quic/quic_http_stream_test.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	net/quic/quic_stream_factory.cc	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	net/quic/test_tools/quic_test_utils.h	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	net/quic/test_tools/quic_test_utils.cc	View	1	1 chunk	+2 lines, -4 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

ramant (doing other things)

Change looks awesome. LGTM https://codereview.chromium.org/12545035/diff/1/net/quic/quic_connection_test.cc File net/quic/quic_connection_test.cc (right): https://codereview.chromium.org/12545035/diff/1/net/quic/quic_connection_test.cc#newcode450 net/quic/quic_connection_test.cc:450: TestConnectionHelper* helper_; // owned by ...

7 years, 9 months ago (2013-03-14 17:08:04 UTC) #2

Ryan Hamilton

On 2013/03/14 17:08:04, ramant wrote: > Change looks awesome. LGTM > > https://codereview.chromium.org/12545035/diff/1/net/quic/quic_connection_test.cc > File ...

7 years, 9 months ago (2013-03-14 22:22:32 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/12545035/5001

7 years, 9 months ago (2013-03-14 22:37:52 UTC) #4

commit-bot: I haz the power

Failed to trigger a try job on win7_aura HTTP Error 400: Bad Request

7 years, 9 months ago (2013-03-15 00:13:55 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/12545035/17001

7 years, 9 months ago (2013-03-15 00:14:20 UTC) #6

Message was sent while issue was closed.

Change committed as 188284

Expand Messages | Collapse Messages