Issue 97433002: Change the defaults QuicConfig::server_initial_congestion_window to be

Issue 97433002: Change the defaults QuicConfig::server_initial_congestion_window to be (Closed)

Created:
7 years ago by ramant (doing other things)

Modified:
7 years ago

Reviewers:
Ryan Hamilton

CC:
chromium-reviews, cbentzel+watch_chromium.org

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

Visibility:
Public.

More Reviews

Description

Change the defaults QuicConfig::server_initial_congestion_window to be kDefaultInitialWindow, kDefaultInitialWindow instead of kMaxInitialWindow, kDefaultInitialWindow. Move the logic for allowing a larger max from SetDefaults to the quic_server and internal QUIC server. Remove the logic for setting "safe" defaults from the client. Merge internal change: 57382959 R=rch@chromium.org

Patch Set 1 #

Total comments: 2

Created: 7 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+5 lines, -5 lines)			Patch
M	net/quic/quic_config.cc	View	1 chunk	+1 line, -1 line	0 comments	Download
M	net/tools/quic/end_to_end_test.cc	View	1 chunk	+2 lines, -0 lines	0 comments	Download
M	net/tools/quic/quic_client.cc	View	1 chunk	+0 lines, -4 lines	2 comments	Download
M	net/tools/quic/quic_server.cc	View	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Ryan Hamilton

lgtm https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc File net/tools/quic/quic_client.cc (left): https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc#oldcode51 net/tools/quic/quic_client.cc:51: kDefaultInitialWindow); There is a line similar to this ...

7 years ago (2013-12-01 18:25:38 UTC) #2

ramant (doing other things)

https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc File net/tools/quic/quic_client.cc (left): https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc#oldcode51 net/tools/quic/quic_client.cc:51: kDefaultInitialWindow); On 2013/12/01 18:25:39, Ryan Hamilton wrote: > There ...

7 years ago (2013-12-02 05:10:50 UTC) #3

Ryan Hamilton

On 2013/12/02 05:10:50, ramant wrote: > https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc > File net/tools/quic/quic_client.cc (left): > > https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc#oldcode51 > ...

7 years ago (2013-12-02 05:15:28 UTC) #4

Ryan Hamilton

7 years ago (2013-12-02 05:15:29 UTC) #5

On 2013/12/02 05:10:50, ramant wrote:
> https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc
> File net/tools/quic/quic_client.cc (left):
> 
>
https://codereview.chromium.org/97433002/diff/1/net/tools/quic/quic_client.cc...
> net/tools/quic/quic_client.cc:51: kDefaultInitialWindow);
> On 2013/12/01 18:25:39, Ryan Hamilton wrote:
> > There is a line similar to this in quic_stream_factory.cc.  Can you remove
it
> > too?
> 
> Didn't see a similar line in quic_stream_factory.cc (might have been deleted
> already). Would like to show you tomorrow. thanks.

Hm.  It was added in this CL:

https://codereview.chromium.org/91623003/diff/1/net/quic/quic_stream_factory.cc

Expand Messages | Collapse Messages