Skip to content

Commit

Permalink
http2: multiple smaller code cleanups
Browse files Browse the repository at this point in the history 
* Add Http2Priority utility class
* Reduces some code duplication
* Other small minor cleanups

PR-URL: #16764
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
  • Loading branch information
@jasnell @evanlucas
jasnell authored and evanlucas committed Nov 13, 2017
1 parent 7964849 commit 8dfd5a5
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 72 deletions.
52 changes: 21 additions & 31 deletions src/node_http2.cc
View file
Expand Up @@ -186,6 +186,18 @@ inline void Http2Settings::RefreshDefaults(Environment* env) {
(1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
}

Http2Priority::Http2Priority(Environment* env,
Local<Value> parent,
Local<Value> weight,
Local<Value> exclusive) {
Local<Context> context = env->context();
int32_t parent_ = parent->Int32Value(context).ToChecked();
int32_t weight_ = weight->Int32Value(context).ToChecked();
bool exclusive_ = exclusive->BooleanValue(context).ToChecked();
DEBUG_HTTP2("Http2Priority: parent: %d, weight: %d, exclusive: %d\n",
parent_, weight_, exclusive_);
nghttp2_priority_spec_init(&spec, parent_, weight_, exclusive_ ? 1 : 0);
}

Http2Session::Http2Session(Environment* env,
Local<Object> wrap,
Expand Down Expand Up @@ -267,12 +279,8 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
buffer[PADDING_BUF_RETURN_VALUE] = frameLen;
MakeCallback(env()->ongetpadding_string(), 0, nullptr);
uint32_t retval = buffer[PADDING_BUF_RETURN_VALUE];
retval = retval <= maxPayloadLen ? retval : maxPayloadLen;
retval = retval >= frameLen ? retval : frameLen;
#if defined(DEBUG) && DEBUG
CHECK_GE(retval, frameLen);
CHECK_LE(retval, maxPayloadLen);
#endif
retval = std::min(retval, static_cast<uint32_t>(maxPayloadLen));
retval = std::max(retval, static_cast<uint32_t>(frameLen));
return retval;
}

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

nghttp2_priority_spec spec;
int32_t id = args[0]->Int32Value(context).ToChecked();
int32_t parent = args[1]->Int32Value(context).ToChecked();
int32_t weight = args[2]->Int32Value(context).ToChecked();
bool exclusive = args[3]->BooleanValue(context).ToChecked();
Http2Priority priority(env, args[1], args[2], args[3]);
bool silent = args[4]->BooleanValue(context).ToChecked();
DEBUG_HTTP2("Http2Session: submitting priority for stream %d: "
"parent: %d, weight: %d, exclusive: %d, silent: %d\n",
id, parent, weight, exclusive, silent);

#if defined(DEBUG) && DEBUG
CHECK_GT(id, 0);
CHECK_GE(parent, 0);
CHECK_GE(weight, 0);
#endif
DEBUG_HTTP2("Http2Session: submitting priority for stream %d", id);

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

args.GetReturnValue().Set(stream->SubmitPriority(&spec, silent));
args.GetReturnValue().Set(stream->SubmitPriority(*priority, silent));
}

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

Local<Array> headers = args[0].As<Array>();
int options = args[1]->IntegerValue(context).ToChecked();
int32_t parent = args[2]->Int32Value(context).ToChecked();
int32_t weight = args[3]->Int32Value(context).ToChecked();
bool exclusive = args[4]->BooleanValue(context).ToChecked();

DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, "
"parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
options, parent, weight, exclusive);
Http2Priority priority(env, args[2], args[3], args[4]);

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

Headers list(isolate, context, headers);

int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
int32_t ret = session->Nghttp2Session::SubmitRequest(*priority,
*list, list.length(),
nullptr, options);
DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret);
Expand Down
14 changes: 14 additions & 0 deletions src/node_http2.h
View file
Expand Up @@ -368,6 +368,20 @@ class Http2Settings {
MaybeStackBuffer<nghttp2_settings_entry, IDX_SETTINGS_COUNT> entries_;
};

class Http2Priority {
public:
Http2Priority(Environment* env,
Local<Value> parent,
Local<Value> weight,
Local<Value> exclusive);

nghttp2_priority_spec* operator*() {
return &spec;
}
private:
nghttp2_priority_spec spec;
};

class Http2Session : public AsyncWrap,
public StreamBase,
public Nghttp2Session {
Expand Down
63 changes: 22 additions & 41 deletions src/node_http2_core-inl.h
View file
Expand Up @@ -22,18 +22,21 @@ inline int Nghttp2Session::OnNghttpError(nghttp2_session* session,
}
#endif

inline int32_t GetFrameID(const nghttp2_frame* frame) {
// If this is a push promise, we want to grab the id of the promised stream
return (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
}

// nghttp2 calls this at the beginning a new HEADERS or PUSH_PROMISE frame.
// We use it to ensure that an Nghttp2Stream instance is allocated to store
// the state.
inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session,
const nghttp2_frame* frame,
void* user_data) {
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
// If this is a push promise frame, we want to grab the handle of
// the promised stream.
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: beginning headers for stream %d\n",
handle->TypeName(), id);

Expand Down Expand Up @@ -79,11 +82,7 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session,
uint8_t flags,
void* user_data) {
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
// If this is a push promise frame, we want to grab the handle of
// the promised stream.
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
int32_t id = GetFrameID(frame);
Nghttp2Stream* stream = handle->FindStream(id);
if (!stream->AddHeader(name, value, flags)) {
// This will only happen if the connected peer sends us more
Expand Down Expand Up @@ -432,7 +431,7 @@ inline void Nghttp2Stream::FlushDataChunks() {
// see if the END_STREAM flag is set, and will flush the queued data chunks
// to JS if the stream is flowing
inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
int32_t id = frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: handling data frame for stream %d\n",
TypeName(), id);
Nghttp2Stream* stream = this->FindStream(id);
Expand All @@ -450,8 +449,7 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
// The headers are collected as the frame is being processed and sent out
// to the JS side only when the frame is fully processed.
inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id : frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: handling headers frame for stream %d\n",
TypeName(), id);
Nghttp2Stream* stream = FindStream(id);
Expand All @@ -469,7 +467,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
// Notifies the JS layer that a PRIORITY frame has been received
inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
nghttp2_priority priority_frame = frame->priority;
int32_t id = frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: handling priority frame for stream %d\n",
TypeName(), id);

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

max_header_pairs_ = maxHeaderPairs;

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

nghttp2_option* opts;
if (options != nullptr) {
opts = options;
} else {
nghttp2_option_new(&opts);
}
CHECK_NE(options, nullptr);

switch (type) {
case NGHTTP2_SESSION_SERVER:
ret = nghttp2_session_server_new3(&session_,
callbacks,
this,
opts,
mem);
break;
case NGHTTP2_SESSION_CLIENT:
ret = nghttp2_session_client_new3(&session_,
callbacks,
this,
opts,
mem);
break;
}
if (opts != options) {
nghttp2_option_del(opts);
}
typedef int (*init_fn)(nghttp2_session** session,
const nghttp2_session_callbacks* callbacks,
void* user_data,
const nghttp2_option* options,
nghttp2_mem* mem);
init_fn fn = type == NGHTTP2_SESSION_SERVER ?
nghttp2_session_server_new3 :
nghttp2_session_client_new3;

return ret;
return fn(&session_, callbacks, this, options, mem);
}

inline void Nghttp2Session::MarkDestroying() {
Expand Down

0 comments on commit 8dfd5a5

Please sign in to comment.