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]

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!

🐱

Caused by: #34278 was opened by birenroy.

see: more, trace.

[repokitteh-read-only]

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

🐱

Caused by: #34278 was opened by birenroy.

see: more, trace.

[birenroy]

/assign @diannahu
/assign @RyanTheOptimist

[diannahu]

Nice, clear mapping for the migration! Not sure if the test results are related, but the change LGTM.

[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