Compiz

Merge lp:~compiz-team/compiz/compiz.fix_1096455 into lp:compiz/0.9.9

  1. compiz.fix_1096455
  2. Merge into 0.9.9
Proposed by Sam Spilsbury
Status: Merged
Approved by: Daniel van Vugt
Approved revision: 3559
Merged at revision: 3569
Proposed branch: lp:~compiz-team/compiz/compiz.fix_1096455
Merge into: lp:compiz/0.9.9
Diff against target: 199 lines (+155/-4)
3 files modified
src/screen.cpp (+3/-4)
tests/system/xorg-gtest/tests/CMakeLists.txt (+8/-0)
tests/system/xorg-gtest/tests/compiz_xorg_gtest_ewmh.cpp (+144/-0)
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_1096455
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+142847@code.launchpad.net

Commit message

Don't set _NET_DESKTOP_GEOMETRY until we've set the default viewport size. (LP: #1096455)

Description of the change

Don't set _NET_DESKTOP_GEOMETRY until we've set the default viewport size. (LP: #1096455)

Added a system test to verify that.

To post a comment you must log in.
lp:~compiz-team/compiz/compiz.fix_1096455 updated
3558. By Sam Spilsbury

Don't leak property

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

PASSED: Continuous integration, rev:3558
http://jenkins.qa.ubuntu.com/job/compiz-ci/359/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/359/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/359//rebuild/?

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

PASSED: Continuous integration, rev:3558
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/10/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/10/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/10//rebuild/?

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

PASSED: Continuous integration, rev:3558
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/11/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/11/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/11//rebuild/?

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Minor issue:
+ * Copyright (C) 2012 Canonical Ltd.

It should probably be 2013 and yourself.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Trying to make this build, I'm missing xig-0 for raring. Doesn't seem to exist for raring or quantal so I can't test it right now.

Robert is working on updating the Xig PPA to support Q and R.

Abstain for now.

review: Abstain
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

On Wed, Jan 16, 2013 at 8:55 AM, Daniel van Vugt
<email address hidden> wrote:
> Review: Abstain
>
> Trying to make this build, I'm missing xig-0 for raring. Doesn't seem to exist for raring or quantal so I can't test it right now.
>
> Robert is working on updating the Xig PPA to support Q and R.
>

This doesn't use Xig, it uses xorg-gtest. It also doesn't require
either of them to build, just for the tests.

I've been thinking of dropping the Xig support for some time. Its a
really neat concept, but AFAICT its not really maintained, and there's
a large maintenance burden because it actually requires us to
re-implement support for the various bits of the protocol.

xorg-gtest gets us most of the way there. It doesn't allow us to truly
verify whats going on server side, but it does allow us to watch what
the server is saying.

xorg-gtest is a bit of a pain to build, but if I remember correctly it
was something like this:

1. sudo apt-get install libx11-dev libxi-dev xutils-dev xserver-xorg-dev
2. install xf86-video-dummy:
git://anongit.freedesktop.org/xorg/driver/xf86-video-dummy
3. install xorg-gtest: http://cgit.freedesktop.org/xorg/test/xorg-gtest/

You can try it out with that. Be aware that the xorg-gtest tests run a
little slowly (about 2 seconds per test), but thats because they start
a full xserver in the background.

> Abstain for now.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1096455/+merge/142847
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

lp:~compiz-team/compiz/compiz.fix_1096455 updated
3559. By Sam Spilsbury

Update copyrights

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

On Wed, Jan 16, 2013 at 8:18 AM, Daniel van Vugt
<email address hidden> wrote:
> Minor issue:
> + * Copyright (C) 2012 Canonical Ltd.
>
> It should probably be 2013 and yourself.

Thanks. Done.

> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1096455/+merge/142847
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1096455 into lp:compiz.

--
Sam Spilsbury

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Hmm, I thought the missing Xig was a problem because I was getting strange FTBFS after I installed xorg-gtest. Lost the log file for now sorry.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

On Wed, Jan 16, 2013 at 10:27 AM, Daniel van Vugt
<email address hidden> wrote:
> Hmm, I thought the missing Xig was a problem because I was getting strange FTBFS after I installed xorg-gtest. Lost the log file for now sorry.

Weird. Can you post details of the ftbfs?

> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1096455/+merge/142847
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

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

PASSED: Continuous integration, rev:3559
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/17/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/17/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/17//rebuild/?

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

PASSED: Continuous integration, rev:3559
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/16/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/16//rebuild/?

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

PASSED: Continuous integration, rev:3559
http://jenkins.qa.ubuntu.com/job/compiz-ci/365/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/365/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/365//rebuild/?

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Ok, I can't build/test this proposal right now because I'm blocked by bug 1100382. Please review that.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Seems I won't be able to build the test for a while. But I did test the code with compiz a while back. So that's OK for now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/screen.cpp'
--- src/screen.cpp 2012-12-30 11:13:36 +0000
+++ src/screen.cpp 2013-01-16 02:06:20 +0000
@@ -4998,7 +4998,6 @@
49984998
4999 updateScreenEdges ();4999 updateScreenEdges ();
50005000
5001 setDesktopHints ();
5002 eventManager.setSupportingWmCheck (dpy, rootWindow());5001 eventManager.setSupportingWmCheck (dpy, rootWindow());
5003 screen->updateSupportedWmHints ();5002 screen->updateSupportedWmHints ();
50045003
@@ -5114,11 +5113,11 @@
5114 if (!init_succeeded)5113 if (!init_succeeded)
5115 return false;5114 return false;
51165115
5117 /* The active plugins list might have been changed - load any5116 /* The viewport geometry depends on the new new plugins loaded
5118 * new plugins */5117 * especially those that modify option values */
5119
5120 viewPort.vpSize.setWidth (optionGetHsize ());5118 viewPort.vpSize.setWidth (optionGetHsize ());
5121 viewPort.vpSize.setHeight (optionGetVsize ());5119 viewPort.vpSize.setHeight (optionGetVsize ());
5120 setDesktopHints ();
51225121
5123 setAudibleBell (optionGetAudibleBell ());5122 setAudibleBell (optionGetAudibleBell ());
51245123
51255124
=== modified file 'tests/system/xorg-gtest/tests/CMakeLists.txt'
--- tests/system/xorg-gtest/tests/CMakeLists.txt 2012-12-05 03:35:37 +0000
+++ tests/system/xorg-gtest/tests/CMakeLists.txt 2013-01-16 02:06:20 +0000
@@ -18,6 +18,9 @@
18 add_executable (compiz_xorg_gtest_test_icccm18 add_executable (compiz_xorg_gtest_test_icccm
19 ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_icccm.cpp)19 ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_icccm.cpp)
2020
21 add_executable (compiz_xorg_gtest_test_ewmh
22 ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_ewmh.cpp)
23
21 set (COMPIZ_XORG_GTEST_LIBRARIES24 set (COMPIZ_XORG_GTEST_LIBRARIES
22 compiz_xorg_gtest_system_test25 compiz_xorg_gtest_system_test
23 xorg_gtest_all26 xorg_gtest_all
@@ -38,4 +41,9 @@
3841
39 compiz_discover_tests (compiz_xorg_gtest_test_icccm)42 compiz_discover_tests (compiz_xorg_gtest_test_icccm)
4043
44 target_link_libraries (compiz_xorg_gtest_test_ewmh
45 ${COMPIZ_XORG_GTEST_LIBRARIES})
46
47 compiz_discover_tests (compiz_xorg_gtest_test_ewmh)
48
41endif (BUILD_XORG_GTEST AND X11_XI_FOUND)49endif (BUILD_XORG_GTEST AND X11_XI_FOUND)
4250
=== added file 'tests/system/xorg-gtest/tests/compiz_xorg_gtest_ewmh.cpp'
--- tests/system/xorg-gtest/tests/compiz_xorg_gtest_ewmh.cpp 1970-01-01 00:00:00 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_ewmh.cpp 2013-01-16 02:06:20 +0000
@@ -0,0 +1,144 @@
1/*
2 * Compiz XOrg GTest, EWMH compliance
3 *
4 * Copyright (C) 2013 Sam Spilsbury
5 *
6 * This library is free software; you can redistribute it and/or
7 * modify it under the terms of the GNU Lesser General Public
8 * License as published by the Free Software Foundation; either
9 * version 2.1 of the License, or (at your option) any later version.
10
11 * This library is distributed in the hope that it will be useful,
12 * but WITHOUT ANY WARRANTY; without even the implied warranty of
13 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14 * Lesser General Public License for more details.
15
16 * You should have received a copy of the GNU Lesser General Public
17 * License along with this library; if not, write to the Free Software
18 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
19 *
20 * Authored By:
21 * Sam Spilsbury <smspillaz@gmail.com>
22 */
23#include <list>
24#include <string>
25#include <stdexcept>
26#include <boost/function.hpp>
27#include <boost/bind.hpp>
28#include <boost/shared_ptr.hpp>
29#include <boost/make_shared.hpp>
30#include <gtest/gtest.h>
31#include <gmock/gmock.h>
32#include <xorg/gtest/xorg-gtest.h>
33#include <compiz-xorg-gtest.h>
34
35#include <gtest_shared_tmpenv.h>
36#include <gtest_shared_characterwrapper.h>
37
38#include <X11/Xlib.h>
39#include <X11/Xatom.h>
40
41using ::testing::MatchResultListener;
42using ::testing::MakeMatcher;
43using ::testing::Matcher;
44
45namespace ct = compiz::testing;
46
47namespace
48{
49unsigned int DEFAULT_VIEWPORT_WIDTH = 4;
50unsigned int DEFAULT_VIEWPORT_HEIGHT = 1;
51
52bool Advance (Display *d, bool r)
53{
54 return ct::AdvanceToNextEventOnSuccess (d, r);
55}
56
57}
58
59class CompizXorgSystemEWMH :
60 public ct::CompizXorgSystemTest
61{
62 public:
63
64 virtual void SetUp ()
65 {
66 ct::CompizXorgSystemTest::SetUp ();
67
68 ::Display *dpy = Display ();
69
70 XSelectInput (dpy, DefaultRootWindow (dpy),
71 PropertyChangeMask);
72
73 Window wDummy;
74 unsigned int uiDummy;
75 int iDummy;
76
77 ASSERT_TRUE (XGetGeometry (dpy, DefaultRootWindow (dpy),
78 &wDummy,
79 &iDummy,
80 &iDummy,
81 &screenWidth,
82 &screenHeight,
83 &uiDummy,
84 &uiDummy));
85 }
86
87 unsigned int screenWidth;
88 unsigned int screenHeight;
89
90 private:
91};
92
93TEST_F (CompizXorgSystemEWMH, InitialViewportGeometry)
94{
95 ::Display *dpy = Display ();
96 StartCompiz (static_cast <ct::CompizProcess::StartupFlags> (
97 ct::CompizProcess::ReplaceCurrentWM));
98
99 ct::PropertyNotifyXEventMatcher desktopHintsProperty (dpy,
100 "_NET_DESKTOP_GEOMETRY");
101
102 /* Assert that we get the property update */
103 ASSERT_TRUE (Advance (dpy,
104 ct::WaitForEventOfTypeOnWindowMatching (dpy,
105 DefaultRootWindow (dpy),
106 PropertyNotify,
107 -1,
108 -1,
109 desktopHintsProperty)));
110
111 unsigned int expectedDefaultWidth = screenWidth * DEFAULT_VIEWPORT_WIDTH;
112 unsigned int expectedDefaultHeight = screenHeight * DEFAULT_VIEWPORT_HEIGHT;
113
114 Atom actualType;
115 int actualFmt;
116 unsigned long nItems, bytesAfter;
117 unsigned char *property;
118
119 ASSERT_EQ (Success,
120 XGetWindowProperty (dpy,
121 DefaultRootWindow (dpy),
122 XInternAtom (dpy, "_NET_DESKTOP_GEOMETRY", False),
123 0L,
124 8L,
125 False,
126 XA_CARDINAL,
127 &actualType,
128 &actualFmt,
129 &nItems,
130 &bytesAfter,
131 &property));
132
133 ASSERT_EQ (XA_CARDINAL, actualType);
134 ASSERT_EQ (32, actualFmt);
135 ASSERT_EQ (2, nItems);
136
137 unsigned long *geometry = reinterpret_cast <unsigned long *> (property);
138
139 EXPECT_EQ (expectedDefaultWidth, geometry[0]);
140 EXPECT_EQ (expectedDefaultHeight, geometry[1]);
141
142 if (property)
143 XFree (property);
144}

Subscribers

People subscribed via source and target branches