[glibmm/glibmm-2-66] Glib::Binding: Add set_manage() to not require a RefPtr

[Kjell Ahlstedt <kjellahl src gnome org>]

commit f648c7fcde4e18b501eff7a1a702b6415d6e949c
Author: Daniel Boles <dboles src gnome org>
Date:   Tue Jan 12 09:48:46 2021 +0100

    Glib::Binding: Add set_manage() to not require a RefPtr
    
    Allow users to call Glib::manage(RefPtr<Binding>) in order to defeat our
    pre-existing and still default behaviour that a user must keep a RefPtr
    to the Binding in order to keep it alive, instead letting it revert to
    the C-style behaviour of staying alive as long as either object does.
    
    We use Glib::manage() to avoid the possible pitfall of a user calling
    ->set_manage() on the result of bind_property*(), which might be empty.
    
    This patch has been modified by Kjell Ahlstedt.
    This modified version stores the new instance data in a GQuark
    instead of new member data, which would have broken ABI.
    
    Fixes https://gitlab.gnome.org/GNOME/glibmm/issues/62

 glib/src/binding.ccg         | 37 ++++++++++++++++++++++------
 glib/src/binding.hg          | 57 +++++++++++++++++++++++++++++++++++++++-----
 tests/glibmm_binding/main.cc | 20 ++++++++++++++++
 3 files changed, 101 insertions(+), 13 deletions(-)
---
diff --git a/glib/src/binding.ccg b/glib/src/binding.ccg
index 732e103d..4dc0f289 100644
--- a/glib/src/binding.ccg
+++ b/glib/src/binding.ccg
@@ -19,6 +19,9 @@
 
 namespace
 {
+// TODO: When we can break ABI, replace this GQuark by a new data member in Glib::Binding.
+GQuark quark_manage = g_quark_from_static_string("glibmm__Glib::Binding::manage");
+
 struct BindingTransformSlots
 {
   BindingTransformSlots(const Glib::Binding::SlotTransform& transform_to,
@@ -110,7 +113,8 @@ Binding::bind_property_value(const PropertyProxy_Base& source_property,
 
   // Take an extra ref. GBinding uses one ref itself, and drops it if
   // either the source object or the target object is finalized.
-  // The GBinding object must not be destroyed while there are RefPtrs around.
+  // The GBinding object must not be destroyed while there are RefPtrs around,
+  // unless set_manage() was called.
   g_object_ref(binding);
   return Glib::RefPtr<Binding>(new Binding(binding));
 }
@@ -160,19 +164,38 @@ Binding::unbind()
 // It calls g_object_unref() itself, if either the source object or the
 // target object is finalized, almost like g_binding_unbind().
 // But the GBinding object shall be destroyed when and only when the last
-// reference from a Glib::RefPtr is dropped.
+// reference from a Glib::RefPtr is dropped, unless set_manage() was called.
 void
 Binding::unreference() const
 {
-  GBinding* const binding = const_cast<GBinding*>(gobj());
+  if (!g_object_get_qdata(gobject_, quark_manage))
+  {
+    GBinding* const binding = const_cast<GBinding*>(gobj());
 
-  // If the last Glib::RefPtr is being deleted, and the binding has not been unbound,
-  // then drop the extra reference that was added by bind_property_value().
-  if (gobject_->ref_count == 2 && g_binding_get_source(binding))
-    g_object_unref(binding);
+    // If the last Glib::RefPtr is being deleted, and the binding has not been unbound,
+    // then drop the extra reference that was added by bind_property_value().
+    if (gobject_->ref_count == 2 && g_binding_get_source(binding))
+      g_object_unref(binding);
+  }
 
   Object::unreference();
 }
 G_GNUC_END_IGNORE_DEPRECATIONS
 
+void
+Binding::set_manage()
+{
+  // Any pointer can be set, just not nullptr.
+  g_object_set_qdata(gobject_, quark_manage, this);
+}
+
+const Glib::RefPtr<Glib::Binding>&
+manage(const Glib::RefPtr<Glib::Binding>& binding)
+{
+  if (binding)
+    binding->set_manage();
+
+  return binding;
+}
+
 } // namespace Glib
diff --git a/glib/src/binding.hg b/glib/src/binding.hg
index 4ad909d8..dd4054fd 100644
--- a/glib/src/binding.hg
+++ b/glib/src/binding.hg
@@ -90,12 +90,19 @@ _WRAP_ENUM(BindingFlags, GBindingFlags, newin "2,44", decl_prefix GLIBMM_API)
  * or g_signal_handler_block().
  *
  * The binding between properties is broken, and its resources freed, when the
- * %Glib::Binding loses its last Glib::RefPtr, the source or target object is
- * deleted, or unbind() is called. If a Glib::RefPtr to the %Glib::Binding
- * remains after the binding is broken another way, get_source()
- * and get_target() return an empty Glib::RefPtr. So, you must
- * keep a Glib::RefPtr to the %Glib::Binding for as long as you want it to bind,
- * but doing that does not guarantee the source/target are still alive or bound.
+ * %Glib::Binding loses its last Glib::RefPtr (by default), the source or target
+ * object is deleted, or unbind() is called. If a Glib::RefPtr to the
+ * %Glib::Binding remains after the binding is broken another way,
+ * get_source() and get_target() return an empty Glib::RefPtr.
+ * So, by default, you must keep a Glib::RefPtr to the
+ * %Glib::Binding for as long as you want it to bind, but doing that does not
+ * guarantee the source/target are still alive or bound.
+ *
+ * If it is not convenient to maintain a Glib::RefPtr to keep a %Glib::Binding
+ * active, you can pass the %Glib::Binding to Glib::manage() to specify that it
+ * should have its lifetime managed by the source/target objects and unbind()
+ * only. In that case, it will stay active as long as the source and target
+ * exist and unbind() is not called, even if no Glib::RefPtr to it is kept.
  *
  * @newin{2,44}
  */
@@ -466,6 +473,19 @@ public:
   _WRAP_PROPERTY("target-property", Glib::ustring, newin "2,44")
 
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
+  /** Sets the binding as having its lifetime managed by the lifetimes of its
+   * source and target objects, rather than requiring a Glib::RefPtr to be kept.
+   *
+   * See the class description for full information on the lifetimes of bindings.
+   *
+   * To manage the result of bind_property() or bind_property_value(), which can
+   * return an empty Glib::RefPtr on error, pass the result to Glib::manage(),
+   * as that specifically avoids calling set_manage() on an empty Glib::RefPtr.
+   *
+   * @newin{2,66}
+   */
+  void set_manage() override final;
+
   /** Decrement the reference count for this object.
    * You should never need to do this manually - use the object via a RefPtr instead.
    */
@@ -504,4 +524,29 @@ private:
   };
 };
 
+/** Sets a Glib::Binding as having its lifetime managed by the lifetimes of its
+ * source and target objects, rather than requiring a Glib::RefPtr to be kept.
+ *
+ * See the class description of Glib::Binding for full information on the
+ * lifetimes of bindings.
+ *
+ * For instance:
+ * @code
+ * void bind_my_properties(const Glib::RefPtr<Source>& source,
+ *                         const Glib::RefPtr<Target>& target)
+ * {
+ *   Glib::manage(Glib::Binding::bind_property(source->property_foo(),
+ *                                             target->property_bar()));
+ *   // don’t keep RefPtr to new Binding as source & target control its lifetime
+ * }
+ * @endcode
+ *
+ * @newin{2,66}
+ * @relates Glib::Binding
+ *
+ * @param binding The Glib::Binding to set as managed, or an empty Glib::RefPtr.
+ * @return The same Glib::Binding or empty Glib::RefPtr.
+ */
+const Glib::RefPtr<Glib::Binding>& manage(const Glib::RefPtr<Glib::Binding>& binding);
+
 } // namespace Glib
diff --git a/tests/glibmm_binding/main.cc b/tests/glibmm_binding/main.cc
index 6b03b5f4..ac8a0bc9 100644
--- a/tests/glibmm_binding/main.cc
+++ b/tests/glibmm_binding/main.cc
@@ -109,6 +109,26 @@ test()
     // With SYNC_CREATE, value of source must sync to target on bind
     g_assert_cmpint(target.property_int(), ==, 89);
   }
+
+  // Ensure the binding was released when its RefPtr went out of scope
+  source.property_string() = "97";
+  g_assert_cmpint(target.property_int(), ==, 89);
+
+  // Ensure that a manage()d binding...
+  {
+    auto binding = Glib::Binding::bind_property(
+      source.property_string(), target.property_int(),
+      Glib::BINDING_DEFAULT, &transform_string_to_int);
+    Glib::manage(binding);
+
+    // (a) still syncs when the source value changes
+    source.property_string() = "1999";
+    g_assert_cmpint(target.property_int(), ==, 1999);
+  }
+
+  // and (b) still binds the properties after our RefPtr to it goes out of scope
+  source.property_string() = "2001";
+  g_assert_cmpint(target.property_int(), ==, 2001);
 }
 
 } // namespace