Rewrite Callback to add Callback::Reset

nodejs · Jun 13, 2016 · c4cf44d · c4cf44d
1 parent bd43cb9
commit c4cf44d
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 33 deletions.
diff --git a/doc/callback.md b/doc/callback.md
@@ -35,6 +35,10 @@ class Callback {
 
   bool IsEmpty() const;
 
+  void Reset(const v8::Local<v8::Function> &fn);
+
+  void Reset();
+
   v8::Local<v8::Value> Call(v8::Local<v8::Object> target,
                             int argc,
                             v8::Local<v8::Value> argv[]) const;

diff --git a/nan.h b/nan.h
@@ -1348,37 +1348,24 @@ typedef void NAN_INDEX_QUERY_RETURN_TYPE;
 
 class Callback {
  public:
-  Callback() {
-    HandleScope scope;
-    v8::Local<v8::Object> obj = New<v8::Object>();
-    handle.Reset(obj);
-  }
+  Callback() {}
 
-  explicit Callback(const v8::Local<v8::Function> &fn) {
-    HandleScope scope;
-    v8::Local<v8::Object> obj = New<v8::Object>();
-    handle.Reset(obj);
-    SetFunction(fn);
-  }
+  explicit Callback(const v8::Local<v8::Function> &fn) : handle_(fn) {}
 
   ~Callback() {
-    if (handle.IsEmpty()) return;
-    handle.Reset();
+    handle_.Reset();
   }
 
   bool operator==(const Callback &other) const {
-    HandleScope scope;
-    v8::Local<v8::Value> a = New(handle)->Get(kCallbackIndex);
-    v8::Local<v8::Value> b = New(other.handle)->Get(kCallbackIndex);
-    return a->StrictEquals(b);
+    return handle_ == other.handle_;
   }
 
   bool operator!=(const Callback &other) const {
-    return !this->operator==(other);
+    return !operator==(other);
   }
 
   inline
-  v8::Local<v8::Function> operator*() const { return this->GetFunction(); }
+  v8::Local<v8::Function> operator*() const { return GetFunction(); }
 
   inline v8::Local<v8::Value> operator()(
       v8::Local<v8::Object> target
@@ -1393,20 +1380,25 @@ class Callback {
     return this->Call(argc, argv);
   }
 
+  // TODO(kkoopa): remove
   inline void SetFunction(const v8::Local<v8::Function> &fn) {
-    HandleScope scope;
-    Set(New(handle), kCallbackIndex, fn);
+    Reset(fn);
+  }
+
+  inline void Reset(const v8::Local<v8::Function> &fn) {
+    handle_.Reset(fn);
+  }
+
+  inline void Reset() {
+    handle_.Reset();
   }
 
   inline v8::Local<v8::Function> GetFunction() const {
-    EscapableHandleScope scope;
-    return scope.Escape(New(handle)->Get(kCallbackIndex)
-        .As<v8::Function>());
+    return New(handle_);
   }
 
   inline bool IsEmpty() const {
-    HandleScope scope;
-    return New(handle)->Get(kCallbackIndex)->IsUndefined();
+    return handle_.IsEmpty();
   }
 
   inline v8::Local<v8::Value>
@@ -1433,8 +1425,7 @@ class Callback {
 
  private:
   NAN_DISALLOW_ASSIGN_COPY_MOVE(Callback)
-  Persistent<v8::Object> handle;
-  static const uint32_t kCallbackIndex = 0;
+  Persistent<v8::Function> handle_;
 
 #if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION)
   v8::Local<v8::Value> Call_(v8::Isolate *isolate
@@ -1443,8 +1434,7 @@ class Callback {
                            , v8::Local<v8::Value> argv[]) const {
     EscapableHandleScope scope;
 
-    v8::Local<v8::Function> callback = New(handle)->
-        Get(kCallbackIndex).As<v8::Function>();
+    v8::Local<v8::Function> callback = New(handle_);
 # if NODE_MODULE_VERSION < IOJS_3_0_MODULE_VERSION
     return scope.Escape(New(node::MakeCallback(
         isolate
@@ -1469,8 +1459,7 @@ class Callback {
                            , v8::Local<v8::Value> argv[]) const {
     EscapableHandleScope scope;
 
-    v8::Local<v8::Function> callback = New(handle)->
-        Get(kCallbackIndex).As<v8::Function>();
+    v8::Local<v8::Function> callback = New(handle_);
     return scope.Escape(New(node::MakeCallback(
         target
       , callback

diff --git a/test/cpp/nancallback.cpp b/test/cpp/nancallback.cpp
@@ -41,6 +41,18 @@ NAN_METHOD(CallAsFunction) {
   Callback(info[0].As<v8::Function>())();
 }
 
+NAN_METHOD(ResetUnset) {
+  Callback callback;
+  callback.Reset();
+  info.GetReturnValue().Set(callback.IsEmpty());
+}
+
+NAN_METHOD(ResetSet) {
+  Callback callback(info[0].As<v8::Function>());
+  callback.Reset();
+  info.GetReturnValue().Set(callback.IsEmpty());
+}
+
 NAN_MODULE_INIT(Init) {
   Set(target
     , New<v8::String>("globalContext").ToLocalChecked()
@@ -66,6 +78,14 @@ NAN_MODULE_INIT(Init) {
     , New<v8::String>("callAsFunction").ToLocalChecked()
     , New<v8::FunctionTemplate>(CallAsFunction)->GetFunction()
   );
+  Set(target
+    , New<v8::String>("resetUnset").ToLocalChecked()
+    , New<v8::FunctionTemplate>(ResetUnset)->GetFunction()
+  );
+  Set(target
+    , New<v8::String>("resetSet").ToLocalChecked()
+    , New<v8::FunctionTemplate>(ResetSet)->GetFunction()
+  );
 }
 
 NODE_MODULE(nancallback, Init)
diff --git a/test/js/nancallback-test.js b/test/js/nancallback-test.js
@@ -12,7 +12,7 @@ const test     = require('tap').test
     , round = Math.round;
 
 test('nancallback', function (t) {
-  t.plan(13)
+  t.plan(17)
 
   var persistent = bindings;
   t.type(persistent.globalContext, 'function');
@@ -21,11 +21,15 @@ test('nancallback', function (t) {
   t.type(persistent.compareCallbacks, 'function');
   t.type(persistent.callDirect, 'function');
   t.type(persistent.callAsFunction, 'function');
+  t.type(persistent.resetUnset, 'function');
+  t.type(persistent.resetSet, 'function');
   persistent.globalContext(function () { t.ok(true); });
   persistent.specificContext(function () { t.ok(true); });
   persistent.customReceiver(function () { t.equal(this, process); }, process);
   persistent.callDirect(function () { t.ok(true); });
   persistent.callAsFunction(function () { t.ok(true); });
+  t.ok(persistent.resetUnset());
+  t.ok(persistent.resetSet(function () {}));
 
   var round2 = Math.round
     , x = function(param) { return param + 1; }