http2: refactor and cleanup http2

* cleanup constants in http2 binding
  The error constants were just doing some weird things. Cleanup
  and improve maintainability.
* simplify settings to reduce duplicate code
* improve style consistency and correctness
  Use snake_case for getters and setters at c++ level,
  avoid unnecessary use of enums,
  use consistent style for exported vs. internal constants,
  avoid unnecessary memory info reporting,
  use setters/getters for flags_ for improved code readability
* make EmitStatistics function private
* un-nest Http2Settings and Http2Ping
* refactoring and cleanup of Http2Settings and Http2Ping
* avoid ** syntax for readability
  The **session and **stream syntax for getting the underlying
  nghttp2 pointers is not ideal for readability
* use const references for Http2Priority
* remove unnecessary GetStream function
* refactor Http2Scope to use BaseObjectPtr
* move utility function to anonymous namespace
* refactor and simplify Origins
* Use an AllocatedBuffer instead of MaybeStackBuffer
* Use a const reference instead of pointer
* use BaseObjectPtr for Http2Streams map
* move MemoryInfo impl to cc

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/32884
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

1 files changed, 479 insertions, 515 deletions

diff --git a/src/node_http2.cc b/src/node_http2.cc
index 7477bfbb6d..385a235204 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -21,6 +21,7 @@ using v8::ArrayBuffer;
 using v8::ArrayBufferView;
 using v8::Boolean;
 using v8::Context;
+using v8::EscapableHandleScope;
 using v8::Float64Array;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -30,7 +31,6 @@ using v8::Integer;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
-using v8::NewStringType;
 using v8::Number;
 using v8::Object;
 using v8::ObjectTemplate;
@@ -48,15 +48,9 @@ namespace {
 
 const char zero_bytes_256[256] = {};
 
-inline Http2Stream* GetStream(Http2Session* session,
-                              int32_t id,
-                              nghttp2_data_source* source) {
-  Http2Stream* stream = static_cast<Http2Stream*>(source->ptr);
-  if (stream == nullptr)
-    stream = session->FindStream(id);
-  CHECK_NOT_NULL(stream);
-  CHECK_EQ(id, stream->id());
-  return stream;
+bool HasHttp2Observer(Environment* env) {
+  AliasedUint32Array& observers = env->performance_state()->observers;
+  return observers[performance::NODE_PERFORMANCE_ENTRY_TYPE_HTTP2] != 0;
 }
 
 }  // anonymous namespace
@@ -75,36 +69,27 @@ const Http2Session::Callbacks Http2Session::callback_struct_saved[2] = {
 // For example:
 //
 // Http2Scope h2scope(session);
-// nghttp2_submit_ping(**session, ... );
+// nghttp2_submit_ping(session->session(), ... );
 //
 // When the Http2Scope passes out of scope and is deconstructed, it will
 // call Http2Session::MaybeScheduleWrite().
 Http2Scope::Http2Scope(Http2Stream* stream) : Http2Scope(stream->session()) {}
 
-Http2Scope::Http2Scope(Http2Session* session) {
-  if (session == nullptr)
-    return;
+Http2Scope::Http2Scope(Http2Session* session) : session_(session) {
+  if (!session_) return;
 
-  if (session->flags_ & (SESSION_STATE_HAS_SCOPE |
-                         SESSION_STATE_WRITE_SCHEDULED)) {
-    // There is another scope further below on the stack, or it is already
-    // known that a write is scheduled. In either case, there is nothing to do.
+  // If there is another scope further below on the stack, or
+  // a write is already scheduled, there's nothing to do.
+  if (session_->is_in_scope() || session_->is_write_scheduled()) {
+    session_.reset();
     return;
   }
-  session->flags_ |= SESSION_STATE_HAS_SCOPE;
-  session_ = session;
-
-  // Always keep the session object alive for at least as long as
-  // this scope is active.
-  session_handle_ = session->object();
-  CHECK(!session_handle_.IsEmpty());
+  session_->set_in_scope();
 }
 
 Http2Scope::~Http2Scope() {
-  if (session_ == nullptr)
-    return;
-
-  session_->flags_ &= ~SESSION_STATE_HAS_SCOPE;
+  if (!session_) return;
+  session_->set_in_scope(false);
   session_->MaybeScheduleWrite();
 }
 
@@ -112,7 +97,7 @@ Http2Scope::~Http2Scope() {
 // instances to configure an appropriate nghttp2_options struct. The class
 // uses a single TypedArray instance that is shared with the JavaScript side
 // to more efficiently pass values back and forth.
-Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) {
+Http2Options::Http2Options(Http2State* http2_state, SessionType type) {
   nghttp2_option* option;
   CHECK_EQ(nghttp2_option_new(&option), 0);
   CHECK_NOT_NULL(option);
@@ -171,10 +156,10 @@ Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) {
   // this is set on a per-session basis, but eventually we may switch to
   // a per-stream setting, giving users greater control
   if (flags & (1 << IDX_OPTIONS_PADDING_STRATEGY)) {
-    padding_strategy_type strategy =
-        static_cast<padding_strategy_type>(
+    PaddingStrategy strategy =
+        static_cast<PaddingStrategy>(
             buffer.GetValue(IDX_OPTIONS_PADDING_STRATEGY));
-    SetPaddingStrategy(strategy);
+    set_padding_strategy(strategy);
   }
 
   // The max header list pairs option controls the maximum number of
@@ -182,7 +167,7 @@ Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) {
   // if the remote peer sends more than this amount, the stream will be
   // automatically closed with an RST_STREAM.
   if (flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS))
-    SetMaxHeaderPairs(buffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS]);
+    set_max_header_pairs(buffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS]);
 
   // The HTTP2 specification places no limits on the number of HTTP2
   // PING frames that can be sent. In order to prevent PINGS from being
@@ -190,7 +175,7 @@ Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) {
   // on the number of unacknowledged PINGS that can be sent at any given
   // time.
   if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS))
-    SetMaxOutstandingPings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS]);
+    set_max_outstanding_pings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS]);
 
   // The HTTP2 specification places no limits on the number of HTTP2
   // SETTINGS frames that can be sent. In order to prevent PINGS from being
@@ -198,7 +183,7 @@ Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) {
   // on the number of unacknowledged SETTINGS that can be sent at any given
   // time.
   if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS))
-    SetMaxOutstandingSettings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS]);
+    set_max_outstanding_settings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS]);
 
   // The HTTP2 specification places no limits on the amount of memory
   // that a session can consume. In order to prevent abuse, we place a
@@ -209,131 +194,133 @@ Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) {
   // Important: The maxSessionMemory option in javascript is expressed in
   //            terms of MB increments (i.e. the value 1 == 1 MB)
   if (flags & (1 << IDX_OPTIONS_MAX_SESSION_MEMORY))
-    SetMaxSessionMemory(buffer[IDX_OPTIONS_MAX_SESSION_MEMORY] * 1000000);
+    set_max_session_memory(buffer[IDX_OPTIONS_MAX_SESSION_MEMORY] * 1000000);
 }
 
-void Http2Session::Http2Settings::Init(Http2State* http2_state) {
+#define GRABSETTING(entries, count, name)                                      \
+  do {                                                                         \
+    if (flags & (1 << IDX_SETTINGS_ ## name)) {                                \
+      uint32_t val = buffer[IDX_SETTINGS_ ## name];                            \
+      entries[count++] =                                                       \
+          nghttp2_settings_entry {NGHTTP2_SETTINGS_ ## name, val};             \
+    } } while (0)
+
+size_t Http2Settings::Init(
+    Http2State* http2_state,
+    nghttp2_settings_entry* entries) {
   AliasedUint32Array& buffer = http2_state->settings_buffer;
   uint32_t flags = buffer[IDX_SETTINGS_COUNT];
 
-  size_t n = 0;
-
-#define GRABSETTING(N, trace)                                                 \
-  if (flags & (1 << IDX_SETTINGS_##N)) {                                      \
-    uint32_t val = buffer[IDX_SETTINGS_##N];                                  \
-    if (session_ != nullptr)                                                  \
-      Debug(session_, "setting " trace ": %d\n", val);                        \
-    entries_[n++] =                                                           \
-        nghttp2_settings_entry {NGHTTP2_SETTINGS_##N, val};                   \
-  }
+  size_t count = 0;
 
-  GRABSETTING(HEADER_TABLE_SIZE, "header table size");
-  GRABSETTING(MAX_CONCURRENT_STREAMS, "max concurrent streams");
-  GRABSETTING(MAX_FRAME_SIZE, "max frame size");
-  GRABSETTING(INITIAL_WINDOW_SIZE, "initial window size");
-  GRABSETTING(MAX_HEADER_LIST_SIZE, "max header list size");
-  GRABSETTING(ENABLE_PUSH, "enable push");
-  GRABSETTING(ENABLE_CONNECT_PROTOCOL, "enable connect protocol");
-
-#undef GRABSETTING
+#define V(name) GRABSETTING(entries, count, name);
+  HTTP2_SETTINGS(V)
+#undef V
 
-  count_ = n;
+  return count;
 }
+#undef GRABSETTING
 
 // The Http2Settings class is used to configure a SETTINGS frame that is
 // to be sent to the connected peer. The settings are set using a TypedArray
 // that is shared with the JavaScript side.
-Http2Session::Http2Settings::Http2Settings(Http2State* http2_state,
-                                           Http2Session* session,
-                                           Local<Object> obj,
-                                           uint64_t start_time)
-    : AsyncWrap(http2_state->env(), obj, PROVIDER_HTTP2SETTINGS),
+Http2Settings::Http2Settings(Http2Session* session,
+                             Local<Object> obj,
+                             Local<Function> callback,
+                             uint64_t start_time)
+    : AsyncWrap(session->env(), obj, PROVIDER_HTTP2SETTINGS),
       session_(session),
       startTime_(start_time) {
-  Init(http2_state);
+  callback_.Reset(env()->isolate(), callback);
+  count_ = Init(session->http2_state(), entries_);
+}
+
+Local<Function> Http2Settings::callback() const {
+  return callback_.Get(env()->isolate());
+}
+
+void Http2Settings::MemoryInfo(MemoryTracker* tracker) const {
+  tracker->TrackField("callback", callback_);
 }
 
 // Generates a Buffer that contains the serialized payload of a SETTINGS
 // frame. This can be used, for instance, to create the Base64-encoded
 // content of an Http2-Settings header field.
-Local<Value> Http2Session::Http2Settings::Pack() {
-  const size_t len = count_ * 6;
-  Local<Value> buf = Buffer::New(env(), len).ToLocalChecked();
+Local<Value> Http2Settings::Pack() {
+  return Pack(session_->env(), count_, entries_);
+}
+
+Local<Value> Http2Settings::Pack(Http2State* state) {
+  nghttp2_settings_entry entries[IDX_SETTINGS_COUNT];
+  size_t count = Init(state, entries);
+  return Pack(state->env(), count, entries);
+}
+
+Local<Value> Http2Settings::Pack(
+    Environment* env,
+    size_t count,
+    const nghttp2_settings_entry* entries) {
+  EscapableHandleScope scope(env->isolate());
+  const size_t size = count * 6;
+  AllocatedBuffer buffer = env->AllocateManaged(size);
   ssize_t ret =
       nghttp2_pack_settings_payload(
-        reinterpret_cast<uint8_t*>(Buffer::Data(buf)), len,
-        &entries_[0], count_);
-  if (ret >= 0)
-    return buf;
-  else
-    return Undefined(env()->isolate());
+          reinterpret_cast<uint8_t*>(buffer.data()),
+          size,
+          entries,
+          count);
+  Local<Value> buf = Undefined(env->isolate());
+  if (ret >= 0) buf = buffer.ToBuffer().ToLocalChecked();
+  return scope.Escape(buf);
 }
 
 // Updates the shared TypedArray with the current remote or local settings for
 // the session.
-void Http2Session::Http2Settings::Update(Http2Session* session,
-                                         get_setting fn) {
+void Http2Settings::Update(Http2Session* session, get_setting fn) {
   AliasedUint32Array& buffer = session->http2_state()->settings_buffer;
-  buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] =
-      fn(**session, NGHTTP2_SETTINGS_HEADER_TABLE_SIZE);
-  buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS] =
-      fn(**session, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
-  buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE] =
-      fn(**session, NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE);
-  buffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
-      fn(**session, NGHTTP2_SETTINGS_MAX_FRAME_SIZE);
-  buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
-      fn(**session, NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE);
-  buffer[IDX_SETTINGS_ENABLE_PUSH] =
-      fn(**session, NGHTTP2_SETTINGS_ENABLE_PUSH);
-  buffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL] =
-      fn(**session, NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL);
+
+#define V(name)                                                                \
+  buffer[IDX_SETTINGS_ ## name] =                                              \
+      fn(session->session(), NGHTTP2_SETTINGS_ ## name);
+  HTTP2_SETTINGS(V)
+#undef V
 }
 
 // Initializes the shared TypedArray with the default settings values.
-void Http2Session::Http2Settings::RefreshDefaults(Http2State* http2_state) {
+void Http2Settings::RefreshDefaults(Http2State* http2_state) {
   AliasedUint32Array& buffer = http2_state->settings_buffer;
+  uint32_t flags = 0;
+
+#define V(name)                                                            \
+  do {                                                                     \
+    buffer[IDX_SETTINGS_ ## name] = DEFAULT_SETTINGS_ ## name;             \
+    flags |= 1 << IDX_SETTINGS_ ## name;                                   \
+  } while (0);
+  HTTP2_SETTINGS(V)
+#undef V
+
+  buffer[IDX_SETTINGS_COUNT] = flags;
+}
 
-  buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] =
-      DEFAULT_SETTINGS_HEADER_TABLE_SIZE;
-  buffer[IDX_SETTINGS_ENABLE_PUSH] =
-      DEFAULT_SETTINGS_ENABLE_PUSH;
-  buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS] =
-      DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS;
-  buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE] =
-      DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE;
-  buffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
-      DEFAULT_SETTINGS_MAX_FRAME_SIZE;
-  buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
-      DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE;
-  buffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL] =
-      DEFAULT_SETTINGS_ENABLE_CONNECT_PROTOCOL;
-  buffer[IDX_SETTINGS_COUNT] =
-    (1 << IDX_SETTINGS_HEADER_TABLE_SIZE) |
-    (1 << IDX_SETTINGS_ENABLE_PUSH) |
-    (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS) |
-    (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE) |
-    (1 << IDX_SETTINGS_MAX_FRAME_SIZE) |
-    (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE) |
-    (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL);
-}
-
-
-void Http2Session::Http2Settings::Send() {
-  Http2Scope h2scope(session_);
-  CHECK_EQ(nghttp2_submit_settings(**session_, NGHTTP2_FLAG_NONE,
-                                   &entries_[0], count_), 0);
-}
-
-void Http2Session::Http2Settings::Done(bool ack) {
+
+void Http2Settings::Send() {
+  Http2Scope h2scope(session_.get());
+  CHECK_EQ(nghttp2_submit_settings(
+      session_->session(),
+      NGHTTP2_FLAG_NONE,
+      &entries_[0],
+      count_), 0);
+}
+
+void Http2Settings::Done(bool ack) {
   uint64_t end = uv_hrtime();
   double duration = (end - startTime_) / 1e6;
 
   Local<Value> argv[] = {
-    Boolean::New(env()->isolate(), ack),
+    ack ? v8::True(env()->isolate()) : v8::False(env()->isolate()),
     Number::New(env()->isolate(), duration)
   };
-  MakeCallback(env()->ondone_string(), arraysize(argv), argv);
+  MakeCallback(callback(), arraysize(argv), argv);
 }
 
 // The Http2Priority class initializes an appropriate nghttp2_priority_spec
@@ -364,34 +351,32 @@ const char* Http2Session::TypeName() const {
   }
 }
 
-Origins::Origins(Isolate* isolate,
-                 Local<Context> context,
-                 Local<String> origin_string,
-                 size_t origin_count) : count_(origin_count) {
+Origins::Origins(
+    Environment* env,
+    Local<String> origin_string,
+    size_t origin_count)
+    : count_(origin_count) {
   int origin_string_len = origin_string->Length();
   if (count_ == 0) {
     CHECK_EQ(origin_string_len, 0);
     return;
   }
 
-  // Allocate a single buffer with count_ nghttp2_nv structs, followed
-  // by the raw header data as passed from JS. This looks like:
-  // | possible padding | nghttp2_nv | nghttp2_nv | ... | header contents |
-  buf_.AllocateSufficientStorage((alignof(nghttp2_origin_entry) - 1) +
-                                 count_ * sizeof(nghttp2_origin_entry) +
-                                 origin_string_len);
+  buf_ = env->AllocateManaged((alignof(nghttp2_origin_entry) - 1) +
+                              count_ * sizeof(nghttp2_origin_entry) +
+                              origin_string_len);
 
   // Make sure the start address is aligned appropriately for an nghttp2_nv*.
   char* start = reinterpret_cast<char*>(
-      RoundUp(reinterpret_cast<uintptr_t>(*buf_),
+      RoundUp(reinterpret_cast<uintptr_t>(buf_.data()),
               alignof(nghttp2_origin_entry)));
   char* origin_contents = start + (count_ * sizeof(nghttp2_origin_entry));
   nghttp2_origin_entry* const nva =
       reinterpret_cast<nghttp2_origin_entry*>(start);
 
-  CHECK_LE(origin_contents + origin_string_len, *buf_ + buf_.length());
+  CHECK_LE(origin_contents + origin_string_len, buf_.data() + buf_.size());
   CHECK_EQ(origin_string->WriteOneByte(
-               isolate,
+               env->isolate(),
                reinterpret_cast<uint8_t*>(origin_contents),
                0,
                origin_string_len,
@@ -469,7 +454,7 @@ void Http2Session::DecreaseAllocatedSize(size_t size) {
 
 Http2Session::Http2Session(Http2State* http2_state,
                            Local<Object> wrap,
-                           nghttp2_session_type type)
+                           SessionType type)
     : AsyncWrap(http2_state->env(), wrap, AsyncWrap::PROVIDER_HTTP2SESSION),
       js_fields_(http2_state->env()->isolate()),
       session_type_(type),
@@ -480,18 +465,18 @@ Http2Session::Http2Session(Http2State* http2_state,
   // Capture the configuration options for this session
   Http2Options opts(http2_state, type);
 
-  max_session_memory_ = opts.GetMaxSessionMemory();
+  max_session_memory_ = opts.max_session_memory();
 
-  uint32_t maxHeaderPairs = opts.GetMaxHeaderPairs();
+  uint32_t maxHeaderPairs = opts.max_header_pairs();
   max_header_pairs_ =
       type == NGHTTP2_SESSION_SERVER
           ? GetServerMaxHeaderPairs(maxHeaderPairs)
           : GetClientMaxHeaderPairs(maxHeaderPairs);
 
-  max_outstanding_pings_ = opts.GetMaxOutstandingPings();
-  max_outstanding_settings_ = opts.GetMaxOutstandingSettings();
+  max_outstanding_pings_ = opts.max_outstanding_pings();
+  max_outstanding_settings_ = opts.max_outstanding_settings();
 
-  padding_strategy_ = opts.GetPaddingStrategy();
+  padding_strategy_ = opts.padding_strategy();
 
   bool hasGetPaddingCallback =
       padding_strategy_ != PADDING_STRATEGY_NONE;
@@ -525,7 +510,7 @@ Http2Session::Http2Session(Http2State* http2_state,
 }
 
 Http2Session::~Http2Session() {
-  CHECK_EQ(flags_ & SESSION_STATE_HAS_SCOPE, 0);
+  CHECK(!is_in_scope());
   Debug(this, "freeing nghttp2 session");
   // Explicitly reset session_ so the subsequent
   // current_nghttp2_memory_ check passes.
@@ -533,16 +518,23 @@ Http2Session::~Http2Session() {
   CHECK_EQ(current_nghttp2_memory_, 0);
 }
 
+void Http2Session::MemoryInfo(MemoryTracker* tracker) const {
+  tracker->TrackField("streams", streams_);
+  tracker->TrackField("outstanding_pings", outstanding_pings_);
+  tracker->TrackField("outstanding_settings", outstanding_settings_);
+  tracker->TrackField("outgoing_buffers", outgoing_buffers_);
+  tracker->TrackFieldWithSize("stream_buf", stream_buf_.len);
+  tracker->TrackFieldWithSize("outgoing_storage", outgoing_storage_.size());
+  tracker->TrackFieldWithSize("pending_rst_streams",
+                              pending_rst_streams_.size() * sizeof(int32_t));
+  tracker->TrackFieldWithSize("nghttp2_memory", current_nghttp2_memory_);
+}
+
 std::string Http2Session::diagnostic_name() const {
   return std::string("Http2Session ") + TypeName() + " (" +
       std::to_string(static_cast<int64_t>(get_async_id())) + ")";
 }
 
-inline bool HasHttp2Observer(Environment* env) {
-  AliasedUint32Array& observers = env->performance_state()->observers;
-  return observers[performance::NODE_PERFORMANCE_ENTRY_TYPE_HTTP2] != 0;
-}
-
 void Http2Stream::EmitStatistics() {
   CHECK_NOT_NULL(session());
   if (!HasHttp2Observer(env()))
@@ -615,13 +607,13 @@ void Http2Session::EmitStatistics() {
 void Http2Session::Close(uint32_t code, bool socket_closed) {
   Debug(this, "closing session");
 
-  if (flags_ & SESSION_STATE_CLOSING)
+  if (is_closing())
     return;
-  flags_ |= SESSION_STATE_CLOSING;
+  set_closing();
 
   // Stop reading on the i/o stream
   if (stream_ != nullptr) {
-    flags_ |= SESSION_STATE_READING_STOPPED;
+    set_reading_stopped();
     stream_->ReadStop();
   }
 
@@ -637,10 +629,10 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
     stream_->RemoveStreamListener(this);
   }
 
-  flags_ |= SESSION_STATE_CLOSED;
+  set_destroyed();
 
   // If we are writing we will get to make the callback in OnStreamAfterWrite.
-  if ((flags_ & SESSION_STATE_WRITE_IN_PROGRESS) == 0) {
+  if (!is_write_in_progress()) {
     Debug(this, "make done session callback");
     HandleScope scope(env()->isolate());
     MakeCallback(env()->ondone_string(), 0, nullptr);
@@ -663,12 +655,12 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
 
 // Locates an existing known stream by ID. nghttp2 has a similar method
 // but this is faster and does not fail if the stream is not found.
-inline Http2Stream* Http2Session::FindStream(int32_t id) {
+BaseObjectPtr<Http2Stream> Http2Session::FindStream(int32_t id) {
   auto s = streams_.find(id);
-  return s != streams_.end() ? s->second : nullptr;
+  return s != streams_.end() ? s->second : BaseObjectPtr<Http2Stream>();
 }
 
-inline bool Http2Session::CanAddStream() {
+bool Http2Session::CanAddStream() {
   uint32_t maxConcurrentStreams =
       nghttp2_session_get_local_settings(
           session_.get(), NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
@@ -677,12 +669,12 @@ inline bool Http2Session::CanAddStream() {
   // We can add a new stream so long as we are less than the current
   // maximum on concurrent streams and there's enough available memory
   return streams_.size() < maxSize &&
-         IsAvailableSessionMemory(sizeof(Http2Stream));
+         has_available_session_memory(sizeof(Http2Stream));
 }
 
-inline void Http2Session::AddStream(Http2Stream* stream) {
+void Http2Session::AddStream(Http2Stream* stream) {
   CHECK_GE(++statistics_.stream_count, 0);
-  streams_[stream->id()] = stream;
+  streams_[stream->id()] = BaseObjectPtr<Http2Stream>(stream);
   size_t size = streams_.size();
   if (size > statistics_.max_concurrent_streams)
     statistics_.max_concurrent_streams = size;
@@ -690,11 +682,16 @@ inline void Http2Session::AddStream(Http2Stream* stream) {
 }
 
 
-inline void Http2Session::RemoveStream(Http2Stream* stream) {
-  if (streams_.empty() || stream == nullptr)
-    return;  // Nothing to remove, item was never added?
-  streams_.erase(stream->id());
-  DecrementCurrentSessionMemory(sizeof(*stream));
+BaseObjectPtr<Http2Stream> Http2Session::RemoveStream(int32_t id) {
+  BaseObjectPtr<Http2Stream> stream;
+  if (streams_.empty())
+    return stream;
+  stream = FindStream(id);
+  if (stream) {
+    streams_.erase(id);
+    DecrementCurrentSessionMemory(sizeof(*stream));
+  }
+  return stream;
 }
 
 // Used as one of the Padding Strategy functions. Will attempt to ensure
@@ -738,7 +735,7 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
   Debug(this, "receiving %d bytes [wants data? %d]",
         read_len,
         nghttp2_session_want_read(session_.get()));
-  flags_ &= ~SESSION_STATE_NGHTTP2_RECV_PAUSED;
+  set_receive_paused(false);
   ssize_t ret =
     nghttp2_session_mem_recv(session_.get(),
                              reinterpret_cast<uint8_t*>(stream_buf_.base) +
@@ -746,8 +743,8 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
                              read_len);
   CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
 
-  if (flags_ & SESSION_STATE_NGHTTP2_RECV_PAUSED) {
-    CHECK_NE(flags_ & SESSION_STATE_READING_STOPPED, 0);
+  if (is_receive_paused()) {
+    CHECK(is_reading_stopped());
 
     CHECK_GT(ret, 0);
     CHECK_LE(static_cast<size_t>(ret), read_len);
@@ -770,14 +767,14 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
     return ret;
 
   // Send any data that was queued up while processing the received data.
-  if (!IsDestroyed()) {
+  if (!is_destroyed()) {
     SendPendingData();
   }
   return ret;
 }
 
 
-inline int32_t GetFrameID(const nghttp2_frame* frame) {
+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 :
@@ -796,10 +793,10 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
   int32_t id = GetFrameID(frame);
   Debug(session, "beginning headers for stream %d", id);
 
-  Http2Stream* stream = session->FindStream(id);
+  BaseObjectPtr<Http2Stream> stream = session->FindStream(id);
   // The common case is that we're creating a new stream. The less likely
   // case is that we're receiving a set of trailers
-  if (LIKELY(stream == nullptr)) {
+  if (LIKELY(!stream)) {
     if (UNLIKELY(!session->CanAddStream() ||
                  Http2Stream::New(session, id, frame->headers.cat) ==
                      nullptr)) {
@@ -807,13 +804,16 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
           session->js_fields_->max_rejected_streams)
         return NGHTTP2_ERR_CALLBACK_FAILURE;
       // Too many concurrent streams being opened
-      nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
-                                NGHTTP2_ENHANCE_YOUR_CALM);
+      nghttp2_submit_rst_stream(
+          session->session(),
+          NGHTTP2_FLAG_NONE,
+          id,
+          NGHTTP2_ENHANCE_YOUR_CALM);
       return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
     }
 
     session->rejected_stream_count_ = 0;
-  } else if (!stream->IsDestroyed()) {
+  } else if (!stream->is_destroyed()) {
     stream->StartHeaders(frame->headers.cat);
   }
   return 0;
@@ -830,15 +830,15 @@ int Http2Session::OnHeaderCallback(nghttp2_session* handle,
                                    void* user_data) {
   Http2Session* session = static_cast<Http2Session*>(user_data);
   int32_t id = GetFrameID(frame);
-  Http2Stream* stream = session->FindStream(id);
+  BaseObjectPtr<Http2Stream> stream = session->FindStream(id);
   // If stream is null at this point, either something odd has happened
   // or the stream was closed locally while header processing was occurring.
   // either way, do not proceed and close the stream.
-  if (UNLIKELY(stream == nullptr))
+  if (UNLIKELY(!stream))
     return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
 
   // If the stream has already been destroyed, ignore.
-  if (!stream->IsDestroyed() && !stream->AddHeader(name, value, flags)) {
+  if (!stream->is_destroyed() && !stream->AddHeader(name, value, flags)) {
     // This will only happen if the connected peer sends us more
     // than the allowed number of header items at any given time
     stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM);
@@ -976,10 +976,10 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
   Local<Context> context = env->context();
   Context::Scope context_scope(context);
   Debug(session, "stream %d closed with code: %d", id, code);
-  Http2Stream* stream = session->FindStream(id);
+  BaseObjectPtr<Http2Stream> stream = session->FindStream(id);
   // Intentionally ignore the callback if the stream does not exist or has
   // already been destroyed
-  if (stream == nullptr || stream->IsDestroyed())
+  if (!stream || stream->is_destroyed())
     return 0;
 
   stream->Close(code);
@@ -991,6 +991,7 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
   MaybeLocal<Value> answer =
     stream->MakeCallback(env->http2session_on_stream_close_function(),
                           1, &arg);
+  Local<Value> def = v8::False(env->isolate());
   if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
     // Skip to destroy
     stream->Destroy();
@@ -1037,9 +1038,10 @@ int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
   // so that it can send a WINDOW_UPDATE frame. This is a critical part of
   // the flow control process in http2
   CHECK_EQ(nghttp2_session_consume_connection(handle, len), 0);
-  Http2Stream* stream = session->FindStream(id);
+  BaseObjectPtr<Http2Stream> stream = session->FindStream(id);
+
   // If the stream has been destroyed, ignore this chunk
-  if (stream->IsDestroyed())
+  if (!stream || stream->is_destroyed())
     return 0;
 
   stream->statistics_.received_bytes += len;
@@ -1071,7 +1073,7 @@ int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
     // If the stream owner (e.g. the JS Http2Stream) wants more data, just
     // tell nghttp2 that all data has been consumed. Otherwise, defer until
     // more data is being requested.
-    if (stream->IsReading())
+    if (stream->is_reading())
       nghttp2_session_consume_stream(handle, id, avail);
     else
       stream->inbound_consumed_data_while_paused_ += avail;
@@ -1085,9 +1087,9 @@ int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
 
   // If we are currently waiting for a write operation to finish, we should
   // tell nghttp2 that we want to wait before we process more input data.
-  if (session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS) {
-    CHECK_NE(session->flags_ & SESSION_STATE_READING_STOPPED, 0);
-    session->flags_ |= SESSION_STATE_NGHTTP2_RECV_PAUSED;
+  if (session->is_write_in_progress()) {
+    CHECK(session->is_reading_stopped());
+    session->set_receive_paused();
     return NGHTTP2_ERR_PAUSE;
   }
 
@@ -1194,10 +1196,10 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
 
   int32_t id = GetFrameID(frame);
   Debug(this, "handle headers frame for stream %d", id);
-  Http2Stream* stream = FindStream(id);
+  BaseObjectPtr<Http2Stream> stream = FindStream(id);
 
   // If the stream has already been destroyed, ignore.
-  if (stream->IsDestroyed())
+  if (!stream || stream->is_destroyed())
     return;
 
   // The headers are stored as a vector of Http2Header instances.
@@ -1263,9 +1265,11 @@ void Http2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
 int Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
   int32_t id = GetFrameID(frame);
   Debug(this, "handling data frame for stream %d", id);
-  Http2Stream* stream = FindStream(id);
+  BaseObjectPtr<Http2Stream> stream = FindStream(id);
 
-  if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
+  if (stream &&
+      !stream->is_destroyed() &&
+      frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
     stream->EmitRead(UV_EOF);
   } else if (frame->hd.length == 0) {
     return 1;  // Consider 0-length frame without END_STREAM an error.
@@ -1292,6 +1296,9 @@ void Http2Session::HandleGoawayFrame(const nghttp2_frame* frame) {
 
   size_t length = goaway_frame.opaque_data_len;
   if (length > 0) {
+    // If the copy fails for any reason here, we just ignore it.
+    // The additional goaway data is completely optional and we
+    // shouldn't fail if we're not able to process it.
     argv[2] = Buffer::Copy(isolate,
                            reinterpret_cast<char*>(goaway_frame.opaque_data),
                            length).ToLocalChecked();
@@ -1317,14 +1324,8 @@ void Http2Session::HandleAltSvcFrame(const nghttp2_frame* frame) {
 
   Local<Value> argv[3] = {
     Integer::New(isolate, id),
-    String::NewFromOneByte(isolate,
-                           altsvc->origin,
-                           NewStringType::kNormal,
-                           altsvc->origin_len).ToLocalChecked(),
-    String::NewFromOneByte(isolate,
-                           altsvc->field_value,
-                           NewStringType::kNormal,
-                           altsvc->field_value_len).ToLocalChecked(),
+    OneByteString(isolate, altsvc->origin, altsvc->origin_len),
+    OneByteString(isolate, altsvc->field_value, altsvc->field_value_len)
   };
 
   MakeCallback(env()->http2session_on_altsvc_function(),
@@ -1347,10 +1348,7 @@ void Http2Session::HandleOriginFrame(const nghttp2_frame* frame) {
 
   for (size_t i = 0; i < nov; ++i) {
     const nghttp2_origin_entry& entry = origin->ov[i];
-    origin_v[i] =
-        String::NewFromOneByte(
-            isolate, entry.origin, NewStringType::kNormal, entry.origin_len)
-            .ToLocalChecked();
+    origin_v[i] = OneByteString(isolate, entry.origin, entry.origin_len);
   }
   Local<Value> holder = Array::New(isolate, origin_v.data(), origin_v.size());
   MakeCallback(env()->http2session_on_origin_function(), 1, &holder);
@@ -1384,9 +1382,10 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
 
   if (!(js_fields_->bitfield & (1 << kSessionHasPingListeners))) return;
   // Notify the session that a ping occurred
-  arg = Buffer::Copy(env(),
-                      reinterpret_cast<const char*>(frame->ping.opaque_data),
-                      8).ToLocalChecked();
+  arg = Buffer::Copy(
+      env(),
+      reinterpret_cast<const char*>(frame->ping.opaque_data),
+      8).ToLocalChecked();
   MakeCallback(env()->http2session_on_ping_function(), 1, &arg);
 }
 
@@ -1430,20 +1429,20 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
 void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
   Debug(this, "write finished with status %d", status);
 
-  CHECK_NE(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
-  flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
+  CHECK(is_write_in_progress());
+  set_write_in_progress(false);
 
   // Inform all pending writes about their completion.
   ClearOutgoing(status);
 
-  if ((flags_ & SESSION_STATE_READING_STOPPED) &&
-      !(flags_ & SESSION_STATE_WRITE_IN_PROGRESS) &&
+  if (is_reading_stopped() &&
+      !is_write_in_progress() &&
       nghttp2_session_want_read(session_.get())) {
-    flags_ &= ~SESSION_STATE_READING_STOPPED;
+    set_reading_stopped(false);
     stream_->ReadStart();
   }
 
-  if ((flags_ & SESSION_STATE_CLOSED) != 0) {
+  if (is_destroyed()) {
     HandleScope scope(env()->isolate());
     MakeCallback(env()->ondone_string(), 0, nullptr);
     return;
@@ -1454,7 +1453,7 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
     ConsumeHTTP2Data();
   }
 
-  if (!(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
+  if (!is_write_scheduled()) {
     // Schedule a new write if nghttp2 wants to send data.
     MaybeScheduleWrite();
   }
@@ -1465,17 +1464,17 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
 // on the next iteration of the Node.js event loop (using the SetImmediate
 // queue), but only if a write has not already been scheduled.
 void Http2Session::MaybeScheduleWrite() {
-  CHECK_EQ(flags_ & SESSION_STATE_WRITE_SCHEDULED, 0);
+  CHECK(!is_write_scheduled());
   if (UNLIKELY(!session_))
     return;
 
   if (nghttp2_session_want_write(session_.get())) {
     HandleScope handle_scope(env()->isolate());
     Debug(this, "scheduling write");
-    flags_ |= SESSION_STATE_WRITE_SCHEDULED;
+    set_write_scheduled();
     BaseObjectPtr<Http2Session> strong_ref{this};
     env()->SetImmediate([this, strong_ref](Environment* env) {
-      if (!session_ || !(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
+      if (!session_ || !is_write_scheduled()) {
         // This can happen e.g. when a stream was reset before this turn
         // of the event loop, in which case SendPendingData() is called early,
         // or the session was destroyed in the meantime.
@@ -1492,11 +1491,11 @@ void Http2Session::MaybeScheduleWrite() {
 }
 
 void Http2Session::MaybeStopReading() {
-  if (flags_ & SESSION_STATE_READING_STOPPED) return;
+  if (is_reading_stopped()) return;
   int want_read = nghttp2_session_want_read(session_.get());
   Debug(this, "wants read? %d", want_read);
-  if (want_read == 0 || (flags_ & SESSION_STATE_WRITE_IN_PROGRESS)) {
-    flags_ |= SESSION_STATE_READING_STOPPED;
+  if (want_read == 0 || is_write_in_progress()) {
+    set_reading_stopped();
     stream_->ReadStop();
   }
 }
@@ -1504,9 +1503,9 @@ void Http2Session::MaybeStopReading() {
 // Unset the sending state, finish up all current writes, and reset
 // storage for data and metadata that was associated with these writes.
 void Http2Session::ClearOutgoing(int status) {
-  CHECK_NE(flags_ & SESSION_STATE_SENDING, 0);
+  CHECK(is_sending());
 
-  flags_ &= ~SESSION_STATE_SENDING;
+  set_sending(false);
 
   if (outgoing_buffers_.size() > 0) {
     outgoing_storage_.clear();
@@ -1534,8 +1533,8 @@ void Http2Session::ClearOutgoing(int status) {
     SendPendingData();
 
     for (int32_t stream_id : current_pending_rst_streams) {
-      Http2Stream* stream = FindStream(stream_id);
-      if (LIKELY(stream != nullptr))
+      BaseObjectPtr<Http2Stream> stream = FindStream(stream_id);
+      if (LIKELY(stream))
         stream->FlushRstStream();
     }
   }
@@ -1573,15 +1572,15 @@ uint8_t Http2Session::SendPendingData() {
   // Do not attempt to send data on the socket if the destroying flag has
   // been set. That means everything is shutting down and the socket
   // will not be usable.
-  if (IsDestroyed())
+  if (is_destroyed())
     return 0;
-  flags_ &= ~SESSION_STATE_WRITE_SCHEDULED;
+  set_write_scheduled(false);
 
   // SendPendingData should not be called recursively.
-  if (flags_ & SESSION_STATE_SENDING)
+  if (is_sending())
     return 1;
   // This is cleared by ClearOutgoing().
-  flags_ |= SESSION_STATE_SENDING;
+  set_sending();
 
   ssize_t src_length;
   const uint8_t* src;
@@ -1635,11 +1634,11 @@ uint8_t Http2Session::SendPendingData() {
 
   chunks_sent_since_last_write_++;
 
-  CHECK_EQ(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
-  flags_ |= SESSION_STATE_WRITE_IN_PROGRESS;
+  CHECK(!is_write_in_progress());
+  set_write_in_progress();
   StreamWriteResult res = underlying_stream()->Write(*bufs, count);
   if (!res.async) {
-    flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
+    set_write_in_progress(false);
     ClearOutgoing(res.err);
   }
 
@@ -1661,7 +1660,8 @@ int Http2Session::OnSendData(
       nghttp2_data_source* source,
       void* user_data) {
   Http2Session* session = static_cast<Http2Session*>(user_data);
-  Http2Stream* stream = GetStream(session, frame->hd.stream_id, source);
+  BaseObjectPtr<Http2Stream> stream = session->FindStream(frame->hd.stream_id);
+  if (!stream) return 0;
 
   // Send the frame header + a byte that indicates padding length.
   session->CopyDataIntoOutgoing(framehd, 9);
@@ -1707,7 +1707,7 @@ int Http2Session::OnSendData(
 
 // Creates a new Http2Stream and submits a new http2 request.
 Http2Stream* Http2Session::SubmitRequest(
-    nghttp2_priority_spec* prispec,
+    const Http2Priority& priority,
     const Http2Headers& headers,
     int32_t* ret,
     int options) {
@@ -1717,7 +1717,7 @@ Http2Stream* Http2Session::SubmitRequest(
   Http2Stream::Provider::Stream prov(options);
   *ret = nghttp2_submit_request(
       session_.get(),
-      prispec,
+      &priority,
       headers.data(),
       headers.length(),
       *prov,
@@ -1768,6 +1768,9 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
     // The data in stream_buf_ is already accounted for, add nread received
     // bytes to session memory but remove the already processed
     // stream_buf_offset_ bytes.
+    // TODO(@jasnell): There are some cases where nread is < stream_buf_offset_
+    // here but things still work. Those need to be investigated.
+    // CHECK_GE(nread, stream_buf_offset_);
     IncrementCurrentSessionMemory(nread - stream_buf_offset_);
 
     buf = std::move(new_buf);
@@ -1846,7 +1849,7 @@ Http2Stream::Http2Stream(Http2Session* session,
   statistics_.start_time = uv_hrtime();
 
   // Limit the number of header pairs
-  max_header_pairs_ = session->GetMaxHeaderPairs();
+  max_header_pairs_ = session->max_header_pairs();
   if (max_header_pairs_ == 0) {
     max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS;
   }
@@ -1861,7 +1864,7 @@ Http2Stream::Http2Stream(Http2Session* session,
       MAX_MAX_HEADER_LIST_SIZE);
 
   if (options & STREAM_OPTION_GET_TRAILERS)
-    flags_ |= NGHTTP2_STREAM_FLAG_TRAILERS;
+    set_has_trailers();
 
   PushStreamListener(&stream_listener_);
 
@@ -1871,11 +1874,12 @@ Http2Stream::Http2Stream(Http2Session* session,
 }
 
 Http2Stream::~Http2Stream() {
-  if (!session_)
-    return;
   Debug(this, "tearing down stream");
-  session_->DecrementCurrentSessionMemory(current_headers_length_);
-  session_->RemoveStream(this);
+}
+
+void Http2Stream::MemoryInfo(MemoryTracker* tracker) const {
+  tracker->TrackField("current_headers", current_headers_);
+  tracker->TrackField("queue", queue_);
 }
 
 std::string Http2Stream::diagnostic_name() const {
@@ -1887,7 +1891,7 @@ std::string Http2Stream::diagnostic_name() const {
 // Notify the Http2Stream that a new block of HEADERS is being processed.
 void Http2Stream::StartHeaders(nghttp2_headers_category category) {
   Debug(this, "starting headers, category: %d", category);
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   session_->DecrementCurrentSessionMemory(current_headers_length_);
   current_headers_length_ = 0;
   current_headers_.clear();
@@ -1895,13 +1899,15 @@ void Http2Stream::StartHeaders(nghttp2_headers_category category) {
 }
 
 
-nghttp2_stream* Http2Stream::operator*() {
-  return nghttp2_session_find_stream(**session_, id_);
+nghttp2_stream* Http2Stream::operator*() const { return stream(); }
+
+nghttp2_stream* Http2Stream::stream() const {
+  return nghttp2_session_find_stream(session_->session(), id_);
 }
 
 void Http2Stream::Close(int32_t code) {
-  CHECK(!this->IsDestroyed());
-  flags_ |= NGHTTP2_STREAM_FLAG_CLOSED;
+  CHECK(!this->is_destroyed());
+  set_closed();
   code_ = code;
   Debug(this, "closed with code %d", code);
 }
@@ -1913,14 +1919,15 @@ ShutdownWrap* Http2Stream::CreateShutdownWrap(v8::Local<v8::Object> object) {
 }
 
 int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
-  if (IsDestroyed())
+  if (is_destroyed())
     return UV_EPIPE;
 
   {
     Http2Scope h2scope(this);
-    flags_ |= NGHTTP2_STREAM_FLAG_SHUT;
-    CHECK_NE(nghttp2_session_resume_data(**session_, id_),
-             NGHTTP2_ERR_NOMEM);
+    set_not_writable();
+    CHECK_NE(nghttp2_session_resume_data(
+        session_->session(), id_),
+        NGHTTP2_ERR_NOMEM);
     Debug(this, "writable side shutdown");
   }
   return 1;
@@ -1931,36 +1938,40 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
 // using the SetImmediate queue.
 void Http2Stream::Destroy() {
   // Do nothing if this stream instance is already destroyed
-  if (IsDestroyed())
+  if (is_destroyed())
     return;
-  if (session_->HasPendingRstStream(id_))
+  if (session_->has_pending_rststream(id_))
     FlushRstStream();
-  flags_ |= NGHTTP2_STREAM_FLAG_DESTROYED;
+  set_destroyed();
 
   Debug(this, "destroying stream");
 
   // Wait until the start of the next loop to delete because there
   // may still be some pending operations queued for this stream.
-  BaseObjectPtr<Http2Stream> strong_ref{this};
-  env()->SetImmediate([this, strong_ref](Environment* env) {
-    // Free any remaining outgoing data chunks here. This should be done
-    // here because it's possible for destroy to have been called while
-    // we still have queued outbound writes.
-    while (!queue_.empty()) {
-      NgHttp2StreamWrite& head = queue_.front();
-      if (head.req_wrap != nullptr)
-        head.req_wrap->Done(UV_ECANCELED);
-      queue_.pop();
-    }
+  BaseObjectPtr<Http2Stream> strong_ref = session_->RemoveStream(id_);
+  if (strong_ref) {
+    env()->SetImmediate([this, strong_ref = std::move(strong_ref)](
+        Environment* env) {
+      // Free any remaining outgoing data chunks here. This should be done
+      // here because it's possible for destroy to have been called while
+      // we still have queued outbound writes.
+      while (!queue_.empty()) {
+        NgHttp2StreamWrite& head = queue_.front();
+        if (head.req_wrap != nullptr)
+          head.req_wrap->Done(UV_ECANCELED);
+        queue_.pop();
+      }
 
-    // We can destroy the stream now if there are no writes for it
-    // already on the socket. Otherwise, we'll wait for the garbage collector
-    // to take care of cleaning up.
-    if (session() == nullptr || !session()->HasWritesOnSocketForStream(this)) {
-      // Delete once strong_ref goes out of scope.
-      Detach();
-    }
-  });
+      // We can destroy the stream now if there are no writes for it
+      // already on the socket. Otherwise, we'll wait for the garbage collector
+      // to take care of cleaning up.
+      if (session() == nullptr ||
+          !session()->HasWritesOnSocketForStream(this)) {
+        // Delete once strong_ref goes out of scope.
+        Detach();
+      }
+    });
+  }
 
   statistics_.end_time = uv_hrtime();
   session_->statistics_.stream_average_duration =
@@ -1973,18 +1984,18 @@ void Http2Stream::Destroy() {
 // Initiates a response on the Http2Stream using data provided via the
 // StreamBase Streams API.
 int Http2Stream::SubmitResponse(const Http2Headers& headers, int options) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   Http2Scope h2scope(this);
   Debug(this, "submitting response");
   if (options & STREAM_OPTION_GET_TRAILERS)
-    flags_ |= NGHTTP2_STREAM_FLAG_TRAILERS;
+    set_has_trailers();
 
-  if (!IsWritable())
+  if (!is_writable())
     options |= STREAM_OPTION_EMPTY_PAYLOAD;
 
   Http2Stream::Provider::Stream prov(this, options);
   int ret = nghttp2_submit_response(
-      **session_,
+      session_->session(),
       id_,
       headers.data(),
       headers.length(),
@@ -1996,11 +2007,11 @@ int Http2Stream::SubmitResponse(const Http2Headers& headers, int options) {
 
 // Submit informational headers for a stream.
 int Http2Stream::SubmitInfo(const Http2Headers& headers) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   Http2Scope h2scope(this);
   Debug(this, "sending %d informational headers", headers.length());
   int ret = nghttp2_submit_headers(
-      **session_,
+      session_->session(),
       NGHTTP2_FLAG_NONE,
       id_,
       nullptr,
@@ -2013,18 +2024,18 @@ int Http2Stream::SubmitInfo(const Http2Headers& headers) {
 
 void Http2Stream::OnTrailers() {
   Debug(this, "let javascript know we are ready for trailers");
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   Isolate* isolate = env()->isolate();
   HandleScope scope(isolate);
   Local<Context> context = env()->context();
   Context::Scope context_scope(context);
-  flags_ &= ~NGHTTP2_STREAM_FLAG_TRAILERS;
+  set_has_trailers(false);
   MakeCallback(env()->http2session_on_stream_trailers_function(), 0, nullptr);
 }
 
 // Submit informational headers for a stream.
 int Http2Stream::SubmitTrailers(const Http2Headers& headers) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   Http2Scope h2scope(this);
   Debug(this, "sending %d trailers", headers.length());
   int ret;
@@ -2033,10 +2044,14 @@ int Http2Stream::SubmitTrailers(const Http2Headers& headers) {
   // to indicate that the stream is ready to be closed.
   if (headers.length() == 0) {
     Http2Stream::Provider::Stream prov(this, 0);
-    ret = nghttp2_submit_data(**session_, NGHTTP2_FLAG_END_STREAM, id_, *prov);
+    ret = nghttp2_submit_data(
+        session_->session(),
+        NGHTTP2_FLAG_END_STREAM,
+        id_,
+        *prov);
   } else {
     ret = nghttp2_submit_trailer(
-        **session_,
+        session_->session(),
         id_,
         headers.data(),
         headers.length());
@@ -2046,17 +2061,20 @@ int Http2Stream::SubmitTrailers(const Http2Headers& headers) {
 }
 
 // Submit a PRIORITY frame to the connected peer.
-int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
+int Http2Stream::SubmitPriority(const Http2Priority& priority,
                                 bool silent) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   Http2Scope h2scope(this);
   Debug(this, "sending priority spec");
   int ret = silent ?
-      nghttp2_session_change_stream_priority(**session_,
-                                             id_, prispec) :
-      nghttp2_submit_priority(**session_,
-                              NGHTTP2_FLAG_NONE,
-                              id_, prispec);
+      nghttp2_session_change_stream_priority(
+          session_->session(),
+          id_,
+          &priority) :
+      nghttp2_submit_priority(
+          session_->session(),
+          NGHTTP2_FLAG_NONE,
+          id_, &priority);
   CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
   return ret;
 }
@@ -2064,7 +2082,7 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
 // Closes the Http2Stream by submitting an RST_STREAM frame to the connected
 // peer.
 void Http2Stream::SubmitRstStream(const uint32_t code) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   code_ = code;
   // If possible, force a purge of any currently pending data here to make sure
   // it is sent before closing the stream. If it returns non-zero then we need
@@ -2079,11 +2097,14 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
 }
 
 void Http2Stream::FlushRstStream() {
-  if (IsDestroyed())
+  if (is_destroyed())
     return;
   Http2Scope h2scope(this);
-  CHECK_EQ(nghttp2_submit_rst_stream(**session_, NGHTTP2_FLAG_NONE,
-                                     id_, code_), 0);
+  CHECK_EQ(nghttp2_submit_rst_stream(
+      session_->session(),
+      NGHTTP2_FLAG_NONE,
+      id_,
+      code_), 0);
 }
 
 
@@ -2091,11 +2112,11 @@ void Http2Stream::FlushRstStream() {
 Http2Stream* Http2Stream::SubmitPushPromise(const Http2Headers& headers,
                                             int32_t* ret,
                                             int options) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
   Http2Scope h2scope(this);
   Debug(this, "sending push promise");
   *ret = nghttp2_submit_push_promise(
-      **session_,
+      session_->session(),
       NGHTTP2_FLAG_NONE,
       id_,
       headers.data(),
@@ -2115,17 +2136,17 @@ Http2Stream* Http2Stream::SubmitPushPromise(const Http2Headers& headers,
 // out to JS land.
 int Http2Stream::ReadStart() {
   Http2Scope h2scope(this);
-  CHECK(!this->IsDestroyed());
-  flags_ |= NGHTTP2_STREAM_FLAG_READ_START;
-  flags_ &= ~NGHTTP2_STREAM_FLAG_READ_PAUSED;
+  CHECK(!this->is_destroyed());
+  set_reading();
 
   Debug(this, "reading starting");
 
   // Tell nghttp2 about our consumption of the data that was handed
   // off to JS land.
-  nghttp2_session_consume_stream(**session_,
-                                 id_,
-                                 inbound_consumed_data_while_paused_);
+  nghttp2_session_consume_stream(
+      session_->session(),
+      id_,
+      inbound_consumed_data_while_paused_);
   inbound_consumed_data_while_paused_ = 0;
 
   return 0;
@@ -2133,10 +2154,10 @@ int Http2Stream::ReadStart() {
 
 // Switch the StreamBase into paused mode.
 int Http2Stream::ReadStop() {
-  CHECK(!this->IsDestroyed());
-  if (!IsReading())
+  CHECK(!this->is_destroyed());
+  if (!is_reading())
     return 0;
-  flags_ |= NGHTTP2_STREAM_FLAG_READ_PAUSED;
+  set_paused();
   Debug(this, "reading stopped");
   return 0;
 }
@@ -2157,7 +2178,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
                          uv_stream_t* send_handle) {
   CHECK_NULL(send_handle);
   Http2Scope h2scope(this);
-  if (!IsWritable() || IsDestroyed()) {
+  if (!is_writable() || is_destroyed()) {
     req_wrap->Done(UV_EOF);
     return 0;
   }
@@ -2171,7 +2192,9 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
     });
     IncrementAvailableOutboundLength(bufs[i].len);
   }
-  CHECK_NE(nghttp2_session_resume_data(**session_, id_), NGHTTP2_ERR_NOMEM);
+  CHECK_NE(nghttp2_session_resume_data(
+      session_->session(),
+      id_), NGHTTP2_ERR_NOMEM);
   return 0;
 }
 
@@ -2183,7 +2206,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
 bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
                             nghttp2_rcbuf* value,
                             uint8_t flags) {
-  CHECK(!this->IsDestroyed());
+  CHECK(!this->is_destroyed());
 
   if (Http2RcBufferPointer::IsZeroLength(name))
     return true;  // Ignore empty headers.
@@ -2192,7 +2215,7 @@ bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
   size_t length = header.length() + 32;
   // A header can only be added if we have not exceeded the maximum number
   // of headers and the session has memory available for it.
-  if (!session_->IsAvailableSessionMemory(length) ||
+  if (!session_->has_available_session_memory(length) ||
       current_headers_.size() == max_header_pairs_ ||
       current_headers_length_ + length > max_header_length_) {
     return false;
@@ -2210,7 +2233,7 @@ bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
 
 // A Provider is the thing that provides outbound DATA frame data.
 Http2Stream::Provider::Provider(Http2Stream* stream, int options) {
-  CHECK(!stream->IsDestroyed());
+  CHECK(!stream->is_destroyed());
   provider_.source.ptr = stream;
   empty_ = options & STREAM_OPTION_EMPTY_PAYLOAD;
 }
@@ -2245,7 +2268,8 @@ ssize_t Http2Stream::Provider::Stream::OnRead(nghttp2_session* handle,
                                               void* user_data) {
   Http2Session* session = static_cast<Http2Session*>(user_data);
   Debug(session, "reading outbound data for stream %d", id);
-  Http2Stream* stream = GetStream(session, id, source);
+  BaseObjectPtr<Http2Stream> stream = session->FindStream(id);
+  if (!stream) return 0;
   if (stream->statistics_.first_byte_sent == 0)
     stream->statistics_.first_byte_sent = uv_hrtime();
   CHECK_EQ(id, stream->id());
@@ -2275,21 +2299,21 @@ ssize_t Http2Stream::Provider::Stream::OnRead(nghttp2_session* handle,
     }
   }
 
-  if (amount == 0 && stream->IsWritable()) {
+  if (amount == 0 && stream->is_writable()) {
     CHECK(stream->queue_.empty());
     Debug(session, "deferring stream %d", id);
     stream->EmitWantsWrite(length);
-    if (stream->available_outbound_length_ > 0 || !stream->IsWritable()) {
+    if (stream->available_outbound_length_ > 0 || !stream->is_writable()) {
       // EmitWantsWrite() did something interesting synchronously, restart:
       return OnRead(handle, id, buf, length, flags, source, user_data);
     }
     return NGHTTP2_ERR_DEFERRED;
   }
 
-  if (stream->queue_.empty() && !stream->IsWritable()) {
+  if (stream->queue_.empty() && !stream->is_writable()) {
     Debug(session, "no more data for stream %d", id);
     *flags |= NGHTTP2_DATA_FLAG_EOF;
-    if (stream->HasTrailers()) {
+    if (stream->has_trailers()) {
       *flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM;
       stream->OnTrailers();
     }
@@ -2299,12 +2323,12 @@ ssize_t Http2Stream::Provider::Stream::OnRead(nghttp2_session* handle,
   return amount;
 }
 
-inline void Http2Stream::IncrementAvailableOutboundLength(size_t amount) {
+void Http2Stream::IncrementAvailableOutboundLength(size_t amount) {
   available_outbound_length_ += amount;
   session_->IncrementCurrentSessionMemory(amount);
 }
 
-inline void Http2Stream::DecrementAvailableOutboundLength(size_t amount) {
+void Http2Stream::DecrementAvailableOutboundLength(size_t amount) {
   available_outbound_length_ -= amount;
   session_->DecrementCurrentSessionMemory(amount);
 }
@@ -2318,10 +2342,9 @@ void HttpErrorString(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   uint32_t val = args[0]->Uint32Value(env->context()).ToChecked();
   args.GetReturnValue().Set(
-      String::NewFromOneByte(
+      OneByteString(
           env->isolate(),
-          reinterpret_cast<const uint8_t*>(nghttp2_strerror(val)),
-          NewStringType::kInternalized).ToLocalChecked());
+          reinterpret_cast<const uint8_t*>(nghttp2_strerror(val))));
 }
 
 
@@ -2330,16 +2353,7 @@ void HttpErrorString(const FunctionCallbackInfo<Value>& args) {
 // output for an HTTP2-Settings header field.
 void PackSettings(const FunctionCallbackInfo<Value>& args) {
   Http2State* state = Unwrap<Http2State>(args.Data());
-  Environment* env = state->env();
-  // TODO(addaleax): We should not be creating a full AsyncWrap for this.
-  Local<Object> obj;
-  if (!env->http2settings_constructor_template()
-           ->NewInstance(env->context())
-           .ToLocal(&obj)) {
-    return;
-  }
-  Http2Session::Http2Settings settings(state, nullptr, obj);
-  args.GetReturnValue().Set(settings.Pack());
+  args.GetReturnValue().Set(Http2Settings::Pack(state));
 }
 
 // A TypedArray instance is shared between C++ and JS land to contain the
@@ -2347,7 +2361,7 @@ void PackSettings(const FunctionCallbackInfo<Value>& args) {
 // default values.
 void RefreshDefaultSettings(const FunctionCallbackInfo<Value>& args) {
   Http2State* state = Unwrap<Http2State>(args.Data());
-  Http2Session::Http2Settings::RefreshDefaults(state);
+  Http2Settings::RefreshDefaults(state);
 }
 
 // Sets the next stream ID the Http2Session. If successful, returns true.
@@ -2356,7 +2370,7 @@ void Http2Session::SetNextStreamID(const FunctionCallbackInfo<Value>& args) {
   Http2Session* session;
   ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
   int32_t id = args[0]->Int32Value(env->context()).ToChecked();
-  if (nghttp2_session_set_next_stream_id(**session, id) < 0) {
+  if (nghttp2_session_set_next_stream_id(session->session(), id) < 0) {
     Debug(session, "failed to set next stream id to %d", id);
     return args.GetReturnValue().Set(false);
   }
@@ -2385,7 +2399,7 @@ void Http2Session::RefreshState(const FunctionCallbackInfo<Value>& args) {
 
   AliasedFloat64Array& buffer = session->http2_state()->session_state_buffer;
 
-  nghttp2_session* s = **session;
+  nghttp2_session* s = session->session();
 
   buffer[IDX_SESSION_STATE_EFFECTIVE_LOCAL_WINDOW_SIZE] =
       nghttp2_session_get_effective_local_window_size(s);
@@ -2413,8 +2427,9 @@ void Http2Session::New(const FunctionCallbackInfo<Value>& args) {
   Http2State* state = Unwrap<Http2State>(args.Data());
   Environment* env = state->env();
   CHECK(args.IsConstructCall());
-  int32_t val = args[0]->Int32Value(env->context()).ToChecked();
-  nghttp2_session_type type = static_cast<nghttp2_session_type>(val);
+  SessionType type =
+      static_cast<SessionType>(
+          args[0]->Int32Value(env->context()).ToChecked());
   Http2Session* session = new Http2Session(state, args.This(), type);
   session->get_async_id();  // avoid compiler warning
   Debug(session, "session created");
@@ -2450,14 +2465,13 @@ void Http2Session::Request(const FunctionCallbackInfo<Value>& args) {
 
   Local<Array> headers = args[0].As<Array>();
   int32_t options = args[1]->Int32Value(env->context()).ToChecked();
-  Http2Priority priority(env, args[2], args[3], args[4]);
 
   Debug(session, "request submitted");
 
   int32_t ret = 0;
   Http2Stream* stream =
       session->Http2Session::SubmitRequest(
-          &priority,
+          Http2Priority(env, args[2], args[3], args[4]),
           Http2Headers(env, headers),
           &ret,
           static_cast<int>(options));
@@ -2478,7 +2492,7 @@ void Http2Session::Goaway(uint32_t code,
                           int32_t lastStreamID,
                           const uint8_t* data,
                           size_t len) {
-  if (IsDestroyed())
+  if (is_destroyed())
     return;
 
   Http2Scope h2scope(this);
@@ -2629,10 +2643,9 @@ void Http2Stream::Priority(const FunctionCallbackInfo<Value>& args) {
   Http2Stream* stream;
   ASSIGN_OR_RETURN_UNWRAP(&stream, args.Holder());
 
-  Http2Priority priority(env, args[0], args[1], args[2]);
-  bool silent = args[3]->IsTrue();
-
-  CHECK_EQ(stream->SubmitPriority(&priority, silent), 0);
+  CHECK_EQ(stream->SubmitPriority(
+      Http2Priority(env, args[0], args[1], args[2]),
+      args[3]->IsTrue()), 0);
   Debug(stream, "priority submitted");
 }
 
@@ -2649,8 +2662,8 @@ void Http2Stream::RefreshState(const FunctionCallbackInfo<Value>& args) {
   AliasedFloat64Array& buffer =
       stream->session()->http2_state()->stream_state_buffer;
 
-  nghttp2_stream* str = **stream;
-  nghttp2_session* s = **(stream->session());
+  nghttp2_stream* str = stream->stream();
+  nghttp2_session* s = stream->session()->session();
 
   if (str == nullptr) {
     buffer[IDX_STREAM_STATE] = NGHTTP2_STREAM_STATE_IDLE;
@@ -2685,12 +2698,13 @@ void Http2Session::AltSvc(int32_t id,
                                  origin, origin_len, value, value_len), 0);
 }
 
-void Http2Session::Origin(nghttp2_origin_entry* ov, size_t count) {
+void Http2Session::Origin(const Origins& origins) {
   Http2Scope h2scope(this);
   CHECK_EQ(nghttp2_submit_origin(
       session_.get(),
       NGHTTP2_FLAG_NONE,
-      ov, count), 0);
+      *origins,
+      origins.length()), 0);
 }
 
 // Submits an AltSvc frame to be sent to the connected peer.
@@ -2705,6 +2719,9 @@ void Http2Session::AltSvc(const FunctionCallbackInfo<Value>& args) {
   Local<String> origin_str = args[1]->ToString(env->context()).ToLocalChecked();
   Local<String> value_str = args[2]->ToString(env->context()).ToLocalChecked();
 
+  if (origin_str.IsEmpty() || value_str.IsEmpty())
+    return;
+
   size_t origin_len = origin_str->Length();
   size_t value_len = value_str->Length();
 
@@ -2728,15 +2745,9 @@ void Http2Session::Origin(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
 
   Local<String> origin_string = args[0].As<String>();
-  int32_t count = args[1]->Int32Value(context).ToChecked();
-
+  size_t count = args[1]->Int32Value(context).ToChecked();
 
-  Origins origins(env->isolate(),
-                  env->context(),
-                  origin_string,
-                  static_cast<int>(count));
-
-  session->Origin(*origins, origins.length());
+  session->Origin(Origins(env, origin_string, count));
 }
 
 // Submits a PING frame to be sent to the connected peer.
@@ -2753,28 +2764,9 @@ void Http2Session::Ping(const FunctionCallbackInfo<Value>& args) {
     CHECK_EQ(payload.length(), 8);
   }
 
-  Local<Object> obj;
-  if (!env->http2ping_constructor_template()
-           ->NewInstance(env->context())
-           .ToLocal(&obj)) {
-    return;
-  }
-  if (obj->Set(env->context(), env->ondone_string(), args[1]).IsNothing())
-    return;
-
-  Http2Ping* ping = session->AddPing(
-      MakeDetachedBaseObject<Http2Ping>(session, obj));
-  // To prevent abuse, we strictly limit the number of unacknowledged PING
-  // frames that may be sent at any given time. This is configurable in the
-  // Options when creating a Http2Session.
-  if (ping == nullptr) return args.GetReturnValue().Set(false);
-
-  // The Ping itself is an Async resource. When the acknowledgement is received,
-  // the callback will be invoked and a notification sent out to JS land. The
-  // notification will include the duration of the ping, allowing the round
-  // trip to be measured.
-  ping->Send(payload.data());
-  args.GetReturnValue().Set(true);
+  CHECK(args[1]->IsFunction());
+  args.GetReturnValue().Set(
+      session->AddPing(payload.data(), args[1].As<Function>()));
 }
 
 // Submits a SETTINGS frame for the Http2Session
@@ -2782,26 +2774,11 @@ void Http2Session::Settings(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Http2Session* session;
   ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
-
-  Local<Object> obj;
-  if (!env->http2settings_constructor_template()
-           ->NewInstance(env->context())
-           .ToLocal(&obj)) {
-    return;
-  }
-  if (obj->Set(env->context(), env->ondone_string(), args[0]).IsNothing())
-    return;
-
-  Http2Settings* settings = session->AddSettings(
-      MakeDetachedBaseObject<Http2Settings>(
-          session->http2_state(), session, obj, 0));
-  if (settings == nullptr) return args.GetReturnValue().Set(false);
-
-  settings->Send();
-  args.GetReturnValue().Set(true);
+  CHECK(args[0]->IsFunction());
+  args.GetReturnValue().Set(session->AddSettings(args[0].As<Function>()));
 }
 
-BaseObjectPtr<Http2Session::Http2Ping> Http2Session::PopPing() {
+BaseObjectPtr<Http2Ping> Http2Session::PopPing() {
   BaseObjectPtr<Http2Ping> ping;
   if (!outstanding_pings_.empty()) {
     ping = std::move(outstanding_pings_.front());
@@ -2811,19 +2788,36 @@ BaseObjectPtr<Http2Session::Http2Ping> Http2Session::PopPing() {
   return ping;
 }
 
-Http2Session::Http2Ping* Http2Session::AddPing(
-    BaseObjectPtr<Http2Session::Http2Ping> ping) {
+bool Http2Session::AddPing(const uint8_t* payload, Local<Function> callback) {
+  Local<Object> obj;
+  if (!env()->http2ping_constructor_template()
+          ->NewInstance(env()->context())
+              .ToLocal(&obj)) {
+    return false;
+  }
+
+  BaseObjectPtr<Http2Ping> ping =
+      MakeDetachedBaseObject<Http2Ping>(this, obj, callback);
+  if (!ping)
+    return false;
+
   if (outstanding_pings_.size() == max_outstanding_pings_) {
     ping->Done(false);
-    return nullptr;
+    return false;
   }
-  Http2Ping* ptr = ping.get();
-  outstanding_pings_.emplace(std::move(ping));
+
   IncrementCurrentSessionMemory(sizeof(*ping));
-  return ptr;
+  // The Ping itself is an Async resource. When the acknowledgement is received,
+  // the callback will be invoked and a notification sent out to JS land. The
+  // notification will include the duration of the ping, allowing the round
+  // trip to be measured.
+  ping->Send(payload);
+
+  outstanding_pings_.emplace(std::move(ping));
+  return true;
 }
 
-BaseObjectPtr<Http2Session::Http2Settings> Http2Session::PopSettings() {
+BaseObjectPtr<Http2Settings> Http2Session::PopSettings() {
   BaseObjectPtr<Http2Settings> settings;
   if (!outstanding_settings_.empty()) {
     settings = std::move(outstanding_settings_.front());
@@ -2833,60 +2827,88 @@ BaseObjectPtr<Http2Session::Http2Settings> Http2Session::PopSettings() {
   return settings;
 }
 
-Http2Session::Http2Settings* Http2Session::AddSettings(
-    BaseObjectPtr<Http2Session::Http2Settings> settings) {
+bool Http2Session::AddSettings(Local<Function> callback) {
+  Local<Object> obj;
+  if (!env()->http2settings_constructor_template()
+          ->NewInstance(env()->context())
+              .ToLocal(&obj)) {
+    return false;
+  }
+
+  BaseObjectPtr<Http2Settings> settings =
+      MakeDetachedBaseObject<Http2Settings>(this, obj, callback, 0);
+  if (!settings)
+    return false;
+
   if (outstanding_settings_.size() == max_outstanding_settings_) {
     settings->Done(false);
-    return nullptr;
+    return false;
   }
-  Http2Settings* ptr = settings.get();
-  outstanding_settings_.emplace(std::move(settings));
+
   IncrementCurrentSessionMemory(sizeof(*settings));
-  return ptr;
+  settings->Send();
+  outstanding_settings_.emplace(std::move(settings));
+  return true;
 }
 
-Http2Session::Http2Ping::Http2Ping(Http2Session* session, Local<Object> obj)
+Http2Ping::Http2Ping(
+    Http2Session* session,
+    Local<Object> obj,
+    Local<Function> callback)
     : AsyncWrap(session->env(), obj, AsyncWrap::PROVIDER_HTTP2PING),
       session_(session),
       startTime_(uv_hrtime()) {
+  callback_.Reset(env()->isolate(), callback);
+}
+
+void Http2Ping::MemoryInfo(MemoryTracker* tracker) const {
+  tracker->TrackField("callback", callback_);
+}
+
+Local<Function> Http2Ping::callback() const {
+  return callback_.Get(env()->isolate());
 }
 
-void Http2Session::Http2Ping::Send(const uint8_t* payload) {
-  CHECK_NOT_NULL(session_);
+void Http2Ping::Send(const uint8_t* payload) {
+  CHECK(session_);
   uint8_t data[8];
   if (payload == nullptr) {
     memcpy(&data, &startTime_, arraysize(data));
     payload = data;
   }
-  Http2Scope h2scope(session_);
-  CHECK_EQ(nghttp2_submit_ping(**session_, NGHTTP2_FLAG_NONE, payload), 0);
+  Http2Scope h2scope(session_.get());
+  CHECK_EQ(nghttp2_submit_ping(
+      session_->session(),
+      NGHTTP2_FLAG_NONE,
+      payload), 0);
 }
 
-void Http2Session::Http2Ping::Done(bool ack, const uint8_t* payload) {
+void Http2Ping::Done(bool ack, const uint8_t* payload) {
   uint64_t duration_ns = uv_hrtime() - startTime_;
   double duration_ms = duration_ns / 1e6;
-  if (session_ != nullptr) session_->statistics_.ping_rtt = duration_ns;
+  if (session_) session_->statistics_.ping_rtt = duration_ns;
 
-  HandleScope handle_scope(env()->isolate());
+  Isolate* isolate = env()->isolate();
+  HandleScope handle_scope(isolate);
   Context::Scope context_scope(env()->context());
 
-  Local<Value> buf = Undefined(env()->isolate());
+  Local<Value> buf = Undefined(isolate);
   if (payload != nullptr) {
-    buf = Buffer::Copy(env()->isolate(),
+    buf = Buffer::Copy(isolate,
                        reinterpret_cast<const char*>(payload),
                        8).ToLocalChecked();
   }
 
   Local<Value> argv[] = {
-    Boolean::New(env()->isolate(), ack),
-    Number::New(env()->isolate(), duration_ms),
+    ack ? v8::True(isolate) : v8::False(isolate),
+    Number::New(isolate, duration_ms),
     buf
   };
-  MakeCallback(env()->ondone_string(), arraysize(argv), argv);
+  MakeCallback(callback(), arraysize(argv), argv);
 }
 
-void Http2Session::Http2Ping::DetachFromSession() {
-  session_ = nullptr;
+void Http2Ping::DetachFromSession() {
+  session_.reset();
 }
 
 void NgHttp2StreamWrite::MemoryInfo(MemoryTracker* tracker) const {
@@ -2970,6 +2992,9 @@ void Initialize(Local<Object> target,
 
   // Method to fetch the nghttp2 string description of an nghttp2 error code
   env->SetMethod(target, "nghttp2ErrorString", HttpErrorString);
+  env->SetMethod(target, "refreshDefaultSettings", RefreshDefaultSettings);
+  env->SetMethod(target, "packSettings", PackSettings);
+  env->SetMethod(target, "setCallbackFunctions", SetCallbackFunctions);
 
   Local<String> http2SessionClassName =
     FIXED_ONE_BYTE_STRING(isolate, "Http2Session");
@@ -2978,7 +3003,7 @@ void Initialize(Local<Object> target,
   ping->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Ping"));
   ping->Inherit(AsyncWrap::GetConstructorTemplate(env));
   Local<ObjectTemplate> pingt = ping->InstanceTemplate();
-  pingt->SetInternalFieldCount(Http2Session::Http2Ping::kInternalFieldCount);
+  pingt->SetInternalFieldCount(Http2Ping::kInternalFieldCount);
   env->set_http2ping_constructor_template(pingt);
 
   Local<FunctionTemplate> setting = FunctionTemplate::New(env->isolate());
@@ -3038,113 +3063,52 @@ void Initialize(Local<Object> target,
               session->GetFunction(env->context()).ToLocalChecked()).Check();
 
   Local<Object> constants = Object::New(isolate);
-  Local<Array> name_for_error_code = Array::New(isolate);
-
-#define NODE_NGHTTP2_ERROR_CODES(V)                       \
-  V(NGHTTP2_SESSION_SERVER);                              \
-  V(NGHTTP2_SESSION_CLIENT);                              \
-  V(NGHTTP2_STREAM_STATE_IDLE);                           \
-  V(NGHTTP2_STREAM_STATE_OPEN);                           \
-  V(NGHTTP2_STREAM_STATE_RESERVED_LOCAL);                 \
-  V(NGHTTP2_STREAM_STATE_RESERVED_REMOTE);                \
-  V(NGHTTP2_STREAM_STATE_HALF_CLOSED_LOCAL);              \
-  V(NGHTTP2_STREAM_STATE_HALF_CLOSED_REMOTE);             \
-  V(NGHTTP2_STREAM_STATE_CLOSED);                         \
-  V(NGHTTP2_NO_ERROR);                                    \
-  V(NGHTTP2_PROTOCOL_ERROR);                              \
-  V(NGHTTP2_INTERNAL_ERROR);                              \
-  V(NGHTTP2_FLOW_CONTROL_ERROR);                          \
-  V(NGHTTP2_SETTINGS_TIMEOUT);                            \
-  V(NGHTTP2_STREAM_CLOSED);                               \
-  V(NGHTTP2_FRAME_SIZE_ERROR);                            \
-  V(NGHTTP2_REFUSED_STREAM);                              \
-  V(NGHTTP2_CANCEL);                                      \
-  V(NGHTTP2_COMPRESSION_ERROR);                           \
-  V(NGHTTP2_CONNECT_ERROR);                               \
-  V(NGHTTP2_ENHANCE_YOUR_CALM);                           \
-  V(NGHTTP2_INADEQUATE_SECURITY);                         \
-  V(NGHTTP2_HTTP_1_1_REQUIRED);                           \
-
-#define V(name)                                                         \
-  NODE_DEFINE_CONSTANT(constants, name);                                \
-  name_for_error_code->Set(env->context(),                              \
-                           static_cast<int>(name),                      \
-                           FIXED_ONE_BYTE_STRING(isolate,               \
-                                                 #name)).Check();
-  NODE_NGHTTP2_ERROR_CODES(V)
+
+  // This does alocate one more slot than needed but it's not used.
+#define V(name) FIXED_ONE_BYTE_STRING(isolate, #name),
+  Local<Value> error_code_names[] = {
+    HTTP2_ERROR_CODES(V)
+  };
 #undef V
 
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_REQUEST);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_RESPONSE);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_PUSH_RESPONSE);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_HEADERS);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_NV_FLAG_NONE);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_NV_FLAG_NO_INDEX);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_ERR_DEFERRED);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_ERR_INVALID_ARGUMENT);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_ERR_STREAM_CLOSED);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_ERR_FRAME_SIZE_ERROR);
-
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, STREAM_OPTION_EMPTY_PAYLOAD);
-  NODE_DEFINE_HIDDEN_CONSTANT(constants, STREAM_OPTION_GET_TRAILERS);
-
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_NONE);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_END_STREAM);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_END_HEADERS);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_ACK);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_PADDED);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_PRIORITY);
-
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_HEADER_TABLE_SIZE);
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_ENABLE_PUSH);
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS);
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE);
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_FRAME_SIZE);
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE);
-  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_ENABLE_CONNECT_PROTOCOL);
-  NODE_DEFINE_CONSTANT(constants, MAX_MAX_FRAME_SIZE);
-  NODE_DEFINE_CONSTANT(constants, MIN_MAX_FRAME_SIZE);
-  NODE_DEFINE_CONSTANT(constants, MAX_INITIAL_WINDOW_SIZE);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_DEFAULT_WEIGHT);
+  Local<Array> name_for_error_code =
+      Array::New(
+          isolate,
+          error_code_names,
+          arraysize(error_code_names));
+
+  target->Set(context,
+              FIXED_ONE_BYTE_STRING(isolate, "nameForErrorCode"),
+              name_for_error_code).Check();
 
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_HEADER_TABLE_SIZE);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_ENABLE_PUSH);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_MAX_FRAME_SIZE);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE);
-  NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL);
+#define V(constant) NODE_DEFINE_HIDDEN_CONSTANT(constants, constant);
+  HTTP2_HIDDEN_CONSTANTS(V)
+#undef V
 
-  NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_NONE);
-  NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_ALIGNED);
-  NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_MAX);
-  NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_CALLBACK);
+#define V(constant) NODE_DEFINE_CONSTANT(constants, constant);
+  HTTP2_CONSTANTS(V)
+#undef V
 
-#define STRING_CONSTANT(NAME, VALUE)                                          \
+  // NGHTTP2_DEFAULT_WEIGHT is a macro and not a regular define
+  // it won't be set properly on the constants object if included
+  // in the HTTP2_CONSTANTS macro.
+  NODE_DEFINE_CONSTANT(constants, NGHTTP2_DEFAULT_WEIGHT);
+
+#define V(NAME, VALUE)                                          \
   NODE_DEFINE_STRING_CONSTANT(constants, "HTTP2_HEADER_" # NAME, VALUE);
-HTTP_KNOWN_HEADERS(STRING_CONSTANT)
-#undef STRING_CONSTANT
+  HTTP_KNOWN_HEADERS(V)
+#undef V
 
-#define STRING_CONSTANT(NAME, VALUE)                                          \
+#define V(NAME, VALUE)                                          \
   NODE_DEFINE_STRING_CONSTANT(constants, "HTTP2_METHOD_" # NAME, VALUE);
-HTTP_KNOWN_METHODS(STRING_CONSTANT)
-#undef STRING_CONSTANT
+  HTTP_KNOWN_METHODS(V)
+#undef V
 
 #define V(name, _) NODE_DEFINE_CONSTANT(constants, HTTP_STATUS_##name);
-HTTP_STATUS_CODES(V)
+  HTTP_STATUS_CODES(V)
 #undef V
 
-  env->SetMethod(target, "refreshDefaultSettings", RefreshDefaultSettings);
-  env->SetMethod(target, "packSettings", PackSettings);
-  env->SetMethod(target, "setCallbackFunctions", SetCallbackFunctions);
-
-  target->Set(context,
-              env->constants_string(),
-              constants).Check();
-  target->Set(context,
-              FIXED_ONE_BYTE_STRING(isolate, "nameForErrorCode"),
-              name_for_error_code).Check();
+  target->Set(context, env->constants_string(), constants).Check();
 }
 }  // namespace http2
 }  // namespace node