http2: migrates DATA frame payloads to the visitor API

[birenroy]

This slightly simplifies the flow of DATA frame payloads; after this change, the StreamDataFrameSource class can be removed.

• Http2Visitor::OnReadyToSendDataForStream() is substantially the same implementation as StreamDataFrameSource::SelectPayloadLength().
• Http2Visitor::SendDataFrame() is substantially the same as StreamDataFrameSource::Send().

Commit Message: http2: migrates DATA frame payloads to the visitor API Additional Description: Risk Level: low Testing: ran unit and integration tests locally Docs Changes: Release Notes: added a note with the runtime feature to disable in case of problems Runtime guard: envoy_reloadable_features_http2_use_visitor_for_data

[repokitteh-read-only[bot]]

As a reminder, PRs marked as draft will not be automatically assigned reviewers, or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34278 was opened by birenroy.

see: more, trace.

[repokitteh-read-only[bot]]

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34278 was opened by birenroy.

see: more, trace.

[birenroy]

/assign @diannahu /assign @RyanTheOptimist

[RyanTheOptimist]

Is this a real failure, or a flake?

[  FAILED  ] IpVersions/Http2FrameIntegrationTest.AdjustUpstreamSettingsMaxStreams/IPv6_Nghttp2, where GetParam() = 8-byte object <01-00 00-00 00-00 00-00>

https://dev.azure.com/cncf/envoy/_build/results?buildId=171029&view=logs&jobId=767be981-567e-57d8-68c3-2140ede0a0bd&j=767be981-567e-57d8-68c3-2140ede0a0bd&t=2181edf2-f610-59f2-c43a-04bb9d0bca00

[birenroy]

Is this a real failure, or a flake?

[  FAILED  ] IpVersions/Http2FrameIntegrationTest.AdjustUpstreamSettingsMaxStreams/IPv6_Nghttp2, where GetParam() = 8-byte object <01-00 00-00 00-00 00-00>

https://dev.azure.com/cncf/envoy/_build/results?buildId=171029&view=logs&jobId=767be981-567e-57d8-68c3-2140ede0a0bd&j=767be981-567e-57d8-68c3-2140ede0a0bd&t=2181edf2-f610-59f2-c43a-04bb9d0bca00

Test passes with --runs_per_test=50 when I run it. I think it's a flake, since the behavior being tested does not seem to rely on DATA frames.

[RyanTheOptimist]

Looks like coverage has dropped. Maybe we need to add a tests where the flag is false?

Per-extension coverage failed: Code coverage for source/common/http is lower than limit of 96.6 (96.4) Code coverage for source/common/http/http2 is lower than limit of 95.6 (93.8)

[birenroy]

/retest