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 29efb02

Browse filesBrowse files
jasnellMylesBorins
jasnell
authored and
MylesBorins
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 dc14c25 commit 29efb02
Copy full SHA for 29efb02

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
@@ -180,6 +180,18 @@ inline void Http2Settings::RefreshDefaults(Environment* env) {
180180
(1 << IDX_SETTINGS_MAX_FRAME_SIZE);
181181
}
182182

183+
Http2Priority::Http2Priority(Environment* env,
184+
Local<Value> parent,
185+
Local<Value> weight,
186+
Local<Value> exclusive) {
187+
Local<Context> context = env->context();
188+
int32_t parent_ = parent->Int32Value(context).ToChecked();
189+
int32_t weight_ = weight->Int32Value(context).ToChecked();
190+
bool exclusive_ = exclusive->BooleanValue(context).ToChecked();
191+
DEBUG_HTTP2("Http2Priority: parent: %d, weight: %d, exclusive: %d\n",
192+
parent_, weight_, exclusive_);
193+
nghttp2_priority_spec_init(&spec, parent_, weight_, exclusive_ ? 1 : 0);
194+
}
183195

184196
Http2Session::Http2Session(Environment* env,
185197
Local<Object> wrap,
@@ -258,12 +270,8 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
258270
buffer[PADDING_BUF_RETURN_VALUE] = frameLen;
259271
MakeCallback(env()->ongetpadding_string(), 0, nullptr);
260272
uint32_t retval = buffer[PADDING_BUF_RETURN_VALUE];
261-
retval = retval <= maxPayloadLen ? retval : maxPayloadLen;
262-
retval = retval >= frameLen ? retval : frameLen;
263-
#if defined(DEBUG) && DEBUG
264-
CHECK_GE(retval, frameLen);
265-
CHECK_LE(retval, maxPayloadLen);
266-
#endif
273+
retval = std::min(retval, static_cast<uint32_t>(maxPayloadLen));
274+
retval = std::max(retval, static_cast<uint32_t>(frameLen));
267275
return retval;
268276
}
269277

@@ -445,30 +453,18 @@ void Http2Session::SubmitPriority(const FunctionCallbackInfo<Value>& args) {
445453
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
446454
Local<Context> context = env->context();
447455

448-
nghttp2_priority_spec spec;
449456
int32_t id = args[0]->Int32Value(context).ToChecked();
450-
int32_t parent = args[1]->Int32Value(context).ToChecked();
451-
int32_t weight = args[2]->Int32Value(context).ToChecked();
452-
bool exclusive = args[3]->BooleanValue(context).ToChecked();
457+
Http2Priority priority(env, args[1], args[2], args[3]);
453458
bool silent = args[4]->BooleanValue(context).ToChecked();
454-
DEBUG_HTTP2("Http2Session: submitting priority for stream %d: "
455-
"parent: %d, weight: %d, exclusive: %d, silent: %d\n",
456-
id, parent, weight, exclusive, silent);
457-
458-
#if defined(DEBUG) && DEBUG
459-
CHECK_GT(id, 0);
460-
CHECK_GE(parent, 0);
461-
CHECK_GE(weight, 0);
462-
#endif
459+
DEBUG_HTTP2("Http2Session: submitting priority for stream %d", id);
463460

464461
Nghttp2Stream* stream;
465462
if (!(stream = session->FindStream(id))) {
466463
// invalid stream
467464
return args.GetReturnValue().Set(NGHTTP2_ERR_INVALID_STREAM_ID);
468465
}
469-
nghttp2_priority_spec_init(&spec, parent, weight, exclusive ? 1 : 0);
470466

471-
args.GetReturnValue().Set(stream->SubmitPriority(&spec, silent));
467+
args.GetReturnValue().Set(stream->SubmitPriority(*priority, silent));
472468
}
473469

474470
void Http2Session::SubmitSettings(const FunctionCallbackInfo<Value>& args) {
@@ -524,20 +520,14 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
524520

525521
Local<Array> headers = args[0].As<Array>();
526522
int options = args[1]->IntegerValue(context).ToChecked();
527-
int32_t parent = args[2]->Int32Value(context).ToChecked();
528-
int32_t weight = args[3]->Int32Value(context).ToChecked();
529-
bool exclusive = args[4]->BooleanValue(context).ToChecked();
530-
531-
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, "
532-
"parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
533-
options, parent, weight, exclusive);
523+
Http2Priority priority(env, args[2], args[3], args[4]);
534524

535-
nghttp2_priority_spec prispec;
536-
nghttp2_priority_spec_init(&prispec, parent, weight, exclusive ? 1 : 0);
525+
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d\n",
526+
headers->Length(), options);
537527

538528
Headers list(isolate, context, headers);
539529

540-
int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
530+
int32_t ret = session->Nghttp2Session::SubmitRequest(*priority,
541531
*list, list.length(),
542532
nullptr, options);
543533
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
@@ -369,6 +369,20 @@ class Http2Settings {
369369
MaybeStackBuffer<nghttp2_settings_entry, IDX_SETTINGS_COUNT> entries_;
370370
};
371371

372+
class Http2Priority {
373+
public:
374+
Http2Priority(Environment* env,
375+
Local<Value> parent,
376+
Local<Value> weight,
377+
Local<Value> exclusive);
378+
379+
nghttp2_priority_spec* operator*() {
380+
return &spec;
381+
}
382+
private:
383+
nghttp2_priority_spec spec;
384+
};
385+
372386
class Http2Session : public AsyncWrap,
373387
public StreamBase,
374388
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
@@ -28,18 +28,21 @@ inline int Nghttp2Session::OnNghttpError(nghttp2_session* session,
2828
}
2929
#endif
3030

31+
inline int32_t GetFrameID(const nghttp2_frame* frame) {
32+
// If this is a push promise, we want to grab the id of the promised stream
33+
return (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
34+
frame->push_promise.promised_stream_id :
35+
frame->hd.stream_id;
36+
}
37+
3138
// nghttp2 calls this at the beginning a new HEADERS or PUSH_PROMISE frame.
3239
// We use it to ensure that an Nghttp2Stream instance is allocated to store
3340
// the state.
3441
inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session,
3542
const nghttp2_frame* frame,
3643
void* user_data) {
3744
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
38-
// If this is a push promise frame, we want to grab the handle of
39-
// the promised stream.
40-
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
41-
frame->push_promise.promised_stream_id :
42-
frame->hd.stream_id;
45+
int32_t id = GetFrameID(frame);
4346
DEBUG_HTTP2("Nghttp2Session %s: beginning headers for stream %d\n",
4447
handle->TypeName(), id);
4548

@@ -62,11 +65,7 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session,
6265
uint8_t flags,
6366
void* user_data) {
6467
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
65-
// If this is a push promise frame, we want to grab the handle of
66-
// the promised stream.
67-
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
68-
frame->push_promise.promised_stream_id :
69-
frame->hd.stream_id;
68+
int32_t id = GetFrameID(frame);
7069
Nghttp2Stream* stream = handle->FindStream(id);
7170
// The header name and value are stored in a reference counted buffer
7271
// provided to us by nghttp2. We need to increment the reference counter
@@ -418,7 +417,7 @@ inline void Nghttp2Stream::FlushDataChunks() {
418417
// see if the END_STREAM flag is set, and will flush the queued data chunks
419418
// to JS if the stream is flowing
420419
inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
421-
int32_t id = frame->hd.stream_id;
420+
int32_t id = GetFrameID(frame);
422421
DEBUG_HTTP2("Nghttp2Session %s: handling data frame for stream %d\n",
423422
TypeName(), id);
424423
Nghttp2Stream* stream = this->FindStream(id);
@@ -436,8 +435,7 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
436435
// The headers are collected as the frame is being processed and sent out
437436
// to the JS side only when the frame is fully processed.
438437
inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
439-
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
440-
frame->push_promise.promised_stream_id : frame->hd.stream_id;
438+
int32_t id = GetFrameID(frame);
441439
DEBUG_HTTP2("Nghttp2Session %s: handling headers frame for stream %d\n",
442440
TypeName(), id);
443441
Nghttp2Stream* stream = FindStream(id);
@@ -454,7 +452,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
454452
// Notifies the JS layer that a PRIORITY frame has been received
455453
inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
456454
nghttp2_priority priority_frame = frame->priority;
457-
int32_t id = frame->hd.stream_id;
455+
int32_t id = GetFrameID(frame);
458456
DEBUG_HTTP2("Nghttp2Session %s: handling priority frame for stream %d\n",
459457
TypeName(), id);
460458

@@ -548,39 +546,22 @@ inline int Nghttp2Session::Init(const nghttp2_session_type type,
548546
session_type_ = type;
549547
DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName());
550548
destroying_ = false;
551-
int ret = 0;
552549

553550
nghttp2_session_callbacks* callbacks
554551
= callback_struct_saved[HasGetPaddingCallback() ? 1 : 0].callbacks;
555552

556-
nghttp2_option* opts;
557-
if (options != nullptr) {
558-
opts = options;
559-
} else {
560-
nghttp2_option_new(&opts);
561-
}
553+
CHECK_NE(options, nullptr);
562554

563-
switch (type) {
564-
case NGHTTP2_SESSION_SERVER:
565-
ret = nghttp2_session_server_new3(&session_,
566-
callbacks,
567-
this,
568-
opts,
569-
mem);
570-
break;
571-
case NGHTTP2_SESSION_CLIENT:
572-
ret = nghttp2_session_client_new3(&session_,
573-
callbacks,
574-
this,
575-
opts,
576-
mem);
577-
break;
578-
}
579-
if (opts != options) {
580-
nghttp2_option_del(opts);
581-
}
555+
typedef int (*init_fn)(nghttp2_session** session,
556+
const nghttp2_session_callbacks* callbacks,
557+
void* user_data,
558+
const nghttp2_option* options,
559+
nghttp2_mem* mem);
560+
init_fn fn = type == NGHTTP2_SESSION_SERVER ?
561+
nghttp2_session_server_new3 :
562+
nghttp2_session_client_new3;
582563

583-
return ret;
564+
return fn(&session_, callbacks, this, options, mem);
584565
}
585566

586567
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.