SPDY - Added enabling of SPDY/3 to about:flags.

SPDY - Added enabling of SPDY/3  and SPDY flow control to
about:flags.

Added command line switches --enable-spdy3 to enable
SPDY/3 and --enable-spdy-flow-control to enable flow control (SPDY/2.1).

Added Field Trials for SPDY/2.1 and SPDY/3. The FieldTrials are enabled
when SPDY/3 and flow control command line options are not chosen.

Removed --use-spdy=v3 and --use-spdy=flow-control options.


BUG=118440, 119205
R=willchan
TEST=Netowrk unit tests and browser tests.

Enable SPDY/3 in about:flags and see in chrome://net-internals
that SPDY/2, SPDY/2.1 and SPDY/3 are displayed as the protocols
supported.

Enable SPDY FLow Control in about:flags and see in chrome://net-internals
that SPDY/2 and SPDY/2.1 are displayed as the protocols supported.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128420

[ramant (doing other things), 2012-03-21 06:23:36]

Hi willchan,
  Would appreciate if you could look at this CL for enabling SPDY/3 via
about:flags. Will submit a separate CL for SPDY/3 field trial.

thanks much,
raman

[willchan no longer on Chromium, 2012-03-21 16:15:32]

http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
chrome/browser/chrome_browser_main.cc:270: if
(parsed_command_line.HasSwitch(switches::kEnableSPDY3)) {
How does this command line flag interact with the --use-spdy flags? For example,
how does it interact with flow control?

http://codereview.chromium.org/9766021/diff/1/chrome/common/chrome_switches.h
File chrome/common/chrome_switches.h (right):

http://codereview.chromium.org/9766021/diff/1/chrome/common/chrome_switches.h...
chrome/common/chrome_switches.h:315: extern const char kEnableSPDY3[];
Let's order this alphabetically, even though others aren't necessarily
alphabetical.

[ramant (doing other things), 2012-03-21 18:46:35]

Hi willchan,
  Ordered enable-spdy3 alphabetically.

thanks for your comments,
raman

http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
chrome/browser/chrome_browser_main.cc:270: if
(parsed_command_line.HasSwitch(switches::kEnableSPDY3)) {
On 2012/03/21 16:15:32, willchan wrote:
> How does this command line flag interact with the --use-spdy flags? For
example,
> how does it interact with flow control?

If --use-sdy=off, then enable-spdy3 is ignored.

Currently HttpStreamFactory::SetNextProtos is accumulating all next_protos. If
flow control is enabled and enable-spdy3 is enabled, then chrome supports
http/1.1, spdy/2, spdy/2.1 and spdy/3.

Would appreciate your opinion on if we should do something different.

thanks much.

http://codereview.chromium.org/9766021/diff/1/chrome/common/chrome_switches.h
File chrome/common/chrome_switches.h (right):

http://codereview.chromium.org/9766021/diff/1/chrome/common/chrome_switches.h...
chrome/common/chrome_switches.h:315: extern const char kEnableSPDY3[];
On 2012/03/21 16:15:32, willchan wrote:
> Let's order this alphabetically, even though others aren't necessarily
> alphabetical.

Done.

[willchan no longer on Chromium, 2012-03-21 23:01:55]

http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
chrome/browser/chrome_browser_main.cc:270: if
(parsed_command_line.HasSwitch(switches::kEnableSPDY3)) {
On 2012/03/21 18:46:36, ramant wrote:
> On 2012/03/21 16:15:32, willchan wrote:
> > How does this command line flag interact with the --use-spdy flags? For
> example,
> > how does it interact with flow control?
> 
> If --use-sdy=off, then enable-spdy3 is ignored.
> 
> Currently HttpStreamFactory::SetNextProtos is accumulating all next_protos. If
> flow control is enabled and enable-spdy3 is enabled, then chrome supports
> http/1.1, spdy/2, spdy/2.1 and spdy/3.
> 
> Would appreciate your opinion on if we should do something different.
> 
> thanks much.

What about --use-spdy=v3? or --use-spdy=flow-control?

[ramant (doing other things), 2012-03-21 23:56:16]

On 2012/03/21 23:01:55, willchan wrote:
>
http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
> File chrome/browser/chrome_browser_main.cc (right):
> 
>
http://codereview.chromium.org/9766021/diff/1/chrome/browser/chrome_browser_m...
> chrome/browser/chrome_browser_main.cc:270: if
> (parsed_command_line.HasSwitch(switches::kEnableSPDY3)) {
> On 2012/03/21 18:46:36, ramant wrote:
> > On 2012/03/21 16:15:32, willchan wrote:
> > > How does this command line flag interact with the --use-spdy flags? For
> > example,
> > > how does it interact with flow control?
> > 
> > If --use-sdy=off, then enable-spdy3 is ignored.
> > 
> > Currently HttpStreamFactory::SetNextProtos is accumulating all next_protos.
If
> > flow control is enabled and enable-spdy3 is enabled, then chrome supports
> > http/1.1, spdy/2, spdy/2.1 and spdy/3.
> > 
> > Would appreciate your opinion on if we should do something different.
> > 
> > thanks much.
> 
> What about --use-spdy=v3? or --use-spdy=flow-control?


enable-spdy3    use-spdy=v3    SDPY/3
Y               Y              Y
N               Y              Y
Y               N              Y
N               N              N

enable-spdy3   use-spdy=flow-control   SDPY/3   SPDY/2.1(flow-control)
Y              Y                       Y        Y
N              Y                       N        Y
Y              N                       Y        Y
N              N                       N        N

Should we retire --use-spdy=v3 and --use-spdy=flow-control and only allow
-enable-spdy3 or enable-flow-control command line options?

thanks much,
raman

[ramant (doing other things), 2012-03-22 05:41:34]

Please ignore my comment#5 (that comment was incorrect). --use-spdy=flow-control
was overwriting the about:flags's enable-spdy3 flag.

Deleted --use-spdy=v3 and --use-spdy=flow-control switches and added 
--enable-spdy3 and --enable-spdy-flow-control command line switches and
about:flags.

[ramant (doing other things), 2012-03-22 07:25:57]

Hi willchan,
  It was confusing for me to have two ways to enable spdy/2.1 or spdy/3. Kept
the command line options (and about:flags) and deleted the use-spdy=v3,
use-spdy=flow-control options. Would appreciate your opinion.

thanks,
raman

[Ryan Hamilton, 2012-03-22 16:30:21]

https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chro...
File chrome/browser/chrome_browser_main.cc (right):

https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chro...
chrome/browser/chrome_browser_main.cc:844: 2012, 10, 30));
October seems like a long time from now, so this is probably fine.  That being
said, I would have thought we would wait to add this code until we we
comfortable that spdy/3 was working as expected.

[ramant (doing other things), 2012-03-22 16:38:24]

Set the SPDY/3 probability to 0.

Thanks much Ryan and willchan for all your comments.
raman

https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chro...
File chrome/browser/chrome_browser_main.cc (right):

https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chro...
chrome/browser/chrome_browser_main.cc:844: 2012, 10, 30));
On 2012/03/22 16:30:21, Ryan Hamilton wrote:
> October seems like a long time from now, so this is probably fine.  That being
> said, I would have thought we would wait to add this code until we we
> comfortable that spdy/3 was working as expected.

Just to be on safe side, set the probability to 0. In my tests, didn't access
any server which advertised spdy/3. Will change this after getting lot more
testing with Hasan with a spdy/3 server.

Done.

[ramant (doing other things), 2012-03-22 22:34:01]

On 2012/03/22 16:38:24, ramant wrote:
> Set the SPDY/3 probability to 0.
> 
> Thanks much Ryan and willchan for all your comments.
> raman
> 
>
https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chro...
> File chrome/browser/chrome_browser_main.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chro...
> chrome/browser/chrome_browser_main.cc:844: 2012, 10, 30));
> On 2012/03/22 16:30:21, Ryan Hamilton wrote:
> > October seems like a long time from now, so this is probably fine.  That
being
> > said, I would have thought we would wait to add this code until we we
> > comfortable that spdy/3 was working as expected.
> 
> Just to be on safe side, set the probability to 0. In my tests, didn't access
> any server which advertised spdy/3. Will change this after getting lot more
> testing with Hasan with a spdy/3 server.
> 
> Done.

hasan said "we're ready for spdy 3"

[hkhalil, 2012-03-22 22:36:15]

I think that we're in a good place to start playing with it at least.

    -Hasan


On Thu, Mar 22, 2012 at 6:34 PM, <rtenneti@chromium.org> wrote:

> On 2012/03/22 16:38:24, ramant wrote:
>
>> Set the SPDY/3 probability to 0.
>>
>
>  Thanks much Ryan and willchan for all your comments.
>> raman
>>
>
>
> https://chromiumcodereview.**appspot.com/9766021/diff/**
>
18007/chrome/browser/chrome_**browser_main.cc<https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chrome_browser_main.cc>
>
>> File chrome/browser/chrome_browser_**main.cc (right):
>>
>
>
> https://chromiumcodereview.**appspot.com/9766021/diff/**
>
18007/chrome/browser/chrome_**browser_main.cc#newcode844<https://chromiumcodereview.appspot.com/9766021/diff/18007/chrome/browser/chrome_browser_main.cc#newcode844>
>
>> chrome/browser/chrome_browser_**main.cc:844: 2012, 10, 30));
>> On 2012/03/22 16:30:21, Ryan Hamilton wrote:
>> > October seems like a long time from now, so this is probably fine.  That
>>
> being
>
>> > said, I would have thought we would wait to add this code until we we
>> > comfortable that spdy/3 was working as expected.
>>
>
>  Just to be on safe side, set the probability to 0. In my tests, didn't
>> access
>> any server which advertised spdy/3. Will change this after getting lot
>> more
>> testing with Hasan with a spdy/3 server.
>>
>
>  Done.
>>
>
> hasan said "we're ready for spdy 3"
>
>
>
https://chromiumcodereview.**appspot.com/9766021/<https://chromiumcodereview....
>

[willchan no longer on Chromium, 2012-03-23 00:41:46]

LGTM, mod some leftovers you had.

https://chromiumcodereview.appspot.com/9766021/diff/20017/net/http/http_netwo...
File net/http/http_network_layer.cc (right):

https://chromiumcodereview.appspot.com/9766021/diff/20017/net/http/http_netwo...
net/http/http_network_layer.cc:95: std::vector<std::string> next_protos;
Wait, what about these guys?

[ramant (doing other things), 2012-03-23 03:06:50]

On 2012/03/23 00:41:46, willchan wrote:
> LGTM, mod some leftovers you had.
> 
>
https://chromiumcodereview.appspot.com/9766021/diff/20017/net/http/http_netwo...
> File net/http/http_network_layer.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9766021/diff/20017/net/http/http_netwo...
> net/http/http_network_layer.cc:95: std::vector<std::string> next_protos;
> Wait, what about these guys?

I will submit a separate CL to move  kEnableNPN options to chrome_switches.

Thanks much willchan.

[commit-bot: I haz the power, 2012-03-23 03:08:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/9766021/13012

[commit-bot: I haz the power, 2012-03-23 05:57:56]

Change committed as 128420