Merge lp:~3v1n0/unity/glib-signal-safer-cb into lp:unity

Proposed by Marco Trevisan (Treviño) on 2014-10-15
Status: Merged
Approved by: Christopher Townsend on 2014-10-15
Approved revision: 3873
Merged at revision: 3880
Proposed branch: lp:~3v1n0/unity/glib-signal-safer-cb
Merge into: lp:unity
Diff against target: 92 lines (+9/-24)
2 files modified
UnityCore/GLibSignal-inl.h (+5/-1)
unity-shared/StaticCairoText.cpp (+4/-23)
To merge this branch: bzr merge lp:~3v1n0/unity/glib-signal-safer-cb
Reviewer Review Type Date Requested Status
Christopher Townsend 2014-10-15 Approve on 2014-10-15
PS Jenkins bot (community) continuous-integration Needs Fixing on 2014-10-15
Review via email: mp+238387@code.launchpad.net

Commit message

GLibSignal: don't call callback function if the object_ value is not matching the CB

To post a comment you must log in.
lp:~3v1n0/unity/glib-signal-safer-cb updated on 2014-10-15
3873. By Marco Trevisan (Treviño) on 2014-10-15

StaticCairoText: use glib::Signal

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Christopher Townsend (townsend) wrote :

Great, looks good and no longer crashes. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'UnityCore/GLibSignal-inl.h'
2--- UnityCore/GLibSignal-inl.h 2012-08-30 18:14:19 +0000
3+++ UnityCore/GLibSignal-inl.h 2014-10-15 05:28:09 +0000
4@@ -52,7 +52,11 @@
5 template <typename R, typename G, typename... Ts>
6 R Signal<R, G, Ts...>::Callback(G object, Ts... vs, Signal* self)
7 {
8- return self->callback_(object, vs...);
9+ /* For some weird reasons, this might not be always true (see bug #1366351) */
10+ if (reinterpret_cast<GObject*>(object) == self->object_)
11+ return self->callback_(object, vs...);
12+
13+ return R();
14 }
15
16 }
17
18=== modified file 'unity-shared/StaticCairoText.cpp'
19--- unity-shared/StaticCairoText.cpp 2014-03-20 05:05:21 +0000
20+++ unity-shared/StaticCairoText.cpp 2014-10-15 05:28:09 +0000
21@@ -34,6 +34,7 @@
22 #include <pango/pangocairo.h>
23
24 #include <UnityCore/GLibWrapper.h>
25+#include <UnityCore/GLibSignal.h>
26
27 #include "CairoTexture.h"
28 #include "UnitySettings.h"
29@@ -45,7 +46,6 @@
30 struct StaticCairoText::Impl : sigc::trackable
31 {
32 Impl(StaticCairoText* parent, std::string const& text);
33- ~Impl();
34
35 PangoEllipsizeMode GetPangoEllipsizeMode() const;
36 PangoAlignment GetPangoAlignment() const;
37@@ -77,8 +77,6 @@
38 void UpdateTexture();
39 void OnFontChanged();
40
41- static void FontChanged(GObject* gobject, GParamSpec* pspec, gpointer data);
42-
43 StaticCairoText* parent_;
44 bool accept_key_nav_focus_;
45 mutable bool need_new_extent_cache_;
46@@ -106,6 +104,8 @@
47 int actual_lines_;
48 float line_spacing_;
49 double scale_;
50+
51+ glib::Signal<void, GtkSettings*, GParamSpec*> font_changed_;
52 };
53
54 StaticCairoText::Impl::Impl(StaticCairoText* parent, std::string const& text)
55@@ -126,21 +126,10 @@
56 , line_spacing_(0.5)
57 , scale_(1.0f)
58 {
59- GtkSettings* settings = gtk_settings_get_default(); // not ref'ed
60- g_signal_connect(settings, "notify::gtk-font-name",
61- (GCallback)FontChanged, this);
62-
63+ font_changed_.Connect(gtk_settings_get_default(), "notify::gtk-font-name", sigc::hide(sigc::hide(sigc::mem_fun(this, &Impl::OnFontChanged))));
64 Settings::Instance().font_scaling.changed.connect(sigc::hide(sigc::mem_fun(this, &Impl::OnFontChanged)));
65 }
66
67-StaticCairoText::Impl::~Impl()
68-{
69- GtkSettings* settings = gtk_settings_get_default(); // not ref'ed
70- g_signal_handlers_disconnect_by_func(settings,
71- (void*)FontChanged,
72- this);
73-}
74-
75 PangoEllipsizeMode StaticCairoText::Impl::GetPangoEllipsizeMode() const
76 {
77 switch (ellipsize_)
78@@ -780,14 +769,6 @@
79 }
80 }
81
82-void StaticCairoText::Impl::FontChanged(GObject* gobject,
83- GParamSpec* pspec,
84- gpointer data)
85-{
86- StaticCairoText::Impl* self = static_cast<StaticCairoText::Impl*>(data);
87- self->OnFontChanged();
88-}
89-
90 void StaticCairoText::Impl::OnFontChanged()
91 {
92 need_new_extent_cache_ = true;