Cygwin/X: Re-order window creation processjturney-race-fix

Be a bit more careful to do things in the right order and set all
the style flags before we show the window.

This is probably the right thing to do in any case as it means we can avoid
the messy appearance of the window changing style just after it is first shown

In this implementation, this is achieved by having WM_CREATE send a WM_WM_CREATE
message to our window manager thread, which then does all the work of discovering
the windows style; it's convenient to do that there as accessing X internals is
awkward in the wndproc; possibly it's more than convenient as there might be some
deadlock issue that thread avoids...

But note that this subtly changes the semantics of winCreateWindowsWindow():
previously the window was visible and drawn before that function returned, now
that happens asychronously; I'm not sure if that could cause problems or not...

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>

4 files changed, 96 insertions, 86 deletions

diff --git a/hw/xwin/winmultiwindowwindow.c b/hw/xwin/winmultiwindowwindow.c
index ee561e274..b8dd3ddc8 100644
--- a/hw/xwin/winmultiwindowwindow.c
+++ b/hw/xwin/winmultiwindowwindow.c
@@ -658,12 +658,6 @@ winUpdateWindowsWindow (WindowPtr pWin)
 	  winCreateWindowsWindow (pWin);
 	  assert (pWinPriv->hWnd != NULL);
 	}
-
-      /* Display the window without activating it */
-      ShowWindow (pWinPriv->hWnd, SW_SHOWNOACTIVATE);
-
-      /* Send first paint message */
-      UpdateWindow (pWinPriv->hWnd);
     }
   else if (hWnd != NULL)
     {
diff --git a/hw/xwin/winmultiwindowwm.c b/hw/xwin/winmultiwindowwm.c
index 9c90543fb..80a34ca69 100644
--- a/hw/xwin/winmultiwindowwm.c
+++ b/hw/xwin/winmultiwindowwm.c
@@ -682,6 +682,35 @@ winMultiWindowWMProc (void *pArg)
       /* Branch on the message type */
       switch (pNode->msg.msg)
 	{
+	case WM_WM_CREATE:
+#if CYGMULTIWINDOW_DEBUG
+	  ErrorF ("\tWM_WM_CREATE\n");
+#endif
+	  /* Put a note as to the HWND associated with this Window */
+	  XChangeProperty (pWMInfo->pDisplay,
+			   pNode->msg.iWindow,
+			   pWMInfo->atmPrivMap,
+			   XA_INTEGER,
+			   32,
+			   PropModeReplace,
+			   (unsigned char *) &(pNode->msg.hwndWindow),
+			   1);
+
+	  /* Determine the Window style, which determines borders and clipping region... */
+	  {
+	    HWND zstyle = HWND_NOTOPMOST;
+	    winApplyHints (pWMInfo->pDisplay, pNode->msg.iWindow, pNode->msg.hwndWindow, &zstyle);
+	    winUpdateWindowPosition (pNode->msg.hwndWindow, TRUE, &zstyle);
+	  }
+
+	  /* Display the window without activating it */
+	  ShowWindow (pNode->msg.hwndWindow, SW_SHOWNOACTIVATE);
+
+	  /* Send first paint message */
+	  UpdateWindow (pNode->msg.hwndWindow);
+
+	  break;
+
 #if 0
 	case WM_WM_MOVE:
 	  ErrorF ("\tWM_WM_MOVE\n");
@@ -716,15 +745,6 @@ winMultiWindowWMProc (void *pArg)
 #if CYGMULTIWINDOW_DEBUG
 	  ErrorF ("\tWM_WM_MAP\n");
 #endif
-	  /* Put a note as to the HWND associated with this Window */
-	  XChangeProperty (pWMInfo->pDisplay,
-			   pNode->msg.iWindow,
-			   pWMInfo->atmPrivMap,
-			   XA_INTEGER,//pWMInfo->atmPrivMap,
-			   32,
-			   PropModeReplace,
-			   (unsigned char *) &(pNode->msg.hwndWindow),
-			   1);
 	  UpdateName (pWMInfo, pNode->msg.iWindow);
 	  UpdateIcon (pWMInfo, pNode->msg.iWindow);
 	  break;
@@ -733,36 +753,7 @@ winMultiWindowWMProc (void *pArg)
 #if CYGMULTIWINDOW_DEBUG
 	  ErrorF ("\tWM_WM_MAP2\n");
 #endif
-	  XChangeProperty (pWMInfo->pDisplay,
-			   pNode->msg.iWindow,
-			   pWMInfo->atmPrivMap,
-			   XA_INTEGER,//pWMInfo->atmPrivMap,
-			   32,
-			   PropModeReplace,
-			   (unsigned char *) &(pNode->msg.hwndWindow),
-			   1);
-	  break;
 
-	case WM_WM_MAP3:
-#if CYGMULTIWINDOW_DEBUG
-	  ErrorF ("\tWM_WM_MAP3\n");
-#endif
-	  /* Put a note as to the HWND associated with this Window */
-	  XChangeProperty (pWMInfo->pDisplay,
-			   pNode->msg.iWindow,
-			   pWMInfo->atmPrivMap,
-			   XA_INTEGER,//pWMInfo->atmPrivMap,
-			   32,
-			   PropModeReplace,
-			   (unsigned char *) &(pNode->msg.hwndWindow),
-			   1);
-	  UpdateName (pWMInfo, pNode->msg.iWindow);
-	  UpdateIcon (pWMInfo, pNode->msg.iWindow);
-	  {
-	    HWND zstyle = HWND_NOTOPMOST;
-	    winApplyHints (pWMInfo->pDisplay, pNode->msg.iWindow, pNode->msg.hwndWindow, &zstyle);
-	    winUpdateWindowPosition (pNode->msg.hwndWindow, TRUE, &zstyle);
-	  }
 	  break;
 
 	case WM_WM_UNMAP:
@@ -1737,22 +1728,51 @@ winApplyHints (Display *pDisplay, Window iWindow, HWND hWnd, HWND *zstyle)
   else if (style & STYLE_NOFRAME)
 	hint = (hint & ~HINT_BORDER & ~HINT_CAPTION & ~HINT_SIZEBOX) | HINT_NOFRAME;
 
-  /* Now apply styles to window */
-  style = GetWindowLongPtr(hWnd, GWL_STYLE) & ~WS_CAPTION & ~WS_SIZEBOX; /* Just in case */
-  if (!style) return;
+  XWindowAttributes wa;
+  wa.override_redirect = FALSE;
+  XGetWindowAttributes(pDisplay, iWindow, &wa);
+
+  if (!wa.override_redirect)
+    {
+      /*
+	Moved from WM_SHOWWINDOW now we are a bit more careful to do things in the right
+	order and set all the style flags before we show the window ...
+	but what exactly are we trying to do here?
+      */
+      if (GetParent(hWnd))
+	/* Set the transient style flags */
+	SetWindowLongPtr (hWnd, GWL_STYLE,
+			  WS_POPUP | WS_OVERLAPPED | WS_SYSMENU | WS_CLIPCHILDREN | WS_CLIPSIBLINGS);
+      else
+	/* Set the window standard style flags */
+	SetWindowLongPtr (hWnd, GWL_STYLE,
+			  (WS_POPUP | WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN | WS_CLIPSIBLINGS)
+			  & ~WS_CAPTION & ~WS_SIZEBOX);
+
+      /* Now apply styles to window */
+      style = GetWindowLongPtr(hWnd, GWL_STYLE) & ~WS_CAPTION & ~WS_SIZEBOX; /* Just in case */
+      if (!style) return;
+
+      if (!hint) /* All on */
+        style = style | WS_CAPTION | WS_SIZEBOX;
+      else if (hint & HINT_NOFRAME) /* All off */
+        style = style & ~WS_CAPTION & ~WS_SIZEBOX;
+      else
+        style = style | ((hint & HINT_BORDER) ? WS_BORDER : 0) |
+          ((hint & HINT_SIZEBOX) ? WS_SIZEBOX : 0) |
+          ((hint & HINT_CAPTION) ? WS_CAPTION : 0);
 
-  if (!hint) /* All on */
-    style = style | WS_CAPTION | WS_SIZEBOX;
-  else if (hint & HINT_NOFRAME) /* All off */
-    style = style & ~WS_CAPTION & ~WS_SIZEBOX;
-  else style = style | ((hint & HINT_BORDER) ? WS_BORDER : 0) |
-		((hint & HINT_SIZEBOX) ? WS_SIZEBOX : 0) |
-		((hint & HINT_CAPTION) ? WS_CAPTION : 0);
+      if (hint & HINT_NOMAXIMIZE)
+        style = style & ~WS_MAXIMIZEBOX;
 
-  if (hint & HINT_NOMAXIMIZE)
-    style = style & ~WS_MAXIMIZEBOX;
+      SetWindowLongPtr (hWnd, GWL_STYLE, style);
 
-  SetWindowLongPtr (hWnd, GWL_STYLE, style);
+      winDebug("winApplyHints: iWindow %d hints %d %d\n", iWindow, hint, maxmin);
+    }
+  else
+    {
+      winDebug("winApplyHints: iWindow %d no hints as override-redirect\n", iWindow);
+    }
 }
 
 void
diff --git a/hw/xwin/winmultiwindowwndproc.c b/hw/xwin/winmultiwindowwndproc.c
index d14ceee9c..cb9b66327 100644
--- a/hw/xwin/winmultiwindowwndproc.c
+++ b/hw/xwin/winmultiwindowwndproc.c
@@ -334,7 +334,23 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
 #if CYGDEBUG
   winDebugWin32Message("winTopLevelWindowProc", hwnd, message, wParam, lParam);
 #endif
-  
+
+  /*
+     If this is WM_CREATE, set up the Windows window properties which point to X window information,
+     before we populate other local variables...
+  */
+  if (message == WM_CREATE)
+    {
+      /* */
+      SetProp (hwnd,
+	       WIN_WINDOW_PROP,
+	       (HANDLE)((LPCREATESTRUCT) lParam)->lpCreateParams);
+      /* */
+      SetProp (hwnd,
+	       WIN_WID_PROP,
+	       (HANDLE)winGetWindowID (((LPCREATESTRUCT) lParam)->lpCreateParams));
+    }
+
   /* Check if the Windows window property for our X window pointer is valid */
   if ((pWin = GetProp (hwnd, WIN_WINDOW_PROP)) != NULL)
     {
@@ -397,17 +413,6 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
   switch (message)
     {
     case WM_CREATE:
-
-      /* */
-      SetProp (hwnd,
-	       WIN_WINDOW_PROP,
-	       (HANDLE)((LPCREATESTRUCT) lParam)->lpCreateParams);
-      
-      /* */
-      SetProp (hwnd,
-	       WIN_WID_PROP,
-	       (HANDLE)winGetWindowID (((LPCREATESTRUCT) lParam)->lpCreateParams));
-
       /*
        * Make X windows' Z orders sync with Windows windows because
        * there can be AlwaysOnTop windows overlapped on the window
@@ -427,6 +432,11 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
 
       SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR)XMING_SIGNATURE);
 
+      /* Tell our Window Manager thread to style the window */
+      wmMsg.msg = WM_WM_CREATE;
+      if (fWMMsgInitialized)
+	winSendMessageToWM (s_pScreenPriv->pWMInfo, &wmMsg);
+
       return 0;
 
     case WM_INIT_SYS_MENU:
@@ -877,25 +887,11 @@ winTopLevelWindowProc (HWND hwnd, UINT message,
 	  /* Flag that this window needs to be made active when clicked */
 	  SetProp (hwnd, WIN_NEEDMANAGE_PROP, (HANDLE) 1);
 
-	  if (!(GetWindowLongPtr (hwnd, GWL_EXSTYLE) & WS_EX_APPWINDOW))
-	    {
 	      HWND		zstyle = HWND_NOTOPMOST;
-
-	      /* Set the window extended style flags */
-	      SetWindowLongPtr (hwnd, GWL_EXSTYLE, WS_EX_APPWINDOW);
-
-	      /* Set the transient style flags */
-	      if (GetParent(hwnd)) SetWindowLongPtr (hwnd, GWL_STYLE,
-		   WS_POPUP | WS_OVERLAPPED | WS_SYSMENU | WS_CLIPCHILDREN | WS_CLIPSIBLINGS);
-	      /* Set the window standard style flags */
-	      else SetWindowLongPtr (hwnd, GWL_STYLE,
-		   (WS_POPUP | WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN | WS_CLIPSIBLINGS)
-		   & ~WS_CAPTION & ~WS_SIZEBOX);
-
 	      winUpdateWindowPosition (hwnd, FALSE, &zstyle);
 	      SetForegroundWindow (hwnd);
-	    }
-	  wmMsg.msg = WM_WM_MAP3;
+
+	  wmMsg.msg = WM_WM_MAP;
 	}
       else /* It is an overridden window so make it top of Z stack */
 	{
diff --git a/hw/xwin/winwindow.h b/hw/xwin/winwindow.h
index 2f731bc78..3c96773fb 100644
--- a/hw/xwin/winwindow.h
+++ b/hw/xwin/winwindow.h
@@ -117,7 +117,7 @@ typedef struct _winWMMessageRec{
 #define		WM_WM_HINTS_EVENT	(WM_USER + 10)
 #define		WM_WM_CHANGE_STATE	(WM_USER + 11)
 #define		WM_WM_MAP2		(WM_USER + 12)
-#define		WM_WM_MAP3		(WM_USER + 13)
+#define		WM_WM_CREATE		(WM_USER + 13)
 #define		WM_MANAGE		(WM_USER + 100)
 #define		WM_UNMANAGE		(WM_USER + 102)