[gjs: 1/13] fundamental: Move to C++-style API

[Cosimo Cecchi <cosimoc src gnome org>]

commit b4149b9e4ae867f9c188db3ec15f5b96049d2cfc
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Jan 20 18:17:02 2019 -0800

    fundamental: Move to C++-style API
    
    This allows cleaning up the public API in fundamental.h a bit, changing
    some methods to be private, adding missing error checks in calling code,
    and creating static FundamentalInstance::object_for_c_ptr() and
    FundamentalInstance::object_for_gvalue() methods in order to fetch a
    JS object given a C pointer or GValue holding a fundamental object.
    
    The idea is to gradually provide more typesafe, C++-style APIs for all
    of the GI wrapper classes, and change calling code to use them rather
    than the old C-style APIs.

 gi/arg.cpp         | 11 ++++---
 gi/fundamental.cpp | 84 +++++++++++++++++++++++++-----------------------------
 gi/fundamental.h   | 41 ++++++++++----------------
 gi/repo.cpp        | 11 ++-----
 gi/value.cpp       |  4 +--
 5 files changed, 66 insertions(+), 85 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 7e060924..c6056ab8 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -2944,8 +2944,8 @@ gjs_value_from_g_argument (JSContext             *context,
             } else if (gtype == G_TYPE_NONE) {
                 gjs_throw(context, "Unexpected unregistered type packing GArgument into JS::Value");
             } else if (G_TYPE_IS_INSTANTIATABLE(gtype) || G_TYPE_IS_INTERFACE(gtype)) {
-                JSObject *obj;
-                obj = gjs_object_from_g_fundamental(context, (GIObjectInfo *)interface_info, arg->v_pointer);
+                JSObject* obj = FundamentalInstance::object_for_c_ptr(
+                    context, arg->v_pointer);
                 if (obj)
                     value = JS::ObjectValue(*obj);
             } else {
@@ -3227,8 +3227,11 @@ gjs_g_arg_release_internal(JSContext  *context,
                     failed = true;
                 }
             } else if (G_TYPE_IS_INSTANTIATABLE(gtype)) {
-                if (transfer != TRANSFER_IN_NOTHING)
-                    gjs_fundamental_unref(context, arg->v_pointer);
+                if (transfer != TRANSFER_IN_NOTHING) {
+                    auto* priv =
+                        FundamentalPrototype::for_gtype(context, gtype);
+                    priv->call_unref_function(arg->v_pointer);
+                }
             } else {
                 gjs_throw(context, "Unhandled GType %s releasing GArgument",
                           g_type_name(gtype));
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index 389d922e..dc961728 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -331,9 +331,8 @@ gjs_lookup_fundamental_prototype(JSContext    *context,
         /* In case we're looking for a private type, and we don't find it,
            we need to define it first.
         */
-        JS::RootedObject ignored(context);
-        if (!gjs_define_fundamental_class(context, in_object, info,
-                                          &constructor, &ignored))
+        if (!FundamentalPrototype::define_class(context, in_object, info,
+                                                &constructor))
             return nullptr;
     } else {
         if (G_UNLIKELY(!value.isObject())) {
@@ -392,19 +391,27 @@ unsigned FundamentalPrototype::constructor_nargs(void) const {
     return g_callable_info_get_n_args(m_constructor_info);
 }
 
-bool
-gjs_define_fundamental_class(JSContext              *context,
-                             JS::HandleObject        in_object,
-                             GIObjectInfo           *info,
-                             JS::MutableHandleObject constructor,
-                             JS::MutableHandleObject prototype)
-{
+/*
+ * FundamentalPrototype::define_class:
+ * @in_object: Object where the constructor is stored, typically a repo object.
+ * @info: Introspection info for the fundamental class.
+ * @constructor: Return location for the constructor object.
+ *
+ * Define a fundamental class constructor and prototype, including all the
+ * necessary methods and properties. Provides the constructor object as an out
+ * parameter, for convenience elsewhere.
+ */
+bool FundamentalPrototype::define_class(JSContext* cx,
+                                        JS::HandleObject in_object,
+                                        GIObjectInfo* info,
+                                        JS::MutableHandleObject constructor) {
     GType gtype;
 
     gtype = g_registered_type_info_get_g_type (info);
 
+    JS::RootedObject prototype(cx);
     FundamentalPrototype* priv = FundamentalPrototype::create_class(
-        context, in_object, info, gtype, constructor, prototype);
+        cx, in_object, info, gtype, constructor, &prototype);
     if (!priv)
         return false;
 
@@ -415,17 +422,22 @@ gjs_define_fundamental_class(JSContext              *context,
                   priv->ns(), priv->name());
     }
 
-    return gjs_define_static_methods<InfoType::Object>(context, constructor,
-                                                       gtype, info);
+    return gjs_define_static_methods<InfoType::Object>(cx, constructor, gtype,
+                                                       info);
 }
 
-JSObject*
-gjs_object_from_g_fundamental(JSContext    *context,
-                              GIObjectInfo *info,
-                              void         *gfundamental)
-{
-    if (gfundamental == NULL)
+/*
+ * FundamentalInstance::object_for_c_ptr:
+ *
+ * Given a pointer to a C fundamental object, returns a JS object. This JS
+ * object may have been cached, or it may be newly created.
+ */
+JSObject* FundamentalInstance::object_for_c_ptr(JSContext* context,
+                                                void* gfundamental) {
+    if (!gfundamental) {
+        gjs_throw(context, "Cannot get JSObject for null fundamental pointer");
         return NULL;
+    }
 
     GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
     auto p = gjs->fundamental_table().lookup(gfundamental);
@@ -433,10 +445,7 @@ gjs_object_from_g_fundamental(JSContext    *context,
         return p->value();
 
     gjs_debug_marshal(GJS_DEBUG_GFUNDAMENTAL,
-                      "Wrapping fundamental %s.%s %p with JSObject",
-                      g_base_info_get_namespace((GIBaseInfo *) info),
-                      g_base_info_get_name((GIBaseInfo *) info),
-                      gfundamental);
+                      "Wrapping fundamental %p with JSObject", gfundamental);
 
     JS::RootedObject proto(context,
         gjs_lookup_fundamental_prototype_from_gtype(context,
@@ -474,23 +483,17 @@ FundamentalPrototype* FundamentalPrototype::for_gtype(JSContext* cx,
     return FundamentalPrototype::for_js(cx, proto);
 }
 
-JSObject *
-gjs_fundamental_from_g_value(JSContext    *context,
-                             const GValue *value,
-                             GType         gtype)
-{
-    void *fobj;
-
-    auto* proto_priv = FundamentalPrototype::for_gtype(context, gtype);
-
-    fobj = proto_priv->call_get_value_function(value);
+JSObject* FundamentalInstance::object_for_gvalue(JSContext* cx,
+                                                 const GValue* value,
+                                                 GType gtype) {
+    auto* proto_priv = FundamentalPrototype::for_gtype(cx, gtype);
+    void* fobj = proto_priv->call_get_value_function(value);
     if (!fobj) {
-        gjs_throw(context,
-                  "Failed to convert GValue to a fundamental instance");
+        gjs_throw(cx, "Failed to convert GValue to a fundamental instance");
         return NULL;
     }
 
-    return gjs_object_from_g_fundamental(context, proto_priv->info(), fobj);
+    return FundamentalInstance::object_for_c_ptr(cx, fobj);
 }
 
 bool FundamentalBase::to_gvalue(JSContext* cx, JS::HandleObject obj,
@@ -508,12 +511,3 @@ void* FundamentalInstance::copy_ptr(JSContext* cx, GType gtype,
     auto* priv = FundamentalPrototype::for_gtype(cx, gtype);
     return priv->call_ref_function(gfundamental);
 }
-
-void
-gjs_fundamental_unref(JSContext    *context,
-                      void         *gfundamental)
-{
-    auto* priv = FundamentalPrototype::for_gtype(
-        context, G_TYPE_FROM_INSTANCE(gfundamental));
-    priv->call_unref_function(gfundamental);
-}
diff --git a/gi/fundamental.h b/gi/fundamental.h
index 9f2960e0..38201d6a 100644
--- a/gi/fundamental.h
+++ b/gi/fundamental.h
@@ -129,6 +129,13 @@ class FundamentalPrototype
     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:
+    GJS_JSAPI_RETURN_CONVENTION
+    static bool define_class(JSContext* cx, JS::HandleObject in_object,
+                             GIObjectInfo* info,
+                             JS::MutableHandleObject constructor);
 };
 
 class FundamentalInstance
@@ -156,6 +163,10 @@ class FundamentalInstance
         get_prototype()->call_set_value_function(gvalue, m_ptr);
     }
 
+    GJS_JSAPI_RETURN_CONVENTION
+    bool associate_js_instance(JSContext* cx, JSObject* object,
+                               void* gfundamental);
+
     // JS constructor
 
     GJS_JSAPI_RETURN_CONVENTION
@@ -164,34 +175,12 @@ class FundamentalInstance
 
  public:
     GJS_JSAPI_RETURN_CONVENTION
-    bool associate_js_instance(JSContext* cx, JSObject* object,
-                               void* gfundamental);
+    static JSObject* object_for_c_ptr(JSContext* cx, void* gfundamental);
+    GJS_JSAPI_RETURN_CONVENTION
+    static JSObject* object_for_gvalue(JSContext* cx, const GValue* gvalue,
+                                       GType gtype);
 
     static void* copy_ptr(JSContext* cx, GType gtype, void* gfundamental);
 };
 
-G_BEGIN_DECLS
-
-GJS_JSAPI_RETURN_CONVENTION
-bool gjs_define_fundamental_class(JSContext              *context,
-                                  JS::HandleObject        in_object,
-                                  GIObjectInfo           *info,
-                                  JS::MutableHandleObject constructor,
-                                  JS::MutableHandleObject prototype);
-
-GJS_JSAPI_RETURN_CONVENTION
-JSObject* gjs_object_from_g_fundamental      (JSContext     *context,
-                                              GIObjectInfo  *info,
-                                              void          *fobj);
-
-GJS_JSAPI_RETURN_CONVENTION
-JSObject *gjs_fundamental_from_g_value       (JSContext     *context,
-                                              const GValue  *value,
-                                              GType          gtype);
-
-void      gjs_fundamental_unref              (JSContext     *context,
-                                              void          *fobj);
-
-G_END_DECLS
-
 #endif  /* __GJS_FUNDAMENTAL_H__ */
diff --git a/gi/repo.cpp b/gi/repo.cpp
index f46d599a..4d69ce6c 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -437,15 +437,10 @@ gjs_define_info(JSContext       *context,
                                                    gtype, &ignored1, &ignored2))
                     return false;
             } else if (G_TYPE_IS_INSTANTIATABLE(gtype)) {
-                JS::RootedObject ignored1(context), ignored2(context);
-                if (!gjs_define_fundamental_class(context, in_object,
-                                                  (GIObjectInfo*)info,
-                                                  &ignored1, &ignored2)) {
-                    gjs_throw (context,
-                               "Unsupported fundamental class creation for type %s",
-                               g_type_name(gtype));
+                JS::RootedObject ignored(context);
+                if (!FundamentalPrototype::define_class(context, in_object,
+                                                        info, &ignored))
                     return false;
-                }
             } else {
                 gjs_throw (context,
                            "Unsupported type %s, deriving from fundamental %s",
diff --git a/gi/value.cpp b/gi/value.cpp
index 75b53d2e..9a25e18e 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -958,8 +958,8 @@ gjs_value_from_g_value_internal(JSContext             *context,
     } else if (G_TYPE_IS_INSTANTIATABLE(gtype)) {
         /* The gtype is none of the above, it should be a custom
            fundamental type. */
-        JSObject *obj;
-        obj = gjs_fundamental_from_g_value(context, (const GValue*)gvalue, gtype);
+        JSObject* obj =
+            FundamentalInstance::object_for_gvalue(context, gvalue, gtype);
         if (obj == NULL)
             return false;
         else