src: extracting OnConnection from pipe_wrap and tcp_wrap

One of the issues in nodejs#4641 concerns OnConnection in pipe_wrap and
tcp_wrap which are very similar with some minor difference in how
they are coded. This commit extracts OnConnection so both these
classes can share the same implementation.

node.gyp

@@ -141,6 +141,7 @@
         'src/env.cc',
         'src/fs_event_wrap.cc',
         'src/cares_wrap.cc',
+        'src/connection_wrap.cc',
         'src/handle_wrap.cc',
         'src/js_stream.cc',
         'src/node.cc',
@@ -177,6 +178,7 @@
         'src/async-wrap-inl.h',
         'src/base-object.h',
         'src/base-object-inl.h',
+        'src/connection_wrap.h',
         'src/debug-agent.h',
         'src/env.h',
         'src/env-inl.h',

src/connection_wrap.cc

@@ -0,0 +1,64 @@
+#include "connection_wrap.h"
+
+#include "env-inl.h"
+#include "env.h"
+#include "pipe_wrap.h"
+#include "tcp_wrap.h"
+#include "util.h"
+#include "util-inl.h"
+
+namespace node {
+
+using v8::Context;
+using v8::HandleScope;
+using v8::Integer;
+using v8::Local;
+using v8::Object;
+using v8::Value;
+
+
+template<typename WrapType, typename UVType>
+void ConnectionWrap::OnConnection(uv_stream_t* handle, int status) {
+  WrapType* wrap_data = static_cast<WrapType*>(handle->data);
+  CHECK_EQ(&wrap_data->handle_, reinterpret_cast<UVType*>(handle));
+
+  Environment* env = wrap_data->env();
+  HandleScope handle_scope(env->isolate());
+  Context::Scope context_scope(env->context());
+
+  // We should not be getting this callback if someone as already called
+  // uv_close() on the handle.
+  CHECK_EQ(wrap_data->persistent().IsEmpty(), false);
+
+  Local<Value> argv[] = {
+    Integer::New(env->isolate(), status),
+    Undefined(env->isolate())
+  };
+
+  if (status == 0) {
+    // Instanciate the client javascript object and handle.
+    Local<Object> client_obj = WrapType::Instantiate(env, wrap_data);
+
+    // Unwrap the client javascript object.
+    WrapType* wrap;
+    ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj);
+    uv_stream_t* client_handle = reinterpret_cast<uv_stream_t*>(&wrap->handle_);
+    if (uv_accept(handle, client_handle))
+      return;
+
+    // Successful accept. Call the onconnection callback in JavaScript land.
+    argv[1] = client_obj;
+  }
+  wrap_data->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
+}
+
+template void ConnectionWrap::OnConnection<PipeWrap, uv_pipe_t>(
+    uv_stream_t* handle,
+    int status);
+
+template void ConnectionWrap::OnConnection<TCPWrap, uv_tcp_t>(
+    uv_stream_t* handle,
+    int status);
+
+
+}  // namespace node

src/connection_wrap.h

@@ -0,0 +1,22 @@
+#ifndef SRC_CONNECTION_WRAP_H_
+#define SRC_CONNECTION_WRAP_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include "env.h"
+#include "v8.h"
+
+namespace node {
+
+class ConnectionWrap {
+ protected:
+  template<typename WrapType, typename UVType>
+  static void OnConnection(uv_stream_t* handle, int status);
+};
+
+
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_CONNECTION_WRAP_H_

src/pipe_wrap.cc

@@ -10,6 +10,7 @@
 #include "req-wrap.h"
 #include "req-wrap-inl.h"
 #include "stream_wrap.h"
+#include "connection_wrap.h"
 #include "util-inl.h"
 #include "util.h"
 
@@ -27,7 +28,6 @@ using v8::Integer;
 using v8::Local;
 using v8::Object;
 using v8::String;
-using v8::Undefined;
 using v8::Value;
 
 
@@ -162,49 +162,11 @@ void PipeWrap::Listen(const FunctionCallbackInfo<Value>& args) {
   int backlog = args[0]->Int32Value();
   int err = uv_listen(reinterpret_cast<uv_stream_t*>(&wrap->handle_),
                       backlog,
-                      OnConnection);
+                      ConnectionWrap::OnConnection<PipeWrap, uv_pipe_t>);
   args.GetReturnValue().Set(err);
 }
 
 
-// TODO(bnoordhuis) maybe share with TCPWrap?
-void PipeWrap::OnConnection(uv_stream_t* handle, int status) {
-  PipeWrap* pipe_wrap = static_cast<PipeWrap*>(handle->data);
-  CHECK_EQ(&pipe_wrap->handle_, reinterpret_cast<uv_pipe_t*>(handle));
-
-  Environment* env = pipe_wrap->env();
-  HandleScope handle_scope(env->isolate());
-  Context::Scope context_scope(env->context());
-
-  // We should not be getting this callback if someone as already called
-  // uv_close() on the handle.
-  CHECK_EQ(pipe_wrap->persistent().IsEmpty(), false);
-
-  Local<Value> argv[] = {
-    Integer::New(env->isolate(), status),
-    Undefined(env->isolate())
-  };
-
-  if (status != 0) {
-    pipe_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
-    return;
-  }
-
-  // Instanciate the client javascript object and handle.
-  Local<Object> client_obj = Instantiate(env, pipe_wrap);
-
-  // Unwrap the client javascript object.
-  PipeWrap* wrap;
-  ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj);
-  uv_stream_t* client_handle = reinterpret_cast<uv_stream_t*>(&wrap->handle_);
-  if (uv_accept(handle, client_handle))
-    return;
-
-  // Successful accept. Call the onconnection callback in JavaScript land.
-  argv[1] = client_obj;
-  pipe_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
-}
-
 // TODO(bnoordhuis) Maybe share this with TCPWrap?
 void PipeWrap::AfterConnect(uv_connect_t* req, int status) {
   PipeConnectWrap* req_wrap = static_cast<PipeConnectWrap*>(req->data);

src/pipe_wrap.h

@@ -6,10 +6,11 @@
 #include "async-wrap.h"
 #include "env.h"
 #include "stream_wrap.h"
+#include "connection_wrap.h"
 
 namespace node {
 
-class PipeWrap : public StreamWrap {
+class PipeWrap : public StreamWrap, ConnectionWrap {
  public:
   uv_pipe_t* UVHandle();
 
@@ -21,6 +22,7 @@ class PipeWrap : public StreamWrap {
   size_t self_size() const override { return sizeof(*this); }
 
  private:
+  friend class ConnectionWrap;  // So handle_ can be accessed
   PipeWrap(Environment* env,
            v8::Local<v8::Object> object,
            bool ipc,
@@ -37,7 +39,6 @@ class PipeWrap : public StreamWrap {
       const v8::FunctionCallbackInfo<v8::Value>& args);
 #endif
 
-  static void OnConnection(uv_stream_t* handle, int status);
   static void AfterConnect(uv_connect_t* req, int status);
 
   uv_pipe_t handle_;

src/tcp_wrap.cc

@@ -8,6 +8,7 @@
 #include "req-wrap.h"
 #include "req-wrap-inl.h"
 #include "stream_wrap.h"
+#include "connection_wrap.h"
 #include "util.h"
 #include "util-inl.h"
 
@@ -28,7 +29,6 @@ using v8::Integer;
 using v8::Local;
 using v8::Object;
 using v8::String;
-using v8::Undefined;
 using v8::Value;
 
 
@@ -253,48 +253,11 @@ void TCPWrap::Listen(const FunctionCallbackInfo<Value>& args) {
   int backlog = args[0]->Int32Value();
   int err = uv_listen(reinterpret_cast<uv_stream_t*>(&wrap->handle_),
                       backlog,
-                      OnConnection);
+                      ConnectionWrap::OnConnection<TCPWrap, uv_tcp_t>);
   args.GetReturnValue().Set(err);
 }
 
 
-void TCPWrap::OnConnection(uv_stream_t* handle, int status) {
-  TCPWrap* tcp_wrap = static_cast<TCPWrap*>(handle->data);
-  CHECK_EQ(&tcp_wrap->handle_, reinterpret_cast<uv_tcp_t*>(handle));
-  Environment* env = tcp_wrap->env();
-
-  HandleScope handle_scope(env->isolate());
-  Context::Scope context_scope(env->context());
-
-  // We should not be getting this callback if someone as already called
-  // uv_close() on the handle.
-  CHECK_EQ(tcp_wrap->persistent().IsEmpty(), false);
-
-  Local<Value> argv[2] = {
-    Integer::New(env->isolate(), status),
-    Undefined(env->isolate())
-  };
-
-  if (status == 0) {
-    // Instantiate the client javascript object and handle.
-    Local<Object> client_obj =
-        Instantiate(env, static_cast<AsyncWrap*>(tcp_wrap));
-
-    // Unwrap the client javascript object.
-    TCPWrap* wrap = Unwrap<TCPWrap>(client_obj);
-    CHECK_NE(wrap, nullptr);
-    uv_stream_t* client_handle = reinterpret_cast<uv_stream_t*>(&wrap->handle_);
-    if (uv_accept(handle, client_handle))
-      return;
-
-    // Successful accept. Call the onconnection callback in JavaScript land.
-    argv[1] = client_obj;
-  }
-
-  tcp_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
-}
-
-
 void TCPWrap::AfterConnect(uv_connect_t* req, int status) {
   TCPConnectWrap* req_wrap = static_cast<TCPConnectWrap*>(req->data);
   TCPWrap* wrap = static_cast<TCPWrap*>(req->handle->data);

src/tcp_wrap.h

@@ -6,10 +6,11 @@
 #include "async-wrap.h"
 #include "env.h"
 #include "stream_wrap.h"
+#include "connection_wrap.h"
 
 namespace node {
 
-class TCPWrap : public StreamWrap {
+class TCPWrap : public StreamWrap, ConnectionWrap {
  public:
   static v8::Local<v8::Object> Instantiate(Environment* env, AsyncWrap* parent);
   static void Initialize(v8::Local<v8::Object> target,
@@ -21,6 +22,7 @@ class TCPWrap : public StreamWrap {
   size_t self_size() const override { return sizeof(*this); }
 
  private:
+  friend class ConnectionWrap;  // So handle_ can be accessed
   typedef uv_tcp_t HandleType;
 
   template <typename T,
@@ -45,7 +47,6 @@ class TCPWrap : public StreamWrap {
       const v8::FunctionCallbackInfo<v8::Value>& args);
 #endif
 
-  static void OnConnection(uv_stream_t* handle, int status);
   static void AfterConnect(uv_connect_t* req, int status);
 
   uv_tcp_t handle_;