GObject: don't use toggle references unless necessary

Many GObject (such as widgets and actors) don't have JS state, but
they're kept alive by C code. We can therefore save some memory
by GCing these objects, and creating new wrappers when needed.
If state is ever set, we transparently switch to toggle refs, so
no change should be visible at the JS level.

https://bugzilla.gnome.org/show_bug.cgi?id=679688

1 files changed, 61 insertions, 14 deletions

diff --git a/gi/object.c b/gi/object.c
index d5652860..0b2f1143 100644
--- a/gi/object.c
+++ b/gi/object.c
@@ -60,6 +60,13 @@ typedef struct {
     /* the GObjectClass wrapped by this JS Object (only used for
        prototypes) */
     GTypeClass *klass;
+
+    /* TRUE if this object has visible JS state, and
+       thus its lifecycle is managed using toggle references.
+       FALSE if this object just keeps a hard ref on the underlying GObject,
+       and may be finalized at will.
+    */
+    gboolean uses_toggle_ref;
 } ObjectInstance;
 
 typedef struct {
@@ -86,6 +93,8 @@ static JSObject*       peek_js_obj  (JSContext *context,
 static void            set_js_obj   (JSContext *context,
                                      GObject   *gobj,
                                      JSObject  *obj);
+static void            ensure_uses_toggle_ref (JSContext *context,
+                                                JSObject  *obj);
 
 typedef enum {
     SOME_ERROR_OCCURRED = JS_FALSE,
@@ -297,8 +306,10 @@ object_instance_set_prop(JSContext *context,
     GParameter param = { NULL, { 0, }};
     JSBool ret = JS_TRUE;
 
-    if (!gjs_get_string_id(context, id, &name))
-        return JS_TRUE; /* not resolved, but no error */
+    if (!gjs_get_string_id(context, id, &name)) {
+        /* not resolved, but no error */
+        goto set_custom;
+    }
 
     priv = priv_from_js(context, obj);
     gjs_debug_jsprop(GJS_DEBUG_GOBJECT,
@@ -318,8 +329,9 @@ object_instance_set_prop(JSContext *context,
                                        FALSE /* constructing */)) {
     case SOME_ERROR_OCCURRED:
         ret = JS_FALSE;
-    case NO_SUCH_G_PROPERTY:
         goto out;
+    case NO_SUCH_G_PROPERTY:
+        goto set_custom;
     case VALUE_WAS_SET:
         break;
     }
@@ -338,6 +350,14 @@ object_instance_set_prop(JSContext *context,
  out:
     g_free(name);
     return ret;
+
+ set_custom:
+    /* We need to keep the wrapper alive in order not
+     * to lose custom "expando" properties
+     */
+    ensure_uses_toggle_ref(context, obj);
+    g_free(name);
+    return JS_TRUE;
 }
 
 static gboolean
@@ -912,6 +932,7 @@ associate_js_gobject (JSContext      *context,
     ObjectInstance *priv;
 
     priv = priv_from_js(context, object);
+    priv->uses_toggle_ref = FALSE;
     priv->gobj = gobj;
 
     g_assert(peek_js_obj(context, gobj) == NULL);
@@ -920,6 +941,18 @@ associate_js_gobject (JSContext      *context,
 #if DEBUG_DISPOSE
     g_object_weak_ref(gobj, wrapped_gobj_dispose_notify, object);
 #endif
+}
+
+static void
+ensure_uses_toggle_ref (JSContext *context,
+                         JSObject  *object)
+{
+    ObjectInstance *priv;
+
+    priv = priv_from_js(context, object);
+
+    if (priv->uses_toggle_ref)
+        return;
 
     /* OK, here is where things get complicated. We want the
      * wrapped gobj to keep the JSObject* wrapper alive, because
@@ -932,6 +965,7 @@ associate_js_gobject (JSContext      *context,
      * the wrapper to be garbage collected (and thus unref the
      * wrappee).
      */
+    priv->uses_toggle_ref = TRUE;
     priv->keep_alive = gjs_keep_alive_get_for_import_global(context);
     gjs_keep_alive_add_child(context,
                              priv->keep_alive,
@@ -939,9 +973,15 @@ associate_js_gobject (JSContext      *context,
                              object,
                              priv);
 
-    g_object_add_toggle_ref(gobj,
+    g_object_add_toggle_ref(priv->gobj,
                             wrapped_gobj_toggle_notify,
                             JS_GetRuntime(context));
+
+    /* We now have both a ref and a toggle ref, we only want the
+     * toggle ref. This may immediately remove the GC root
+     * we just added, since refcount may drop to 1.
+     */
+    g_object_unref(priv->gobj);
 }
 
 static JSBool
@@ -989,6 +1029,8 @@ object_instance_init (JSContext *context,
          * we're not actually using it, so just let it get collected. Avoiding
          * this would require a non-trivial amount of work.
          * */
+
+        ensure_uses_toggle_ref(context, old_jsobj);
         *object = old_jsobj;
         g_object_unref(gobj); /* We already own a reference */
         gobj = NULL;
@@ -1016,11 +1058,6 @@ object_instance_init (JSContext *context,
 
     if (priv->gobj == NULL)
         associate_js_gobject(context, *object, gobj);
-    /* We now have both a ref and a toggle ref, we only want the
-     * toggle ref. This may immediately remove the GC root
-     * we just added, since refcount may drop to 1.
-     */
-    g_object_unref(gobj);
 
     gjs_debug_lifecycle(GJS_DEBUG_GOBJECT,
                         "JSObject created with GObject %p %s",
@@ -1120,8 +1157,12 @@ object_instance_finalize(JSContext *context,
                     priv->info ? g_base_info_get_name((GIBaseInfo*) priv->info) : g_type_name(priv->gtype));
         }
         set_js_obj(context, priv->gobj, NULL);
-        g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify,
-                                   JS_GetRuntime(context));
+
+        if (priv->uses_toggle_ref)
+            g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify,
+                                       JS_GetRuntime(context));
+        else
+            g_object_unref(priv->gobj);
         priv->gobj = NULL;
     }
 
@@ -1209,6 +1250,9 @@ gjs_object_associate_closure (JSContext *context,
     if (priv == NULL)
         return JS_FALSE;
 
+    if (priv->gobj != NULL)
+        ensure_uses_toggle_ref(context, object);
+
     do_associate_closure(priv, closure);
 
     return JS_TRUE;
@@ -1248,6 +1292,8 @@ real_connect_func(JSContext *context,
         return JS_FALSE;
     }
 
+    ensure_uses_toggle_ref(context, obj);
+
     /* Best I can tell, there is no way to know if argv[1] is really
      * callable other than to just try it. Checking whether it's a
      * function will not detect native objects that provide
@@ -1941,9 +1987,6 @@ gjs_object_from_g_object(JSContext    *context,
         g_object_ref_sink(gobj);
         associate_js_gobject(context, obj, gobj);
 
-        /* see the comment in init_object_instance() for this */
-        g_object_unref(gobj);
-
         g_assert(peek_js_obj(context, gobj) == obj);
     }
 
@@ -2270,6 +2313,10 @@ gjs_object_set_gproperty (GObject      *object,
     if (property_id == PROP_JS_OBJECT) {
         js_obj = g_value_get_pointer (value);
         associate_js_gobject(context, js_obj, object);
+
+        /* Custom JS objects will most likely have visible state, so
+           just do this from the start */
+        ensure_uses_toggle_ref(context, js_obj);
         return;
     }