Adds a Http2VisitorInterface method for when the library wants to send serialized frame data to the peer.
This replaces Http2Adapter's GetBytesToWrite(), and will make compatibility with existing usage a little easier.
PiperOrigin-RevId: 375197346
diff --git a/http2/adapter/callback_visitor.cc b/http2/adapter/callback_visitor.cc
index eaf4b61..8baba58 100644
--- a/http2/adapter/callback_visitor.cc
+++ b/http2/adapter/callback_visitor.cc
@@ -36,6 +36,11 @@
namespace http2 {
namespace adapter {
+ssize_t CallbackVisitor::OnReadyToSend(absl::string_view serialized) {
+ return callbacks_->send_callback(nullptr, ToUint8Ptr(serialized.data()),
+ serialized.size(), 0, user_data_);
+}
+
void CallbackVisitor::OnConnectionError() {
QUICHE_LOG(FATAL) << "Not implemented";
}
diff --git a/http2/adapter/callback_visitor.h b/http2/adapter/callback_visitor.h
index 94746bf..5516008 100644
--- a/http2/adapter/callback_visitor.h
+++ b/http2/adapter/callback_visitor.h
@@ -23,6 +23,7 @@
callbacks_(std::move(callbacks)),
user_data_(user_data) {}
+ ssize_t OnReadyToSend(absl::string_view serialized) override;
void OnConnectionError() override;
void OnFrameHeader(Http2StreamId stream_id,
size_t length,
diff --git a/http2/adapter/http2_adapter.h b/http2/adapter/http2_adapter.h
index c97fd27..c18f1bb 100644
--- a/http2/adapter/http2_adapter.h
+++ b/http2/adapter/http2_adapter.h
@@ -65,10 +65,8 @@
// END_METADATA flag set.
virtual void SubmitMetadata(Http2StreamId stream_id, bool fin) = 0;
- // Returns serialized bytes for writing to the wire.
- // Writes should be submitted to Http2Adapter first, so that Http2Adapter
- // has data to serialize and return in this method.
- virtual std::string GetBytesToWrite(absl::optional<size_t> max_bytes) = 0;
+ // Invokes the visitor's OnReadyToSend() method for serialized frame data.
+ virtual void Send() = 0;
// Returns the connection-level flow control window for the peer.
virtual int GetPeerConnectionWindow() const = 0;
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index 12729e7..a7e34e9 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -50,6 +50,11 @@
Http2VisitorInterface& operator=(const Http2VisitorInterface&) = delete;
virtual ~Http2VisitorInterface() = default;
+ // Called when there are serialized frames to send. Should return how many
+ // bytes were actually sent. Returning 0 indicates that sending is blocked.
+ // Returning -1 indicates an error.
+ virtual ssize_t OnReadyToSend(absl::string_view serialized) = 0;
+
// Called when a connection-level processing error has been encountered.
virtual void OnConnectionError() = 0;
diff --git a/http2/adapter/mock_http2_visitor.h b/http2/adapter/mock_http2_visitor.h
index 171d40f..3c4ee2f 100644
--- a/http2/adapter/mock_http2_visitor.h
+++ b/http2/adapter/mock_http2_visitor.h
@@ -13,6 +13,10 @@
public:
MockHttp2Visitor() = default;
+ MOCK_METHOD(ssize_t,
+ OnReadyToSend,
+ (absl::string_view serialized),
+ (override));
MOCK_METHOD(void, OnConnectionError, (), (override));
MOCK_METHOD(
void,
diff --git a/http2/adapter/nghttp2_adapter.cc b/http2/adapter/nghttp2_adapter.cc
index 878c040..f9fe6fe 100644
--- a/http2/adapter/nghttp2_adapter.cc
+++ b/http2/adapter/nghttp2_adapter.cc
@@ -84,21 +84,11 @@
QUICHE_LOG(DFATAL) << "Not implemented";
}
-std::string NgHttp2Adapter::GetBytesToWrite(absl::optional<size_t> max_bytes) {
- ssize_t num_bytes = 0;
- std::string result;
- do {
- const uint8_t* data = nullptr;
- num_bytes = nghttp2_session_mem_send(session_->raw_ptr(), &data);
- if (num_bytes > 0) {
- absl::StrAppend(
- &result,
- absl::string_view(reinterpret_cast<const char*>(data), num_bytes));
- } else if (num_bytes < 0) {
- visitor_.OnConnectionError();
- }
- } while (num_bytes > 0);
- return result;
+void NgHttp2Adapter::Send() {
+ const int result = nghttp2_session_send(session_->raw_ptr());
+ if (result != 0) {
+ visitor_.OnConnectionError();
+ }
}
int NgHttp2Adapter::GetPeerConnectionWindow() const {
diff --git a/http2/adapter/nghttp2_adapter.h b/http2/adapter/nghttp2_adapter.h
index 55f8d3d..1fa4de8 100644
--- a/http2/adapter/nghttp2_adapter.h
+++ b/http2/adapter/nghttp2_adapter.h
@@ -62,10 +62,8 @@
// END_METADATA flag set.
void SubmitMetadata(Http2StreamId stream_id, bool end_metadata) override;
- // Returns serialized bytes for writing to the wire. Writes should be
- // submitted to Nghttp2Adapter first, so that Nghttp2Adapter has data to
- // serialize and return in this method.
- std::string GetBytesToWrite(absl::optional<size_t> max_bytes) override;
+ // Invokes the visitor's OnReadyToSend() method for serialized frame data.
+ void Send() override;
// Returns the connection-level flow control window for the peer.
int GetPeerConnectionWindow() const override;
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 3aec115..ede56f1 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -33,10 +33,12 @@
}
TEST(NgHttp2AdapterTest, ClientHandlesFrames) {
- testing::StrictMock<MockHttp2Visitor> visitor;
+ DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
- std::string serialized = adapter->GetBytesToWrite(absl::nullopt);
- EXPECT_THAT(serialized, testing::StrEq(spdy::kHttp2ConnectionHeaderPrefix));
+ adapter->Send();
+ EXPECT_THAT(visitor.data(),
+ testing::StrEq(spdy::kHttp2ConnectionHeaderPrefix));
+ visitor.Clear();
const std::string initial_frames = TestFrameSequence()
.ServerPreface()
@@ -61,9 +63,10 @@
EXPECT_EQ(adapter->GetPeerConnectionWindow(),
kDefaultInitialStreamWindowSize + 1000);
// Some bytes should have been serialized.
- serialized = adapter->GetBytesToWrite(absl::nullopt);
- EXPECT_THAT(serialized, EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::PING}));
+ adapter->Send();
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::PING}));
+ visitor.Clear();
const std::vector<const Header> headers1 =
ToHeaders({{":method", "GET"},
@@ -104,10 +107,11 @@
ASSERT_GT(stream_id3, 0);
QUICHE_LOG(INFO) << "Created stream: " << stream_id3;
- serialized = adapter->GetBytesToWrite(absl::nullopt);
- EXPECT_THAT(serialized, EqualsFrames({spdy::SpdyFrameType::HEADERS,
- spdy::SpdyFrameType::HEADERS,
- spdy::SpdyFrameType::HEADERS}));
+ adapter->Send();
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS,
+ spdy::SpdyFrameType::HEADERS,
+ spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
const std::string stream_frames =
TestFrameSequence()
@@ -160,8 +164,8 @@
// Client will not have anything else to write.
EXPECT_FALSE(adapter->session().want_write());
- serialized = adapter->GetBytesToWrite(absl::nullopt);
- EXPECT_THAT(serialized, testing::IsEmpty());
+ adapter->Send();
+ EXPECT_THAT(visitor.data(), testing::IsEmpty());
}
TEST(NgHttp2AdapterTest, ServerConstruction) {
@@ -173,7 +177,7 @@
}
TEST(NgHttp2AdapterTest, ServerHandlesFrames) {
- testing::StrictMock<MockHttp2Visitor> visitor;
+ DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
const std::string frames = TestFrameSequence()
@@ -242,11 +246,11 @@
EXPECT_TRUE(adapter->session().want_write());
// Some bytes should have been serialized.
- std::string serialized = adapter->GetBytesToWrite(absl::nullopt);
+ adapter->Send();
// SETTINGS ack, two PING acks.
- EXPECT_THAT(serialized, EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::PING,
- spdy::SpdyFrameType::PING}));
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::PING,
+ spdy::SpdyFrameType::PING}));
}
} // namespace
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index 8774873..c3e1dc8 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -16,6 +16,22 @@
namespace adapter {
namespace callbacks {
+ssize_t OnReadyToSend(nghttp2_session* /* session */,
+ const uint8_t* data,
+ size_t length,
+ int flags,
+ void* user_data) {
+ auto* visitor = static_cast<Http2VisitorInterface*>(user_data);
+ const ssize_t result = visitor->OnReadyToSend(ToStringView(data, length));
+ if (result < 0) {
+ return NGHTTP2_ERR_CALLBACK_FAILURE;
+ } else if (result == 0) {
+ return NGHTTP2_ERR_WOULDBLOCK;
+ } else {
+ return result;
+ }
+}
+
int OnBeginFrame(nghttp2_session* /* session */,
const nghttp2_frame_hd* header,
void* user_data) {
@@ -190,6 +206,7 @@
nghttp2_session_callbacks* callbacks;
nghttp2_session_callbacks_new(&callbacks);
+ nghttp2_session_callbacks_set_send_callback(callbacks, &OnReadyToSend);
nghttp2_session_callbacks_set_on_begin_frame_callback(callbacks,
&OnBeginFrame);
nghttp2_session_callbacks_set_on_frame_recv_callback(callbacks,
diff --git a/http2/adapter/nghttp2_callbacks.h b/http2/adapter/nghttp2_callbacks.h
index 5fbaee0..f4621c1 100644
--- a/http2/adapter/nghttp2_callbacks.h
+++ b/http2/adapter/nghttp2_callbacks.h
@@ -13,6 +13,13 @@
// beginning of its lifetime. It is expected that |user_data| holds an
// Http2VisitorInterface.
+// Callback once the library is ready to send serialized frames.
+ssize_t OnReadyToSend(nghttp2_session* session,
+ const uint8_t* data,
+ size_t length,
+ int flags,
+ void* user_data);
+
// Callback once a frame header has been received.
int OnBeginFrame(nghttp2_session* session, const nghttp2_frame_hd* header,
void* user_data);
diff --git a/http2/adapter/nghttp2_session_test.cc b/http2/adapter/nghttp2_session_test.cc
index 169d2f2..5e0fd23 100644
--- a/http2/adapter/nghttp2_session_test.cc
+++ b/http2/adapter/nghttp2_session_test.cc
@@ -26,16 +26,6 @@
WINDOW_UPDATE,
};
-ssize_t SaveSessionOutput(nghttp2_session* /* session*/,
- const uint8_t* data,
- size_t length,
- int /* flags */,
- void* user_data) {
- auto visitor = static_cast<DataSavingVisitor*>(user_data);
- visitor->Save(ToStringView(data, length));
- return length;
-}
-
class NgHttp2SessionTest : public testing::Test {
public:
nghttp2_option* CreateOptions() {
@@ -47,8 +37,6 @@
nghttp2_session_callbacks_unique_ptr CreateCallbacks() {
nghttp2_session_callbacks_unique_ptr callbacks = callbacks::Create();
- nghttp2_session_callbacks_set_send_callback(callbacks.get(),
- &SaveSessionOutput);
return callbacks;
}
diff --git a/http2/adapter/oghttp2_adapter.cc b/http2/adapter/oghttp2_adapter.cc
index 9f287fa..788310a 100644
--- a/http2/adapter/oghttp2_adapter.cc
+++ b/http2/adapter/oghttp2_adapter.cc
@@ -71,8 +71,8 @@
QUICHE_BUG(oghttp2_submit_metadata) << "Not implemented";
}
-std::string OgHttp2Adapter::GetBytesToWrite(absl::optional<size_t> max_bytes) {
- return session_->GetBytesToWrite(max_bytes);
+void OgHttp2Adapter::Send() {
+ session_->Send();
}
int OgHttp2Adapter::GetPeerConnectionWindow() const {
diff --git a/http2/adapter/oghttp2_adapter.h b/http2/adapter/oghttp2_adapter.h
index 572ba04..90b4391 100644
--- a/http2/adapter/oghttp2_adapter.h
+++ b/http2/adapter/oghttp2_adapter.h
@@ -32,7 +32,7 @@
void SubmitWindowUpdate(Http2StreamId stream_id,
int window_increment) override;
void SubmitMetadata(Http2StreamId stream_id, bool fin) override;
- std::string GetBytesToWrite(absl::optional<size_t> max_bytes) override;
+ void Send() override;
int GetPeerConnectionWindow() const override;
void MarkDataConsumedForStream(Http2StreamId stream_id,
size_t num_bytes) override;
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 4c849f6..0f185d2 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -18,7 +18,7 @@
adapter_ = OgHttp2Adapter::Create(http2_visitor_, options);
}
- testing::StrictMock<MockHttp2Visitor> http2_visitor_;
+ DataSavingVisitor http2_visitor_;
std::unique_ptr<OgHttp2Adapter> adapter_;
};
@@ -62,8 +62,9 @@
adapter_->SubmitWindowUpdate(3, 127);
EXPECT_TRUE(adapter_->session().want_write());
+ adapter_->Send();
EXPECT_THAT(
- adapter_->GetBytesToWrite(absl::nullopt),
+ http2_visitor_.data(),
EqualsFrames(
{spdy::SpdyFrameType::SETTINGS, spdy::SpdyFrameType::PRIORITY,
spdy::SpdyFrameType::RST_STREAM, spdy::SpdyFrameType::PING,
@@ -80,12 +81,15 @@
adapter_->SubmitPing(42);
EXPECT_TRUE(adapter_->session().want_write());
- const std::string first_part = adapter_->GetBytesToWrite(10);
+ http2_visitor_.set_send_limit(20);
+ adapter_->Send();
EXPECT_TRUE(adapter_->session().want_write());
- const std::string second_part = adapter_->GetBytesToWrite(absl::nullopt);
+ adapter_->Send();
+ EXPECT_TRUE(adapter_->session().want_write());
+ adapter_->Send();
EXPECT_FALSE(adapter_->session().want_write());
EXPECT_THAT(
- absl::StrCat(first_part, second_part),
+ http2_visitor_.data(),
EqualsFrames({spdy::SpdyFrameType::SETTINGS, spdy::SpdyFrameType::GOAWAY,
spdy::SpdyFrameType::PING}));
}
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 2222fed..e45502e 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -75,20 +75,28 @@
frames_.push_back(std::move(frame));
}
-std::string OgHttp2Session::GetBytesToWrite(absl::optional<size_t> max_bytes) {
- const size_t serialized_max =
- max_bytes ? max_bytes.value() : std::numeric_limits<size_t>::max();
- std::string serialized = std::move(serialized_prefix_);
- while (serialized.size() < serialized_max && !frames_.empty()) {
+void OgHttp2Session::Send() {
+ ssize_t result = std::numeric_limits<ssize_t>::max();
+ // Flush any serialized prefix.
+ while (result > 0 && !serialized_prefix_.empty()) {
+ result = visitor_.OnReadyToSend(serialized_prefix_);
+ if (result > 0) {
+ serialized_prefix_.erase(0, result);
+ }
+ }
+ // Serialize and send frames in the queue.
+ // TODO(birenroy): wake streams for DATA frame writes
+ while (result > 0 && !frames_.empty()) {
spdy::SpdySerializedFrame frame = framer_.SerializeFrame(*frames_.front());
- absl::StrAppend(&serialized, absl::string_view(frame));
frames_.pop_front();
+ result = visitor_.OnReadyToSend(absl::string_view(frame));
+ if (result < 0) {
+ visitor_.OnConnectionError();
+ } else if (result < frame.size()) {
+ serialized_prefix_.assign(frame.data() + result, frame.size() - result);
+ break;
+ }
}
- if (serialized.size() > serialized_max) {
- serialized_prefix_ = serialized.substr(serialized_max);
- serialized.resize(serialized_max);
- }
- return serialized;
}
void OgHttp2Session::OnError(http2::Http2DecoderAdapter::SpdyFramerError error,
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index b0bc28b..39f4cdc 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -28,9 +28,8 @@
// Enqueues a frame for transmission to the peer.
void EnqueueFrame(std::unique_ptr<spdy::SpdyFrameIR> frame);
- // If |want_write()| returns true, this method will return a non-empty string
- // containing serialized HTTP/2 frames to write to the peer.
- std::string GetBytesToWrite(absl::optional<size_t> max_bytes);
+ // Invokes the visitor's OnReadyToSend() method for serialized frame data.
+ void Send();
// From Http2Session.
ssize_t ProcessBytes(absl::string_view bytes) override;
diff --git a/http2/adapter/recording_http2_visitor.cc b/http2/adapter/recording_http2_visitor.cc
index 78ec423..4cae3c0 100644
--- a/http2/adapter/recording_http2_visitor.cc
+++ b/http2/adapter/recording_http2_visitor.cc
@@ -7,6 +7,11 @@
namespace adapter {
namespace test {
+ssize_t RecordingHttp2Visitor::OnReadyToSend(absl::string_view serialized) {
+ events_.push_back(absl::StrFormat("OnReadyToSend %d", serialized.size()));
+ return serialized.size();
+}
+
void RecordingHttp2Visitor::OnConnectionError() {
events_.push_back("OnConnectionError");
}
diff --git a/http2/adapter/recording_http2_visitor.h b/http2/adapter/recording_http2_visitor.h
index 452b451..feac2de 100644
--- a/http2/adapter/recording_http2_visitor.h
+++ b/http2/adapter/recording_http2_visitor.h
@@ -18,6 +18,7 @@
using EventSequence = std::list<Event>;
// From Http2VisitorInterface
+ ssize_t OnReadyToSend(absl::string_view serialized) override;
void OnConnectionError() override;
void OnFrameHeader(Http2StreamId stream_id,
size_t length,
diff --git a/http2/adapter/test_utils.h b/http2/adapter/test_utils.h
index ef1ae29..dccdfd9 100644
--- a/http2/adapter/test_utils.h
+++ b/http2/adapter/test_utils.h
@@ -17,13 +17,20 @@
class DataSavingVisitor : public testing::StrictMock<MockHttp2Visitor> {
public:
- void Save(absl::string_view data) { absl::StrAppend(&data_, data); }
+ ssize_t OnReadyToSend(absl::string_view data) override {
+ const size_t to_accept = std::min(send_limit_, data.size());
+ absl::StrAppend(&data_, data.substr(0, to_accept));
+ return to_accept;
+ }
const std::string& data() { return data_; }
void Clear() { data_.clear(); }
+ void set_send_limit(size_t limit) { send_limit_ = limit; }
+
private:
std::string data_;
+ size_t send_limit_ = std::numeric_limits<size_t>::max();
};
// These matchers check whether a string consists entirely of HTTP/2 frames of
