FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: multiple smaller code cleanups · nodejs/node@29efb02 · GitHub

/ node Public

Commit 29efb02

Browse files
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

3 files changed

Lines changed: 57 additions & 72 deletions

File tree

‎src/node_http2.cc‎

Lines 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);

‎src/node_http2.h‎

Lines 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 {

‎src/node_http2_core-inl.h‎

Lines 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)
Back | FazBrowse Home | New Git URL