New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 622737 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

Support "connection_options" equivalent for HTTP/2 settings

Project Member Reported by mpw@chromium.org, Jun 23 2016

Issue description

The "connection_options" field in QUIC field trial configuration is useful for running QUIC field trials with different connection-level settings.  It would be useful to have the same ability for HTTP/2, by giving field trials the ability to set parameters in the HTTP/2 SETTINGS frame.

These parameters could be standard HTTP/2 parameters (e.g. those defined in RFC 7540 section 6.5.2) or non-standard parameters understood only by the server. In either case, the parameter is just a 16-bit numeric key, and 32-bit numeric value.

More concretely, one could imagine having a "http2_settings" entry in the field trials params map, similar to QUIC's "connection_options", whose value is a comma-separated list of <parameter-id>:<parameter-value> pairs.  E.g., an "http2_settings" value of "6:1234,25:5678" would have the effect of setting HTTP/2 settings frame key SETTINGS_MAX_HEADER_LIST_SIZE (ID value 6) to 1234, and non-standard setting 25 to value 5678.

(Having syntactic sugar for standard HTTP/2 settings, e.g. specifying "max_header_list_size" instead of "6", would also be nice, but is not essential.)

Having this functionality will enable running field trials to gauge the effects of different HTTP/2 settings (e.g. HPACK table size), as well as field trials comparing different server-side behavior (keyed off non-standard HTTP/2 settings frame parameters) where the behavior must apply from the start of the HTTP/2 connection. One example of this is different HTTP/2 priority scheduling schemes.
 

Comment 1 by mpw@chromium.org, Jun 23 2016

Labels: -Pri-3 Pri-2

Comment 2 by mpw@chromium.org, Jun 23 2016

Components: Internals>Network>HTTP2

Comment 3 by b...@chromium.org, Jun 27 2016

Status: Available (was: Untriaged)

Comment 4 by b...@chromium.org, Nov 9 2016

Cc: -b...@chromium.org
Owner: b...@chromium.org
Status: Started (was: Available)

Comment 5 by b...@chromium.org, Nov 10 2016

Just a bit of complication: Currently SpdySettingsIds enum values do not correspond to on-the-wire values, and they are translated at https://cs.chromium.org/chromium/src/net/spdy/spdy_protocol.cc?q=SerializeSettingId&l=327, which would need to be updated to handle non-standard setting values.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08e5b8e42482ea6f11711333306df76c8ff24d80

commit 08e5b8e42482ea6f11711333306df76c8ff24d80
Author: bnc <bnc@chromium.org>
Date: Sun Dec 04 19:48:12 2016

Use wire values in SpdySettingsIds.

- Use wire values in SpdySettingsIds,
- remove SPDY/3.1-only setting values from SpdySettingsIds,
- rename ParseSettingId() to ParseSettingsId(),
- merge IsValidSettingId() into ParseSettingsId(),
- remove SerializeSettingId(),
- factor out SettingsIdToString(); remove default case
  to make sure every enum value is handled.

These values are not persisted to disk (that was a SPDY/3.1 feature),
so the only downside to the value change is Chrome net-log incompatibility.

This lands server side change 140592266 by bnc.

BUG= 622737 , 488484

Review-Url: https://codereview.chromium.org/2544343003
Cr-Commit-Position: refs/heads/master@{#436200}

[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/quic/core/quic_headers_stream_test.cc
[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/spdy/spdy_framer.cc
[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/spdy/spdy_framer_test.cc
[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/spdy/spdy_protocol.cc
[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/spdy/spdy_protocol.h
[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/spdy/spdy_protocol_test.cc
[modify] https://crrev.com/08e5b8e42482ea6f11711333306df76c8ff24d80/net/spdy/spdy_session.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/903e8c18c8722819356c5f161b08672226651ee8

commit 903e8c18c8722819356c5f161b08672226651ee8
Author: bnc <bnc@chromium.org>
Date: Fri Dec 09 20:50:05 2016

Remove two SpdySessionPoolPeer methods.

Remove SpdySessionPoolPeer::SetSessionMaxRecvWindowSize() and
SpdySessionPoolPeer::SetStreamInitialRecvWindowSize(): |session_deps_| has
corresponding members and is more widely used in tests, so use them
instead.

BUG= 622737 

Review-Url: https://codereview.chromium.org/2567493002
Cr-Commit-Position: refs/heads/master@{#437639}

[modify] https://crrev.com/903e8c18c8722819356c5f161b08672226651ee8/net/spdy/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/903e8c18c8722819356c5f161b08672226651ee8/net/spdy/spdy_session_unittest.cc
[modify] https://crrev.com/903e8c18c8722819356c5f161b08672226651ee8/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/903e8c18c8722819356c5f161b08672226651ee8/net/spdy/spdy_test_util_common.h

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2

commit 3171a243c6b5fbaba264a71e78dd36a6b5eb47d2
Author: bnc <bnc@chromium.org>
Date: Wed Dec 28 18:40:26 2016

Implement HTTP/2 settings field trial parameters.

* Add |http2_settings| field trial parameter to HTTP/2 trial group.
* Set max header table size, max concurrent pushed streams, and max
  receiving window size values in |http2_settings| in
  HttpNetworkSession.
* Pass |http2_settings| from HttpNetworkSession to SpdySessionPool to
  SpdySession.  Remove |stream_max_recv_window_size| from this path.
* Extract max header table size, max concurrent pushed streams, and max
  receiving window size values from |http2_settings| in SpdySession.
* Move max header table size and max concurrent pushed streams default
  value from SpdySession to HttpNetworkSession.
* Add |http2_settings| to SpdySessionDependencies; remove
  |stream_max_recv_window_size|.

BUG= 622737 

Review-Url: https://codereview.chromium.org/2600973002
Cr-Commit-Position: refs/heads/master@{#440871}

[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/components/network_session_configurator/network_session_configurator.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/components/network_session_configurator/network_session_configurator_unittest.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/http/http_network_session.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/http/http_network_session.h
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_session.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_session.h
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_session_pool.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_session_pool.h
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_session_unittest.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/3171a243c6b5fbaba264a71e78dd36a6b5eb47d2/net/spdy/spdy_test_util_common.h

Comment 9 by b...@chromium.org, Dec 29 2016

Status: Fixed (was: Started)

Comment 10 by b...@chromium.org, Dec 29 2016

Labels: M-57
FYI this was picked up by Canary 57.0.2966.0.

Sign in to add a comment