Simplify handling of adjustments

The relationship between adjustments and scrollbars and
scrollable widgets was much more complex than it needed to be.

StScrollView: Have the scroll view own a pair of adjustments,
  set them on the child on add(), remove unnecessary
  change notification signal connections.
StBoxLayout: Remove auto-create of adjustments, just take the
  adjustments from the scrollbars and set them on the scrollable
  child. Notify for hadjustment/vadjustment properties.
StScrollBar: Notify adjustment property.
StScrollable: Document how adjustment setting works.

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

src/st/st-box-layout.c

@@ -111,6 +111,8 @@ scrollable_set_adjustments (StScrollable *scrollable,
 {
   StBoxLayoutPrivate *priv = ST_BOX_LAYOUT (scrollable)->priv;
 
+  g_object_freeze_notify (G_OBJECT (scrollable));
+
   if (hadjustment != priv->hadjustment)
     {
       if (priv->hadjustment)
@@ -130,6 +132,7 @@ scrollable_set_adjustments (StScrollable *scrollable,
         }
 
       priv->hadjustment = hadjustment;
+      g_object_notify (G_OBJECT (scrollable), "hadjustment");
     }
 
   if (vadjustment != priv->vadjustment)
@@ -151,7 +154,10 @@ scrollable_set_adjustments (StScrollable *scrollable,
         }
 
       priv->vadjustment = vadjustment;
+      g_object_notify (G_OBJECT (scrollable), "vadjustment");
     }
+
+  g_object_thaw_notify (G_OBJECT (scrollable));
 }
 
 static void
@@ -160,82 +166,14 @@ scrollable_get_adjustments (StScrollable  *scrollable,
                             StAdjustment **vadjustment)
 {
   StBoxLayoutPrivate *priv;
-  ClutterActor *actor, *stage;
 
   priv = (ST_BOX_LAYOUT (scrollable))->priv;
 
-  actor = CLUTTER_ACTOR (scrollable);
-  stage = clutter_actor_get_stage (actor);
-
-  if (hadjustment)
-    {
   if (priv->hadjustment)
     *hadjustment = priv->hadjustment;
-      else
-        {
-          StAdjustment *adjustment;
-          gdouble width, stage_width, increment;
-
-          if (stage)
-            {
-              width = clutter_actor_get_width (actor);
-              stage_width = clutter_actor_get_width (stage);
-              increment = MAX (1.0, MIN (stage_width, width));
-            }
-          else
-            {
-              width = increment = 1.0;
-            }
-
-          adjustment = st_adjustment_new (0,
-                                          0,
-                                          width,
-                                          1.0,
-                                          increment,
-                                          increment);
-
-          scrollable_set_adjustments (scrollable,
-                                      adjustment,
-                                      priv->vadjustment);
-
-          *hadjustment = adjustment;
-        }
-    }
 
   if (vadjustment)
-    {
-      if (priv->vadjustment)
     *vadjustment = priv->vadjustment;
-      else
-        {
-          StAdjustment *adjustment;
-          gdouble height, stage_height, increment;
-
-          if (stage)
-            {
-              height = clutter_actor_get_height (actor);
-              stage_height = clutter_actor_get_height (stage);
-              increment = MAX (1.0, MIN (stage_height, height));
-            }
-          else
-            {
-              height = increment = 1.0;
-            }
-
-          adjustment = st_adjustment_new (0,
-                                          0,
-                                          height,
-                                          1.0,
-                                          increment,
-                                          increment);
-
-          scrollable_set_adjustments (scrollable,
-                                      priv->hadjustment,
-                                      adjustment);
-
-          *vadjustment = adjustment;
-        }
-    }
 }
 
 

src/st/st-scroll-bar.c

@@ -1143,6 +1143,10 @@ st_scroll_bar_set_adjustment (StScrollBar  *bar,
   g_return_if_fail (ST_IS_SCROLL_BAR (bar));
 
   priv = bar->priv;
+
+  if (adjustment == priv->adjustment)
+    return;
+
   if (priv->adjustment)
     {
       g_signal_handlers_disconnect_by_func (priv->adjustment,
@@ -1168,6 +1172,8 @@ st_scroll_bar_set_adjustment (StScrollBar  *bar,
 
       clutter_actor_queue_relayout (CLUTTER_ACTOR (bar));
     }
+
+  g_object_notify (G_OBJECT (bar), "adjustment");
 }
 
 /**

src/st/st-scroll-view.c

@@ -91,11 +91,10 @@ struct _StScrollViewPrivate
    */
   ClutterActor *child;
 
-  ClutterActor *hscroll;
-  ClutterActor *vscroll;
-
   StAdjustment *hadjustment;
+  ClutterActor *hscroll;
   StAdjustment *vadjustment;
+  ClutterActor *vscroll;
 
   GtkPolicyType hscrollbar_policy;
   GtkPolicyType vscrollbar_policy;
@@ -165,7 +164,7 @@ update_shadow_visibility (StScrollView *scroll)
 {
   StScrollViewPrivate *priv = scroll->priv;
 
-  if (priv->vshadows && priv->vadjustment)
+  if (priv->vshadows)
     {
       gdouble value, lower, upper, page_size;
 
@@ -272,6 +271,25 @@ st_scroll_view_dispose (GObject *object)
   if (priv->hscroll)
     clutter_actor_destroy (priv->hscroll);
 
+  /* For most reliable freeing of memory, an object with signals
+   * like StAdjustment should be explicitly disposed. Since we own
+   * the adjustments, we take care of that. This also disconnects
+   * the signal handlers that we established on creation.
+   */
+  if (priv->hadjustment)
+    {
+      g_object_run_dispose (G_OBJECT (priv->hadjustment));
+      g_object_unref (priv->hadjustment);
+      priv->hadjustment = NULL;
+    }
+
+  if (priv->vadjustment)
+    {
+      g_object_run_dispose (G_OBJECT (priv->vadjustment));
+      g_object_unref (priv->vadjustment);
+      priv->hadjustment = NULL;
+    }
+
   /* since it's impossible to get a handle to these actors, we can
    * just directly unparent them and not go through destroy/remove */
   if (priv->top_shadow)
@@ -712,40 +730,30 @@ st_scroll_view_scroll_event (ClutterActor       *self,
 {
   StScrollViewPrivate *priv = ST_SCROLL_VIEW (self)->priv;
   gdouble lower, value, upper, step;
-  StAdjustment *vadjustment, *hadjustment;
 
   /* don't handle scroll events if requested not to */
   if (!priv->mouse_scroll)
     return FALSE;
 
-  hadjustment = st_scroll_bar_get_adjustment (ST_SCROLL_BAR (priv->hscroll));
-  vadjustment = st_scroll_bar_get_adjustment (ST_SCROLL_BAR (priv->vscroll));
-
   switch (event->direction)
     {
     case CLUTTER_SCROLL_UP:
     case CLUTTER_SCROLL_DOWN:
-      if (vadjustment)
+      g_object_get (priv->vadjustment,
-        g_object_get (vadjustment,
                     "lower", &lower,
                     "step-increment", &step,
                     "value", &value,
                     "upper", &upper,
                     NULL);
-      else
-        return FALSE;
       break;
     case CLUTTER_SCROLL_LEFT:
     case CLUTTER_SCROLL_RIGHT:
-      if (vadjustment)
+      g_object_get (priv->hadjustment,
-        g_object_get (hadjustment,
                     "lower", &lower,
                     "step-increment", &step,
                     "value", &value,
                     "upper", &upper,
                     NULL);
-      else
-        return FALSE;
       break;
     }
 
@@ -755,25 +763,25 @@ st_scroll_view_scroll_event (ClutterActor       *self,
       if (value == lower)
         return FALSE;
       else
-        st_adjustment_set_value (vadjustment, value - step);
+        st_adjustment_set_value (priv->vadjustment, value - step);
       break;
     case CLUTTER_SCROLL_DOWN:
       if (value == upper)
         return FALSE;
       else
-        st_adjustment_set_value (vadjustment, value + step);
+        st_adjustment_set_value (priv->vadjustment, value + step);
       break;
     case CLUTTER_SCROLL_LEFT:
       if (value == lower)
         return FALSE;
       else
-        st_adjustment_set_value (hadjustment, value - step);
+        st_adjustment_set_value (priv->hadjustment, value - step);
       break;
     case CLUTTER_SCROLL_RIGHT:
       if (value == upper)
         return FALSE;
       else
-        st_adjustment_set_value (hadjustment, value + step);
+        st_adjustment_set_value (priv->hadjustment, value + step);
       break;
     }
 
@@ -855,14 +863,6 @@ st_scroll_view_class_init (StScrollViewClass *klass)
                                    pspec);
 }
 
-static void
-disconnect_hadjustment (StScrollView *scroll)
-{
-  StScrollViewPrivate *priv = scroll->priv;
-
-  priv->hadjustment = NULL;
-}
-
 static void
 child_adjustment_changed_cb (StAdjustment *adjustment,
                              StScrollView *scroll)
@@ -878,80 +878,6 @@ child_adjustment_notify_value (GObject      *gobject,
   update_shadow_visibility (scroll);
 }
 
-static void
-disconnect_vadjustment (StScrollView *scroll)
-{
-  StScrollViewPrivate *priv = scroll->priv;
-
-  if (priv->vadjustment)
-    {
-      g_signal_handlers_disconnect_by_func (priv->vadjustment,
-                                            child_adjustment_notify_value,
-                                            scroll);
-      g_signal_handlers_disconnect_by_func (priv->vadjustment,
-                                            child_adjustment_changed_cb,
-                                            scroll);
-      priv->vadjustment = NULL;
-    }
-}
-
-static void
-child_hadjustment_notify_cb (GObject      *gobject,
-                             GParamSpec   *arg1,
-                             StScrollView *scroll)
-{
-  ClutterActor *actor = CLUTTER_ACTOR (gobject);
-  StScrollViewPrivate *priv = scroll->priv;
-
-  disconnect_hadjustment (scroll);
-
-  st_scrollable_get_adjustments (ST_SCROLLABLE (actor), &priv->hadjustment, NULL);
-  st_scroll_bar_set_adjustment (ST_SCROLL_BAR (priv->hscroll), priv->hadjustment);
-
-  if (priv->hadjustment)
-    {
-      /* Force scroll step if needed. */
-      if (priv->column_size_set)
-        {
-          g_object_set (priv->hadjustment,
-                        "step-increment", priv->column_size,
-                        NULL);
-        }
-    }
-}
-
-static void
-child_vadjustment_notify_cb (GObject    *gobject,
-                             GParamSpec *arg1,
-                             gpointer    user_data)
-{
-  StScrollView *scroll = ST_SCROLL_VIEW (user_data);
-  StScrollViewPrivate *priv = scroll->priv;
-
-  disconnect_vadjustment (scroll);
-
-  st_scrollable_get_adjustments (ST_SCROLLABLE (priv->child), NULL, &priv->vadjustment);
-  st_scroll_bar_set_adjustment (ST_SCROLL_BAR (priv->vscroll), priv->vadjustment);
-
-  if (priv->vadjustment)
-    {
-      /* Force scroll step if needed. */
-      if (priv->row_size_set)
-        {
-          g_object_set (priv->vadjustment,
-                        "step-increment", priv->row_size,
-                        NULL);
-        }
-
-      g_signal_connect (priv->vadjustment, "changed",
-                        G_CALLBACK (child_adjustment_changed_cb), scroll);
-      g_signal_connect (priv->vadjustment, "notify::value",
-                        G_CALLBACK (child_adjustment_notify_value), scroll);
-    }
-
-  update_shadow_visibility (scroll);
-}
-
 static void
 st_scroll_view_init (StScrollView *self)
 {
@@ -960,8 +886,21 @@ st_scroll_view_init (StScrollView *self)
   priv->hscrollbar_policy = GTK_POLICY_AUTOMATIC;
   priv->vscrollbar_policy = GTK_POLICY_AUTOMATIC;
 
-  priv->hscroll = CLUTTER_ACTOR (st_scroll_bar_new (NULL));
+  priv->hadjustment = g_object_new (ST_TYPE_ADJUSTMENT, NULL);
-  priv->vscroll = g_object_new (ST_TYPE_SCROLL_BAR, "vertical", TRUE, NULL);
+  priv->hscroll = g_object_new (ST_TYPE_SCROLL_BAR,
+                                "adjustment", priv->hadjustment,
+                                "vertical", FALSE,
+                                NULL);
+
+  priv->vadjustment = g_object_new (ST_TYPE_ADJUSTMENT, NULL);
+  g_signal_connect (priv->vadjustment, "changed",
+                    G_CALLBACK (child_adjustment_changed_cb), self);
+  g_signal_connect (priv->vadjustment, "notify::value",
+                    G_CALLBACK (child_adjustment_notify_value), self);
+  priv->vscroll = g_object_new (ST_TYPE_SCROLL_BAR,
+                                "adjustment", priv->vadjustment,
+                                "vertical", TRUE,
+                                NULL);
 
   clutter_actor_set_parent (priv->hscroll, CLUTTER_ACTOR (self));
   clutter_actor_set_parent (priv->vscroll, CLUTTER_ACTOR (self));
@@ -985,15 +924,8 @@ st_scroll_view_add (ClutterContainer *container,
       /* chain up to StBin::add() */
       st_scroll_view_parent_iface->add (container, actor);
 
-      /* Get adjustments for scroll-bars */
+      st_scrollable_set_adjustments (ST_SCROLLABLE (actor),
-      g_signal_connect (actor, "notify::hadjustment",
+                                     priv->hadjustment, priv->vadjustment);
-                        G_CALLBACK (child_hadjustment_notify_cb),
-                        container);
-      g_signal_connect (actor, "notify::vadjustment",
-                        G_CALLBACK (child_vadjustment_notify_cb),
-                        container);
-      child_hadjustment_notify_cb (G_OBJECT (actor), NULL, self);
-      child_vadjustment_notify_cb (G_OBJECT (actor), NULL, self);
     }
   else
     {
@@ -1018,15 +950,8 @@ st_scroll_view_remove (ClutterContainer *container,
       /* chain up to StBin::remove() */
       st_scroll_view_parent_iface->remove (container, actor);
 
-      disconnect_hadjustment (self);
+      st_scrollable_set_adjustments (ST_SCROLLABLE (priv->child),
-      disconnect_vadjustment (self);
+                                     NULL, NULL);
-
-      g_signal_handlers_disconnect_by_func (priv->child,
-                                            child_hadjustment_notify_cb,
-                                            container);
-      g_signal_handlers_disconnect_by_func (priv->child,
-                                            child_vadjustment_notify_cb,
-                                            container);
 
       g_object_unref (priv->child);
       priv->child = NULL;
@@ -1122,14 +1047,11 @@ st_scroll_view_get_vscroll_bar (StScrollView *scroll)
 gfloat
 st_scroll_view_get_column_size (StScrollView *scroll)
 {
-  StAdjustment *adjustment;
   gdouble column_size;
 
   g_return_val_if_fail (scroll, 0);
 
-  adjustment = st_scroll_bar_get_adjustment (
+  g_object_get (scroll->priv->hadjustment,
-    ST_SCROLL_BAR (scroll->priv->hscroll));
-  g_object_get (adjustment,
                 "step-increment", &column_size,
                 NULL);
 
@@ -1140,8 +1062,6 @@ void
 st_scroll_view_set_column_size (StScrollView *scroll,
                                 gfloat        column_size)
 {
-  StAdjustment *adjustment;
-
   g_return_if_fail (scroll);
 
   if (column_size < 0)
@@ -1154,11 +1074,7 @@ st_scroll_view_set_column_size (StScrollView *scroll,
       scroll->priv->column_size_set = TRUE;
       scroll->priv->column_size = column_size;
 
-      adjustment = st_scroll_bar_get_adjustment (
+      g_object_set (scroll->priv->hadjustment,
-        ST_SCROLL_BAR (scroll->priv->hscroll));
-
-      if (adjustment)
-        g_object_set (adjustment,
                     "step-increment", (gdouble) scroll->priv->column_size,
                     NULL);
     }
@@ -1167,14 +1083,11 @@ st_scroll_view_set_column_size (StScrollView *scroll,
 gfloat
 st_scroll_view_get_row_size (StScrollView *scroll)
 {
-  StAdjustment *adjustment;
   gdouble row_size;
 
   g_return_val_if_fail (scroll, 0);
 
-  adjustment = st_scroll_bar_get_adjustment (
+  g_object_get (scroll->priv->vadjustment,
-    ST_SCROLL_BAR (scroll->priv->vscroll));
-  g_object_get (adjustment,
                 "step-increment", &row_size,
                 NULL);
 
@@ -1185,8 +1098,6 @@ void
 st_scroll_view_set_row_size (StScrollView *scroll,
                              gfloat        row_size)
 {
-  StAdjustment *adjustment;
-
   g_return_if_fail (scroll);
 
   if (row_size < 0)
@@ -1199,11 +1110,7 @@ st_scroll_view_set_row_size (StScrollView *scroll,
       scroll->priv->row_size_set = TRUE;
       scroll->priv->row_size = row_size;
 
-      adjustment = st_scroll_bar_get_adjustment (
+      g_object_set (scroll->priv->vadjustment,
-        ST_SCROLL_BAR (scroll->priv->vscroll));
-
-      if (adjustment)
-        g_object_set (adjustment,
                     "step-increment", (gdouble) scroll->priv->row_size,
                     NULL);
     }

src/st/st-scrollable.c

@@ -33,7 +33,10 @@
  *
  * The interface contains methods for getting and setting the adjustments
  * for scrolling; these adjustments will be used to hook the scrolled
- * position up to scrollbars or other external controls.
+ * position up to scrollbars or other external controls. When a #StScrollable
+ * is added to a parent container, the parent container is responsible
+ * for setting the adjustments. The parent container then sets the adjustments
+ * back to %NULL when the scrollable is removed.
  *
  * For #StScrollable supporting height-for-width size negotation, size
  * negotation works as follows: