libical-glib: Simplify memory management around ICalCompIter

Previously, before this change, the components returned from
the ICalCompIter structures did not hold the reference to the parent
component, thus the parent component could be freed before the returned
instance, effectively destroying the internal libical native structure.
There was required to set the owner manually before, but it breaks
abstraction and is easy to forget.

This change makes all these things transparent. The old code does not need
to be changed, because it should set the same owner anyway, thus the call
results in no change on the ownership of the returned components.

3 files changed, 155 insertions, 11 deletions

diff --git a/src/libical-glib/api/i-cal-component.xml b/src/libical-glib/api/i-cal-component.xml
index fcde0f42..ebe9e11e 100644
--- a/src/libical-glib/api/i-cal-component.xml
+++ b/src/libical-glib/api/i-cal-component.xml
@@ -182,32 +182,89 @@
         <returns type="ICalComponent *" annotation="transfer full, nullable" translator_argus="(GObject *)component" comment="The next #ICalComponent."/>
         <comment xml:space="preserve">Gets the next #ICalComponent with specific kind in #ICalComponent.</comment>
     </method>
-    <method name="i_cal_component_begin_component" corresponds="icalcomponent_begin_component" since="1.0">
+    <method name="i_cal_component_begin_component" corresponds="CUSTOM" since="1.0">
         <parameter type="ICalComponent *" name="component" comment="A #ICalComponent"/>
         <parameter type="ICalComponentKind" name="kind" comment="A #ICalComponentKind"/>
         <returns type="ICalCompIter *" annotation="transfer full" comment="A #ICalCompIter"/>
-        <comment xml:space="preserve">Gets the #ICalCompIter pointing to the first child #ICalComponent. Use #ICalCompIter when you need remove the child component from the parent. For traversing, i_cal_object_set_owner needs to be called after this API. See component.py in tests for details.</comment>
+        <comment xml:space="preserve">Gets the #ICalCompIter pointing to the first child #ICalComponent.</comment>
+        <custom>    ICalCompIter *iter;
+
+    g_return_val_if_fail (I_CAL_IS_COMPONENT (component), NULL);
+
+    iter = i_cal_comp_iter_new_full(icalcomponent_begin_component((icalcomponent *)i_cal_object_get_native(I_CAL_OBJECT (component)), (icalcomponent_kind) (kind)));
+    if (iter) {
+        i_cal_object_set_owner(I_CAL_OBJECT(iter), G_OBJECT(component));
+        i_cal_object_set_always_destroy(I_CAL_OBJECT(iter), TRUE);
+    }
+
+    return iter;</custom>
     </method>
-    <method name="i_cal_component_end_component" corresponds="icalcomponent_end_component" since="1.0">
+    <method name="i_cal_component_end_component" corresponds="CUSTOM" since="1.0">
         <parameter type="ICalComponent *" name="component" comment="A #ICalComponent"/>
         <parameter type="ICalComponentKind" name="kind" comment="A #ICalComponentKind"/>
         <returns type="ICalCompIter *" annotation="transfer full" comment="A #ICalCompIter"/>
-        <comment xml:space="preserve">Gets the #ICalCompIter pointing to the end child #ICalComponent. Use #ICalCompIter when you need remove the child component from the parent. For traversing, i_cal_object_set_owner needs to be called after this API. See component.py in tests for details.</comment>
+        <comment xml:space="preserve">Gets the #ICalCompIter pointing to the end child #ICalComponent.</comment>
+        <custom>    ICalCompIter *iter;
+    g_return_val_if_fail (I_CAL_IS_COMPONENT (component), NULL);
+
+    iter = i_cal_comp_iter_new_full(icalcomponent_end_component((icalcomponent *)i_cal_object_get_native(I_CAL_OBJECT (component)), (icalcomponent_kind) (kind)));
+    if (iter) {
+        i_cal_object_set_owner(I_CAL_OBJECT(iter), G_OBJECT(component));
+        i_cal_object_set_always_destroy(I_CAL_OBJECT(iter), TRUE);
+    }
+
+    return iter;</custom>
     </method>
-    <method name="i_cal_comp_iter_next" corresponds="icalcompiter_next" since="1.0">
+    <method name="i_cal_comp_iter_next" corresponds="CUSTOM" since="1.0">
         <parameter type="ICalCompIter *" name="i" native_op="POINTER" comment="A #ICalCompIter"/>
         <returns type="ICalComponent *" annotation="transfer full" comment="A #ICalCompIter"/>
-        <comment xml:space="preserve">Gets the next #ICalComponent pointed by #ICalCompIter. Use #ICalCompIter when you need remove the child component from the parent. For traversing, i_cal_object_set_owner needs to be called after this API. See component.py in tests for details.</comment>
+        <comment xml:space="preserve">Gets the next #ICalComponent pointed by #ICalCompIter.</comment>
+        <custom>    ICalComponent *comp;
+
+    g_return_val_if_fail (I_CAL_IS_COMP_ITER (i), NULL);
+
+    comp = i_cal_component_new_full(icalcompiter_next((struct icalcompiter *)i_cal_object_get_native(I_CAL_OBJECT (i))), NULL);
+    if (comp) {
+        GObject *owner = i_cal_object_ref_owner(I_CAL_OBJECT(i));
+        i_cal_object_set_owner(I_CAL_OBJECT(comp), owner);
+        g_clear_object(&amp;owner);
+    }
+
+    return comp;</custom>
     </method>
-    <method name="i_cal_comp_iter_prior" corresponds="icalcompiter_prior" since="1.0">
+    <method name="i_cal_comp_iter_prior" corresponds="CUSTOM" since="1.0">
         <parameter type="ICalCompIter *" name="i" native_op="POINTER" comment="A #ICalCompIter"/>
         <returns type="ICalComponent *" annotation="transfer full" comment="A #ICalCompIter"/>
-        <comment xml:space="preserve">Gets the prior #ICalComponent pointed by #ICalCompIter. Use #ICalCompIter when you need remove the child component from the parent. For traversing, i_cal_object_set_owner needs to be called after this API. See component.py in tests for details.</comment>
+        <comment xml:space="preserve">Gets the prior #ICalComponent pointed by #ICalCompIter.</comment>
+        <custom>    ICalComponent *comp;
+
+    g_return_val_if_fail (I_CAL_IS_COMP_ITER (i), NULL);
+
+    comp = i_cal_component_new_full(icalcompiter_prior((struct icalcompiter *)i_cal_object_get_native(I_CAL_OBJECT (i))), NULL);
+    if (comp) {
+        GObject *owner = i_cal_object_ref_owner(I_CAL_OBJECT(i));
+        i_cal_object_set_owner(I_CAL_OBJECT(comp), owner);
+        g_clear_object(&amp;owner);
+    }
+
+    return comp;</custom>
     </method>
-    <method name="i_cal_comp_iter_deref" corresponds="icalcompiter_deref" since="1.0">
+    <method name="i_cal_comp_iter_deref" corresponds="CUSTOM" since="1.0">
         <parameter type="ICalCompIter *" name="i" native_op="POINTER" comment="A #ICalCompIter"/>
         <returns type="ICalComponent *" annotation="transfer full" comment="A #ICalComponent"/>
-        <comment xml:space="preserve">Gets the current #ICalComponent pointed by #ICalCompIter. Use #ICalCompIter when you need remove the child component from the parent. For traversing, i_cal_object_set_owner needs to be called after this API. See component.py in tests for details.</comment>
+        <comment xml:space="preserve">Gets the current #ICalComponent pointed by #ICalCompIter.</comment>
+        <custom>    ICalComponent *comp;
+
+    g_return_val_if_fail (I_CAL_IS_COMP_ITER (i), NULL);
+
+    comp = i_cal_component_new_full(icalcompiter_deref((struct icalcompiter *)i_cal_object_get_native(I_CAL_OBJECT (i))), NULL);
+    if (comp) {
+        GObject *owner = i_cal_object_ref_owner(I_CAL_OBJECT(i));
+        i_cal_object_set_owner(I_CAL_OBJECT(comp), owner);
+        g_clear_object(&amp;owner);
+    }
+
+    return comp;</custom>
     </method>
     <method name="i_cal_component_check_restrictions" corresponds="icalcomponent_check_restrictions" kind="other" since="1.0">
         <parameter type="ICalComponent *" name="comp" comment="The #ICalComponent to be checked"/>
diff --git a/src/libical-glib/i-cal-object.c.in b/src/libical-glib/i-cal-object.c.in
index 1f593a94..f6633919 100644
--- a/src/libical-glib/i-cal-object.c.in
+++ b/src/libical-glib/i-cal-object.c.in
@@ -110,6 +110,7 @@ struct _ICalObjectPrivate
     gpointer native;
     GDestroyNotify native_destroy_func;
     gboolean is_global_memory;
+    gboolean always_destroy;
     GObject *owner;
     GSList *dependers;  /* referenced GObject-s */
 };
@@ -122,6 +123,7 @@ enum
     PROP_NATIVE,
     PROP_NATIVE_DESTROY_FUNC,
     PROP_IS_GLOBAL_MEMORY,
+    PROP_ALWAYS_DESTROY,
     PROP_OWNER
 };
 
@@ -150,6 +152,10 @@ static void i_cal_object_set_property(GObject *object, guint property_id,
         iobject->priv->is_global_memory = g_value_get_boolean(value);
         return;
 
+    case PROP_ALWAYS_DESTROY:
+        i_cal_object_set_always_destroy(iobject, g_value_get_boolean(value));
+        return;
+
     case PROP_OWNER:
         i_cal_object_set_owner(iobject, g_value_get_object(value));
         return;
@@ -180,6 +186,10 @@ static void i_cal_object_get_property(GObject *object, guint property_id,
         g_value_set_boolean(value, i_cal_object_get_is_global_memory(iobject));
         return;
 
+    case PROP_ALWAYS_DESTROY:
+        g_value_set_boolean(value, i_cal_object_get_always_destroy(iobject));
+        return;
+
     case PROP_OWNER:
         g_value_take_object(value, i_cal_object_ref_owner(iobject));
         return;
@@ -192,7 +202,8 @@ static void i_cal_object_finalize(GObject *object)
 {
     ICalObject *iobject = I_CAL_OBJECT(object);
 
-    if (!iobject->priv->owner && !iobject->priv->is_global_memory &&
+    if ((iobject->priv->always_destroy || !iobject->priv->owner) &&
+        !iobject->priv->is_global_memory &&
         iobject->priv->native && iobject->priv->native_destroy_func) {
         iobject->priv->native_destroy_func(iobject->priv->native);
     }
@@ -270,6 +281,25 @@ static void i_cal_object_class_init(ICalObjectClass * class)
             G_PARAM_STATIC_STRINGS));
 
     /**
+     * ICalObject:always-destroy:
+     *
+     * Whether free the native libical structure on #ICalObject's finalize even
+     * if the object has set an owner.
+     *
+     * Since: 3.0.11
+     **/
+    g_object_class_install_property(
+        object_class,
+        PROP_ALWAYS_DESTROY,
+        g_param_spec_boolean(
+            "always-destroy",
+            "Always-Destroy",
+            "Whether the native libical structure is freed even when the owner is set",
+            FALSE,
+            G_PARAM_READWRITE |
+            G_PARAM_STATIC_STRINGS));
+
+    /**
      * ICalObject:owner:
      *
      * Owner of the native libical structure. If set, then it is
@@ -663,3 +693,56 @@ void i_cal_object_remove_depender(ICalObject *iobject, GObject *depender)
 
     UNLOCK_PROPS(iobject);
 }
+
+/**
+ * i_cal_object_set_always_destroy:
+ * @iobject: an #ICalObject
+ * @value: value to set
+ *
+ * Sets the @ICalObject::always-destroy property value. When %TRUE, the native
+ * libical structure is always freed, even when an owner of the @iobject is set.
+ *
+ * Since: 3.0.11
+ **/
+void i_cal_object_set_always_destroy(ICalObject *iobject, gboolean value)
+{
+    gboolean changed;
+
+    g_return_if_fail(I_CAL_IS_OBJECT(iobject));
+
+    LOCK_PROPS(iobject);
+
+    changed = (value ? 1 : 0) != (iobject->priv->always_destroy ? 1 : 0);
+    if (changed)
+        iobject->priv->always_destroy = value;
+
+    UNLOCK_PROPS(iobject);
+
+    if (changed)
+        g_object_notify(G_OBJECT(iobject), "always-destroy");
+}
+
+/**
+ * i_cal_object_get_always_destroy:
+ * @iobject: an #ICalObject
+ *
+ * Obtain the @ICalObject::always-destroy property value.
+ *
+ * Returns: Whether the native libical structure is freed even when an owner is set.
+ *
+ * Since: 3.0.11
+ **/
+gboolean i_cal_object_get_always_destroy(ICalObject *iobject)
+{
+    gboolean value;
+
+    g_return_val_if_fail(I_CAL_IS_OBJECT(iobject), FALSE);
+
+    LOCK_PROPS(iobject);
+
+    value = iobject->priv->always_destroy;
+
+    UNLOCK_PROPS(iobject);
+
+    return value;
+}
diff --git a/src/libical-glib/i-cal-object.h.in b/src/libical-glib/i-cal-object.h.in
index bc53e9e0..19438fd4 100644
--- a/src/libical-glib/i-cal-object.h.in
+++ b/src/libical-glib/i-cal-object.h.in
@@ -107,6 +107,10 @@ LIBICAL_ICAL_EXPORT void i_cal_object_add_depender(ICalObject *iobject, GObject
 
 LIBICAL_ICAL_EXPORT void i_cal_object_remove_depender(ICalObject *iobject, GObject *depender);
 
+LIBICAL_ICAL_EXPORT void i_cal_object_set_always_destroy(ICalObject *iobject, gboolean value);
+
+LIBICAL_ICAL_EXPORT gboolean i_cal_object_get_always_destroy(ICalObject *iobject);
+
 LIBICAL_ICAL_EXPORT void i_cal_object_free_global_objects(void);
 
 G_END_DECLS