[glibmm/wip/dboles/Binding-Issue#62: 3/3] Binding: Add set_manage() to not require a RefPtr

[Daniel Boles <dboles src gnome org>]

commit 4b6b0664ab838a6f9c4b8d794671a0bd8f6fcefb
Author: Daniel Boles <dboles src gmail com>
Date:   Fri Nov 15 18:05:05 2019 +0000

    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.
    
    https://gitlab.gnome.org/GNOME/glibmm/issues/62

 glib/src/binding.ccg         | 33 ++++++++++++++++++++++++++-------
 glib/src/binding.hg          | 29 +++++++++++++++++++++++++++++
 tests/glibmm_binding/main.cc | 32 ++++++++++++++++++++++++++------
 3 files changed, 81 insertions(+), 13 deletions(-)
---
diff --git a/glib/src/binding.ccg b/glib/src/binding.ccg
index dfad8a66..56938f05 100644
--- a/glib/src/binding.ccg
+++ b/glib/src/binding.ccg
@@ -112,7 +112,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::make_refptr_for_instance<Binding>(new Binding(binding));
 }
@@ -131,18 +132,36 @@ 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 (!m_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();
 }
 
+void
+Binding::set_manage()
+{
+  m_manage = true;
+}
+
+Glib::RefPtr<Glib::Binding> const&
+manage(Glib::RefPtr<Glib::Binding> const& binding)
+{
+  if (binding)
+    binding->set_manage();
+
+  return binding;
+}
+
 } // namespace Glib
diff --git a/glib/src/binding.hg b/glib/src/binding.hg
index 1c963f36..eb2de747 100644
--- a/glib/src/binding.hg
+++ b/glib/src/binding.hg
@@ -156,6 +156,12 @@ public:
    * Glib::Binding for as long as you want to bind, but that alone does not
    * necessarily mean the binding between properties itself is still active.
    *
+   * 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.
+   *
    * A Glib::ObjectBase instance can have multiple bindings.
    *
    * If you supply transformation functions, it is usually easier to use one of the
@@ -415,9 +421,20 @@ public:
    * You should never need to do this manually - use the object via a RefPtr instead.
    */
   void unreference() const override;
+
+  /** 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 bind_property_value() for full information on the lifetimes of bindings.
+   *
+   * @newin{2,64}
+   */
+  void set_manage();
 #endif /* DOXYGEN_SHOULD_SKIP_THIS */
 
 private:
+  bool m_manage = false;
+
   // The functor TransformProp can be implicitly converted to a SlotTransform
   // and used in a call to bind_property_value().
   template <typename T_from, typename T_to>
@@ -447,4 +464,16 @@ 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 Glib::Binding::bind_property_value() for full information on the
+ * lifetimes of bindings.
+ *
+ * @newin{2,64}
+ * @param binding the Glib::Binding to set as managed, or an empty Glib::RefPtr.
+ * @return the same Glib::Binding.
+ */
+Glib::RefPtr<Glib::Binding> const& manage(Glib::RefPtr<Glib::Binding> const& binding);
+
 } // namespace Glib
diff --git a/tests/glibmm_binding/main.cc b/tests/glibmm_binding/main.cc
index 9b727e51..3c55d1f2 100644
--- a/tests/glibmm_binding/main.cc
+++ b/tests/glibmm_binding/main.cc
@@ -56,15 +56,20 @@ test()
   auto source = StringSource{};
   auto target = IntTarget{};
 
+  auto const create_binding = [&source, &target](Glib::Binding::Flags flags)
+  {
+    return Glib::Binding::bind_property(
+      source.property_string(), target.property_int(),
+      flags, &transform_string_to_int);
+  };
+
   // We should obviously not change the target before it has been bound!
   target.property_int() = 7;
   source.property_string() = "42";
   g_assert_cmpint(target.property_int(), ==, 7);
 
   {
-    auto binding = Glib::Binding::bind_property(
-      source.property_string(), target.property_int(),
-      Glib::Binding::Flags::DEFAULT, &transform_string_to_int);
+    auto binding = create_binding(Glib::Binding::Flags::DEFAULT);
 
     // Without SYNC_CREATE, only changes after bound will be synced
     g_assert_cmpint(target.property_int(), ==, 7);
@@ -87,13 +92,28 @@ test()
   g_assert_cmpint(target.property_int(), ==, 47);
 
   {
-    auto binding = Glib::Binding::bind_property(
-      source.property_string(), target.property_int(),
-      Glib::Binding::Flags::SYNC_CREATE, &transform_string_to_int);
+    auto binding = create_binding(Glib::Binding::Flags::SYNC_CREATE);
 
     // 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 a manage()d binding (a) works
+  {
+    auto binding = create_binding(Glib::Binding::Flags::DEFAULT);
+    Glib::manage(binding);
+
+    source.property_string() = "1999";
+    g_assert_cmpint(target.property_int(), ==, 1999);
+  }
+
+  // and (b) stays active after our RefPtr to it goes out of scope
+  source.property_string() = "2001";
+  g_assert_cmpint(target.property_int(), ==, 2001);
 }
 
 } // namespace