[gjs/april-maintenance: 8/11] function: Merge two ways of invoking constructor from C

[Philip Chimento <pchimento src gnome org>]

commit 85a3fe3e36a2293e2e852456a281e92d05b7b350
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Apr 29 22:11:12 2020 -0700

    function: Merge two ways of invoking constructor from C
    
    We had two separate ways of invoking a JS constructor function, one was
    used in union_new and the other in Fundamental::invoke_constructor. The
    original one, gjs_invoke_c_function_uncached(), was added by Colin in
    commit 47381c920222f0502a9328b664669edf9fc8c6ca and was intended for
    constructors that didn't need the fast path of caching their function
    data.
    
    The other, gjs_invoke_constructor_from_c(), was added later for
    fundamental type support and did cache the function data, but did not do
    argument marshalling on the return value, just returning the C pointer
    instead.
    
    Both of these features are useful for both situations, so we merge the
    two functions into one gjs_invoke_constructor_from_c() which does not
    cache the function data and does not do argument marshalling on the
    return value.
    
    Since we now invoke the constructor by its GIFunctionInfo, we don't need
    to store its name on FundamentalPrototype, which allows us to remove
    actually a lot of code for storing and tracing the property key.

 gi/function.cpp    | 46 +++++++++++----------------------------
 gi/function.h      | 14 +++---------
 gi/fundamental.cpp | 63 +++++++++---------------------------------------------
 gi/fundamental.h   |  7 ------
 gi/union.cpp       | 40 ++++++++--------------------------
 5 files changed, 35 insertions(+), 135 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index 5956834a..e65d09ff 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -1856,38 +1856,18 @@ gjs_define_function(JSContext       *context,
     return function;
 }
 
+bool gjs_invoke_constructor_from_c(JSContext* context, GIFunctionInfo* info,
+                                   JS::HandleObject obj,
+                                   const JS::HandleValueArray& args,
+                                   GIArgument* rvalue) {
+    Function function;
+
+    memset(&function, 0, sizeof(Function));
+    if (!init_cached_function_data(context, &function, 0, info))
+        return false;
 
-bool
-gjs_invoke_c_function_uncached(JSContext                  *context,
-                               GIFunctionInfo             *info,
-                               JS::HandleObject            obj,
-                               const JS::HandleValueArray& args,
-                               JS::MutableHandleValue      rval)
-{
-  Function function;
-  bool result;
-
-  memset (&function, 0, sizeof (Function));
-  if (!init_cached_function_data (context, &function, 0, info))
-      return false;
-
-  result =
-      gjs_invoke_c_function(context, &function, obj, args, mozilla::Some(rval));
-  uninit_cached_function_data (&function);
-  return result;
-}
-
-bool
-gjs_invoke_constructor_from_c(JSContext                  *context,
-                              JS::HandleObject            constructor,
-                              JS::HandleObject            obj,
-                              const JS::HandleValueArray& args,
-                              GIArgument                 *rvalue)
-{
-    Function *priv;
-
-    priv = priv_from_js(context, constructor);
-
-    return gjs_invoke_c_function(context, priv, obj, args, mozilla::Nothing(),
-                                 rvalue);
+    bool result = gjs_invoke_c_function(context, &function, obj, args,
+                                        mozilla::Nothing(), rvalue);
+    uninit_cached_function_data(&function);
+    return result;
 }
diff --git a/gi/function.h b/gi/function.h
index 1038f5fb..eef77afd 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -74,17 +74,9 @@ JSObject *gjs_define_function(JSContext       *context,
                               GICallableInfo  *info);
 
 GJS_JSAPI_RETURN_CONVENTION
-bool gjs_invoke_c_function_uncached(JSContext                  *context,
-                                    GIFunctionInfo             *info,
-                                    JS::HandleObject            obj,
-                                    const JS::HandleValueArray& args,
-                                    JS::MutableHandleValue      rval);
-
-GJS_JSAPI_RETURN_CONVENTION
-bool gjs_invoke_constructor_from_c(JSContext                  *context,
-                                   JS::HandleObject            constructor,
-                                   JS::HandleObject            obj,
+bool gjs_invoke_constructor_from_c(JSContext* cx, GIFunctionInfo* info,
+                                   JS::HandleObject this_obj,
                                    const JS::HandleValueArray& args,
-                                   GIArgument                 *rvalue);
+                                   GIArgument* rvalue);
 
 #endif  // GI_FUNCTION_H_
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index d667d938..bde80edd 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -82,10 +82,7 @@ bool FundamentalInstance::associate_js_instance(JSContext* cx, JSObject* object,
 
 /* Find the first constructor */
 GJS_USE
-static bool find_fundamental_constructor(
-    JSContext* context, GIObjectInfo* info,
-    JS::MutableHandleId constructor_name,
-    GjsAutoFunctionInfo* constructor_info) {
+static GIFunctionInfo* find_fundamental_constructor(GIObjectInfo* info) {
     int i, n_methods;
 
     n_methods = g_object_info_get_n_methods(info);
@@ -97,22 +94,13 @@ static bool find_fundamental_constructor(
         func_info = g_object_info_get_method(info, i);
 
         flags = g_function_info_get_flags(func_info);
-        if ((flags & GI_FUNCTION_IS_CONSTRUCTOR) != 0) {
-            const char *name;
-
-            name = g_base_info_get_name((GIBaseInfo *) func_info);
-            constructor_name.set(gjs_intern_string_to_id(context, name));
-            if (constructor_name == JSID_VOID)
-                return false;
-
-            constructor_info->reset(func_info);
-            return true;
-        }
+        if ((flags & GI_FUNCTION_IS_CONSTRUCTOR) != 0)
+            return func_info;
 
         g_base_info_unref((GIBaseInfo *) func_info);
     }
 
-    return true;
+    return nullptr;
 }
 
 /**/
@@ -194,34 +182,22 @@ bool FundamentalPrototype::resolve_impl(JSContext* cx, JS::HandleObject obj,
  * FundamentalInstance::invoke_constructor:
  *
  * Finds the type's static constructor method (the static method given by
- * FundamentalPrototype::constructor_name()) and invokes it with the given
+ * FundamentalPrototype::constructor_info()) and invokes it with the given
  * arguments.
  */
 bool FundamentalInstance::invoke_constructor(JSContext* context,
                                              JS::HandleObject obj,
                                              const JS::HandleValueArray& args,
                                              GIArgument* rvalue) {
-    JS::RootedObject js_constructor(context);
-    JS::RootedId constructor_name(context, get_prototype()->constructor_name());
-
-    const GjsAtoms& atoms = GjsContextPrivate::atoms(context);
-    if (!gjs_object_require_property(context, obj, nullptr, atoms.constructor(),
-                                     &js_constructor) ||
-        constructor_name == JSID_VOID) {
-        gjs_throw(context, "Couldn't find a constructor for type %s.%s", ns(),
-                  name());
-        return false;
-    }
-
-    JS::RootedObject constructor(context);
-    if (!gjs_object_require_property(context, js_constructor, nullptr,
-                                     constructor_name, &constructor)) {
+    GIFunctionInfo* constructor_info = get_prototype()->constructor_info();
+    if (!constructor_info) {
         gjs_throw(context, "Couldn't find a constructor for type %s.%s", ns(),
                   name());
         return false;
     }
 
-    return gjs_invoke_constructor_from_c(context, constructor, obj, args, rvalue);
+    return gjs_invoke_constructor_from_c(context, constructor_info, obj, args,
+                                         rvalue);
 }
 
 // See GIWrapperBase::constructor().
@@ -257,37 +233,18 @@ FundamentalPrototype::FundamentalPrototype(GIObjectInfo* info, GType gtype)
       m_unref_function(g_object_info_get_unref_function_pointer(info)),
       m_get_value_function(g_object_info_get_get_value_function_pointer(info)),
       m_set_value_function(g_object_info_get_set_value_function_pointer(info)),
-      m_constructor_info(nullptr) {
+      m_constructor_info(find_fundamental_constructor(info)) {
     g_assert(m_ref_function);
     g_assert(m_unref_function);
     g_assert(m_set_value_function);
     g_assert(m_get_value_function);
 }
 
-// Overrides GIWrapperPrototype::init().
-bool FundamentalPrototype::init(JSContext* cx) {
-    JS::RootedId constructor_name(cx);
-    GjsAutoFunctionInfo constructor_info;
-    if (!find_fundamental_constructor(cx, info(), &constructor_name,
-                                      &constructor_info))
-        return false;
-
-    m_constructor_name = constructor_name;
-    m_constructor_info = constructor_info.release();
-    return true;
-}
-
 FundamentalPrototype::~FundamentalPrototype(void) {
     g_clear_pointer(&m_constructor_info, g_base_info_unref);
     GJS_DEC_COUNTER(fundamental_prototype);
 }
 
-// Overrides GIWrapperPrototype::trace_impl().
-void FundamentalPrototype::trace_impl(JSTracer* trc) {
-    JS::TraceEdge<jsid>(trc, &m_constructor_name,
-                        "Fundamental::constructor_name");
-}
-
 // clang-format off
 const struct JSClassOps FundamentalBase::class_ops = {
     nullptr,  // addProperty
diff --git a/gi/fundamental.h b/gi/fundamental.h
index 00c38225..a1a9f363 100644
--- a/gi/fundamental.h
+++ b/gi/fundamental.h
@@ -91,15 +91,11 @@ class FundamentalPrototype
     GIObjectInfoUnrefFunction m_unref_function;
     GIObjectInfoGetValueFunction m_get_value_function;
     GIObjectInfoSetValueFunction m_set_value_function;
-
-    JS::Heap<jsid> m_constructor_name;
     GICallableInfo* m_constructor_info;
 
     explicit FundamentalPrototype(GIObjectInfo* info, GType gtype);
     ~FundamentalPrototype(void);
 
-    GJS_JSAPI_RETURN_CONVENTION bool init(JSContext* cx);
-
     static constexpr InfoType::Tag info_type_tag = InfoType::Object;
 
  public:
@@ -108,8 +104,6 @@ class FundamentalPrototype
 
     // Accessors
 
-    GJS_USE
-    jsid constructor_name(void) const { return m_constructor_name.get(); }
     GJS_USE
     GICallableInfo* constructor_info(void) const { return m_constructor_info; }
 
@@ -139,7 +133,6 @@ class FundamentalPrototype
     GJS_JSAPI_RETURN_CONVENTION
     bool resolve_impl(JSContext* cx, JS::HandleObject obj, JS::HandleId id,
                       const char* prop_name, bool* resolved);
-    void trace_impl(JSTracer* trc);
 
     // Public API
  public:
diff --git a/gi/union.cpp b/gi/union.cpp
index a58e90e9..34720f38 100644
--- a/gi/union.cpp
+++ b/gi/union.cpp
@@ -115,28 +115,21 @@ union_new(JSContext       *context,
         flags = g_function_info_get_flags(func_info);
         if ((flags & GI_FUNCTION_IS_CONSTRUCTOR) != 0 &&
             g_callable_info_get_n_args((GICallableInfo*) func_info) == 0) {
-
-            JS::RootedValue rval(context, JS::NullValue());
-
-            if (!gjs_invoke_c_function_uncached(context, func_info, obj,
-                                                JS::HandleValueArray::empty(),
-                                                &rval))
+            GIArgument rval;
+            if (!gjs_invoke_constructor_from_c(context, func_info, obj,
+                                               JS::HandleValueArray::empty(),
+                                               &rval))
                 return nullptr;
 
-            /* We are somewhat wasteful here; invoke_c_function() above
-             * creates a JSObject wrapper for the union that we immediately
-             * discard.
-             */
-            if (rval.isNull()) {
+            if (!rval.v_pointer) {
                 gjs_throw(context,
                           "Unable to construct union type %s as its"
                           "constructor function returned null",
                           g_base_info_get_name(info));
                 return nullptr;
-            } else {
-                JS::RootedObject rval_obj(context, &rval.toObject());
-                return UnionBase::to_c_ptr(context, rval_obj);
             }
+
+            return rval.v_pointer;
         }
     }
 
@@ -155,23 +148,8 @@ bool UnionInstance::constructor_impl(JSContext* context,
                       name()))
         return false;
 
-    /* union_new happens to be implemented by calling
-     * gjs_invoke_c_function(), which returns a JS::Value.
-     * The returned "gboxed" here is owned by that JS::Value,
-     * not by us.
-     */
-    void* gboxed = union_new(context, object, info());
-
-    if (!gboxed)
-        return false;
-
-    /* Because "gboxed" is owned by a JS::Value and will
-     * be garbage collected, we make a copy here to be
-     * owned by us.
-     */
-    copy_union(gboxed);
-
-    return true;
+    m_ptr = union_new(context, object, info());
+    return !!m_ptr;
 }
 
 // clang-format off