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
=== modified file 'cmake/CompizCommon.cmake'
--- cmake/CompizCommon.cmake 2013-02-15 11:28:27 +0000
+++ cmake/CompizCommon.cmake 2013-06-05 16:13:36 +0000
@@ -56,6 +56,11 @@
56 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-unused-private-field")56 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-unused-private-field")
57endif ()57endif ()
5858
59option (COMPIZ_UNUSED_LOCAL_TYPEDEFS_WARNINGS "Warn about unused local typedefs" OFF)
60if (NOT COMPIZ_UNUSED_LOCAL_TYPEDEFS_WARNINGS)
61 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-unused-local-typedefs")
62endif (NOT COMPIZ_UNUSED_LOCAL_TYPEDEFS_WARNINGS)
63
59option (COMPIZ_DEPRECATED_WARNINGS "Warn about declarations marked deprecated" OFF)64option (COMPIZ_DEPRECATED_WARNINGS "Warn about declarations marked deprecated" OFF)
60if (NOT COMPIZ_DEPRECATED_WARNINGS)65if (NOT COMPIZ_DEPRECATED_WARNINGS)
61 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-deprecated-declarations")66 set (COMMON_FLAGS "${COMMON_FLAGS} -Wno-deprecated-declarations")
6267
=== modified file 'gtk/window-decorator/tests/test_gwd_settings.cpp'
--- gtk/window-decorator/tests/test_gwd_settings.cpp 2013-03-06 13:44:06 +0000
+++ gtk/window-decorator/tests/test_gwd_settings.cpp 2013-06-05 16:13:36 +0000
@@ -258,8 +258,6 @@
258{258{
259};259};
260260
261const GValue referenceGValue = G_VALUE_INIT;
262
263namespace261namespace
264{262{
265 void gwd_settings_storage_unref (GWDSettingsStorage *storage)263 void gwd_settings_storage_unref (GWDSettingsStorage *storage)
@@ -287,7 +285,11 @@
287285
288 AutoUnsetGValue (GType type)286 AutoUnsetGValue (GType type)
289 {287 {
290 memcpy (&mValue, &referenceGValue, sizeof (GValue));288 /* This is effectively G_VALUE_INIT, we can't use that here
289 * because this is not a C++11 project */
290 mValue.g_type = 0;
291 mValue.data[0].v_int = 0;
292 mValue.data[1].v_int = 0;
291 g_value_init (&mValue, type);293 g_value_init (&mValue, type);
292 }294 }
293295
@@ -423,109 +425,154 @@
423 EXPECT_CALL (settingsGMock, dispose ());425 EXPECT_CALL (settingsGMock, dispose ());
424 EXPECT_CALL (settingsGMock, finalize ());426 EXPECT_CALL (settingsGMock, finalize ());
425427
428 /* The order of evaluation of matchers in Google Mock appears to be undefined and
429 * the way GValueMatch is written makes it particularly unsafe when used with
430 * matchers of multiple types on the same function, since there's no guaruntee
431 * that the matchers will be traversed in any order. If a type is passed to
432 * any of the matchers that it doesn't know how to handle then it will
433 * call directly through to GValueCmp which will run into undefined behaviour
434 * in itself.
435 *
436 * In reality, the API for GValueMatch is probably a little bit broken in this
437 * sense, but just satisfying each expectation as soon as its set seems to do
438 * the job here
439 */
440
426 /* calling g_object_get_property actually resets441 /* calling g_object_get_property actually resets
427 * the value so expecting 0x0 is correct */442 * the value so expecting 0x0 is correct */
428 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_SHADOW,443 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_SHADOW,
429 GValueMatch <gpointer> (0x0, g_value_get_pointer),444 GValueMatch <gpointer> (0x0, g_value_get_pointer),
430 _));445 _));
446
447 g_object_get_property (G_OBJECT (settingsMock.get ()),
448 "active-shadow",
449 &pointerGValue);
450
431 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_SHADOW,451 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_SHADOW,
432 GValueMatch <gpointer> (0x0, g_value_get_pointer),452 GValueMatch <gpointer> (0x0, g_value_get_pointer),
433 _));453 _));
454
455 g_object_get_property (G_OBJECT (settingsMock.get ()),
456 "inactive-shadow",
457 &pointerGValue);
458
434 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_USE_TOOLTIPS,459 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_USE_TOOLTIPS,
435 GValueMatch <gboolean> (FALSE, g_value_get_boolean),460 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
436 _));461 _));
462
463 g_object_get_property (G_OBJECT (settingsMock.get ()),
464 "use-tooltips",
465 &booleanGValue);
466
437 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_DRAGGABLE_BORDER_WIDTH,467 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_DRAGGABLE_BORDER_WIDTH,
438 GValueMatch <gint> (0, g_value_get_int),468 GValueMatch <gint> (0, g_value_get_int),
439 _));469 _));
470
471 g_object_get_property (G_OBJECT (settingsMock.get ()),
472 "draggable-border-width",
473 &integerGValue);
474
440 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ATTACH_MODAL_DIALOGS,475 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ATTACH_MODAL_DIALOGS,
441 GValueMatch <gboolean> (FALSE, g_value_get_boolean),476 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
442 _));477 _));
478
479 g_object_get_property (G_OBJECT (settingsMock.get ()),
480 "attach-modal-dialogs",
481 &booleanGValue);
482
443 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_BLUR_CHANGED,483 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_BLUR_CHANGED,
444 GValueMatch <gint> (0, g_value_get_int),484 GValueMatch <gint> (0, g_value_get_int),
445 _));485 _));
486
487 g_object_get_property (G_OBJECT (settingsMock.get ()),
488 "blur",
489 &integerGValue);
490
446 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_METACITY_THEME,491 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_METACITY_THEME,
447 GValueMatch <const gchar *> (NULL, g_value_get_string),492 GValueMatch <const gchar *> (NULL, g_value_get_string),
448 _));493 _));
494
495 g_object_get_property (G_OBJECT (settingsMock.get ()),
496 "metacity-theme",
497 &stringGValue);
498
449 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_OPACITY,499 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_OPACITY,
450 GValueMatch <gdouble> (0.0, g_value_get_double),500 GValueMatch <gdouble> (0.0, g_value_get_double),
451 _));501 _));
502
503 g_object_get_property (G_OBJECT (settingsMock.get ()),
504 "metacity-active-opacity",
505 &doubleGValue);
506
452 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_OPACITY,507 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_OPACITY,
453 GValueMatch <gdouble> (0.0, g_value_get_double),508 GValueMatch <gdouble> (0.0, g_value_get_double),
454 _));509 _));
510
511 g_object_get_property (G_OBJECT (settingsMock.get ()),
512 "metacity-inactive-opacity",
513 &doubleGValue);
514
455 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_SHADE_OPACITY,515 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_ACTIVE_SHADE_OPACITY,
456 GValueMatch <gboolean> (FALSE, g_value_get_boolean),516 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
457 _));517 _));
518
519 g_object_get_property (G_OBJECT (settingsMock.get ()),
520 "metacity-active-shade-opacity",
521 &booleanGValue);
522
458 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_SHADE_OPACITY,523 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_INACTIVE_SHADE_OPACITY,
459 GValueMatch <gboolean> (FALSE, g_value_get_boolean),524 GValueMatch <gboolean> (FALSE, g_value_get_boolean),
460 _));525 _));
526
527 g_object_get_property (G_OBJECT (settingsMock.get ()),
528 "metacity-inactive-shade-opacity",
529 &booleanGValue);
530
461 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_BUTTON_LAYOUT,531 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_BUTTON_LAYOUT,
462 GValueMatch <const gchar *> (NULL, g_value_get_string),532 GValueMatch <const gchar *> (NULL, g_value_get_string),
463 _));533 _));
534
535 g_object_get_property (G_OBJECT (settingsMock.get ()),
536 "metacity-button-layout",
537 &stringGValue);
538
464 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_DOUBLE_CLICK,539 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_DOUBLE_CLICK,
465 GValueMatch <gint> (0, g_value_get_int),540 GValueMatch <gint> (0, g_value_get_int),
466 _));541 _));
542
543 g_object_get_property (G_OBJECT (settingsMock.get ()),
544 "titlebar-double-click-action",
545 &integerGValue);
546
467 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_MIDDLE_CLICK,547 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_MIDDLE_CLICK,
468 GValueMatch <gint> (0, g_value_get_int),548 GValueMatch <gint> (0, g_value_get_int),
469 _));549 _));
550
551 g_object_get_property (G_OBJECT (settingsMock.get ()),
552 "titlebar-middle-click-action",
553 &integerGValue);
554
470 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_RIGHT_CLICK,555 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_ACTION_RIGHT_CLICK,
471 GValueMatch <gint> (0, g_value_get_int),556 GValueMatch <gint> (0, g_value_get_int),
472 _));557 _));
558
559 g_object_get_property (G_OBJECT (settingsMock.get ()),
560 "titlebar-right-click-action",
561 &integerGValue);
562
473 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_MOUSE_WHEEL_ACTION,563 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_MOUSE_WHEEL_ACTION,
474 GValueMatch <gint> (0, g_value_get_int),564 GValueMatch <gint> (0, g_value_get_int),
475 _));565 _));
566
567 g_object_get_property (G_OBJECT (settingsMock.get ()),
568 "mouse-wheel-action",
569 &integerGValue);
570
476 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_FONT,571 EXPECT_CALL (settingsGMock, getProperty (GWD_MOCK_SETTINGS_PROPERTY_TITLEBAR_FONT,
477 GValueMatch <const gchar *> (NULL, g_value_get_string),572 GValueMatch <const gchar *> (NULL, g_value_get_string),
478 _));573 _));
479574
480 g_object_get_property (G_OBJECT (settingsMock.get ()),575 g_object_get_property (G_OBJECT (settingsMock.get ()),
481 "active-shadow",
482 &pointerGValue);
483 g_object_get_property (G_OBJECT (settingsMock.get ()),
484 "inactive-shadow",
485 &pointerGValue);
486 g_object_get_property (G_OBJECT (settingsMock.get ()),
487 "use-tooltips",
488 &booleanGValue);
489 g_object_get_property (G_OBJECT (settingsMock.get ()),
490 "draggable-border-width",
491 &integerGValue);
492 g_object_get_property (G_OBJECT (settingsMock.get ()),
493 "attach-modal-dialogs",
494 &booleanGValue);
495 g_object_get_property (G_OBJECT (settingsMock.get ()),
496 "blur",
497 &integerGValue);
498 g_object_get_property (G_OBJECT (settingsMock.get ()),
499 "metacity-theme",
500 &stringGValue);
501 g_object_get_property (G_OBJECT (settingsMock.get ()),
502 "metacity-active-opacity",
503 &doubleGValue);
504 g_object_get_property (G_OBJECT (settingsMock.get ()),
505 "metacity-inactive-opacity",
506 &doubleGValue);
507 g_object_get_property (G_OBJECT (settingsMock.get ()),
508 "metacity-active-shade-opacity",
509 &booleanGValue);
510 g_object_get_property (G_OBJECT (settingsMock.get ()),
511 "metacity-inactive-shade-opacity",
512 &booleanGValue);
513 g_object_get_property (G_OBJECT (settingsMock.get ()),
514 "metacity-button-layout",
515 &stringGValue);
516 g_object_get_property (G_OBJECT (settingsMock.get ()),
517 "titlebar-double-click-action",
518 &integerGValue);
519 g_object_get_property (G_OBJECT (settingsMock.get ()),
520 "titlebar-middle-click-action",
521 &integerGValue);
522 g_object_get_property (G_OBJECT (settingsMock.get ()),
523 "titlebar-right-click-action",
524 &integerGValue);
525 g_object_get_property (G_OBJECT (settingsMock.get ()),
526 "mouse-wheel-action",
527 &integerGValue);
528 g_object_get_property (G_OBJECT (settingsMock.get ()),
529 "titlebar-font",576 "titlebar-font",
530 &stringGValue);577 &stringGValue);
531}578}

Subscribers

People subscribed via source and target branches

to all changes: