Compiz

Merge lp:~compiz-team/compiz/compiz.fix_1187168 into lp:compiz/0.9.10

  1. compiz.fix_1187168
  2. Merge into 0.9.10
Proposed by Sam Spilsbury
Status: Merged
Approved by: Andrea Azzarone
Approved revision: 3735
Merged at revision: 3734
Proposed branch: lp:~compiz-team/compiz/compiz.fix_1187168
Merge into: lp:compiz/0.9.10
Diff against target: 244 lines (+103/-51)
2 files modified
cmake/CompizCommon.cmake (+5/-0)
gtk/window-decorator/tests/test_gwd_settings.cpp (+98/-51)
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_1187168
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Andrea Azzarone Approve
Sam Spilsbury Pending
Review via email: mp+167508@code.launchpad.net

This proposal supersedes a proposal from 2013-06-04.

Commit message

Satisfy matchers on getProperty as soon as they are set.

  The order of evaluation for matchers in Google Mock appears to be
  undefined - this means that we can't rely on the first argument being
  matched first and the second argument being matched afterwards. In turn,
  this means that any GValue may be passed to a GValueMatch which, by a
  design flaw, is unable to handle any values of a type it does not expect
  (at least not without an API change). It will silently pass the incorrect
  type to g_type_get_* which causes internal assertion failures.

  At the moment we're just interleaving the calls to getProperty and
  get_property - that way the expectations are satisfied and go away
  as soon as they're set. This in turn means that Google Mock only
  has to traverse one matcher rather than multiple matchers.

  (LP: #1187468)

Description of the change

  Satisfy matchers on getProperty as soon as they are set.

  The order of evaluation for matchers in Google Mock appears to be
  undefined - this means that we can't rely on the first argument being
  matched first and the second argument being matched afterwards. In turn,
  this means that any GValue may be passed to a GValueMatch which, by a
  design flaw, is unable to handle any values of a type it does not expect
  (at least not without an API change). It will silently pass the incorrect
  type to g_type_get_* which causes internal assertion failures.

  At the moment we're just interleaving the calls to getProperty and
  get_property - that way the expectations are satisfied and go away
  as soon as they're set. This in turn means that Google Mock only
  has to traverse one matcher rather than multiple matchers.

  (LP: #1187468)

To post a comment you must log in.
Revision history for this message
Andrea Azzarone (azzar1) wrote : Posted in a previous version of this proposal

Proposed by Sam Spilsbury 17 hours ago - Updating diff...

What??

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Launchpad is being weird. I'll resubmit this.

review: Needs Resubmitting
Revision history for this message
MC Return (mc-return) wrote :

Sam - it is not your fault.
Happens here also: https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1185819-cube-gears-obstruct-inside-view.1/+merge/167507

I do not think resubmitting will fix it -> seems to be a launchpad issue...

Revision history for this message
MC Return (mc-return) wrote :

Nope, now the other branch updated the diff correctly...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3734
http://jenkins.qa.ubuntu.com/job/compiz-ci/192/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/233
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-saucy-amd64-ci/5
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-saucy-armhf-ci/5/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-saucy-i386-ci/5

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/192/rebuild

review: Needs Fixing (continuous-integration)
lp:~compiz-team/compiz/compiz.fix_1187168 updated
3735. By Andrea Azzarone

Merge lp:~compiz-team/compiz/compiz.fix_1185719

Revision history for this message
Andrea Azzarone (azzar1) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmake/CompizCommon.cmake'
2--- cmake/CompizCommon.cmake 2013-02-15 11:28:27 +0000
3+++ cmake/CompizCommon.cmake 2013-06-05 16:13:36 +0000
4@@ -56,6 +56,11 @@
5 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-unused-private-field")
6 endif ()
7
8+option (COMPIZ_UNUSED_LOCAL_TYPEDEFS_WARNINGS "Warn about unused local typedefs" OFF)
9+if (NOT COMPIZ_UNUSED_LOCAL_TYPEDEFS_WARNINGS)
10+ set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-unused-local-typedefs")
11+endif (NOT COMPIZ_UNUSED_LOCAL_TYPEDEFS_WARNINGS)
12+
13 option (COMPIZ_DEPRECATED_WARNINGS "Warn about declarations marked deprecated" OFF)
14 if (NOT COMPIZ_DEPRECATED_WARNINGS)
15 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-deprecated-declarations")
16
17=== modified file 'gtk/window-decorator/tests/test_gwd_settings.cpp'
18--- gtk/window-decorator/tests/test_gwd_settings.cpp 2013-03-06 13:44:06 +0000
19+++ gtk/window-decorator/tests/test_gwd_settings.cpp 2013-06-05 16:13:36 +0000
20@@ -258,8 +258,6 @@
21 {
22 };
23
24-const GValue referenceGValue = G_VALUE_INIT;
25-
26 namespace
27 {
28 void gwd_settings_storage_unref (GWDSettingsStorage *storage)
29@@ -287,7 +285,11 @@
30
31 AutoUnsetGValue (GType type)
32 {
33- memcpy (&mValue, &referenceGValue, sizeof (GValue));
34+ /* This is effectively G_VALUE_INIT, we can't use that here
35+ * because this is not a C++11 project */
36+ mValue.g_type = 0;
37+ mValue.data[0].v_int = 0;
38+ mValue.data[1].v_int = 0;
39 g_value_init (&mValue, type);
40 }
41
42@@ -423,109 +425,154 @@
43 EXPECT_CALL (settingsGMock, dispose ());
44 EXPECT_CALL (settingsGMock, finalize ());
45
46+ /* The order of evaluation of matchers in Google Mock appears to be undefined and
47+ * the way GValueMatch is written makes it particularly unsafe when used with
48+ * matchers of multiple types on the same function, since there's no guaruntee
49+ * that the matchers will be traversed in any order. If a type is passed to
50+ * any of the matchers that it doesn't know how to handle then it will
51+ * call directly through to GValueCmp which will run into undefined behaviour
52+ * in itself.
53+ *
54+ * In reality, the API for GValueMatch is probably a little bit broken in this
55+ * sense, but just satisfying each expectation as soon as its set seems to do
56+ * the job here
57+ */
58+
59 /* calling g_object_get_property actually resets
60 * the value so expecting 0x0 is correct */
61 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_SHADOW,
62 GValueMatch <gpointer> (0x0, g_value_get_pointer),
63 _));
64+
65+ g_object_get_property (G_OBJECT (settingsMock.get ()),
66+ "active-shadow",
67+ &pointerGValue);
68+
69 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_SHADOW,
70 GValueMatch <gpointer> (0x0, g_value_get_pointer),
71 _));
72+
73+ g_object_get_property (G_OBJECT (settingsMock.get ()),
74+ "inactive-shadow",
75+ &pointerGValue);
76+
77 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_USE_TOOLTIPS,
78 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
79 _));
80+
81+ g_object_get_property (G_OBJECT (settingsMock.get ()),
82+ "use-tooltips",
83+ &booleanGValue);
84+
85 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_DRAGGABLE_BORDER_WIDTH,
86 GValueMatch <gint> (0, g_value_get_int),
87 _));
88+
89+ g_object_get_property (G_OBJECT (settingsMock.get ()),
90+ "draggable-border-width",
91+ &integerGValue);
92+
93 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ATTACH_MODAL_DIALOGS,
94 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
95 _));
96+
97+ g_object_get_property (G_OBJECT (settingsMock.get ()),
98+ "attach-modal-dialogs",
99+ &booleanGValue);
100+
101 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_BLUR_CHANGED,
102 GValueMatch <gint> (0, g_value_get_int),
103 _));
104+
105+ g_object_get_property (G_OBJECT (settingsMock.get ()),
106+ "blur",
107+ &integerGValue);
108+
109 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_METACITY_THEME,
110 GValueMatch <const gchar *> (NULL, g_value_get_string),
111 _));
112+
113+ g_object_get_property (G_OBJECT (settingsMock.get ()),
114+ "metacity-theme",
115+ &stringGValue);
116+
117 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_OPACITY,
118 GValueMatch <gdouble> (0.0, g_value_get_double),
119 _));
120+
121+ g_object_get_property (G_OBJECT (settingsMock.get ()),
122+ "metacity-active-opacity",
123+ &doubleGValue);
124+
125 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_OPACITY,
126 GValueMatch <gdouble> (0.0, g_value_get_double),
127 _));
128+
129+ g_object_get_property (G_OBJECT (settingsMock.get ()),
130+ "metacity-inactive-opacity",
131+ &doubleGValue);
132+
133 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_SHADE_OPACITY,
134 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
135 _));
136+
137+ g_object_get_property (G_OBJECT (settingsMock.get ()),
138+ "metacity-active-shade-opacity",
139+ &booleanGValue);
140+
141 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_SHADE_OPACITY,
142 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
143 _));
144+
145+ g_object_get_property (G_OBJECT (settingsMock.get ()),
146+ "metacity-inactive-shade-opacity",
147+ &booleanGValue);
148+
149 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_BUTTON_LAYOUT,
150 GValueMatch <const gchar *> (NULL, g_value_get_string),
151 _));
152+
153+ g_object_get_property (G_OBJECT (settingsMock.get ()),
154+ "metacity-button-layout",
155+ &stringGValue);
156+
157 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_DOUBLE_CLICK,
158 GValueMatch <gint> (0, g_value_get_int),
159 _));
160+
161+ g_object_get_property (G_OBJECT (settingsMock.get ()),
162+ "titlebar-double-click-action",
163+ &integerGValue);
164+
165 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_MIDDLE_CLICK,
166 GValueMatch <gint> (0, g_value_get_int),
167 _));
168+
169+ g_object_get_property (G_OBJECT (settingsMock.get ()),
170+ "titlebar-middle-click-action",
171+ &integerGValue);
172+
173 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_RIGHT_CLICK,
174 GValueMatch <gint> (0, g_value_get_int),
175 _));
176+
177+ g_object_get_property (G_OBJECT (settingsMock.get ()),
178+ "titlebar-right-click-action",
179+ &integerGValue);
180+
181 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_MOUSE_WHEEL_ACTION,
182 GValueMatch <gint> (0, g_value_get_int),
183 _));
184+
185+ g_object_get_property (G_OBJECT (settingsMock.get ()),
186+ "mouse-wheel-action",
187+ &integerGValue);
188+
189 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_FONT,
190 GValueMatch <const gchar *> (NULL, g_value_get_string),
191 _));
192
193 g_object_get_property (G_OBJECT (settingsMock.get ()),
194- "active-shadow",
195- &pointerGValue);
196- g_object_get_property (G_OBJECT (settingsMock.get ()),
197- "inactive-shadow",
198- &pointerGValue);
199- g_object_get_property (G_OBJECT (settingsMock.get ()),
200- "use-tooltips",
201- &booleanGValue);
202- g_object_get_property (G_OBJECT (settingsMock.get ()),
203- "draggable-border-width",
204- &integerGValue);
205- g_object_get_property (G_OBJECT (settingsMock.get ()),
206- "attach-modal-dialogs",
207- &booleanGValue);
208- g_object_get_property (G_OBJECT (settingsMock.get ()),
209- "blur",
210- &integerGValue);
211- g_object_get_property (G_OBJECT (settingsMock.get ()),
212- "metacity-theme",
213- &stringGValue);
214- g_object_get_property (G_OBJECT (settingsMock.get ()),
215- "metacity-active-opacity",
216- &doubleGValue);
217- g_object_get_property (G_OBJECT (settingsMock.get ()),
218- "metacity-inactive-opacity",
219- &doubleGValue);
220- g_object_get_property (G_OBJECT (settingsMock.get ()),
221- "metacity-active-shade-opacity",
222- &booleanGValue);
223- g_object_get_property (G_OBJECT (settingsMock.get ()),
224- "metacity-inactive-shade-opacity",
225- &booleanGValue);
226- g_object_get_property (G_OBJECT (settingsMock.get ()),
227- "metacity-button-layout",
228- &stringGValue);
229- g_object_get_property (G_OBJECT (settingsMock.get ()),
230- "titlebar-double-click-action",
231- &integerGValue);
232- g_object_get_property (G_OBJECT (settingsMock.get ()),
233- "titlebar-middle-click-action",
234- &integerGValue);
235- g_object_get_property (G_OBJECT (settingsMock.get ()),
236- "titlebar-right-click-action",
237- &integerGValue);
238- g_object_get_property (G_OBJECT (settingsMock.get ()),
239- "mouse-wheel-action",
240- &integerGValue);
241- g_object_get_property (G_OBJECT (settingsMock.get ()),
242 "titlebar-font",
243 &stringGValue);
244 }

Subscribers

People subscribed via source and target branches

to all changes: