Merge lp:~sil2100/compiz/quickfix_1141079_raring into lp:compiz/raring

[Łukasz Zemczak]

Remove the override_redirect xcreatewindow event assignment from revision 3606. This fixes the Qt menu regressions (LP: #1141079)

Merge from compiz trunk:
https://code.launchpad.net/~sil2100/compiz/quickfix_1141079/+merge/153427

- Problem:

The fix introduced in revision 3606 triggers a regression with Qt menus. It seems that passing on the override_redirect state from xcreatewindow results in this regression happening. Pop-up menus in Qt apps sometimes are detected as a separate window.

- Fix:

Remove the line that seems to trigger the regression (probably implicitly). Comments needed?

- Tests:

N/A

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3637
http://jenkins.qa.ubuntu.com/job/compiz-ci/111/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/148/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/500/console

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

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

Dunno what happened with those tests, might just want to run them again (unless gsettings changed upstream, in which case we need to fix that).

See my main commentary on the main review.

[lp:~sil2100/compiz/quickfix_1141079_raring]

3638. By Łukasz Zemczak on 2013-04-02

Backport the changes from the lp:compiz branch of the same fix.

[Daniel van Vugt (vanvugt)]

Please target all fixes to lp:compiz first (0.9.10). And then lp:compiz/0.9.9 (the new raring branch) second.

review: Needs Resubmitting

[Łukasz Zemczak (sil2100)]

Resubmitted as lp:~sil2100/compiz/quickfix_1141079_0.9.9 .

Unmerged revisions

3638. By Łukasz Zemczak on 2013-04-02

Backport the changes from the lp:compiz branch of the same fix.

3637. By Łukasz Zemczak on 2013-03-14

Remove the override_redirect xcreatewindow event assignment from revision 3606. This fixes the Qt menu regressions (LP: #1141079)

3636. By Sam Spilsbury on 2013-03-13

Use the actual change in window size to determine if we need to change
  the frame window size, don't just use the difference in frame extents
  to determine that.

  (LP: #1140505). Fixes: https://bugs.launchpad.net/bugs/1140505.

Approved by PS Jenkins bot, Andrea Azzarone.

3635. By Sam Spilsbury on 2013-03-13

Don't set lastFrameExtents unless the window geometry actually changed - as
that variable only exists to track changes in the actual geometry of
the window and not the apparant frame extents.

Added tests to verify that behaviour.

(LP: #1138517). Fixes: https://bugs.launchpad.net/bugs/1138517.

Approved by PS Jenkins bot, Timo Jyrinki, Andrea Azzarone.

3634. By PS Jenkins bot on 2013-03-08

Releasing 1:0.9.9~daily13.03.08-0ubuntu1 to ubuntu.

Approved by PS Jenkins bot.

3633. By Brandon Schaefer on 2013-03-07

Instead of not grabbing all the keys when IgnoreTap is set, add a patch to only not grab Super+P. Allowing part of this bug to say fix (LP:950160), while fixing bugs such as this (LP:1075207). This is only a patch for unity, otherwise it will not cause any problems with trunk compiz. Fixes: https://bugs.launchpad.net/bugs/1075207.

Approved by PS Jenkins bot, Andrea Azzarone.

3632. By PS Jenkins bot on 2013-03-06

Releasing 1:0.9.9~daily13.03.06-0ubuntu1 to ubuntu.

Approved by PS Jenkins bot.

3631. By Łukasz Zemczak on 2013-03-06

 Revert revision 3616, as the proposed fix causes serious regressions with Java Swing applications (LP: #1138517). That's no good!. Fixes: https://bugs.launchpad.net/bugs/1138517.

Approved by PS Jenkins bot, Didier Roche.

3630. By PS Jenkins bot on 2013-03-01

Releasing 1:0.9.9~daily13.03.01-0ubuntu1 to ubuntu.

Approved by PS Jenkins bot.

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-29 07:23:54 +0000
+++ debian/changelog	2013-04-02 14:45:27 +0000
@@ -69,7 +69,7 @@
 
  -- Automatic PS uploader <[e-mail]>  Fri, 01 Mar 2013 04:02:19 +0000
 
-compiz (1:0.9.9~daily13.02.28-0ubuntu1) raring; urgency=low
+compiz (1:0.9.9~daily13.02.28-0ubuntu1~test1) raring; urgency=low
 
   [ Sam Spilsbury ]
   * [nvidia] Moving windows freezes and stutters on nvidia (especially

=== modified file 'src/event.cpp'
--- src/event.cpp	2013-02-10 05:01:50 +0000
+++ src/event.cpp	2013-04-02 14:45:27 +0000
@@ -1285,19 +1285,20 @@
 	 * the window to the window list as we might get configure requests
 	 * which require us to stack other windows relative to it. Setting
 	 * some default values if this is the case. */
-	if (!XGetWindowAttributes (privateScreen.dpy, event->xcreatewindow.window, &wa))
+	if (!XGetWindowAttributes (privateScreen.dpy, event->xcreatewindow.window, &wa)) {
 	    privateScreen.setDefaultWindowAttributes (&wa);
 
-	/* That being said, we should store as much information as possible
-	 * about it. There may be requests relative to this window that could
-	 * use the data in the XCreateWindowEvent structure, especially the
-	 * override redirect state */
-	wa.x = event->xcreatewindow.x;
-	wa.y = event->xcreatewindow.y;
-	wa.width = event->xcreatewindow.width;
-	wa.height = event->xcreatewindow.height;
-	wa.border_width = event->xcreatewindow.border_width;
-	wa.override_redirect = event->xcreatewindow.override_redirect;
+	    /* That being said, we should store as much information as possible
+	     * about it. There may be requests relative to this window that could
+	     * use the data in the XCreateWindowEvent structure, especially the
+	     * override redirect state */
+	    wa.x = event->xcreatewindow.x;
+	    wa.y = event->xcreatewindow.y;
+	    wa.width = event->xcreatewindow.width;
+	    wa.height = event->xcreatewindow.height;
+	    wa.border_width = event->xcreatewindow.border_width;
+	    wa.override_redirect = event->xcreatewindow.override_redirect;
+	}
 
 	foreach (CompWindow *w, screen->windows ())
 	{
@@ -1475,16 +1476,17 @@
 		 * the window to the window list as we might get configure requests
 		 * which require us to stack other windows relative to it. Setting
 		 * some default values if this is the case. */
-		if (!XGetWindowAttributes (privateScreen.dpy, event->xcreatewindow.window, &wa))
+		if (!XGetWindowAttributes (privateScreen.dpy, event->xcreatewindow.window, &wa)) {
 		    privateScreen.setDefaultWindowAttributes (&wa);
 
-		/* That being said, we should store as much information as possible
-		 * about it. There may be requests relative to this window that could
-		 * use the data in the XCreateWindowEvent structure, especially the
-		 * override redirect state */
-		wa.x = event->xreparent.x;
-		wa.y = event->xreparent.y;
-		wa.override_redirect = event->xreparent.override_redirect;
+		    /* That being said, we should store as much information as possible
+		     * about it. There may be requests relative to this window that could
+		     * use the data in the XCreateWindowEvent structure, especially the
+		     * override redirect state */
+		    wa.x = event->xreparent.x;
+		    wa.y = event->xreparent.y;
+		    wa.override_redirect = event->xreparent.override_redirect;
+		}
 
 		PrivateWindow::createCompWindow (getTopWindow ()->id (), getTopServerWindow ()->id (), wa, event->xreparent.window);
 		break;

=== modified file 'tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp'
--- tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp	2013-03-06 13:18:07 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp	2013-04-02 14:45:27 +0000
@@ -212,6 +212,12 @@
 	bool VerifySetFrameExtentsResponse (Window w, int left, int right, int top, int bottom);
 	bool VerifyWindowSize (Window w, int x, int y, int width, int height);
 
+	/* Helper function for the Create*WindowOverrideRedirect* tests */
+	void CreateWithAttrAndTest(int overrideCreate, 
+				   bool shouldChangeOverride, 
+				   int overrideChange, 
+				   bool overrideAssert);
+
     protected:
 
 	ReparentedWindow CreateWindow (::Display *);
@@ -750,3 +756,79 @@
 				   currentWidth,
 				   currentHeight));
 }
+
+void
+CompizXorgSystemConfigureWindowTest::CreateWithAttrAndTest(int overrideCreate, 
+							   bool shouldChangeOverride, 
+							   int overrideChange, 
+							   bool overrideAssert)
+{
+    ::Display *dpy = Display ();
+
+    XGrabServer (dpy);
+
+    /* Create window attributes, set override as overrideCreate */
+    XSetWindowAttributes attr;
+    attr.override_redirect = overrideCreate;
+
+    /* Create a window with our custom attributes */
+    unsigned int mask = CWOverrideRedirect;
+    Window w = XCreateWindow (dpy, DefaultRootWindow (dpy),
+			      0, 0, 100, 100, 0, CopyFromParent,
+			      InputOutput, CopyFromParent, mask,
+			      &attr);
+
+    if (shouldChangeOverride) 
+    {
+	/* If we test the case of a attribute change, we don't destroy the window */
+	attr.override_redirect = overrideChange;
+	XChangeWindowAttributes (dpy, w, mask, &attr);
+    }
+    else 
+    {
+	/* ...otherwise, destroy it serwer-side */
+	XDestroyWindow (dpy, w);
+    }
+
+    XSync (dpy, false);
+
+    XUngrabServer (dpy);
+
+    XSync (dpy, false);
+
+    /* Check if the created window had the attributes passed correctly */
+    std::vector <long> data = WaitForWindowCreation (w);
+    EXPECT_EQ (overrideAssert, IsOverrideRedirect (data));
+
+    /* In case we didn't do this before, destroy the window now */
+    if (shouldChangeOverride)
+	XDestroyWindow (dpy, w);
+}
+
+TEST_F (CompizXorgSystemConfigureWindowTest, CreateDestroyWindowOverrideRedirectTrue)
+{
+    /* Create window with override_redirect = 1, destroy the window and check the
+     * override */
+    CreateWithAttrAndTest(1, false, 0, true);
+}
+
+TEST_F (CompizXorgSystemConfigureWindowTest, CreateDestroyWindowOverrideRedirectFalse)
+{
+    /* Create window with override_redirect = 0, destroy the window and check the
+     * override */
+    CreateWithAttrAndTest(0, false, 0, false);
+}
+
+TEST_F (CompizXorgSystemConfigureWindowTest, CreateChangeWindowOverrideRedirectTrue)
+{
+    /* Create window with override_redirect = 0, change the attributes after creation
+     * to override_redirect = 1 and assert the override */
+    CreateWithAttrAndTest(0, true, 1, true);
+}
+
+TEST_F (CompizXorgSystemConfigureWindowTest, CreateChangeWindowOverrideRedirectFalse)
+{
+    /* Create window with override_redirect = 1, change the attributes after creation
+     * to override_redirect = 0 and assert the override */
+    CreateWithAttrAndTest(1, true, 0, false);
+}

=== modified file 'tests/xorg-gtest/include/compiz-xorg-gtest.h'
--- tests/xorg-gtest/include/compiz-xorg-gtest.h	2013-02-23 10:47:18 +0000
+++ tests/xorg-gtest/include/compiz-xorg-gtest.h	2013-04-02 14:45:27 +0000
@@ -214,7 +214,8 @@
 	    protected:
 
 		Atom FetchAtom (const char *);
-		void WaitForWindowCreation (Window w);
+		std::vector <long> WaitForWindowCreation (Window w);
+		bool IsOverrideRedirect (std::vector <long> &data);
 
 		virtual int  GetEventMask ();
 

=== modified file 'tests/xorg-gtest/plugins/testhelper/src/testhelper.cpp'
--- tests/xorg-gtest/plugins/testhelper/src/testhelper.cpp	2013-02-14 12:12:53 +0000
+++ tests/xorg-gtest/plugins/testhelper/src/testhelper.cpp	2013-04-02 14:45:27 +0000
@@ -192,6 +192,7 @@
 
     std::vector <long> data;
     data.push_back (static_cast <long> (window->id ()));
+    data.push_back (static_cast <long> (window->overrideRedirect ()));
     ct::SendClientMessage (screen->dpy (),
 			   ts->fetchAtom (ctm::TEST_HELPER_WINDOW_READY),
 			   screen->root (),

=== modified file 'tests/xorg-gtest/src/compiz-xorg-gtest.cpp'
--- tests/xorg-gtest/src/compiz-xorg-gtest.cpp	2013-02-23 08:23:05 +0000
+++ tests/xorg-gtest/src/compiz-xorg-gtest.cpp	2013-04-02 14:45:27 +0000
@@ -738,7 +738,7 @@
 	std::auto_ptr <ct::MessageAtoms> mMessages;
 };
 
-void
+std::vector <long>
 ct::AutostartCompizXorgSystemTestWithTestHelper::WaitForWindowCreation (Window w)
 {
     ::Display *dpy = Display ();
@@ -758,7 +758,19 @@
 
     }
 
-    ASSERT_TRUE (requestAcknowledged);
+    EXPECT_TRUE (requestAcknowledged);
+
+    std::vector <long> data;
+    for (int i = 0; i < 5; ++i)
+	data.push_back(static_cast <long> (event.xclient.data.l[i]));
+
+    return data;
+}
+
+bool
+ct::AutostartCompizXorgSystemTestWithTestHelper::IsOverrideRedirect (std::vector <long> &data)
+{
+    return (data[1] > 0) ? true : false;
 }
 
 Atom