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

[Sam Spilsbury]

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

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

Added a system test to verify that.

[lp:~compiz-team/compiz/compiz.fix_1096455]

3558. By Sam Spilsbury on 2013-01-11

Don't leak property

[PS Jenkins bot (ps-jenkins)]

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)

[PS Jenkins bot (ps-jenkins)]

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)

[PS Jenkins bot (ps-jenkins)]

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)

[Daniel van Vugt (vanvugt)]

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

It should probably be 2013 and yourself.

[Daniel van Vugt (vanvugt)]

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

[Sam Spilsbury (smspillaz)]

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]

3559. By Sam Spilsbury on 2013-01-16

Update copyrights

[Sam Spilsbury (smspillaz)]

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

[Daniel van Vugt (vanvugt)]

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.

[Sam Spilsbury (smspillaz)]

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

[PS Jenkins bot (ps-jenkins)]

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)

[PS Jenkins bot (ps-jenkins)]

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)

[PS Jenkins bot (ps-jenkins)]

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)

[Daniel van Vugt (vanvugt)]

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

review: Abstain

[Daniel van Vugt (vanvugt)]

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

=== 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 @@
 
     updateScreenEdges ();
 
-    setDesktopHints ();
     eventManager.setSupportingWmCheck (dpy, rootWindow());
     screen->updateSupportedWmHints ();
 
@@ -5114,11 +5113,11 @@
     if (!init_succeeded)
       return false;
 
-    /* The active plugins list might have been changed - load any
-     * new plugins */
-
+    /* The viewport geometry depends on the new new plugins loaded
+     * especially those that modify option values */
     viewPort.vpSize.setWidth (optionGetHsize ());
     viewPort.vpSize.setHeight (optionGetVsize ());
+    setDesktopHints ();
 
     setAudibleBell (optionGetAudibleBell ());
 

=== 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 @@
     add_executable (compiz_xorg_gtest_test_icccm
 		    ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_icccm.cpp)
 
+    add_executable (compiz_xorg_gtest_test_ewmh
+		    ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_ewmh.cpp)
+
     set (COMPIZ_XORG_GTEST_LIBRARIES
 	 compiz_xorg_gtest_system_test
 	 xorg_gtest_all
@@ -38,4 +41,9 @@
 
     compiz_discover_tests (compiz_xorg_gtest_test_icccm)
 
+    target_link_libraries (compiz_xorg_gtest_test_ewmh
+			   ${COMPIZ_XORG_GTEST_LIBRARIES})
+
+    compiz_discover_tests (compiz_xorg_gtest_test_ewmh)
+
 endif (BUILD_XORG_GTEST AND X11_XI_FOUND)

=== 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 @@
+/*
+ * Compiz XOrg GTest, EWMH compliance
+ *
+ * Copyright (C) 2013 Sam Spilsbury
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#include <list>
+#include <string>
+#include <stdexcept>
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+#include <boost/shared_ptr.hpp>
+#include <boost/make_shared.hpp>
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include <xorg/gtest/xorg-gtest.h>
+#include <compiz-xorg-gtest.h>
+
+#include <gtest_shared_tmpenv.h>
+#include <gtest_shared_characterwrapper.h>
+
+#include <X11/Xlib.h>
+#include <X11/Xatom.h>
+
+using ::testing::MatchResultListener;
+using ::testing::MakeMatcher;
+using ::testing::Matcher;
+
+namespace ct = compiz::testing;
+
+namespace
+{
+unsigned int DEFAULT_VIEWPORT_WIDTH = 4;
+unsigned int DEFAULT_VIEWPORT_HEIGHT = 1;
+
+bool Advance (Display *d, bool r)
+{
+    return ct::AdvanceToNextEventOnSuccess (d, r);
+}
+
+}
+
+class CompizXorgSystemEWMH :
+    public ct::CompizXorgSystemTest
+{
+    public:
+
+	virtual void SetUp ()
+	{
+	    ct::CompizXorgSystemTest::SetUp ();
+
+	    ::Display *dpy = Display ();
+
+	    XSelectInput (dpy, DefaultRootWindow (dpy),
+			  PropertyChangeMask);
+
+	    Window wDummy;
+	    unsigned int uiDummy;
+	    int iDummy;
+
+	    ASSERT_TRUE (XGetGeometry (dpy, DefaultRootWindow (dpy),
+				       &wDummy,
+				       &iDummy,
+				       &iDummy,
+				       &screenWidth,
+				       &screenHeight,
+				       &uiDummy,
+				       &uiDummy));
+	}
+
+	unsigned int screenWidth;
+	unsigned int screenHeight;
+
+    private:
+};
+
+TEST_F (CompizXorgSystemEWMH, InitialViewportGeometry)
+{
+    ::Display *dpy = Display ();
+    StartCompiz (static_cast <ct::CompizProcess::StartupFlags> (
+		     ct::CompizProcess::ReplaceCurrentWM));
+
+    ct::PropertyNotifyXEventMatcher desktopHintsProperty (dpy,
+							  "_NET_DESKTOP_GEOMETRY");
+
+    /* Assert that we get the property update */
+    ASSERT_TRUE (Advance (dpy,
+			  ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								  DefaultRootWindow (dpy),
+								  PropertyNotify,
+								  -1,
+								  -1,
+								  desktopHintsProperty)));
+
+    unsigned int expectedDefaultWidth = screenWidth * DEFAULT_VIEWPORT_WIDTH;
+    unsigned int expectedDefaultHeight = screenHeight * DEFAULT_VIEWPORT_HEIGHT;
+
+    Atom actualType;
+    int  actualFmt;
+    unsigned long nItems, bytesAfter;
+    unsigned char *property;
+
+    ASSERT_EQ (Success,
+	       XGetWindowProperty (dpy,
+				   DefaultRootWindow (dpy),
+				   XInternAtom (dpy, "_NET_DESKTOP_GEOMETRY", False),
+				   0L,
+				   8L,
+				   False,
+				   XA_CARDINAL,
+				   &actualType,
+				   &actualFmt,
+				   &nItems,
+				   &bytesAfter,
+				   &property));
+
+    ASSERT_EQ (XA_CARDINAL, actualType);
+    ASSERT_EQ (32, actualFmt);
+    ASSERT_EQ (2, nItems);
+
+    unsigned long *geometry = reinterpret_cast <unsigned long *> (property);
+
+    EXPECT_EQ (expectedDefaultWidth, geometry[0]);
+    EXPECT_EQ (expectedDefaultHeight, geometry[1]);
+
+    if (property)
+	XFree (property);
+}