Accepts nghttp2 options when constructing an NgHttp2Adapter. Also avoids taking ownership of the passed in nghttp2_option* within NgHttp2Session. Options are only required to exist during construction, so it is simpler to accept a const pointer to the options rather than taking ownership. PiperOrigin-RevId: 380226716
diff --git a/http2/adapter/nghttp2_adapter.cc b/http2/adapter/nghttp2_adapter.cc index c6fe12a..ad2d142 100644 --- a/http2/adapter/nghttp2_adapter.cc +++ b/http2/adapter/nghttp2_adapter.cc
@@ -14,16 +14,16 @@ /* static */ std::unique_ptr<NgHttp2Adapter> NgHttp2Adapter::CreateClientAdapter( - Http2VisitorInterface& visitor) { - auto adapter = new NgHttp2Adapter(visitor, Perspective::kClient); + Http2VisitorInterface& visitor, const nghttp2_option* options) { + auto adapter = new NgHttp2Adapter(visitor, Perspective::kClient, options); adapter->Initialize(); return absl::WrapUnique(adapter); } /* static */ std::unique_ptr<NgHttp2Adapter> NgHttp2Adapter::CreateServerAdapter( - Http2VisitorInterface& visitor) { - auto adapter = new NgHttp2Adapter(visitor, Perspective::kServer); + Http2VisitorInterface& visitor, const nghttp2_option* options) { + auto adapter = new NgHttp2Adapter(visitor, Perspective::kServer, options); adapter->Initialize(); return absl::WrapUnique(adapter); } @@ -218,23 +218,34 @@ } NgHttp2Adapter::NgHttp2Adapter(Http2VisitorInterface& visitor, - Perspective perspective) - : Http2Adapter(visitor), visitor_(visitor), perspective_(perspective) {} + Perspective perspective, + const nghttp2_option* options) + : Http2Adapter(visitor), + visitor_(visitor), + options_(options), + perspective_(perspective) {} NgHttp2Adapter::~NgHttp2Adapter() {} void NgHttp2Adapter::Initialize() { - nghttp2_option* options; - nghttp2_option_new(&options); - // Set some common options for compatibility. - nghttp2_option_set_no_closed_streams(options, 1); - nghttp2_option_set_no_auto_window_update(options, 1); - nghttp2_option_set_max_send_header_block_length(options, 0x2000000); - nghttp2_option_set_max_outbound_ack(options, 10000); + nghttp2_option* owned_options = nullptr; + if (options_ == nullptr) { + nghttp2_option_new(&owned_options); + // Set some common options for compatibility. + nghttp2_option_set_no_closed_streams(owned_options, 1); + nghttp2_option_set_no_auto_window_update(owned_options, 1); + nghttp2_option_set_max_send_header_block_length(owned_options, 0x2000000); + nghttp2_option_set_max_outbound_ack(owned_options, 10000); + options_ = owned_options; + } - session_ = - absl::make_unique<NgHttp2Session>(perspective_, callbacks::Create(), - options, static_cast<void*>(&visitor_)); + session_ = absl::make_unique<NgHttp2Session>(perspective_, + callbacks::Create(), options_, + static_cast<void*>(&visitor_)); + if (owned_options != nullptr) { + nghttp2_option_del(owned_options); + } + options_ = nullptr; } } // namespace adapter
diff --git a/http2/adapter/nghttp2_adapter.h b/http2/adapter/nghttp2_adapter.h index e9fa7d2..b9900dd 100644 --- a/http2/adapter/nghttp2_adapter.h +++ b/http2/adapter/nghttp2_adapter.h
@@ -14,13 +14,15 @@ public: ~NgHttp2Adapter() override; - // Creates an adapter that functions as a client. + // Creates an adapter that functions as a client. Does not take ownership of + // |options|. static std::unique_ptr<NgHttp2Adapter> CreateClientAdapter( - Http2VisitorInterface& visitor); + Http2VisitorInterface& visitor, const nghttp2_option* options = nullptr); - // Creates an adapter that functions as a server. + // Creates an adapter that functions as a server. Does not take ownership of + // |options|. static std::unique_ptr<NgHttp2Adapter> CreateServerAdapter( - Http2VisitorInterface& visitor); + Http2VisitorInterface& visitor, const nghttp2_option* options = nullptr); bool IsServerSession() const override; @@ -85,7 +87,8 @@ NgHttp2Session& session() { return *session_; } private: - NgHttp2Adapter(Http2VisitorInterface& visitor, Perspective perspective); + NgHttp2Adapter(Http2VisitorInterface& visitor, Perspective perspective, + const nghttp2_option* options); // Performs any necessary initialization of the underlying HTTP/2 session, // such as preparing initial SETTINGS. @@ -93,6 +96,7 @@ std::unique_ptr<NgHttp2Session> session_; Http2VisitorInterface& visitor_; + const nghttp2_option* options_; Perspective perspective_; absl::flat_hash_map<int32_t, std::unique_ptr<DataFrameSource>> sources_;
diff --git a/http2/adapter/nghttp2_session.cc b/http2/adapter/nghttp2_session.cc index d434b06..1809f3e 100644 --- a/http2/adapter/nghttp2_session.cc +++ b/http2/adapter/nghttp2_session.cc
@@ -4,32 +4,18 @@ namespace http2 { namespace adapter { -namespace { - -void DeleteOptions(nghttp2_option* options) { - if (options) { - nghttp2_option_del(options); - } -} - -} // namespace NgHttp2Session::NgHttp2Session(Perspective perspective, nghttp2_session_callbacks_unique_ptr callbacks, - nghttp2_option* options, - void* userdata) - : session_(MakeSessionPtr(nullptr)), - options_(options, DeleteOptions), - perspective_(perspective) { + const nghttp2_option* options, void* userdata) + : session_(MakeSessionPtr(nullptr)), perspective_(perspective) { nghttp2_session* session; switch (perspective) { case Perspective::kClient: - nghttp2_session_client_new2(&session, callbacks.get(), userdata, - options_.get()); + nghttp2_session_client_new2(&session, callbacks.get(), userdata, options); break; case Perspective::kServer: - nghttp2_session_server_new2(&session, callbacks.get(), userdata, - options_.get()); + nghttp2_session_server_new2(&session, callbacks.get(), userdata, options); break; } session_.reset(session);
diff --git a/http2/adapter/nghttp2_session.h b/http2/adapter/nghttp2_session.h index d446a07..5dd08f8 100644 --- a/http2/adapter/nghttp2_session.h +++ b/http2/adapter/nghttp2_session.h
@@ -11,11 +11,10 @@ // A C++ wrapper around common nghttp2_session operations. class NgHttp2Session : public Http2Session { public: - // Takes ownership of |options|. + // Does not take ownership of |options|. NgHttp2Session(Perspective perspective, nghttp2_session_callbacks_unique_ptr callbacks, - nghttp2_option* options, - void* userdata); + const nghttp2_option* options, void* userdata); ~NgHttp2Session() override; ssize_t ProcessBytes(absl::string_view bytes) override; @@ -29,10 +28,7 @@ nghttp2_session* raw_ptr() const { return session_.get(); } private: - using OptionsDeleter = void (&)(nghttp2_option*); - nghttp2_session_unique_ptr session_; - std::unique_ptr<nghttp2_option, OptionsDeleter> options_; Perspective perspective_; };
diff --git a/http2/adapter/nghttp2_session_test.cc b/http2/adapter/nghttp2_session_test.cc index 4973b3b..d4e5ea1 100644 --- a/http2/adapter/nghttp2_session_test.cc +++ b/http2/adapter/nghttp2_session_test.cc
@@ -28,24 +28,25 @@ class NgHttp2SessionTest : public testing::Test { public: - nghttp2_option* CreateOptions() { - nghttp2_option* options; - nghttp2_option_new(&options); - nghttp2_option_set_no_auto_window_update(options, 1); - return options; + void SetUp() override { + nghttp2_option_new(&options_); + nghttp2_option_set_no_auto_window_update(options_, 1); } + void TearDown() override { nghttp2_option_del(options_); } + nghttp2_session_callbacks_unique_ptr CreateCallbacks() { nghttp2_session_callbacks_unique_ptr callbacks = callbacks::Create(); return callbacks; } DataSavingVisitor visitor_; + nghttp2_option* options_ = nullptr; }; TEST_F(NgHttp2SessionTest, ClientConstruction) { - NgHttp2Session session(Perspective::kClient, CreateCallbacks(), - CreateOptions(), &visitor_); + NgHttp2Session session(Perspective::kClient, CreateCallbacks(), options_, + &visitor_); EXPECT_TRUE(session.want_read()); EXPECT_FALSE(session.want_write()); EXPECT_EQ(session.GetRemoteWindowSize(), kInitialFlowControlWindowSize); @@ -53,8 +54,8 @@ } TEST_F(NgHttp2SessionTest, ClientHandlesFrames) { - NgHttp2Session session(Perspective::kClient, CreateCallbacks(), - CreateOptions(), &visitor_); + NgHttp2Session session(Perspective::kClient, CreateCallbacks(), options_, + &visitor_); ASSERT_EQ(0, nghttp2_session_send(session.raw_ptr())); ASSERT_GT(visitor_.data().size(), 0); @@ -191,8 +192,8 @@ } TEST_F(NgHttp2SessionTest, ServerConstruction) { - NgHttp2Session session(Perspective::kServer, CreateCallbacks(), - CreateOptions(), &visitor_); + NgHttp2Session session(Perspective::kServer, CreateCallbacks(), options_, + &visitor_); EXPECT_TRUE(session.want_read()); EXPECT_FALSE(session.want_write()); EXPECT_EQ(session.GetRemoteWindowSize(), kInitialFlowControlWindowSize); @@ -200,8 +201,8 @@ } TEST_F(NgHttp2SessionTest, ServerHandlesFrames) { - NgHttp2Session session(Perspective::kServer, CreateCallbacks(), - CreateOptions(), &visitor_); + NgHttp2Session session(Perspective::kServer, CreateCallbacks(), options_, + &visitor_); const std::string frames = TestFrameSequence() .ClientPreface()