[clutter] model: Make sure to emit ::row-changed

[Emmanuele Bassi <ebassi src gnome org>]

commit e470fd7d82502dbdc5084c08583d8764f95813f1
Author: Emmanuele Bassi <ebassi linux intel com>
Date:   Thu Jul 28 15:00:18 2011 +0100

    model: Make sure to emit ::row-changed
    
    Currently, only clutter_model_iter_set_valist() is in charge of emitting
    the ClutterModel::row-changed signal. Both the set() and the
    set_valist() functions can be called with multiple columns, so we
    coalesce the signal emission at the end of the set_valist(), to have a
    single ::row-changed emission per change.
    
    The clutter_model_iter_set_value() function is just a thin wrapper
    around the set_value() virtual function, but since it's called
    internally we cannot add the signal emission there as well, as we'd
    break the signal coalescing.
    
    For this reason, we need some code refactoring inside the various set()
    variants of ClutterModelIter:
    
      - we only use the internal va_arg variant for both the set() and
        set_valist() public functions, to avoid multiple type checks;
      - the internal set_valist() calls an internal set_value() method
        which calls the virtual function from the iterator vtable;
      - a new internal emit_row_changed() method is needed to retrieve
        the ClutterModel from the iterator, and emit the signal;
    
    Now, all three variants of the value setter will call an internal
    ClutterModelIter::set_value() wrapper, and emit the ::row-changed
    signal.
    
    To check that the intended behaviour has been implemented, and it's not
    going to be broken, the test suite has grown a new unit which populates
    a model and changes a random row.

 clutter/clutter-model.c           |   60 ++++++++++++++---------
 tests/conform/test-conform-main.c |    1 +
 tests/conform/test-model.c        |   95 +++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 24 deletions(-)
---
diff --git a/clutter/clutter-model.c b/clutter/clutter-model.c
index 17fcb05..9b5b4e4 100644
--- a/clutter/clutter-model.c
+++ b/clutter/clutter-model.c
@@ -1980,17 +1980,27 @@ clutter_model_iter_init (ClutterModelIter *self)
  *  Public functions
  */
 
+static inline void
+clutter_model_iter_set_value_internal (ClutterModelIter *iter,
+                                       guint             column,
+                                       const GValue     *value)
+{
+  ClutterModelIterClass *klass;
+
+  klass = CLUTTER_MODEL_ITER_GET_CLASS (iter);
+  if (klass && klass->set_value)
+    klass->set_value (iter, column, value);
+}
+
 static void
 clutter_model_iter_set_internal_valist (ClutterModelIter *iter,
                                         va_list           args)
 {
-  ClutterModel *model;
-  ClutterModelIterPrivate *priv;
+  ClutterModelIterPrivate *priv = iter->priv;
+  ClutterModel *model = priv->model;
   guint column = 0;
   gboolean sort = FALSE;
   
-  priv = iter->priv;
-  model = priv->model;
   g_assert (CLUTTER_IS_MODEL (model));
 
   column = va_arg (args, gint);
@@ -2002,6 +2012,7 @@ clutter_model_iter_set_internal_valist (ClutterModelIter *iter,
     {
       GValue value = { 0, };
       gchar *error = NULL;
+      GType col_type;
 
       if (column < 0 || column >= clutter_model_get_n_columns (model))
         { 
@@ -2010,9 +2021,10 @@ clutter_model_iter_set_internal_valist (ClutterModelIter *iter,
                      G_STRLOC, column);
           break;
         }
-      g_value_init (&value, clutter_model_get_column_type (model, column));
 
-      G_VALUE_COLLECT (&value, args, 0, &error);
+      col_type = clutter_model_get_column_type (model, column);
+
+      G_VALUE_COLLECT_INIT (&value, col_type, args, 0, &error);
       if (error)
         {
           g_warning ("%s: %s", G_STRLOC, error);
@@ -2022,7 +2034,7 @@ clutter_model_iter_set_internal_valist (ClutterModelIter *iter,
           break;
         }
 
-      clutter_model_iter_set_value (iter, column, &value);
+      clutter_model_iter_set_value_internal (iter, column, &value);
       
       g_value_unset (&value);
       
@@ -2037,6 +2049,17 @@ clutter_model_iter_set_internal_valist (ClutterModelIter *iter,
     clutter_model_resort (model);
 }
 
+static void inline
+clutter_model_iter_emit_row_changed (ClutterModelIter *iter)
+{
+  ClutterModelIterPrivate *priv = iter->priv;
+  ClutterModel *model = priv->model;
+
+  g_assert (CLUTTER_IS_MODEL (model));
+
+  g_signal_emit (model, model_signals[ROW_CHANGED], 0, iter);
+}
+
 /**
  * clutter_model_iter_set_valist:
  * @iter: a #ClutterModelIter
@@ -2051,18 +2074,10 @@ void
 clutter_model_iter_set_valist (ClutterModelIter *iter,
                                va_list           args)
 {
-  ClutterModelIterPrivate *priv;
-  ClutterModel *model;
-
   g_return_if_fail (CLUTTER_IS_MODEL_ITER (iter));
 
   clutter_model_iter_set_internal_valist (iter, args);
-
-  priv = iter->priv;
-  model = priv->model;
-  g_assert (CLUTTER_IS_MODEL (model));
-
-  g_signal_emit (model, model_signals[ROW_CHANGED], 0, iter);
+  clutter_model_iter_emit_row_changed (iter);
 }
 
 /**
@@ -2214,11 +2229,11 @@ clutter_model_iter_set (ClutterModelIter *iter,
   g_return_if_fail (CLUTTER_IS_MODEL_ITER (iter));
 
   va_start (args, iter);
-  clutter_model_iter_set_valist (iter, args);
+  clutter_model_iter_set_internal_valist (iter, args);
+  clutter_model_iter_emit_row_changed (iter);
   va_end (args);
 }
 
-
 /**
  * clutter_model_iter_set_value:
  * @iter: a #ClutterModelIter
@@ -2235,13 +2250,10 @@ clutter_model_iter_set_value (ClutterModelIter *iter,
                               guint             column,
                               const GValue     *value)
 {
-  ClutterModelIterClass *klass;
-
   g_return_if_fail (CLUTTER_IS_MODEL_ITER (iter));
-  
-  klass = CLUTTER_MODEL_ITER_GET_CLASS (iter);
-  if (klass && klass->set_value)
-    klass->set_value (iter, column, value);
+
+  clutter_model_iter_set_value_internal (iter, column, value);
+  clutter_model_iter_emit_row_changed (iter);
 }
 
 /**
diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
index f2b1165..4872ef1 100644
--- a/tests/conform/test-conform-main.c
+++ b/tests/conform/test-conform-main.c
@@ -178,6 +178,7 @@ main (int argc, char **argv)
   TEST_CONFORM_SIMPLE ("/model", test_list_model_iterate);
   TEST_CONFORM_SIMPLE ("/model", test_list_model_filter);
   TEST_CONFORM_SIMPLE ("/model", test_list_model_from_script);
+  TEST_CONFORM_SIMPLE ("/model", test_list_model_row_changed);
 
   TEST_CONFORM_SIMPLE ("/color", test_color_from_string);
   TEST_CONFORM_SIMPLE ("/color", test_color_to_string);
diff --git a/tests/conform/test-model.c b/tests/conform/test-model.c
index e702063..4ce7891 100644
--- a/tests/conform/test-model.c
+++ b/tests/conform/test-model.c
@@ -11,6 +11,18 @@ typedef struct _ModelData
   guint n_row;
 } ModelData;
 
+typedef struct _ChangedData
+{
+  ClutterModel *model;
+
+  ClutterModelIter *iter;
+
+  guint row;
+  guint n_emissions;
+
+  gint value_check;
+} ChangedData;
+
 enum
 {
   COLUMN_FOO,   /* G_TYPE_STRING */
@@ -433,3 +445,86 @@ test_list_model_from_script (TestConformSimpleFixture *fixture,
   g_value_unset (&value);
   g_object_unref (iter);
 }
+
+static void
+on_row_changed (ClutterModel *model,
+                ClutterModelIter *iter,
+                ChangedData *data)
+{
+  gint value = -1;
+
+  clutter_model_iter_get (iter, COLUMN_BAR, &value, -1);
+
+  if (g_test_verbose ())
+    g_print ("row-changed value-check: %d, expected: %d\n",
+             value, data->value_check);
+
+  g_assert_cmpint (value, ==, data->value_check);
+
+  data->n_emissions += 1;
+}
+
+void
+test_list_model_row_changed (TestConformSimpleFixture *fixture,
+                             gconstpointer             data)
+{
+  ChangedData test_data = { NULL, NULL, 0, 0 };
+  GValue value = { 0, };
+  gint i;
+
+  test_data.model = clutter_list_model_new (N_COLUMNS,
+                                            G_TYPE_STRING, "Foo",
+                                            G_TYPE_INT,    "Bar");
+  for (i = 1; i < 10; i++)
+    {
+      gchar *foo = g_strdup_printf ("String %d", i);
+
+      clutter_model_append (test_data.model,
+                            COLUMN_FOO, foo,
+                            COLUMN_BAR, i,
+                            -1);
+
+      g_free (foo);
+    }
+
+  g_signal_connect (test_data.model, "row-changed",
+                    G_CALLBACK (on_row_changed),
+                    &test_data);
+
+  test_data.row = g_random_int_range (0, 9);
+  test_data.iter = clutter_model_get_iter_at_row (test_data.model,
+                                                  test_data.row);
+  g_assert (CLUTTER_IS_MODEL_ITER (test_data.iter));
+
+  test_data.value_check = 47;
+
+  g_value_init (&value, G_TYPE_INT);
+  g_value_set_int (&value, test_data.value_check);
+
+  clutter_model_iter_set_value (test_data.iter, COLUMN_BAR, &value);
+
+  g_value_unset (&value);
+
+  if (g_test_verbose ())
+    g_print ("iter.set_value() emissions: %d, expected: 1\n",
+             test_data.n_emissions);
+
+  g_assert_cmpint (test_data.n_emissions, ==, 1);
+
+  test_data.n_emissions = 0;
+  test_data.value_check = 42;
+
+  clutter_model_iter_set (test_data.iter,
+                          COLUMN_FOO, "changed",
+                          COLUMN_BAR, test_data.value_check,
+                          -1);
+
+  if (g_test_verbose ())
+    g_print ("iter.set() emissions: %d, expected: 1\n",
+             test_data.n_emissions);
+
+  g_assert_cmpint (test_data.n_emissions, ==, 1);
+
+  g_object_unref (test_data.iter);
+  g_object_unref (test_data.model);
+}