Remove about:flags#enable-spdy4 flag.

Remove about:flags#enable-spdy4 flag.

BUG=486780
Committed: https://crrev.com/c21dedb00ba88b0679d03467e510062bc2d82282
Cr-Commit-Position: refs/heads/master@{#329254}

[Bence, 2015-05-11 16:35:08]

bnc@chromium.org changed reviewers:
+ rch@chromium.org

[Bence, 2015-05-11 16:35:08]

Ryan: PTAL.  Thanks.

[Ryan Hamilton, 2015-05-11 18:05:20]

https://codereview.chromium.org/1131513003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1131513003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:817: switches::kEnableNpnHttpOnly)
Why does kEnableNpnHttpOnly represent "disabled"? Do we have no other disable
spdy flag?

Does disabling SPDY/4 aka HTTP/2 imply disabling SPDY/3.1?

I think I'm a bit unclear on the semantics of this flag...

[Bence, 2015-05-11 19:23:17]

Ryan, WDYT?

https://codereview.chromium.org/1131513003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1131513003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:817: switches::kEnableNpnHttpOnly)
On 2015/05/11 18:05:20, Ryan Hamilton wrote:
> Why does kEnableNpnHttpOnly represent "disabled"? Do we have no other disable
> spdy flag?
> 
> Does disabling SPDY/4 aka HTTP/2 imply disabling SPDY/3.1?
> 
> I think I'm a bit unclear on the semantics of this flag...

Good point.  Now this flag does whatever we want it to do.  Ideally one might
want two flags, one to disable HTTP/2 only, the other to disable SPDY/3.1 only,
in such a fashion that together they disable both.  However, we do not have
corresponding command line flags, and are not planning to have those.

Do you want me to set this flag to disable HTTP/2 only?  In that case, "disable"
would enable SPDY/3.1, even if the browser is in the no-SPDY control group.  Is
that not an issue?  Or shall a separate command line flag be created that only
removes HTTP/2 from the given options without messing with SPDY/3.1?  Or shall I
just remove this flag altogether for simplicity?

[Ryan Hamilton, 2015-05-11 19:40:48]

On 2015/05/11 19:23:17, Bence wrote:
> Ryan, WDYT?
> 
>
https://codereview.chromium.org/1131513003/diff/1/chrome/browser/about_flags.cc
> File chrome/browser/about_flags.cc (right):
> 
>
https://codereview.chromium.org/1131513003/diff/1/chrome/browser/about_flags....
> chrome/browser/about_flags.cc:817: switches::kEnableNpnHttpOnly)
> On 2015/05/11 18:05:20, Ryan Hamilton wrote:
> > Why does kEnableNpnHttpOnly represent "disabled"? Do we have no other
disable
> > spdy flag?
> > 
> > Does disabling SPDY/4 aka HTTP/2 imply disabling SPDY/3.1?
> > 
> > I think I'm a bit unclear on the semantics of this flag...
> 
> Good point.  Now this flag does whatever we want it to do.  Ideally one might
> want two flags, one to disable HTTP/2 only, the other to disable SPDY/3.1
only,
> in such a fashion that together they disable both.  However, we do not have
> corresponding command line flags, and are not planning to have those.
> 
> Do you want me to set this flag to disable HTTP/2 only?  In that case,
"disable"
> would enable SPDY/3.1, even if the browser is in the no-SPDY control group. 
Is
> that not an issue?  Or shall a separate command line flag be created that only
> removes HTTP/2 from the given options without messing with SPDY/3.1?  Or shall
I
> just remove this flag altogether for simplicity?

Yeah, I'm inclined to remove all flags that we don't have a compelling reason to
keep. HTTP/2 is already enabled by default right? Let's just nuke this flag.

[Bence, 2015-05-11 20:50:28]

PTAL.

> Yeah, I'm inclined to remove all flags that we don't have a compelling reason
to
> keep. HTTP/2 is already enabled by default right? Let's just nuke this flag.

Sounds like a plat to me.

[Ryan Hamilton, 2015-05-11 20:51:21]

On 2015/05/11 20:50:28, Bence wrote:
> PTAL.
> 
> > Yeah, I'm inclined to remove all flags that we don't have a compelling
reason
> to
> > keep. HTTP/2 is already enabled by default right? Let's just nuke this flag.
> 
> Sounds like a plat to me.

LGTM!

[Bence, 2015-05-11 20:57:03]

bnc@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Bence, 2015-05-11 20:57:04]

Alexei: PTAL at histograms.xml.  Thank you.

[Alexei Svitkine (slow), 2015-05-11 21:01:59]

https://codereview.chromium.org/1131513003/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/1131513003/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:56346: -  <int value="401983950"
label="enable-spdy4"/>
Don't remove this, as it's still needed to decode data from old versions.

[Bence, 2015-05-11 21:17:42]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-05-11 21:17:43]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1131513003/#ps80001
(title: "Revert "Remove flag from LoginCustomFlags.".")

[Bence, 2015-05-11 21:18:01]

Ryan, Alexei: thank you both for reviewing.

[Alexei Svitkine (slow), 2015-05-11 21:19:22]

lgtm

[commit-bot: I haz the power, 2015-05-11 21:20:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1131513003/80001

[commit-bot: I haz the power, 2015-05-11 22:28:22]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-05-11 22:29:16]

Patchset 5 (id:??) landed as
https://crrev.com/c21dedb00ba88b0679d03467e510062bc2d82282
Cr-Commit-Position: refs/heads/master@{#329254}