Merge lp:~compiz-team/compiz/raring.fix_1138517 into lp:compiz/raring

[Sam Spilsbury]

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)

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)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3633
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~compiz-team/compiz/raring.fix_1138517/+merge/151940/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/compiz-ci/106/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/143/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/495/console

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

review: Needs Fixing (continuous-integration)

[Timo Jyrinki (timo-jyrinki)]

Not sure what was jenkins' problem, but this branch has the same issue of non-moveable/resizeable Java windows as https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1138517/+merge/151941

review: Needs Fixing

[lp:~compiz-team/compiz/raring.fix_1138517]

3634. By Sam Spilsbury on 2013-03-11

Merge lp:compiz/raring

3635. By Sam Spilsbury on 2013-03-11

Only set the decoration if we actually meant to decorate the window.

This allows us to set frame extents and not set the whole decoration
property

Unmerged revisions

3635. By Sam Spilsbury on 2013-03-11

Only set the decoration if we actually meant to decorate the window.

This allows us to set frame extents and not set the whole decoration
property

3634. By Sam Spilsbury on 2013-03-11

Merge lp:compiz/raring

3633. By Sam Spilsbury on 2013-03-06

Unmerge lp:~sil2100/compiz/raring_revert_3616

3632. By Sam Spilsbury on 2013-03-06

Merge lp:~sil2100/compiz/raring_revert_3616

3631. By Sam Spilsbury on 2013-03-06

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)

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 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp	2013-03-06 09:15:38 +0000
+++ plugins/decor/src/decor.cpp	2013-03-11 06:47:17 +0000
@@ -1585,35 +1585,44 @@
      */
     if (decoration)
     {
-	wd = WindowDecoration::create (decoration);
-	if (!wd)
-	    return false;
-
 	/* Set extents based on maximize/unmaximize state
 	 * FIXME: With the new type system, this should be
 	 * removed */
 	if ((window->state () & MAXIMIZE_STATE))
-	    window->setWindowFrameExtents (&wd->decor->maxBorder,
-					   &wd->decor->maxInput);
+	    window->setWindowFrameExtents (&decoration->maxBorder,
+					   &decoration->maxInput);
 	else if (!window->hasUnmapReference ())
-	    window->setWindowFrameExtents (&wd->decor->border,
-					   &wd->decor->input);
-
-	movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
-	movement -= oldShift;
-
-	/* Update the input and output frame */
+	    window->setWindowFrameExtents (&decoration->border,
+					   &decoration->input);
+
+	/* We actually need to decorate this window */
 	if (decorate)
+	{
+	    wd = WindowDecoration::create (decoration);
+	    if (!wd)
+	    {
+		/* Error condition, reset frame extents */
+		CompWindowExtents emptyExtents;
+		memset (&emptyExtents, 0, sizeof (CompWindowExtents));
+		window->setWindowFrameExtents (&emptyExtents, &emptyExtents);
+		return false;
+	    }
+
+	    movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
+	    movement -= oldShift;
+
+	    /* Update the input and output frame */
 	    updateFrame ();
-	window->updateWindowOutputExtents ();
+	    window->updateWindowOutputExtents ();
 
-	updateReg = true;
-	updateMatrix = true;
-	mOutputRegion = CompRegion (window->outputRect ());
-	updateGroupShadows ();
-	if (dScreen->cmActive)
-	    cWindow->damageOutputExtents ();
-	updateDecorationScale ();
+	    updateReg = true;
+	    updateMatrix = true;
+	    mOutputRegion = CompRegion (window->outputRect ());
+	    updateGroupShadows ();
+	    if (dScreen->cmActive)
+		cWindow->damageOutputExtents ();
+	    updateDecorationScale ();
+	}
     }
     else
     {
@@ -2418,7 +2427,14 @@
 	    {
 		w = screen->findWindow (event->xclient.window);
 		if (w)
-		    DecorWindow::get (w)->update (true);
+		{
+		    DecorWindow *dw = DecorWindow::get (w);
+
+		    /* Set the frameExtentsRequested flag so that we know to
+		     * at least update _NET_WM_FRAME_EXTENTS (LP: #1110138) */
+		    dw->frameExtentsRequested = true;
+		    dw->update (true);
+		}
 	    }
 
 	    mCommunicator.handleClientMessage (event->xclient);

=== modified file 'src/privatewindow.h'
--- src/privatewindow.h	2013-02-10 05:01:50 +0000
+++ src/privatewindow.h	2013-03-11 06:47:17 +0000
@@ -155,7 +155,7 @@
 
 	void recalcNormalHints ();
 
-	void updateFrameWindow ();
+	bool updateFrameWindow ();
 
 	void setWindowMatrix ();
 
@@ -302,7 +302,7 @@
 						       int            gravity,
 						       int	      direction);
 
-	void updateSize ();
+	bool updateSize ();
 
 	bool getUserTime (Time& time);
 	void setUserTime (Time time);

=== modified file 'src/window.cpp'
--- src/window.cpp	2013-02-27 03:24:45 +0000
+++ src/window.cpp	2013-03-11 06:47:17 +0000
@@ -807,12 +807,11 @@
     priv->type = type;
 }
 
-
-void
+bool
 PrivateWindow::updateFrameWindow ()
 {
     if (!serverFrame)
-	return;
+	return false;
 
     XWindowChanges xwc = XWINDOWCHANGES_INIT;
     unsigned int   valueMask = CWX | CWY | CWWidth | CWHeight;
@@ -826,6 +825,8 @@
     window->configureXWindow (valueMask, &xwc);
     window->windowNotify (CompWindowNotifyFrameUpdate);
     window->recalcActions ();
+
+    return true;
 }
 
 
@@ -4109,11 +4110,11 @@
 	priv->placed = true;
 }
 
-void
+bool
 PrivateWindow::updateSize ()
 {
     if (window->overrideRedirect () || !managed)
-	return;
+	return false;
 
     XWindowChanges xwc = XWINDOWCHANGES_INIT;
 
@@ -4124,7 +4125,10 @@
 	    window->sendSyncRequest ();
 
 	window->configureXWindow (mask, &xwc);
+	return true;
     }
+
+    return false;
 }
 
 int
@@ -6780,8 +6784,10 @@
 
 	recalcActions ();
 
-	priv->updateSize ();
-	priv->updateFrameWindow ();
+	bool sizeUpdated = false;
+
+	sizeUpdated |= priv->updateSize ();
+	sizeUpdated |= priv->updateFrameWindow ();
 
 	/* Always send a moveNotify
 	 * whenever the frame extents update
@@ -6789,7 +6795,8 @@
 	moveNotify (0, 0, true);
 
 	/* Once we have updated everything, re-set lastServerInput */
-	priv->lastServerInput = priv->serverInput;
+	if (sizeUpdated)
+	    priv->lastServerInput = priv->serverInput;
     }
 
     /* Use b for _NET_WM_FRAME_EXTENTS here because
@@ -6909,6 +6916,9 @@
      * but that's all */
     XSelectInput (dpy, screen->root (), SubstructureNotifyMask);
 
+    /* Gravity here is assumed to be SouthEast, clients can update
+     * that if need be */
+
     /* Awaiting a new frame to be given to us */
     frame = None;
     serverFrame = XCreateWindow (dpy, screen->root (), 0, 0,
@@ -6918,10 +6928,17 @@
     /* Do not get any events from here on */
     XSelectInput (dpy, screen->root (), NoEventMask);
 
-    wrapper = XCreateWindow (dpy, serverFrame, 0, 0,
-			    wa.width, wa.height, 0, wa.depth,
+    /* If we have some frame extents, we should apply them here and
+     * set lastFrameExtents */
+    wrapper = XCreateWindow (dpy, serverFrame,
+			    serverInput.left, serverInput.top,
+			    wa.width - (serverInput.left + serverInput.right),
+			    wa.height - (serverInput.top + serverInput.bottom),
+			    0, wa.depth,
 			    InputOutput, visual, mask, &attr);
 
+    lastServerInput = serverInput;
+
     xwc.stack_mode = Above;
 
     /* Look for the client in the current server side stacking

=== 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-02-26 00:52:32 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp	2013-03-11 06:47:17 +0000
@@ -55,27 +55,41 @@
     return ct::AdvanceToNextEventOnSuccess (d, r);
 }
 
+Window GetImmediateParent (Display *display,
+			   Window  w,
+			   Window  &rootReturn)
+{
+    Window parentReturn = w;
+    Window *childrenReturn;
+    unsigned int nChildrenReturn;
+
+    XQueryTree (display,
+		w,
+		&rootReturn,
+		&parentReturn,
+		&childrenReturn,
+		&nChildrenReturn);
+    XFree (childrenReturn);
+
+    return parentReturn;
+}
+
+
 Window GetTopParent (Display *display,
 		     Window w)
 {
-    Window rootReturn;
+    Window rootReturn = 0;
     Window parentReturn = w;
-    Window *childrenReturn;
-    unsigned int nChildrenReturn;
-
     Window lastParent = 0;
 
     do
     {
 	lastParent = parentReturn;
 
-	XQueryTree (display,
-		    lastParent,
-		    &rootReturn,
-		    &parentReturn,
-		    &childrenReturn,
-		    &nChildrenReturn);
-	XFree (childrenReturn);
+	parentReturn = GetImmediateParent (display,
+					   lastParent,
+					   rootReturn);
+	
     } while (parentReturn != rootReturn);
 
     return lastParent;
@@ -565,3 +579,138 @@
 				   currentWidth + (left + right),
 				   currentHeight + (top + bottom)));
 }
+
+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsUnmapped)
+{
+    ::Display *dpy = Display ();
+
+    Window client = ct::CreateNormalWindow (dpy);
+    WaitForWindowCreation (client);
+
+    /* Set frame extents and get a response */
+    int left = 1;
+    int right = 1;
+    int top = 1;
+    int bottom = 1;
+
+    int currentX, currentY;
+    unsigned int currentWidth, currentHeight;
+    ASSERT_TRUE (QueryGeometry (dpy,
+				client,
+				currentX,
+				currentY,
+				currentWidth,
+				currentHeight));
+
+    /* We should get a response with our frame extents but it shouldn't actually
+     * do anything to the client as it is unmapped */
+    SendSetFrameExtentsRequest (client, left, right, top, bottom);
+    ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
+
+    ASSERT_TRUE (VerifyWindowSize (client,
+				   currentX,
+				   currentY,
+				   currentWidth,
+				   currentHeight));
+}
+
+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsCorrectMapBehaviour)
+{
+    ::Display *dpy = Display ();
+
+    Window client = ct::CreateNormalWindow (dpy);
+    WaitForWindowCreation (client);
+
+    /* Set frame extents and get a response */
+    int left = 1;
+    int right = 1;
+    int top = 1;
+    int bottom = 1;
+
+    int currentX, currentY;
+    unsigned int currentWidth, currentHeight;
+    ASSERT_TRUE (QueryGeometry (dpy,
+				client,
+				currentX,
+				currentY,
+				currentWidth,
+				currentHeight));
+
+    SendSetFrameExtentsRequest (client, left, right, top, bottom);
+    ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
+
+    /* Map the window */
+    XMapRaised (dpy, client);
+    WaitForReparentAndMap (dpy, client);
+
+    /* Check the geometry of the frame */
+    Window frame = GetTopParent (dpy, client);
+    ASSERT_TRUE (VerifyWindowSize (frame,
+				   currentX,
+				   currentY,
+				   currentWidth + (left + right),
+				   currentHeight + (top + bottom)));
+
+    /* Wrapper geometry is extents.xy, size.wh */
+    Window root;
+    Window wrapper = GetImmediateParent (dpy, client, root);
+    ASSERT_TRUE (VerifyWindowSize (wrapper,
+				   left,
+				   top,
+				   currentWidth,
+				   currentHeight));
+}
+
+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsConsistentBehaviourAfterMap)
+{
+    ::Display *dpy = Display ();
+
+    Window client = ct::CreateNormalWindow (dpy);
+    WaitForWindowCreation (client);
+
+    /* Set frame extents and get a response */
+    int left = 1;
+    int right = 1;
+    int top = 1;
+    int bottom = 1;
+
+    int currentX, currentY;
+    unsigned int currentWidth, currentHeight;
+    ASSERT_TRUE (QueryGeometry (dpy,
+				client,
+				currentX,
+				currentY,
+				currentWidth,
+				currentHeight));
+
+    SendSetFrameExtentsRequest (client, left, right, top, bottom);
+    ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
+
+    /* Map the window */
+    XMapRaised (dpy, client);
+    WaitForReparentAndMap (dpy, client);
+
+    /* Send it another frame extents request */
+    right = right + 1;
+    bottom = bottom + 1;
+
+    SendSetFrameExtentsRequest (client, left, right, top, bottom);
+    ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
+
+    /* Check the geometry of the frame */
+    Window frame = GetTopParent (dpy, client);
+    ASSERT_TRUE (VerifyWindowSize (frame,
+				   currentX,
+				   currentY,
+				   currentWidth + (left + right),
+				   currentHeight + (top + bottom)));
+
+    /* Wrapper geometry is extents.xy, size.wh */
+    Window root;
+    Window wrapper = GetImmediateParent (dpy, client, root);
+    ASSERT_TRUE (VerifyWindowSize (wrapper,
+				   left,
+				   top,
+				   currentWidth,
+				   currentHeight));
+}