[gjs/ewlsh/refactor-prototype-resolution] gi: Refactor for resolving prototypes

[Evan Welsh <ewlsh src gnome org>]

commit e19d3d3ae382e8d84cabb1bfaadb203f916a10f4
Author: Evan Welsh <contact evanwelsh com>
Date:   Thu Dec 30 21:14:51 2021 -0800

    gi: Refactor for resolving prototypes

 gi/boxed.cpp       |  4 ++--
 gi/boxed.h         |  2 +-
 gi/fundamental.cpp |  5 ++--
 gi/fundamental.h   |  3 ++-
 gi/gerror.cpp      |  4 ++--
 gi/gerror.h        |  2 +-
 gi/interface.h     |  5 ++--
 gi/object.cpp      | 30 ++++++++++++++++-------
 gi/object.h        |  3 ++-
 gi/union.cpp       |  4 ++--
 gi/union.h         |  2 +-
 gi/wrapperutils.h  | 70 +++++++++++++++++++++++++++++++++++++++++-------------
 12 files changed, 93 insertions(+), 41 deletions(-)
---
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index 531544df..92af1366 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -39,8 +39,8 @@
 #include "gjs/mem-private.h"
 #include "util/log.h"
 
-BoxedInstance::BoxedInstance(JSContext* cx, JS::HandleObject obj)
-    : GIWrapperInstance(cx, obj),
+BoxedInstance::BoxedInstance(BoxedPrototype* prototype, JS::HandleObject obj)
+    : GIWrapperInstance(prototype, obj),
       m_allocated_directly(false),
       m_owning_ptr(false) {
     GJS_INC_COUNTER(boxed_instance);
diff --git a/gi/boxed.h b/gi/boxed.h
index fc053a6f..29a0d9d6 100644
--- a/gi/boxed.h
+++ b/gi/boxed.h
@@ -164,7 +164,7 @@ class BoxedInstance
     bool m_owning_ptr : 1;  // if set, the JS wrapper owns the C memory referred
                             // to by m_ptr.
 
-    explicit BoxedInstance(JSContext* cx, JS::HandleObject obj);
+    explicit BoxedInstance(BoxedPrototype* prototype, JS::HandleObject obj);
     ~BoxedInstance(void);
 
     // Don't set GIWrapperBase::m_ptr directly. Instead, use one of these
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index 72b69259..b776fdc1 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -33,8 +33,9 @@ namespace JS {
 class CallArgs;
 }
 
-FundamentalInstance::FundamentalInstance(JSContext* cx, JS::HandleObject obj)
-    : GIWrapperInstance(cx, obj) {
+FundamentalInstance::FundamentalInstance(FundamentalPrototype* prototype,
+                                         JS::HandleObject obj)
+    : GIWrapperInstance(prototype, obj) {
     GJS_INC_COUNTER(fundamental_instance);
 }
 
diff --git a/gi/fundamental.h b/gi/fundamental.h
index 7d2f8de0..3d7caef6 100644
--- a/gi/fundamental.h
+++ b/gi/fundamental.h
@@ -144,7 +144,8 @@ class FundamentalInstance
     friend class GIWrapperBase<FundamentalBase, FundamentalPrototype,
                                FundamentalInstance>;
 
-    explicit FundamentalInstance(JSContext* cx, JS::HandleObject obj);
+    explicit FundamentalInstance(FundamentalPrototype* prototype,
+                                 JS::HandleObject obj);
     ~FundamentalInstance(void);
 
     // Helper methods
diff --git a/gi/gerror.cpp b/gi/gerror.cpp
index 2e62cddb..737de4ce 100644
--- a/gi/gerror.cpp
+++ b/gi/gerror.cpp
@@ -43,8 +43,8 @@ ErrorPrototype::ErrorPrototype(GIEnumInfo* info, GType gtype)
 
 ErrorPrototype::~ErrorPrototype(void) { GJS_DEC_COUNTER(gerror_prototype); }
 
-ErrorInstance::ErrorInstance(JSContext* cx, JS::HandleObject obj)
-    : GIWrapperInstance(cx, obj) {
+ErrorInstance::ErrorInstance(ErrorPrototype* prototype, JS::HandleObject obj)
+    : GIWrapperInstance(prototype, obj) {
     GJS_INC_COUNTER(gerror_instance);
 }
 
diff --git a/gi/gerror.h b/gi/gerror.h
index 8841a994..c2224889 100644
--- a/gi/gerror.h
+++ b/gi/gerror.h
@@ -128,7 +128,7 @@ class ErrorInstance : public GIWrapperInstance<ErrorBase, ErrorPrototype,
                                    GError>;
     friend class GIWrapperBase<ErrorBase, ErrorPrototype, ErrorInstance>;
 
-    explicit ErrorInstance(JSContext* cx, JS::HandleObject obj);
+    explicit ErrorInstance(ErrorPrototype* prototype, JS::HandleObject obj);
     ~ErrorInstance(void);
 
  public:
diff --git a/gi/interface.h b/gi/interface.h
index 490b7093..9cc2853b 100644
--- a/gi/interface.h
+++ b/gi/interface.h
@@ -106,8 +106,9 @@ class InterfaceInstance
     friend class GIWrapperBase<InterfaceBase, InterfacePrototype,
                                InterfaceInstance>;
 
-    [[noreturn]] InterfaceInstance(JSContext* cx, JS::HandleObject obj)
-        : GIWrapperInstance(cx, obj) {
+    [[noreturn]] InterfaceInstance(InterfacePrototype* prototype,
+                                   JS::HandleObject obj)
+        : GIWrapperInstance(prototype, obj) {
         g_assert_not_reached();
     }
     [[noreturn]] ~InterfaceInstance(void) { g_assert_not_reached(); }
diff --git a/gi/object.cpp b/gi/object.cpp
index 0effe95f..d1880966 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1516,8 +1516,9 @@ void ObjectInstance::prepare_shutdown(void) {
         std::mem_fn(&ObjectInstance::release_native_object));
 }
 
-ObjectInstance::ObjectInstance(JSContext* cx, JS::HandleObject object)
-    : GIWrapperInstance(cx, object),
+ObjectInstance::ObjectInstance(ObjectPrototype* prototype,
+                               JS::HandleObject object)
+    : GIWrapperInstance(prototype, object),
       m_wrapper_finalized(false),
       m_gobj_disposed(false),
       m_gobj_finalized(false),
@@ -2553,6 +2554,15 @@ bool ObjectPrototype::get_parent_constructor(
     return true;
 }
 
+void ObjectPrototype::set_interfaces(GType* interface_gtypes,
+                                     uint32_t n_interface_gtypes) {
+    if (interface_gtypes) {
+        for (uint32_t n = 0; n < n_interface_gtypes; n++) {
+            m_interface_gtypes.push_back(interface_gtypes[n]);
+        }
+    }
+}
+
 /*
  * ObjectPrototype::define_class:
  * @in_object: Object where the constructor is stored, typically a repo object.
@@ -2575,11 +2585,7 @@ bool ObjectPrototype::define_class(
     if (!priv)
         return false;
 
-    if (interface_gtypes) {
-        for (uint32_t n = 0; n < n_interface_gtypes; n++) {
-            priv->m_interface_gtypes.push_back(interface_gtypes[n]);
-        }
-    }
+    priv->set_interfaces(interface_gtypes, n_interface_gtypes);
 
     JS::RootedObject parent_constructor(context);
     if (!priv->get_parent_constructor(context, &parent_constructor))
@@ -2672,11 +2678,17 @@ ObjectInstance* ObjectInstance::new_for_gobject(JSContext* cx, GObject* gobj) {
         return nullptr;
 
     JS::RootedObject obj(
-        cx, JS_NewObjectWithGivenProto(cx, JS_GetClass(proto), proto));
+        cx, JS_NewObjectWithGivenProto(cx, &ObjectBase::klass, proto));
     if (!obj)
         return nullptr;
 
-    ObjectInstance* priv = ObjectInstance::new_for_js_object(cx, obj);
+    ObjectPrototype* prototype = resolve_prototype(cx, proto);
+    if (!prototype)
+        return nullptr;
+
+    ObjectInstance* priv = new ObjectInstance(prototype, obj);
+
+    JS_SetPrivate(obj, priv);
 
     g_object_ref_sink(gobj);
     priv->associate_js_gobject(cx, obj, gobj);
diff --git a/gi/object.h b/gi/object.h
index 8bd88957..886f13ea 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -236,6 +236,7 @@ class ObjectPrototype
                           const char* name, bool* resolved);
 
  public:
+    void set_interfaces(GType* interface_gtypes, uint32_t n_interface_gtypes);
     void set_type_qdata(void);
     GJS_JSAPI_RETURN_CONVENTION
     GParamSpec* find_param_spec_from_id(JSContext* cx, JS::HandleString key);
@@ -310,7 +311,7 @@ class ObjectInstance : public GIWrapperInstance<ObjectBase, ObjectPrototype,
     /* Constructors */
 
  private:
-    ObjectInstance(JSContext* cx, JS::HandleObject obj);
+    ObjectInstance(ObjectPrototype* prototype, JS::HandleObject obj);
     ~ObjectInstance();
 
     GJS_JSAPI_RETURN_CONVENTION
diff --git a/gi/union.cpp b/gi/union.cpp
index a91225d4..dca23592 100644
--- a/gi/union.cpp
+++ b/gi/union.cpp
@@ -29,8 +29,8 @@ UnionPrototype::UnionPrototype(GIUnionInfo* info, GType gtype)
 
 UnionPrototype::~UnionPrototype(void) { GJS_DEC_COUNTER(union_prototype); }
 
-UnionInstance::UnionInstance(JSContext* cx, JS::HandleObject obj)
-    : GIWrapperInstance(cx, obj) {
+UnionInstance::UnionInstance(UnionPrototype* prototype, JS::HandleObject obj)
+    : GIWrapperInstance(prototype, obj) {
     GJS_INC_COUNTER(union_instance);
 }
 
diff --git a/gi/union.h b/gi/union.h
index 269cf156..4b490811 100644
--- a/gi/union.h
+++ b/gi/union.h
@@ -65,7 +65,7 @@ class UnionInstance
     friend class GIWrapperInstance<UnionBase, UnionPrototype, UnionInstance>;
     friend class GIWrapperBase<UnionBase, UnionPrototype, UnionInstance>;
 
-    explicit UnionInstance(JSContext* cx, JS::HandleObject obj);
+    explicit UnionInstance(UnionPrototype* prototype, JS::HandleObject obj);
     ~UnionInstance(void);
 
     GJS_JSAPI_RETURN_CONVENTION
diff --git a/gi/wrapperutils.h b/gi/wrapperutils.h
index 777fb72c..1a20b1b5 100644
--- a/gi/wrapperutils.h
+++ b/gi/wrapperutils.h
@@ -10,6 +10,7 @@
 
 #include <stdint.h>
 
+#include <new>
 #include <string>
 
 #include <girepository.h>
@@ -297,6 +298,19 @@ class GIWrapperBase : public CWrapperPointerOps<Base> {
     }
 
  protected:
+    /**
+     * GIWrapperBase::resolve_prototype:
+     */
+    [[nodiscard]] static Prototype* resolve_prototype(JSContext* cx,
+                                                      JS::HandleObject proto) {
+        if (JS_GetClass(proto) != &Base::klass) {
+            gjs_throw(cx, "Tried to construct an object without a GType");
+            return nullptr;
+        }
+
+        return Prototype::for_js(cx, proto);
+    }
+
     /*
      * GIWrapperBase::resolve:
      *
@@ -429,14 +443,14 @@ class GIWrapperBase : public CWrapperPointerOps<Base> {
         JS::RootedObject proto(cx);
         if (!JS_GetPrototype(cx, obj, &proto))
             return false;
-        if (JS_GetClass(proto) != &Base::klass) {
-            gjs_throw(cx, "Tried to construct an object without a GType");
+
+        Prototype* prototype = resolve_prototype(cx, proto);
+        if (!prototype)
             return false;
-        }
 
         args.rval().setUndefined();
 
-        Instance* priv = Instance::new_for_js_object(cx, obj);
+        Instance* priv = Instance::new_for_js_object(prototype, obj);
 
         {
             std::string fullName = priv->format_name();
@@ -641,6 +655,9 @@ class GIWrapperBase : public CWrapperPointerOps<Base> {
 template <class Base, class Prototype, class Instance,
           typename Info = GIObjectInfo>
 class GIWrapperPrototype : public Base {
+    using GjsAutoPrototype =
+        GjsAutoPointer<Prototype, void, g_atomic_rc_box_release>;
+
  protected:
     // m_info may be null in the case of JS-defined types, or internal types
     // not exposed through introspection, such as GLocalFile. Not all subclasses
@@ -798,6 +815,22 @@ class GIWrapperPrototype : public Base {
             cx, constructor, m_gtype, m_info);
     }
 
+    GJS_JSAPI_RETURN_CONVENTION
+    static Prototype* create_prototype(Info* info, GType gtype) {
+        g_assert(gtype != G_TYPE_INVALID);
+
+        // We have to keep the Prototype in an arcbox because some of its
+        // members are needed in some Instance destructors, e.g. m_gtype to
+        // figure out how to free the Instance's m_ptr, and m_info to figure out
+        // how many bytes to free if it is allocated directly. Storing a
+        // refcount on the prototype is cheaper than storing pointers to m_info
+        // and m_gtype on each instance.
+        Prototype* priv = g_atomic_rc_box_new0(Prototype);
+        new (priv) Prototype(info, gtype);
+
+        return priv;
+    }
+
  public:
     /**
      * GIWrapperPrototype::create_class:
@@ -828,17 +861,8 @@ class GIWrapperPrototype : public Base {
                                    JS::MutableHandleObject constructor,
                                    JS::MutableHandleObject prototype) {
         g_assert(in_object);
-        g_assert(gtype != G_TYPE_INVALID);
 
-        // We have to keep the Prototype in an arcbox because some of its
-        // members are needed in some Instance destructors, e.g. m_gtype to
-        // figure out how to free the Instance's m_ptr, and m_info to figure out
-        // how many bytes to free if it is allocated directly. Storing a
-        // refcount on the prototype is cheaper than storing pointers to m_info
-        // and m_gtype on each instance.
-        GjsAutoPointer<Prototype, void, g_atomic_rc_box_release> priv =
-            g_atomic_rc_box_new0(Prototype);
-        new (priv) Prototype(info, gtype);
+        GjsAutoPrototype priv = create_prototype(info, gtype);
         if (!priv->init(cx))
             return nullptr;
 
@@ -954,11 +978,12 @@ class GIWrapperInstance : public Base {
  protected:
     GjsSmartPointer<Wrapped> m_ptr;
 
-    explicit GIWrapperInstance(JSContext* cx, JS::HandleObject obj)
-        : Base(Prototype::for_js_prototype(cx, obj)), m_ptr(nullptr) {
+    explicit GIWrapperInstance(Prototype* prototype, JS::HandleObject obj)
+        : Base(prototype), m_ptr(nullptr) {
         Base::m_proto->acquire();
         Base::GIWrapperBase::debug_lifecycle(obj, "Instance constructor");
     }
+
     ~GIWrapperInstance(void) { Base::m_proto->release(); }
 
  public:
@@ -971,7 +996,8 @@ class GIWrapperInstance : public Base {
     [[nodiscard]] static Instance* new_for_js_object(JSContext* cx,
                                                      JS::HandleObject obj) {
         g_assert(!JS_GetPrivate(obj));
-        auto* priv = new Instance(cx, obj);
+        Prototype* prototype = Prototype::for_js_prototype(cx, obj);
+        auto* priv = new Instance(prototype, obj);
 
         // Init the private variable before we do anything else. If a garbage
         // collection happens when calling the constructor, then this object
@@ -981,6 +1007,16 @@ class GIWrapperInstance : public Base {
         return priv;
     }
 
+    [[nodiscard]] static Instance* new_for_js_object(Prototype* prototype,
+                                                     JS::HandleObject obj) {
+        g_assert(!JS_GetPrivate(obj));
+        auto* priv = new Instance(prototype, obj);
+
+        JS_SetPrivate(obj, priv);
+
+        return priv;
+    }
+
     // Method to get an existing Instance
 
     /*