[gjs/ewlsh/refactor-prototype-resolution] gi: Refactor resolving prototypes in GIWrapperInstance constructors




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

    gi: Refactor resolving prototypes in GIWrapperInstance constructors
    
    This commit enables usecases where the JavaScript prototype may not
    be the same as the native Prototype object.
    
    Additionally this commit refactors ObjectInstance construction to
    match the new GIWrapperInstance constructor pattern.

 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
 
     /*


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]