Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8dfd5a5

Browse filesBrowse files
jasnellevanlucas
jasnell
authored and
evanlucas
committed
http2: multiple smaller code cleanups
* Add Http2Priority utility class * Reduces some code duplication * Other small minor cleanups PR-URL: #16764 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
1 parent 7964849 commit 8dfd5a5
Copy full SHA for 8dfd5a5

File tree

Expand file treeCollapse file tree

3 files changed

+57
-72
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+57
-72
lines changed
Open diff view settings
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+21-31Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,18 @@ inline void Http2Settings::RefreshDefaults(Environment* env) {
186186
(1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
187187
}
188188

189+
Http2Priority::Http2Priority(Environment* env,
190+
Local<Value> parent,
191+
Local<Value> weight,
192+
Local<Value> exclusive) {
193+
Local<Context> context = env->context();
194+
int32_t parent_ = parent->Int32Value(context).ToChecked();
195+
int32_t weight_ = weight->Int32Value(context).ToChecked();
196+
bool exclusive_ = exclusive->BooleanValue(context).ToChecked();
197+
DEBUG_HTTP2("Http2Priority: parent: %d, weight: %d, exclusive: %d\n",
198+
parent_, weight_, exclusive_);
199+
nghttp2_priority_spec_init(&spec, parent_, weight_, exclusive_ ? 1 : 0);
200+
}
189201

190202
Http2Session::Http2Session(Environment* env,
191203
Local<Object> wrap,
@@ -267,12 +279,8 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
267279
buffer[PADDING_BUF_RETURN_VALUE] = frameLen;
268280
MakeCallback(env()->ongetpadding_string(), 0, nullptr);
269281
uint32_t retval = buffer[PADDING_BUF_RETURN_VALUE];
270-
retval = retval <= maxPayloadLen ? retval : maxPayloadLen;
271-
retval = retval >= frameLen ? retval : frameLen;
272-
#if defined(DEBUG) && DEBUG
273-
CHECK_GE(retval, frameLen);
274-
CHECK_LE(retval, maxPayloadLen);
275-
#endif
282+
retval = std::min(retval, static_cast<uint32_t>(maxPayloadLen));
283+
retval = std::max(retval, static_cast<uint32_t>(frameLen));
276284
return retval;
277285
}
278286

@@ -454,30 +462,18 @@ void Http2Session::SubmitPriority(const FunctionCallbackInfo<Value>& args) {
454462
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
455463
Local<Context> context = env->context();
456464

457-
nghttp2_priority_spec spec;
458465
int32_t id = args[0]->Int32Value(context).ToChecked();
459-
int32_t parent = args[1]->Int32Value(context).ToChecked();
460-
int32_t weight = args[2]->Int32Value(context).ToChecked();
461-
bool exclusive = args[3]->BooleanValue(context).ToChecked();
466+
Http2Priority priority(env, args[1], args[2], args[3]);
462467
bool silent = args[4]->BooleanValue(context).ToChecked();
463-
DEBUG_HTTP2("Http2Session: submitting priority for stream %d: "
464-
"parent: %d, weight: %d, exclusive: %d, silent: %d\n",
465-
id, parent, weight, exclusive, silent);
466-
467-
#if defined(DEBUG) && DEBUG
468-
CHECK_GT(id, 0);
469-
CHECK_GE(parent, 0);
470-
CHECK_GE(weight, 0);
471-
#endif
468+
DEBUG_HTTP2("Http2Session: submitting priority for stream %d", id);
472469

473470
Nghttp2Stream* stream;
474471
if (!(stream = session->FindStream(id))) {
475472
// invalid stream
476473
return args.GetReturnValue().Set(NGHTTP2_ERR_INVALID_STREAM_ID);
477474
}
478-
nghttp2_priority_spec_init(&spec, parent, weight, exclusive ? 1 : 0);
479475

480-
args.GetReturnValue().Set(stream->SubmitPriority(&spec, silent));
476+
args.GetReturnValue().Set(stream->SubmitPriority(*priority, silent));
481477
}
482478

483479
void Http2Session::SubmitSettings(const FunctionCallbackInfo<Value>& args) {
@@ -533,20 +529,14 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
533529

534530
Local<Array> headers = args[0].As<Array>();
535531
int options = args[1]->IntegerValue(context).ToChecked();
536-
int32_t parent = args[2]->Int32Value(context).ToChecked();
537-
int32_t weight = args[3]->Int32Value(context).ToChecked();
538-
bool exclusive = args[4]->BooleanValue(context).ToChecked();
539-
540-
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, "
541-
"parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
542-
options, parent, weight, exclusive);
532+
Http2Priority priority(env, args[2], args[3], args[4]);
543533

544-
nghttp2_priority_spec prispec;
545-
nghttp2_priority_spec_init(&prispec, parent, weight, exclusive ? 1 : 0);
534+
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d\n",
535+
headers->Length(), options);
546536

547537
Headers list(isolate, context, headers);
548538

549-
int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
539+
int32_t ret = session->Nghttp2Session::SubmitRequest(*priority,
550540
*list, list.length(),
551541
nullptr, options);
552542
DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret);
Collapse file

‎src/node_http2.h‎

Copy file name to clipboardExpand all lines: src/node_http2.h
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,20 @@ class Http2Settings {
368368
MaybeStackBuffer<nghttp2_settings_entry, IDX_SETTINGS_COUNT> entries_;
369369
};
370370

371+
class Http2Priority {
372+
public:
373+
Http2Priority(Environment* env,
374+
Local<Value> parent,
375+
Local<Value> weight,
376+
Local<Value> exclusive);
377+
378+
nghttp2_priority_spec* operator*() {
379+
return &spec;
380+
}
381+
private:
382+
nghttp2_priority_spec spec;
383+
};
384+
371385
class Http2Session : public AsyncWrap,
372386
public StreamBase,
373387
public Nghttp2Session {
Collapse file

‎src/node_http2_core-inl.h‎

Copy file name to clipboardExpand all lines: src/node_http2_core-inl.h
+22-41Lines changed: 22 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,21 @@ inline int Nghttp2Session::OnNghttpError(nghttp2_session* session,
2222
}
2323
#endif
2424

25+
inline int32_t GetFrameID(const nghttp2_frame* frame) {
26+
// If this is a push promise, we want to grab the id of the promised stream
27+
return (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
28+
frame->push_promise.promised_stream_id :
29+
frame->hd.stream_id;
30+
}
31+
2532
// nghttp2 calls this at the beginning a new HEADERS or PUSH_PROMISE frame.
2633
// We use it to ensure that an Nghttp2Stream instance is allocated to store
2734
// the state.
2835
inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session,
2936
const nghttp2_frame* frame,
3037
void* user_data) {
3138
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
32-
// If this is a push promise frame, we want to grab the handle of
33-
// the promised stream.
34-
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
35-
frame->push_promise.promised_stream_id :
36-
frame->hd.stream_id;
39+
int32_t id = GetFrameID(frame);
3740
DEBUG_HTTP2("Nghttp2Session %s: beginning headers for stream %d\n",
3841
handle->TypeName(), id);
3942

@@ -79,11 +82,7 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session,
7982
uint8_t flags,
8083
void* user_data) {
8184
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
82-
// If this is a push promise frame, we want to grab the handle of
83-
// the promised stream.
84-
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
85-
frame->push_promise.promised_stream_id :
86-
frame->hd.stream_id;
85+
int32_t id = GetFrameID(frame);
8786
Nghttp2Stream* stream = handle->FindStream(id);
8887
if (!stream->AddHeader(name, value, flags)) {
8988
// This will only happen if the connected peer sends us more
@@ -432,7 +431,7 @@ inline void Nghttp2Stream::FlushDataChunks() {
432431
// see if the END_STREAM flag is set, and will flush the queued data chunks
433432
// to JS if the stream is flowing
434433
inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
435-
int32_t id = frame->hd.stream_id;
434+
int32_t id = GetFrameID(frame);
436435
DEBUG_HTTP2("Nghttp2Session %s: handling data frame for stream %d\n",
437436
TypeName(), id);
438437
Nghttp2Stream* stream = this->FindStream(id);
@@ -450,8 +449,7 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
450449
// The headers are collected as the frame is being processed and sent out
451450
// to the JS side only when the frame is fully processed.
452451
inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
453-
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
454-
frame->push_promise.promised_stream_id : frame->hd.stream_id;
452+
int32_t id = GetFrameID(frame);
455453
DEBUG_HTTP2("Nghttp2Session %s: handling headers frame for stream %d\n",
456454
TypeName(), id);
457455
Nghttp2Stream* stream = FindStream(id);
@@ -469,7 +467,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
469467
// Notifies the JS layer that a PRIORITY frame has been received
470468
inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
471469
nghttp2_priority priority_frame = frame->priority;
472-
int32_t id = frame->hd.stream_id;
470+
int32_t id = GetFrameID(frame);
473471
DEBUG_HTTP2("Nghttp2Session %s: handling priority frame for stream %d\n",
474472
TypeName(), id);
475473

@@ -571,41 +569,24 @@ inline int Nghttp2Session::Init(const nghttp2_session_type type,
571569
session_type_ = type;
572570
DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName());
573571
destroying_ = false;
574-
int ret = 0;
575572

576573
max_header_pairs_ = maxHeaderPairs;
577574

578575
nghttp2_session_callbacks* callbacks
579576
= callback_struct_saved[HasGetPaddingCallback() ? 1 : 0].callbacks;
580577

581-
nghttp2_option* opts;
582-
if (options != nullptr) {
583-
opts = options;
584-
} else {
585-
nghttp2_option_new(&opts);
586-
}
578+
CHECK_NE(options, nullptr);
587579

588-
switch (type) {
589-
case NGHTTP2_SESSION_SERVER:
590-
ret = nghttp2_session_server_new3(&session_,
591-
callbacks,
592-
this,
593-
opts,
594-
mem);
595-
break;
596-
case NGHTTP2_SESSION_CLIENT:
597-
ret = nghttp2_session_client_new3(&session_,
598-
callbacks,
599-
this,
600-
opts,
601-
mem);
602-
break;
603-
}
604-
if (opts != options) {
605-
nghttp2_option_del(opts);
606-
}
580+
typedef int (*init_fn)(nghttp2_session** session,
581+
const nghttp2_session_callbacks* callbacks,
582+
void* user_data,
583+
const nghttp2_option* options,
584+
nghttp2_mem* mem);
585+
init_fn fn = type == NGHTTP2_SESSION_SERVER ?
586+
nghttp2_session_server_new3 :
587+
nghttp2_session_client_new3;
607588

608-
return ret;
589+
return fn(&session_, callbacks, this, options, mem);
609590
}
610591

611592
inline void Nghttp2Session::MarkDestroying() {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.