-
Notifications
You must be signed in to change notification settings - Fork 6k
[Keyboard, Linux] Disconnect keymap signal on dispose #34488
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@robert-ancell I implemented disconnection on disposal. However, I don't think we should also disconnect in |
I suggested it should also be disconnected in remove because the reference to keymap is dropped, so it wasn't clear to me if signals should be received about it anymore. But that code is no longer there, so I don't see any reason to do that anymore. |
robert-ancell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| g_clear_object(&self->engine); | ||
| g_clear_object(&self->accessibility_plugin); | ||
| g_clear_object(&self->keyboard_manager); | ||
| g_signal_handler_disconnect(self->keymap, self->keymap_keys_changed_cb_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I've started seeing warnings like
(bug:74019): GLib-GObject-WARNING **: 11:15:08.697: ../../../gobject/gsignal.c:2731: instance '0x55e1c3ea0200' has no handler with id '255'
when closing Flutter apps because GObject's dispose() method may be called multiple times. Here's a fix: #35490
This PR disconnects a signal callback that is registered on
FlView::keymap. Since #34351 removes the disposal of this object, the signal is no longer automatically disconnected.Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.