Add a configuration option for a custom WINDOW_UPDATE strategy in OgHttp2Session. This CL adds an entry to Http2Options for a custom WindowManager::ShouldWindowUpdateFn, as a WINDOW_UPDATE sending strategy, that the OgHttp2Session WindowManager (for connection and streams) would use in place of the default strategy. This CL introduces DeltaAtLeastHalfLimit in the spirit of the nghttp2 analogue: http://google3/third_party/nghttp2/src/lib/nghttp2_helper.c;l=248-251;rcl=431825270 Http2CodecImplFlowControlTest.TestFlowControlInPendingSendData passes with these changes using oghttp2: http://sponge2/ff7fda20-364a-4594-95c2-d3cccd70ac69 PiperOrigin-RevId: 432298189
diff --git a/http2/adapter/http2_util.cc b/http2/adapter/http2_util.cc index 0419952..fd21173 100644 --- a/http2/adapter/http2_util.cc +++ b/http2/adapter/http2_util.cc
@@ -122,5 +122,9 @@ return "UnknownInvalidFrameError"; } +bool DeltaAtLeastHalfLimit(int64_t limit, int64_t /*size*/, int64_t delta) { + return delta > 0 && delta >= limit / 2; +} + } // namespace adapter } // namespace http2
diff --git a/http2/adapter/http2_util.h b/http2/adapter/http2_util.h index 3e25b8c..feea543 100644 --- a/http2/adapter/http2_util.h +++ b/http2/adapter/http2_util.h
@@ -1,6 +1,8 @@ #ifndef QUICHE_HTTP2_ADAPTER_HTTP2_UTIL_H_ #define QUICHE_HTTP2_ADAPTER_HTTP2_UTIL_H_ +#include <cstdint> + #include "http2/adapter/http2_protocol.h" #include "http2/adapter/http2_visitor_interface.h" #include "common/platform/api/quiche_export.h" @@ -20,6 +22,12 @@ QUICHE_EXPORT_PRIVATE absl::string_view InvalidFrameErrorToString( Http2VisitorInterface::InvalidFrameError error); +// A WINDOW_UPDATE sending strategy that returns true if the `delta` to be sent +// is positive and at least half of the window `limit`. +QUICHE_EXPORT_PRIVATE bool DeltaAtLeastHalfLimit(int64_t limit, + int64_t /*size*/, + int64_t delta); + } // namespace adapter } // namespace http2
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index 91768f6..e797e48 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -4742,11 +4742,11 @@ // without recursion guards in OgHttp2Session. TEST(OgHttp2AdapterInteractionTest, ClientServerInteractionTest) { MockHttp2Visitor client_visitor; - auto client_adapter = - OgHttp2Adapter::Create(client_visitor, {Perspective::kClient}); + auto client_adapter = OgHttp2Adapter::Create( + client_visitor, {.perspective = Perspective::kClient}); MockHttp2Visitor server_visitor; - auto server_adapter = - OgHttp2Adapter::Create(server_visitor, {Perspective::kServer}); + auto server_adapter = OgHttp2Adapter::Create( + server_visitor, {.perspective = Perspective::kServer}); // Feeds bytes sent from the client into the server's ProcessBytes. EXPECT_CALL(client_visitor, OnReadyToSend(_)) @@ -6018,6 +6018,68 @@ SpdyFrameType::RST_STREAM})); } +TEST(OgHttp2AdapterTest, ServerUsesCustomWindowUpdateStrategy) { + // Test the use of a custom WINDOW_UPDATE strategy. + DataSavingVisitor visitor; + OgHttp2Adapter::Options options{ + .should_window_update_fn = [](int64_t /*limit*/, int64_t /*size*/, + int64_t /*delta*/) { return true; }, + .perspective = Perspective::kServer}; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "POST"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}, + /*fin=*/false) + .Data(1, "This is the request body.", + /*fin=*/true) + .Serialize(); + testing::InSequence s; + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x1)); + EXPECT_CALL(visitor, OnBeginDataForStream(1, _)); + EXPECT_CALL(visitor, OnDataForStream(1, "This is the request body.")); + EXPECT_CALL(visitor, OnEndStream(1)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(static_cast<int64_t>(frames.size()), result); + + // Mark a small number of bytes for the stream as consumed. Because of the + // custom WINDOW_UPDATE strategy, the session should send WINDOW_UPDATEs. + adapter->MarkDataConsumedForStream(1, 5); + + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 1, 4, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 1, 4, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, 4, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, 4, 0x0, 0)); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), + EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS, + SpdyFrameType::WINDOW_UPDATE, + SpdyFrameType::WINDOW_UPDATE})); +} + } // namespace } // namespace test } // namespace adapter
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc index 89d6d6f..35f498c 100644 --- a/http2/adapter/oghttp2_session.cc +++ b/http2/adapter/oghttp2_session.cc
@@ -334,7 +334,7 @@ [this](size_t window_update_delta) { SendWindowUpdate(kConnectionStreamId, window_update_delta); }, - /*should_window_update_fn=*/{}, + options.should_window_update_fn, /*update_window_on_notify=*/false), options_(options) { decoder_.set_visitor(&receive_logger_); @@ -1650,8 +1650,9 @@ SendWindowUpdate(stream_id, window_update_delta); }; auto [iter, inserted] = stream_map_.try_emplace( - stream_id, StreamState(initial_stream_receive_window_, - initial_stream_send_window_, std::move(listener))); + stream_id, + StreamState(initial_stream_receive_window_, initial_stream_send_window_, + std::move(listener), options_.should_window_update_fn)); if (inserted) { // Add the stream to the write scheduler. const WriteScheduler::StreamPrecedenceType precedence(3);
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h index c9eed29..045eac4 100644 --- a/http2/adapter/oghttp2_session.h +++ b/http2/adapter/oghttp2_session.h
@@ -38,6 +38,11 @@ public spdy::ExtensionVisitorInterface { public: struct QUICHE_EXPORT_PRIVATE Options { + // Returns whether to send a WINDOW_UPDATE based on the window limit, window + // size, and delta that would be sent in the WINDOW_UPDATE. + WindowManager::ShouldWindowUpdateFn should_window_update_fn = + DeltaAtLeastHalfLimit; + // The perspective of this session. Perspective perspective = Perspective::kClient; // The maximum HPACK table size to use. absl::optional<size_t> max_hpack_encoding_table_capacity = absl::nullopt; @@ -213,9 +218,10 @@ struct QUICHE_EXPORT_PRIVATE StreamState { StreamState(int32_t stream_receive_window, int32_t stream_send_window, - WindowManager::WindowUpdateListener listener) + WindowManager::WindowUpdateListener listener, + WindowManager::ShouldWindowUpdateFn should_window_update_fn) : window_manager(stream_receive_window, std::move(listener), - /*should_window_update_fn=*/{}, + std::move(should_window_update_fn), /*update_window_on_notify=*/false), send_window(stream_send_window) {}