From 8a0640716b91bdbfdf9a687710574e8e649b3528 Mon Sep 17 00:00:00 2001
From: Andy Street <andrews@fb.com>
Date: Wed, 6 Jul 2016 06:30:25 -0700
Subject: [PATCH] Add more Value APIs

Summary:
Adds:
 - callAsConstructor
 - Ability to provide `this` object to callAsFunction
 - getPropertyNames() now returns Strings (which can be converted by the caller to std::string if they want).

Fixes:
 - double free issue with the String move constructor

Reviewed By: lexs

Differential Revision: D3515398

fbshipit-source-id: afa1342044e41fdd833dd27b8a244a58d4078442
---
 ReactCommon/cxxreact/Value.cpp | 36 +++++++++++++++++++++++++++-------
 ReactCommon/cxxreact/Value.h   | 19 ++++++++++++++----
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/ReactCommon/cxxreact/Value.cpp b/ReactCommon/cxxreact/Value.cpp
index 40990ad13..c328701ec 100644
--- a/ReactCommon/cxxreact/Value.cpp
+++ b/ReactCommon/cxxreact/Value.cpp
@@ -73,19 +73,41 @@ Object::operator Value() const {
 }
 
 Value Object::callAsFunction(std::initializer_list<JSValueRef> args) const {
-  return callAsFunction(args.size(), args.begin());
+  return callAsFunction(nullptr, args.size(), args.begin());
+}
+
+Value Object::callAsFunction(const Object& thisObj, std::initializer_list<JSValueRef> args) const {
+  return callAsFunction((JSObjectRef) thisObj, args.size(), args.begin());
 }
 
 Value Object::callAsFunction(int nArgs, const JSValueRef args[]) const {
+  return callAsFunction(nullptr, nArgs, args);
+}
+
+Value Object::callAsFunction(const Object& thisObj, int nArgs, const JSValueRef args[]) const {
+  return callAsFunction((JSObjectRef) thisObj, nArgs, args);
+}
+
+Value Object::callAsFunction(JSObjectRef thisObj, int nArgs, const JSValueRef args[]) const {
   JSValueRef exn;
-  JSValueRef result = JSObjectCallAsFunction(m_context, m_obj, NULL, nArgs, args, &exn);
+  JSValueRef result = JSObjectCallAsFunction(m_context, m_obj, thisObj, nArgs, args, &exn);
   if (!result) {
     std::string exceptionText = Value(m_context, exn).toString().str();
-    throwJSExecutionException("Exception calling JS function: %s", exceptionText.c_str());
+    throwJSExecutionException("Exception calling object as function: %s", exceptionText.c_str());
   }
   return Value(m_context, result);
 }
 
+Object Object::callAsConstructor(std::initializer_list<JSValueRef> args) const {
+  JSValueRef exn;
+  JSObjectRef result = JSObjectCallAsConstructor(m_context, m_obj, args.size(), args.begin(), &exn);
+  if (!result) {
+    std::string exceptionText = Value(m_context, exn).toString().str();
+    throwJSExecutionException("Exception calling object as constructor: %s", exceptionText.c_str());
+  }
+  return Object(m_context, result);
+}
+
 Value Object::getProperty(const String& propName) const {
   JSValueRef exn;
   JSValueRef property = JSObjectGetProperty(m_context, m_obj, propName, &exn);
@@ -123,13 +145,13 @@ void Object::setProperty(const char *propName, const Value& value) const {
   setProperty(String(propName), value);
 }
 
-std::vector<std::string> Object::getPropertyNames() const {
-  std::vector<std::string> names;
+std::vector<String> Object::getPropertyNames() const {
   auto namesRef = JSObjectCopyPropertyNames(m_context, m_obj);
   size_t count = JSPropertyNameArrayGetCount(namesRef);
+  std::vector<String> names;
+  names.reserve(count);
   for (size_t i = 0; i < count; i++) {
-    auto string = String::ref(JSPropertyNameArrayGetNameAtIndex(namesRef, i));
-    names.emplace_back(string.str());
+    names.emplace_back(String::ref(JSPropertyNameArrayGetNameAtIndex(namesRef, i)));
   }
   JSPropertyNameArrayRelease(namesRef);
   return names;
diff --git a/ReactCommon/cxxreact/Value.h b/ReactCommon/cxxreact/Value.h
index a5cb7bf65..c43a523a7 100644
--- a/ReactCommon/cxxreact/Value.h
+++ b/ReactCommon/cxxreact/Value.h
@@ -53,7 +53,9 @@ public:
 
   String(String&& other) :
     m_string(other.m_string)
-  {}
+  {
+    other.m_string = nullptr;
+  }
 
   String(const String& other) :
     m_string(other.m_string)
@@ -168,15 +170,18 @@ public:
   }
 
   Value callAsFunction(std::initializer_list<JSValueRef> args) const;
-
+  Value callAsFunction(const Object& thisObj, std::initializer_list<JSValueRef> args) const;
   Value callAsFunction(int nArgs, const JSValueRef args[]) const;
+  Value callAsFunction(const Object& thisObj, int nArgs, const JSValueRef args[]) const;
+
+  Object callAsConstructor(std::initializer_list<JSValueRef> args) const;
 
   Value getProperty(const String& propName) const;
   Value getProperty(const char *propName) const;
   Value getPropertyAtIndex(unsigned index) const;
   void setProperty(const String& propName, const Value& value) const;
   void setProperty(const char *propName, const Value& value) const;
-  std::vector<std::string> getPropertyNames() const;
+  std::vector<String> getPropertyNames() const;
   std::unordered_map<std::string, std::string> toJSONMap() const;
 
   void makeProtected() {
@@ -186,6 +191,10 @@ public:
     }
   }
 
+  JSContextRef context() const {
+    return m_context;
+  }
+
   static Object getGlobalObject(JSContextRef ctx) {
     auto globalObj = JSContextGetGlobalObject(ctx);
     return Object(ctx, globalObj);
@@ -200,6 +209,8 @@ private:
   JSContextRef m_context;
   JSObjectRef m_obj;
   bool m_isProtected = false;
+
+  Value callAsFunction(JSObjectRef thisObj, int nArgs, const JSValueRef args[]) const;
 };
 
 class Value : public noncopyable {
@@ -265,8 +276,8 @@ public:
   std::string toJSONString(unsigned indent = 0) const throw(JSException);
   static Value fromJSON(JSContextRef ctx, const String& json) throw(JSException);
   static Value fromDynamic(JSContextRef ctx, folly::dynamic value) throw(JSException);
-protected:
   JSContextRef context() const;
+protected:
   JSContextRef m_context;
   JSValueRef m_value;
 };