Merge lp:~unity-team/unity/unity.fix-731096 into lp:unity
Proposed by
Mirco Müller
Status: | Merged |
---|---|
Merge reported by: | Neil J. Patel |
Merged at revision: | not available |
Proposed branch: | lp:~unity-team/unity/unity.fix-731096 |
Merge into: | lp:unity |
Diff against target: |
25 lines (+9/-4) 1 file modified
src/Launcher.cpp (+9/-4) |
To merge this branch: | bzr merge lp:~unity-team/unity/unity.fix-731096 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Neil J. Patel (community) | Approve | ||
Loïc Molinari (community) | Needs Fixing | ||
Review via email: mp+52527@code.launchpad.net |
Description of the change
Fixes the segfault of LP: #731096 when reordering icons on the launcher.
To post a comment you must log in.
NULL-testing the LauncherIcon makes perfect sense and is the bulk of the fix for #731096 as Launcher: :MouseIconInter section can return NULL. Regarding the other change, I'd say that since the GSource's user_data is the Launcher instance and is queued in the GMainLoop from a Launcher method, NULL-testing the user_data pointer is useless because it's never going to be NULL, and if by any chance it's NULL it means something went really wrong and should have been catched earlier.
What do you think about removing that test?
That last point leads to a more general discussion. The common practice with GSources is to increase the refcount of the object associated to the GSource's callback before queueing it in a GMainLoop, and then decrease the refcount once consumed at the end of the callback. That prevents objects associated to a GSource queued in a GMainLoop to be disposed before the GSource's callback is dispatched. As far as I can see, that is not done in Unity. It makes sense for objects with controlled lifetime like the Launcher. My personal take on that is to always ref/unref otherwise it becomes a mess knowing when to do that or not.
Do you know if that's intended and what's the policy?