Only allow HTTP/0.9 support on default ports.

Only allow HTTP/0.9 support on default ports.

Also add a group policy to allow HTTP/0.9 support on other
ports, which I plan to remove in a couple revisions.

BUG=624462, 637017
Committed: https://crrev.com/398c105ea8391473126021104554d70fda08324b
Cr-Commit-Position: refs/heads/master@{#415643}

[mmenke, 2016-08-16 17:40:54]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 17:41:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 19:05:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-16 19:05:08]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mmenke, 2016-08-23 21:16:40]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 21:17:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 21:28:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 21:28:17]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[mmenke, 2016-08-23 21:49:13]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 21:49:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 22:05:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 22:05:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2016-08-29 17:47:43]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 17:50:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2016-08-29 19:29:42]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 19:41:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2016-08-29 20:35:03]

Patchset #6 (id:100001) has been deleted

[mmenke, 2016-08-29 20:35:12]

Patchset #5 (id:80001) has been deleted

[mmenke, 2016-08-29 20:35:24]

Patchset #4 (id:60001) has been deleted

[mmenke, 2016-08-29 20:37:51]

mmenke@chromium.org changed reviewers:
+ eroman@chromium.org

[mmenke, 2016-08-29 20:37:52]

Eroman:  PTAL.  If this makes it successfully to stable, I'm thinking I'll try
and remove HTTP/0.9 support over SSL connections, though that may run into
issues with talking to HTTP/0.9 servers over SSL proxies.

[mmenke, 2016-08-29 20:38:31]

Description was changed from

==========
Only allow HTTP/0.9 support on default ports.

BUG=624462,637017
==========

to

==========
Only allow HTTP/0.9 support on default ports.

Also add a group policy to allow HTTP/0.9 support on other
ports, which I plan to remove in a couple revisions.

BUG=624462,637017
==========

[mmenke, 2016-08-29 20:39:02]

The CQ bit was unchecked by mmenke@chromium.org

[eroman, 2016-08-29 22:11:55]

lgtm.

that is an impressive amount of plumbing needed!

https://codereview.chromium.org/2253653002/diff/140001/chrome/browser/net/err...
File chrome/browser/net/errorpage_browsertest.cc (right):

https://codereview.chromium.org/2253653002/diff/140001/chrome/browser/net/err...
chrome/browser/net/errorpage_browsertest.cc:1531: // Make sure HTTP/0.9 is works
on non-default ports when enabled by policy.
nit: drop the "is", it's cleaner.

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_network_...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_network_...
net/http/http_network_transaction_unittest.cc:759: // Don't bother to read the
body - that's verified elsewhere, importand thing
importand

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
File net/http/http_stream_parser.cc (right):

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
net/http/http_stream_parser.cc:1007: // TODO(mmenke):  Further restrict the
cases in which we allow HTTP/0.9.
Can you reference a bug? i.e. TODO(crbug.com/XXX)

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
File net/http/http_stream_parser.h (right):

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
net/http/http_stream_parser.h:55: void
set_http_09_on_non_default_ports_enabled() {
I think this would be clearer if it took a boolean parameter
(and the client of it simply calls it always, rather than conditionally).

https://codereview.chromium.org/2253653002/diff/140001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2253653002/diff/140001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:75758: +      label="Send network
packets to the management server to monitor online
Unrelated changes from running the script?

[mmenke, 2016-08-30 17:49:54]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-30 17:50:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2016-08-30 17:50:47]

Thanks, Eric!

https://codereview.chromium.org/2253653002/diff/140001/chrome/browser/net/err...
File chrome/browser/net/errorpage_browsertest.cc (right):

https://codereview.chromium.org/2253653002/diff/140001/chrome/browser/net/err...
chrome/browser/net/errorpage_browsertest.cc:1531: // Make sure HTTP/0.9 is works
on non-default ports when enabled by policy.
On 2016/08/29 22:11:54, eroman wrote:
> nit: drop the "is", it's cleaner.

Done.

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_network_...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_network_...
net/http/http_network_transaction_unittest.cc:759: // Don't bother to read the
body - that's verified elsewhere, importand thing
On 2016/08/29 22:11:55, eroman wrote:
> importand

Done.

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
File net/http/http_stream_parser.cc (right):

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
net/http/http_stream_parser.cc:1007: // TODO(mmenke):  Further restrict the
cases in which we allow HTTP/0.9.
On 2016/08/29 22:11:55, eroman wrote:
> Can you reference a bug? i.e. TODO(crbug.com/XXX)

Done

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
File net/http/http_stream_parser.h (right):

https://codereview.chromium.org/2253653002/diff/140001/net/http/http_stream_p...
net/http/http_stream_parser.h:55: void
set_http_09_on_non_default_ports_enabled() {
On 2016/08/29 22:11:55, eroman wrote:
> I think this would be clearer if it took a boolean parameter
> (and the client of it simply calls it always, rather than conditionally).

Done.  I didn't make it take a bool to emphasize its default value, but I'm
fine, either way.

https://codereview.chromium.org/2253653002/diff/140001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2253653002/diff/140001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:75758: +      label="Send network
packets to the management server to monitor online
On 2016/08/29 22:11:55, eroman wrote:
> Unrelated changes from running the script?

Yea...Someone probably manually modified the file used to generate these, and
didn't re-run the script....Or I didn't run it again after merging.  I'll run it
again, see what happens.

[mmenke, 2016-08-30 17:52:29]

mmenke@chromium.org changed reviewers:
+ holte@chromium.org, pastarmovj@chromium.org

[mmenke, 2016-08-30 17:52:30]

[+pastarmovj]:  Please review components/policy
[+holte]:  Please review histograms.xml (Just updated by running the policy
update script)

[commit-bot: I haz the power, 2016-08-30 17:52:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 17:52:55]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[mmenke, 2016-08-30 18:00:21]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-30 18:00:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 19:32:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 19:32:01]

Dry run: This issue passed the CQ dry run.

[Steven Holte, 2016-08-30 22:13:39]

histograms lgtm

[pastarmovj, 2016-08-31 12:02:12]

lgtm with two nits.

https://codereview.chromium.org/2253653002/diff/180001/chrome/test/data/polic...
File chrome/test/data/policy/policy_test_cases.json (right):

https://codereview.chromium.org/2253653002/diff/180001/chrome/test/data/polic...
chrome/test/data/policy/policy_test_cases.json:158:
"Http09OnNonDefaultPortsEnabled": {
You can as well leave the braces empty as nothing is tested here right now. If
there is a corresponding pref (I think there isn't) or some UI in the settings
that is changed when this policy is triggered it can be tested here but
otherwise leaving just 

"Http09OnNonDefaultPortsEnabled": {}

to satisfy the presubmit is good enough.

https://codereview.chromium.org/2253653002/diff/180001/components/policy/reso...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/2253653002/diff/180001/components/policy/reso...
components/policy/resources/policy_templates.json:542: 'label': '''List of
disabled protocol schemes''',
Please fix the label - this seems to have slipped from copying the one above. :)

[mmenke, 2016-08-31 14:30:29]

Thanks for catching those!

https://codereview.chromium.org/2253653002/diff/180001/chrome/test/data/polic...
File chrome/test/data/policy/policy_test_cases.json (right):

https://codereview.chromium.org/2253653002/diff/180001/chrome/test/data/polic...
chrome/test/data/policy/policy_test_cases.json:158:
"Http09OnNonDefaultPortsEnabled": {
On 2016/08/31 12:02:11, pastarmovj wrote:
> You can as well leave the braces empty as nothing is tested here right now. If
> there is a corresponding pref (I think there isn't) or some UI in the settings
> that is changed when this policy is triggered it can be tested here but
> otherwise leaving just 
> 
> "Http09OnNonDefaultPortsEnabled": {}
> 
> to satisfy the presubmit is good enough.

Done (Yea, there's no pref, no UI surface area)

https://codereview.chromium.org/2253653002/diff/180001/components/policy/reso...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/2253653002/diff/180001/components/policy/reso...
components/policy/resources/policy_templates.json:542: 'label': '''List of
disabled protocol schemes''',
On 2016/08/31 12:02:12, pastarmovj wrote:
> Please fix the label - this seems to have slipped from copying the one above.
:)

Done (Just removed it, so it defaults to the caption)

[mmenke, 2016-08-31 14:30:51]

The CQ bit was checked by mmenke@chromium.org

[mmenke, 2016-08-31 14:30:51]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org,
pastarmovj@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2253653002/#ps200001
(title: "Response to comments")

[commit-bot: I haz the power, 2016-08-31 14:31:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 16:07:17]

Description was changed from

==========
Only allow HTTP/0.9 support on default ports.

Also add a group policy to allow HTTP/0.9 support on other
ports, which I plan to remove in a couple revisions.

BUG=624462,637017
==========

to

==========
Only allow HTTP/0.9 support on default ports.

Also add a group policy to allow HTTP/0.9 support on other
ports, which I plan to remove in a couple revisions.

BUG=624462,637017
==========

[commit-bot: I haz the power, 2016-08-31 16:07:19]

Committed patchset #8 (id:200001)

[commit-bot: I haz the power, 2016-08-31 16:08:40]

Description was changed from

==========
Only allow HTTP/0.9 support on default ports.

Also add a group policy to allow HTTP/0.9 support on other
ports, which I plan to remove in a couple revisions.

BUG=624462,637017
==========

to

==========
Only allow HTTP/0.9 support on default ports.

Also add a group policy to allow HTTP/0.9 support on other
ports, which I plan to remove in a couple revisions.

BUG=624462,637017

Committed: https://crrev.com/398c105ea8391473126021104554d70fda08324b
Cr-Commit-Position: refs/heads/master@{#415643}
==========

[commit-bot: I haz the power, 2016-08-31 16:08:41]

Patchset 8 (id:??) landed as
https://crrev.com/398c105ea8391473126021104554d70fda08324b
Cr-Commit-Position: refs/heads/master@{#415643}