[texture] Fix error reporting on ::load-finished

The load-finished signal has a GError* argument which is meant to
signify whether the loading was successful. However many of the
places in ClutterTexture that emit this signal directly pass their
'error' variable which is a GError** and will be NULL or not
completely independently of whether there was an error. If the
argument was dereferenced it would probably crash.

The test-texture-async interactive test case should also verify
that the ::load-finished signal is correctly emitted.

Fixes bug:

  http://bugzilla.openedhand.com/show_bug.cgi?id=1622

diff --git a/clutter/clutter-texture.c b/clutter/clutter-texture.c
index 3a5d2d2..c619cba 100644 (file)
--- a/clutter/clutter-texture.c
+++ b/clutter/clutter-texture.c
@@ -1471,6 +1471,8 @@ clutter_texture_set_from_data (ClutterTexture     *texture,
                    CLUTTER_TEXTURE_ERROR_BAD_FORMAT,
                    "Failed to create COGL texture");
 
+      g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0, *error);
+
       return FALSE;
     }
 
@@ -1478,7 +1480,7 @@ clutter_texture_set_from_data (ClutterTexture     *texture,
 
   cogl_handle_unref (new_texture);
 
-  g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0, error);
+  g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0, NULL);
 
   return TRUE;
 }
@@ -1654,6 +1656,7 @@ clutter_texture_async_load_complete (ClutterTexture *self,
                          cogl_texture_get_width(handle),
                          cogl_texture_get_height (handle));
         }
+
       cogl_handle_unref (handle);
     }
 
@@ -1892,19 +1895,21 @@ clutter_texture_set_from_file (ClutterTexture *texture,
                                             flags,
                                             COGL_PIXEL_FORMAT_ANY,
                                             &internal_error);
-  if (new_texture == COGL_INVALID_HANDLE)
+
+  /* If COGL didn't give an error then make one up */
+  if (internal_error == NULL && new_texture == COGL_INVALID_HANDLE)
     {
-      /* If COGL didn't give an error then make one up */
-      if (internal_error == NULL)
-       {
-         g_set_error (error, CLUTTER_TEXTURE_ERROR,
-                      CLUTTER_TEXTURE_ERROR_BAD_FORMAT,
-                      "Failed to create COGL texture");
-       }
-      else
-        g_propagate_error (error, internal_error);
+      g_set_error (&internal_error, CLUTTER_TEXTURE_ERROR,
+                   CLUTTER_TEXTURE_ERROR_BAD_FORMAT,
+                  "Failed to create COGL texture");
+    }
+
+  if (internal_error != NULL)
+    {
+      g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0,
+                     internal_error);
 
-      g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0, error);
+      g_propagate_error (error, internal_error);
 
       return FALSE;
     }
@@ -1913,7 +1918,7 @@ clutter_texture_set_from_file (ClutterTexture *texture,
 
   cogl_handle_unref (new_texture);
 
-  g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0, error);
+  g_signal_emit (texture, texture_signals[LOAD_FINISHED], 0, NULL);
 
   return TRUE;
 }

diff --git a/tests/interactive/test-texture-async.c b/tests/interactive/test-texture-async.c
index 4625a35..7c976d2 100644 (file)
--- a/tests/interactive/test-texture-async.c
+++ b/tests/interactive/test-texture-async.c
@@ -2,6 +2,41 @@
 #include <gmodule.h>
 #include <clutter/clutter.h>
 
+enum
+{
+  LOAD_SYNC,
+  LOAD_DATA_ASYNC,
+  LOAD_ASYNC
+};
+
+static void
+on_load_finished (ClutterTexture *texture,
+                  const GError   *error,
+                  gpointer        user_data)
+{
+  gint load_type = GPOINTER_TO_INT (user_data);
+  const gchar *load_str = NULL;
+
+  switch (load_type)
+    {
+    case LOAD_SYNC:
+      load_str = "synchronous loading";
+      break;
+
+    case LOAD_DATA_ASYNC:
+      load_str = "asynchronous data loading";
+      break;
+
+    case LOAD_ASYNC:
+      load_str = "asynchronous loading";
+      break;
+    }
+
+  if (error != NULL)
+    g_print ("%s failed: %s\n", load_str, error->message);
+  else
+    g_print ("%s successful\n", load_str);
+}
 
 static void size_change_cb (ClutterTexture *texture,
                             gint            width,
@@ -24,6 +59,7 @@ static gboolean task (gpointer foo)
   gint i;
 
   stage = clutter_stage_get_default ();
+
 #if 0
   for (i=0;i<4;i++)
     image[i] = g_object_new (CLUTTER_TYPE_TEXTURE,
@@ -32,23 +68,35 @@ static gboolean task (gpointer foo)
                              NULL);
 #else
   /*for (i=0;i<4;i++)*/
-    image[0] = g_object_new (CLUTTER_TYPE_TEXTURE,
-                             "filename", path,
-                             NULL);
+  image[0] = g_object_new (CLUTTER_TYPE_TEXTURE, NULL);
+  g_signal_connect (image[0], "load-finished",
+                    G_CALLBACK (on_load_finished),
+                    GINT_TO_POINTER (LOAD_SYNC));
+
   image[1] = g_object_new (CLUTTER_TYPE_TEXTURE,
-                           "filename", path,
                            "load-data-async", TRUE,
                            NULL);
+  g_signal_connect (image[1], "load-finished",
+                    G_CALLBACK (on_load_finished),
+                    GINT_TO_POINTER (LOAD_DATA_ASYNC));
   image[2] = g_object_new (CLUTTER_TYPE_TEXTURE,
-                           "filename", path,
                            "load-async", TRUE,
                            NULL);
-
+  g_signal_connect (image[2], "load-finished",
+                    G_CALLBACK (on_load_finished),
+                    GINT_TO_POINTER (LOAD_ASYNC));
 #endif
+
+  for (i=0;i<3;i++)
+    {
+      clutter_texture_set_from_file (CLUTTER_TEXTURE (image[i]), path, NULL);
+    }
+
   for (i=0;i<3;i++)
     {
       clutter_container_add (CLUTTER_CONTAINER (stage), image[i], NULL);
     }
+
   for (i=0;i<3;i++)
     {
       clutter_actor_set_position (image[i], 50+i*100, 0+i*50);