Merge lp:~compiz-team/compiz-core/compiz-core.server_windows into lp:compiz-core/0.9.5

[Sam Spilsbury]

Adds a serverWindows and serverNext + serverPrev members. Update serverWindows whenever we send stacking requests to the X Server. This should fix race conditions where the server is slow to send us stacking updates and we have messed up the internal stacking order.

[Sam Spilsbury (smspillaz)]

This is dead code

Unmerged revisions

2798. By Sam Spilsbury on 2011-08-29

Add window list of windows last sent to server and update this list whenever
a call to XLowerWindow, XRaiseWindow or XConfigureWindow is made.

This breaks the API, and plugins that wish to call a->restackAbove (b);
and then read the window list should probably read screen->serverWindows () instead.

Do not use CompScreen::serverInsertWindow and CompScreen::serverUnhookWindow, these
will be removed.

Should fix some stacking race conditions (but not all of them)

2797. By Sam Spilsbury on 2011-08-23

Merge lp:~compiz-team/compiz-core/compiz-core.fix_831987

=== modified file 'include/core/core.h'
--- include/core/core.h	2011-07-04 07:35:02 +0000
+++ include/core/core.h	2011-08-29 13:21:25 +0000
@@ -27,7 +27,7 @@
 #define _COMPIZ_CORE_H
 
 
-#define CORE_ABIVERSION 20110703
+#define CORE_ABIVERSION 20110828
 
 #include <stdio.h>
 #include <assert.h>

=== modified file 'include/core/screen.h'
--- include/core/screen.h	2011-06-25 02:31:02 +0000
+++ include/core/screen.h	2011-08-29 13:21:25 +0000
@@ -250,6 +250,8 @@
 
 	CompWindowList & windows ();
 
+	CompWindowList & serverWindows ();
+
 	void warpPointer (int dx, int dy);
 
 	Time getCurrentTime ();
@@ -261,8 +263,9 @@
 	void focusDefaultWindow ();
 
 	void insertWindow (CompWindow *w, Window aboveId);
-
+	void insertServerWindow (CompWindow *w, Window aboveId);
 	void unhookWindow (CompWindow *w);
+	void unhookServerWindow (CompWindow *w);
 
 	Cursor normalCursor ();
 

=== modified file 'include/core/window.h'
--- include/core/window.h	2011-04-30 14:07:20 +0000
+++ include/core/window.h	2011-08-29 13:21:25 +0000
@@ -272,7 +272,7 @@
      * A mutable object about the dimensions and location of a CompWindow.
      */
 	class Geometry : public CompRect
-    {
+	{
 	    public:
 		Geometry ();
 		Geometry (int, int, int, int, int);
@@ -293,6 +293,9 @@
 	CompWindow *next;
 	CompWindow *prev;
 
+	CompWindow *serverNext;
+	CompWindow *serverPrev;
+
     public:
 	~CompWindow ();
 

=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2011-08-19 14:25:11 +0000
+++ src/privatescreen.h	2011-08-29 13:21:25 +0000
@@ -362,6 +362,11 @@
 	CompScreen  *screen;
 
 	std::list <CoreWindow *> createdWindows;
+
+	/* Window list last sent to server */
+	CompWindowList serverWindows;
+
+	/* Window list last recv from server */
 	CompWindowList windows;
 	CompWindow::Map windowsMap;
 

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2011-08-23 06:16:21 +0000
+++ src/screen.cpp	2011-08-29 13:21:25 +0000
@@ -2560,6 +2560,56 @@
 }
 
 void
+CompScreen::insertServerWindow (CompWindow *w, Window	aboveId)
+{
+    w->serverPrev = NULL;
+    w->serverNext = NULL;
+
+    if (!aboveId || priv->serverWindows.empty ())
+    {
+	if (!priv->serverWindows.empty ())
+	{
+	    priv->serverWindows.front ()->prev = w;
+	    w->serverNext = priv->serverWindows.front ();
+	}
+	priv->serverWindows.push_front (w);
+
+	return;
+    }
+
+    CompWindowList::iterator it = priv->serverWindows.begin ();
+
+    while (it != priv->serverWindows.end ())
+    {
+	if ((*it)->id () == aboveId ||
+	    ((*it)->frame () && (*it)->frame () == aboveId))
+	{
+	    break;
+	}
+	it++;
+    }
+
+    if (it == priv->serverWindows.end ())
+    {
+#ifdef DEBUG
+	abort ();
+#endif
+	return;
+    }
+
+    w->serverNext = (*it)->serverNext;
+    w->serverPrev = (*it);
+    (*it)->serverNext = w;
+
+    if (w->serverNext)
+    {
+	w->serverNext->serverPrev = w;
+    }
+
+    priv->serverWindows.insert (++it, w);
+}
+
+void
 PrivateScreen::eraseWindowFromMap (Window id)
 {
     if (id != 1)
@@ -2588,6 +2638,27 @@
 	lastFoundWindow = NULL;
 }
 
+void
+CompScreen::unhookServerWindow (CompWindow *w)
+{
+    CompWindowList::iterator it =
+	std::find (priv->serverWindows.begin (), priv->serverWindows.end (), w);
+
+    priv->serverWindows.erase (it);
+
+    if (w->serverNext)
+	w->serverNext->serverPrev = w->serverPrev;
+
+    if (w->serverPrev)
+	w->serverPrev->serverNext = w->serverNext;
+
+    w->serverNext = NULL;
+    w->serverPrev = NULL;
+
+    if (w == lastFoundWindow)
+	lastFoundWindow = NULL;
+}
+
 Cursor
 CompScreen::normalCursor ()
 {

=== modified file 'src/window.cpp'
--- src/window.cpp	2011-08-23 13:15:20 +0000
+++ src/window.cpp	2011-08-29 13:21:25 +0000
@@ -2493,37 +2493,27 @@
 	xwc->y = 0;
     }
 
-    XConfigureWindow (screen->dpy (), id, valueMask, xwc);
-
-    /* Compiz's window list is immediately restacked on reconfigureXWindow
-       in order to ensure correct operation of the raise, lower and restacking
-       functions. This function should only recieve stack_mode == Above
-       but warn incase something else does get through, to make the cause
-       of any potential misbehaviour obvious. */
+    /* Update the server side window list on raise, lower and restack functions.
+     * This function should only recieve stack_mode == Above
+     * but warn incase something else does get through, to make the cause
+     * of any potential misbehaviour obvious. */
     if (valueMask & (CWSibling | CWStackMode))
     {
 	if (xwc->stack_mode == Above)
 	{
-	    /* FIXME: We shouldn't need to sync here, however
-	     * considering the fact that we are immediately
-	     * restacking the windows, we need to ensure
-	     * that when a client tries to restack a window
-	     * relative to this window that the window
-	     * actually goes where the client expects it to go
-	     * and not anywhere else
-	     *
-	     * The real solution here is to have a list of windows
-	     * last sent to server and a list of windows last
-	     * received from server or to have a sync () function
-	     * on the stack which looks through the last recieved
-	     * window stack and the current window stack then
-	     * sends the changes */
-	    XSync (screen->dpy (), false);
-	    restack (xwc->sibling);
+	    if (xwc->sibling)
+	    {
+		CompWindow *w = screen->findWindow (id);
+
+		screen->unhookServerWindow (w);
+		screen->insertServerWindow (w, xwc->sibling);
+	    }
 	}
 	else
 	    compLogMessage ("core", CompLogLevelWarn, "restack_mode not Above");
     }
+
+    XConfigureWindow (screen->dpy (), id, valueMask, xwc);
 }
 
 bool
@@ -3193,26 +3183,9 @@
 		    XLowerWindow (screen->dpy (), frame);
 		XLowerWindow (screen->dpy (), id);
 
-		/* Restacking of compiz's window list happens
-		 * immediately and since this path doesn't call
-		 * reconfigureXWindow, restack must be called here.
-		 *
-		 * FIXME: We shouldn't need to sync here, however
-		 * considering the fact that we are immediately
-		 * restacking the windows, we need to ensure
-		 * that when a client tries to restack a window
-		 * relative to this window that the window
-		 * actually goes where the client expects it to go
-		 * and not anywhere else
-		 *
-		 * The real solution here is to have a list of windows
-		 * last sent to server and a list of windows last
-		 * received from server or to have a sync () function
-		 * on the stack which looks through the last recieved
-		 * window stack and the current window stack then
-		 * sends the changes */
-		XSync (screen->dpy (), false);
-		restack (0);
+		/* Update the list of windows last sent to the server */
+		screen->unhookServerWindow (window);
+		screen->insertServerWindow (window, 0);
 	    }
 	    else if (sibling->priv->id != window->prev->priv->id)
 	    {
@@ -3258,9 +3231,9 @@
 PrivateScreen::focusTopMostWindow ()
 {
     CompWindow  *focus = NULL;
-    CompWindowList::reverse_iterator it = windows.rbegin ();
+    CompWindowList::reverse_iterator it = serverWindows.rbegin ();
 
-    for (; it != windows.rend (); it++)
+    for (; it != serverWindows.rend (); it++)
     {
 	CompWindow *w = *it;
 
@@ -3301,10 +3274,7 @@
        the click-to-focus option is on */
     if ((screen->priv->optionGetClickToFocus ()))
     {
-	Window aboveWindowId = prev ? prev->id () : None;
-	screen->unhookWindow (this);
 	CompWindow *focusedWindow = screen->priv->focusTopMostWindow ();
-	screen->insertWindow (this , aboveWindowId);
 
 	/* if the newly focused window is a desktop window,
 	   give the focus back to w */
@@ -3491,22 +3461,6 @@
 	   it was created */
 	if (mask & CWStackMode)
 	{
-	    /* FIXME: We shouldn't need to sync here, however
-	     * considering the fact that we are immediately
-	     * restacking the windows, we need to ensure
-	     * that when a client tries to restack a window
-	     * relative to this window that the window
-	     * actually goes where the client expects it to go
-	     * and not anywhere else
-	     *
-	     * The real solution here is to have a list of windows
-	     * last sent to server and a list of windows last
-	     * received from server or to have a sync () function
-	     * on the stack which looks through the last recieved
-	     * window stack and the current window stack then
-	     * sends the changes */
-	    XSync (screen->dpy (), false);
-
 	    Window above = (mask & CWSibling) ? xwc.sibling : 0;
 	    priv->restack (above);
 	}
@@ -5314,6 +5268,7 @@
     priv->window = this;
 
     screen->insertWindow (this, aboveId);
+    screen->insertServerWindow (this, aboveId);
 
     priv->attrib = wa;
     priv->serverGeometry.set (priv->attrib.x, priv->attrib.y,
@@ -5498,6 +5453,7 @@
 CompWindow::~CompWindow ()
 {
     screen->unhookWindow (this);
+    screen->unhookServerWindow (this);
 
     if (!priv->destroyed)
     {