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) {}
