After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 671687 - Cannot use cairo context from GtkDrawingArea::draw signal
Cannot use cairo context from GtkDrawingArea::draw signal
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2012-03-08 23:26 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2012-05-25 20:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
foreign: Remove type_info from foreign APIs (10.76 KB, patch)
2012-03-08 23:26 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
value: Handle foreign types correctly when converting from a GValue (1.13 KB, patch)
2012-03-08 23:26 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
value: Fix memory leak when constructing a GValue from a Boxed (843 bytes, patch)
2012-04-10 03:53 UTC, Jasper St. Pierre (not reading bugmail)
accepted-commit_now Details | Review
value: Handle foreign types correctly when converting from a GValue (1.20 KB, patch)
2012-04-10 03:53 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
tests: Fix warnings (927 bytes, patch)
2012-05-25 19:54 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
tests: Add a foreign struct signal test (4.95 KB, patch)
2012-05-25 19:54 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
value: Fix memory leak when constructing a GValue from a Boxed (1.02 KB, patch)
2012-05-25 19:54 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
tests: Fix a memory leak (927 bytes, patch)
2012-05-25 20:01 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2012-03-08 23:26:13 UTC
let w = new Gtk.Window();
  let da = new Gtk.DrawingArea();
  da.connect('draw', function(d, cr) {
    cr.fill();
  });
  w.add(da);
  w.show_all();

Spits out an error. Fix that.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-03-08 23:26:15 UTC
Created attachment 209287 [details] [review]
foreign: Remove type_info from foreign APIs

If we don't require a type_info in a foreign API, this will make it easier
to use the foreign API from another place in the code. Swap out the use
of type_info with the interface info that the type info contains.
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-03-08 23:26:18 UTC
Created attachment 209288 [details] [review]
value: Handle foreign types correctly when converting from a GValue
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-03-31 14:46:43 UTC
Giovanni, Colin: can somebody review this?
Comment 4 Giovanni Campagna 2012-04-10 00:56:41 UTC
Review of attachment 209287 [details] [review]:

Better late than never, they say...
Anyway, this definitely makes sense.

(a-c_n is not a statement of committability, check Colin for freezes :) )
Comment 5 Giovanni Campagna 2012-04-10 00:57:03 UTC
Review of attachment 209287 [details] [review]:

Better late than never, they say...
Anyway, this definitely makes sense.

(a-c_n is not a statement of committability, check Colin for freezes :) )
Comment 6 Giovanni Campagna 2012-04-10 01:00:21 UTC
Review of attachment 209288 [details] [review]:

::: gi/value.c
@@ +682,3 @@
+            GIArgument arg;
+            arg.v_pointer = gboxed;
+            return gjs_struct_foreign_convert_from_g_argument(context, value_p, info, &arg);

Don't you need to free info here (and in the non foreign path too)?
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-04-10 03:53:37 UTC
Created attachment 211690 [details] [review]
value: Fix memory leak when constructing a GValue from a Boxed

Nice catch.
Comment 8 Jasper St. Pierre (not reading bugmail) 2012-04-10 03:53:43 UTC
Created attachment 211691 [details] [review]
value: Handle foreign types correctly when converting from a GValue
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-05-17 12:40:48 UTC
Ping?
Comment 10 Colin Walters 2012-05-25 19:09:15 UTC
Review of attachment 211690 [details] [review]:

::: gi/value.c
@@ +695,3 @@
         }
         *value_p = OBJECT_TO_JSVAL(obj);
+        g_base_info_unref(info);

We leak in the error path above too...but I guess that one's not a big deal.
Comment 11 Colin Walters 2012-05-25 19:16:19 UTC
Review of attachment 211691 [details] [review]:

Can we have a test case for this one?

Otherwise looks OK.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-05-25 19:54:22 UTC
Created attachment 214996 [details] [review]
tests: Fix warnings
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-05-25 19:54:29 UTC
Created attachment 214997 [details] [review]
tests: Add a foreign struct signal test
Comment 14 Jasper St. Pierre (not reading bugmail) 2012-05-25 19:54:43 UTC
Created attachment 214998 [details] [review]
value: Fix memory leak when constructing a GValue from a Boxed

Added test.
Comment 15 Jasper St. Pierre (not reading bugmail) 2012-05-25 20:01:41 UTC
Created attachment 214999 [details] [review]
tests: Fix a memory leak

We need to unref the cairo surface here, as we are making the context
be the sole owner of it.
Comment 16 Colin Walters 2012-05-25 20:03:38 UTC
Review of attachment 214996 [details] [review]:

OK
Comment 17 Colin Walters 2012-05-25 20:07:12 UTC
Review of attachment 214997 [details] [review]:

::: configure.ac
@@ +133,3 @@
 AC_ARG_ENABLE(tests,[  --disable-tests           disable test libraries ], enable_tests=$enableval,enable_tests=yes)
 have_cairo=no
+PKG_CHECK_MODULES(CAIRO, [cairo cairo-gobject], have_cairo=yes, have_cairo=no)

Totally minor: cairo-gobject requires cairo, so listing the latter is redundant.
Comment 18 Colin Walters 2012-05-25 20:08:26 UTC
Review of attachment 214998 [details] [review]:

Yep.
Comment 19 Colin Walters 2012-05-25 20:09:23 UTC
Review of attachment 214999 [details] [review]:

Yep.
Comment 20 Jasper St. Pierre (not reading bugmail) 2012-05-25 20:37:20 UTC
Attachment 214996 [details] pushed as fed7b25 - tests: Fix warnings
Attachment 214997 [details] pushed as a7bfba3 - tests: Add a foreign struct signal test
Attachment 214999 [details] pushed as 9d9000c - tests: Fix a memory leak
Comment 21 Jasper St. Pierre (not reading bugmail) 2012-05-25 20:38:05 UTC
Attachment 209287 [details] pushed as b7ffb73 - foreign: Remove type_info from foreign APIs
Attachment 211691 [details] pushed as 085fb4d - value: Handle foreign types correctly when converting from a GValue
Attachment 214998 [details] pushed as e0a9fb9 - value: Fix memory leak when constructing a GValue from a Boxed