src: pull AfterConnect from pipe_wrap and tcp_wrap

This commit attempts to address one of the items in #4641 which is related to src/pipe_wrap.cc and src/tcp_wrap.cc. Currently both pipe_wrap.cc and tcp_wrap.cc contain an AfterConnect function that are almost identical. This commit extracts this function into ConnectionWrap so that that both can share it. PR-URL: #8448 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
nodejs · Sep 14, 2016 · 0d24247 · 0d24247
1 parent 1620226
commit 0d24247
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 72 deletions.
diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc
@@ -1,5 +1,6 @@
 #include "connection_wrap.h"
 
+#include "connect_wrap.h"
 #include "env-inl.h"
 #include "env.h"
 #include "pipe_wrap.h"
@@ -10,6 +11,7 @@
 
 namespace node {
 
+using v8::Boolean;
 using v8::Context;
 using v8::HandleScope;
 using v8::Integer;
@@ -71,6 +73,46 @@ void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
   wrap_data->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
 }
 
+
+template <typename WrapType, typename UVType>
+void ConnectionWrap<WrapType, UVType>::AfterConnect(uv_connect_t* req,
+                                                    int status) {
+  ConnectWrap* req_wrap = static_cast<ConnectWrap*>(req->data);
+  CHECK_NE(req_wrap, nullptr);
+  WrapType* wrap = static_cast<WrapType*>(req->handle->data);
+  CHECK_EQ(req_wrap->env(), wrap->env());
+  Environment* env = wrap->env();
+
+  HandleScope handle_scope(env->isolate());
+  Context::Scope context_scope(env->context());
+
+  // The wrap and request objects should still be there.
+  CHECK_EQ(req_wrap->persistent().IsEmpty(), false);
+  CHECK_EQ(wrap->persistent().IsEmpty(), false);
+
+  bool readable, writable;
+
+  if (status) {
+    readable = writable = 0;
+  } else {
+    readable = uv_is_readable(req->handle) != 0;
+    writable = uv_is_writable(req->handle) != 0;
+  }
+
+  Local<Object> req_wrap_obj = req_wrap->object();
+  Local<Value> argv[5] = {
+    Integer::New(env->isolate(), status),
+    wrap->object(),
+    req_wrap_obj,
+    Boolean::New(env->isolate(), readable),
+    Boolean::New(env->isolate(), writable)
+  };
+
+  req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv);
+
+  delete req_wrap;
+}
+
 template ConnectionWrap<PipeWrap, uv_pipe_t>::ConnectionWrap(
     Environment* env,
     Local<Object> object,
@@ -89,5 +131,11 @@ template void ConnectionWrap<PipeWrap, uv_pipe_t>::OnConnection(
 template void ConnectionWrap<TCPWrap, uv_tcp_t>::OnConnection(
     uv_stream_t* handle, int status);
 
+template void ConnectionWrap<PipeWrap, uv_pipe_t>::AfterConnect(
+    uv_connect_t* handle, int status);
+
+template void ConnectionWrap<TCPWrap, uv_tcp_t>::AfterConnect(
+    uv_connect_t* handle, int status);
+
 
 }  // namespace node
diff --git a/src/connection_wrap.h b/src/connection_wrap.h
@@ -17,6 +17,7 @@ class ConnectionWrap : public StreamWrap {
   }
 
   static void OnConnection(uv_stream_t* handle, int status);
+  static void AfterConnect(uv_connect_t* req, int status);
 
  protected:
   ConnectionWrap(Environment* env,

diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc
@@ -15,15 +15,13 @@
 
 namespace node {
 
-using v8::Boolean;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::External;
 using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
-using v8::Integer;
 using v8::Local;
 using v8::Object;
 using v8::Value;
@@ -141,44 +139,6 @@ void PipeWrap::Listen(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-// TODO(bnoordhuis) Maybe share this with TCPWrap?
-void PipeWrap::AfterConnect(uv_connect_t* req, int status) {
-  ConnectWrap* req_wrap = static_cast<ConnectWrap*>(req->data);
-  PipeWrap* wrap = static_cast<PipeWrap*>(req->handle->data);
-  CHECK_EQ(req_wrap->env(), wrap->env());
-  Environment* env = wrap->env();
-
-  HandleScope handle_scope(env->isolate());
-  Context::Scope context_scope(env->context());
-
-  // The wrap and request objects should still be there.
-  CHECK_EQ(req_wrap->persistent().IsEmpty(), false);
-  CHECK_EQ(wrap->persistent().IsEmpty(), false);
-
-  bool readable, writable;
-
-  if (status) {
-    readable = writable = 0;
-  } else {
-    readable = uv_is_readable(req->handle) != 0;
-    writable = uv_is_writable(req->handle) != 0;
-  }
-
-  Local<Object> req_wrap_obj = req_wrap->object();
-  Local<Value> argv[5] = {
-    Integer::New(env->isolate(), status),
-    wrap->object(),
-    req_wrap_obj,
-    Boolean::New(env->isolate(), readable),
-    Boolean::New(env->isolate(), writable)
-  };
-
-  req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv);
-
-  delete req_wrap;
-}
-
-
 void PipeWrap::Open(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 

diff --git a/src/pipe_wrap.h b/src/pipe_wrap.h
@@ -34,8 +34,6 @@ class PipeWrap : public ConnectionWrap<PipeWrap, uv_pipe_t> {
   static void SetPendingInstances(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 #endif
-
-  static void AfterConnect(uv_connect_t* req, int status);
 };
 
 

diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -235,34 +235,6 @@ void TCPWrap::Listen(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-void TCPWrap::AfterConnect(uv_connect_t* req, int status) {
-  ConnectWrap* req_wrap = static_cast<ConnectWrap*>(req->data);
-  TCPWrap* wrap = static_cast<TCPWrap*>(req->handle->data);
-  CHECK_EQ(req_wrap->env(), wrap->env());
-  Environment* env = wrap->env();
-
-  HandleScope handle_scope(env->isolate());
-  Context::Scope context_scope(env->context());
-
-  // The wrap and request objects should still be there.
-  CHECK_EQ(req_wrap->persistent().IsEmpty(), false);
-  CHECK_EQ(wrap->persistent().IsEmpty(), false);
-
-  Local<Object> req_wrap_obj = req_wrap->object();
-  Local<Value> argv[5] = {
-    Integer::New(env->isolate(), status),
-    wrap->object(),
-    req_wrap_obj,
-    v8::True(env->isolate()),
-    v8::True(env->isolate())
-  };
-
-  req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv);
-
-  delete req_wrap;
-}
-
-
 void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 

diff --git a/src/tcp_wrap.h b/src/tcp_wrap.h
@@ -42,8 +42,6 @@ class TCPWrap : public ConnectionWrap<TCPWrap, uv_tcp_t> {
   static void SetSimultaneousAccepts(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 #endif
-
-  static void AfterConnect(uv_connect_t* req, int status);
 };