Re: treeview-refactor branch has landed.

[Tristan Van Berkom <tristanvb openismus com>]

On Sun, 2010-12-19 at 22:32 +0000, Rob Bradford wrote:
> On 17 December 2010 11:24, Kristian Rietveld <kris gtk org> wrote:
> > On Fri, Dec 17, 2010 at 7:57 AM, Tristan Van Berkom
> > <tristanvb openismus com> wrote:
> >
> > Furthermore, I would encourage people who use complex tree views in
> > their libraries or applications to test these as soon as possible and
> > report any issues they find.  Although we have put significant time in
> > checking all the corner cases, there is a good chance we might have
> > missed something.
> >
> 
> Great work!
> 
> I was debugging some warnings in Evolution ... and I spotted that if
> you use gtk_tree_view_column_set_cell_data_func (which just calls
> gtk_cell_layout_set_cell_data_func casting GtkTreeViewColumn to a
> GtkCellLayout) then the data function gets called not with a
> GtkTreeViewColumn but a GtkCellBoxArea instead. Both implement
> GtkCellLayout but a GtkTreeViewColumn isn't a GtkCellBoxArea.

Ah yes another thing I overlooked.

I'm attaching a short patch that addresses this elegantly without
the need for adding heaps of code to GtkCellLayout implementers.

I'm adding it to treeview-refactor for now, if there's no objections
I'll land it on master in a day or two.

Cheers,
         -Tristan

> 
> (evolution:12620): Gtk-CRITICAL **:
> gtk_tree_view_column_get_tree_view: assertion `GTK_IS_TREE_VIEW_COLUMN
> (tree_column)' failed
> (evolution:12620): Gtk-CRITICAL **: gtk_tree_view_get_selection:
> assertion `GTK_IS_TREE_VIEW (tree_view)' failed
> (evolution:12620): Gtk-CRITICAL **:
> gtk_tree_selection_iter_is_selected: assertion `GTK_IS_TREE_SELECTION
> (selection)' failed
> (evolution:12620): Gtk-CRITICAL **: gtk_tree_view_get_drag_dest_row:
> assertion `GTK_IS_TREE_VIEW (tree_view)' failed
> ==12620== Conditional jump or move depends on uninitialised value(s)
> ==12620==    at 0x16851764: render_icon (em-folder-tree.c:1240)
> ==12620==    by 0x9927ACC: apply_cell_attributes (gtkcellarea.c:1211)
> ==12620==    by 0xC741BFA: g_hash_table_foreach (ghash.c:1328)
> ==12620==    by 0x9927B5C: gtk_cell_area_real_apply_attributes
> (gtkcellarea.c:1240)
> ==12620==    by 0x9A2D528:
> _gtk_marshal_VOID__OBJECT_BOXED_BOOLEAN_BOOLEAN (gtkmarshalers.c:2156)
> ==12620==    by 0xBEB55E2: g_type_class_meta_marshal (gclosure.c:877)
> ==12620==    by 0xBEB52AE: g_closure_invoke (gclosure.c:766)
> ==12620==    by 0xBECF954: signal_emit_unlocked_R (gsignal.c:3182)
> ==12620==    by 0xBECEBE2: g_signal_emit_valist (gsignal.c:2983)
> ==12620==    by 0xBECF160: g_signal_emit (gsignal.c:3040)
> ==12620==    by 0x992AA9A: gtk_cell_area_apply_attributes (gtkcellarea.c:2381)
> ==12620==    by 0x9BB2771: gtk_tree_view_column_cell_set_cell_data
> (gtktreeviewcolumn.c:2748)
> ==12620==
> 
> Cheerio,
> 
> Rob

diff --git a/gtk/gtkcellarea.c b/gtk/gtkcellarea.c
index fcf550e..246a629 100644
--- a/gtk/gtkcellarea.c
+++ b/gtk/gtkcellarea.c
@@ -479,6 +479,7 @@ typedef struct {
   GtkCellLayoutDataFunc  func;
   gpointer               data;
   GDestroyNotify         destroy;
+  GtkCellLayout         *proxy;
 } CellInfo;
 
 static CellInfo       *cell_info_new       (GtkCellLayoutDataFunc  func,
@@ -1188,7 +1189,7 @@ apply_cell_attributes (GtkCellRenderer *renderer,
   /* Call any GtkCellLayoutDataFunc that may have been set by the user
    */
   if (info->func)
-    info->func (GTK_CELL_LAYOUT (data->area), renderer,
+    info->func (info->proxy ? info->proxy : GTK_CELL_LAYOUT (data->area), renderer,
 		data->model, data->iter, info->data);
 
   g_object_thaw_notify (G_OBJECT (renderer));
@@ -1399,36 +1400,9 @@ gtk_cell_area_set_cell_data_func (GtkCellLayout         *cell_layout,
 				  gpointer               func_data,
 				  GDestroyNotify         destroy)
 {
-  GtkCellArea        *area   = GTK_CELL_AREA (cell_layout);
-  GtkCellAreaPrivate *priv   = area->priv;
-  CellInfo           *info;
-
-  info = g_hash_table_lookup (priv->cell_info, renderer);
-
-  if (info)
-    {
-      if (info->destroy && info->data)
-	info->destroy (info->data);
-
-      if (func)
-	{
-	  info->func    = func;
-	  info->data    = func_data;
-	  info->destroy = destroy;
-	}
-      else
-	{
-	  info->func    = NULL;
-	  info->data    = NULL;
-	  info->destroy = NULL;
-	}
-    }
-  else
-    {
-      info = cell_info_new (func, func_data, destroy);
+  GtkCellArea *area   = GTK_CELL_AREA (cell_layout);
 
-      g_hash_table_insert (priv->cell_info, renderer, info);
-    }
+  _gtk_cell_area_set_cell_data_func_with_proxy (area, renderer, (GFunc)func, func_data, destroy, NULL);
 }
 
 static void
@@ -3616,3 +3590,53 @@ gtk_cell_area_request_renderer (GtkCellArea        *area,
   *minimum_size += focus_line_width;
   *natural_size += focus_line_width;
 }
+
+void
+_gtk_cell_area_set_cell_data_func_with_proxy (GtkCellArea           *area,
+					      GtkCellRenderer       *cell,
+					      GFunc                  func,
+					      gpointer               func_data,
+					      GDestroyNotify         destroy,
+					      gpointer               proxy)
+{
+  GtkCellAreaPrivate *priv;
+  CellInfo           *info;
+
+  g_return_if_fail (GTK_IS_CELL_AREA (area));
+  g_return_if_fail (GTK_IS_CELL_RENDERER (cell));
+
+  priv = area->priv;
+
+  info = g_hash_table_lookup (priv->cell_info, cell);
+
+  /* Note we do not take a reference to the proxy, the proxy is a GtkCellLayout
+   * that is forwarding it's implementation to a delegate GtkCellArea therefore
+   * it's life-cycle is longer than the area's life cycle. 
+   */
+  if (info)
+    {
+      if (info->destroy && info->data)
+	info->destroy (info->data);
+
+      if (func)
+	{
+	  info->func    = (GtkCellLayoutDataFunc)func;
+	  info->data    = func_data;
+	  info->destroy = destroy;
+	  info->proxy   = proxy;
+	}
+      else
+	{
+	  info->func    = NULL;
+	  info->data    = NULL;
+	  info->destroy = NULL;
+	  info->proxy   = NULL;
+	}
+    }
+  else
+    {
+      info = cell_info_new ((GtkCellLayoutDataFunc)func, func_data, destroy);
+
+      g_hash_table_insert (priv->cell_info, cell, info);
+    }
+}
diff --git a/gtk/gtkcellarea.h b/gtk/gtkcellarea.h
index f9c1c45..7a187d8 100644
--- a/gtk/gtkcellarea.h
+++ b/gtk/gtkcellarea.h
@@ -458,6 +458,19 @@ void                  gtk_cell_area_request_renderer               (GtkCellArea
 								    gint               *minimum_size,
 								    gint               *natural_size);
 
+/* For api stability, this is called from gtkcelllayout.c in order to ensure the correct
+ * object is passed to the user function in gtk_cell_layout_set_cell_data_func.
+ *
+ * This private api takes gpointer & GFunc arguments to circumvent circular header file
+ * dependancies.
+ */
+void                 _gtk_cell_area_set_cell_data_func_with_proxy  (GtkCellArea           *area,
+								    GtkCellRenderer       *cell,
+								    GFunc                  func,
+								    gpointer               func_data,
+								    GDestroyNotify         destroy,
+								    gpointer               proxy);
+
 G_END_DECLS
 
 #endif /* __GTK_CELL_AREA_H__ */
diff --git a/gtk/gtkcelllayout.c b/gtk/gtkcelllayout.c
index c54a875..5a0ae59 100644
--- a/gtk/gtkcelllayout.c
+++ b/gtk/gtkcelllayout.c
@@ -357,7 +357,16 @@ gtk_cell_layout_set_cell_data_func (GtkCellLayout         *cell_layout,
       area = iface->get_area (cell_layout);
 
       if (area)
-	gtk_cell_layout_set_cell_data_func (GTK_CELL_LAYOUT (area), cell, func, func_data, destroy);
+	{
+	  /* Ensure that the correct proxy object is sent to 'func' */
+	  if (GTK_CELL_LAYOUT (area) != cell_layout)
+	    _gtk_cell_area_set_cell_data_func_with_proxy (area, cell, 
+							  (GFunc)func, func_data, destroy, 
+							  cell_layout);
+	  else
+	    gtk_cell_layout_set_cell_data_func (GTK_CELL_LAYOUT (area), cell, 
+						func, func_data, destroy);
+	}
     }
 }