Fix bug 1951abf33d: Aqua sometimes segfaults when destroying a transient.

2 files changed, 99 insertions, 104 deletions

diff --git a/macosx/tkMacOSXWm.c b/macosx/tkMacOSXWm.c
index 84d4709..69c72f4 100644
--- a/macosx/tkMacOSXWm.c
+++ b/macosx/tkMacOSXWm.c
@@ -669,7 +669,7 @@ TkWmNewWindow(
     wmPtr->reparent = None;
     wmPtr->titleUid = NULL;
     wmPtr->iconName = NULL;
-    wmPtr->master = None;
+    wmPtr->master = NULL;
     wmPtr->hints.flags = InputHint | StateHint;
     wmPtr->hints.input = True;
     wmPtr->hints.initial_state = NormalState;
@@ -679,7 +679,6 @@ TkWmNewWindow(
     wmPtr->hints.icon_mask = None;
     wmPtr->hints.window_group = None;
     wmPtr->leaderName = NULL;
-    wmPtr->masterWindowName = NULL;
     wmPtr->icon = NULL;
     wmPtr->iconFor = NULL;
     wmPtr->transientPtr = NULL;
@@ -884,18 +883,16 @@ TkWmDeadWindow(
     TkWindow *winPtr)		/* Top-level window that's being deleted. */
 {
     WmInfo *wmPtr = winPtr->wmInfoPtr, *wmPtr2;
-    TkDisplay *dispPtr = TkGetDisplayList();
 
     if (wmPtr == NULL) {
 	return;
     }
-    
-    /* 
+
+    /*
      *If the dead window is a transient, remove it from the master's list.
      */
 
     RemoveTransient(winPtr);
-
     Tk_ManageGeometry((Tk_Window) winPtr, NULL, NULL);
     Tk_DeleteEventHandler((Tk_Window) winPtr, StructureNotifyMask,
 	    TopLevelEventProc, winPtr);
@@ -911,9 +908,6 @@ TkWmDeadWindow(
     if (wmPtr->leaderName != NULL) {
 	ckfree(wmPtr->leaderName);
     }
-    if (wmPtr->masterWindowName != NULL) {
-	ckfree(wmPtr->masterWindowName);
-    }
     if (wmPtr->icon != NULL) {
 	wmPtr2 = ((TkWindow *) wmPtr->icon)->wmInfoPtr;
 	wmPtr2->iconFor = NULL;
@@ -942,17 +936,14 @@ TkWmDeadWindow(
      * If the dead window has a transient, remove references to it from
      * the transient.
      */
-    
+
     for (Transient *transientPtr = wmPtr->transientPtr;
     	 transientPtr != NULL; transientPtr = transientPtr->nextPtr) {
     	TkWindow *winPtr2 = transientPtr->winPtr;
-    	Window master = TkGetTransientMaster(winPtr2);
-    	TkWindow *masterPtr = (TkWindow *)Tk_IdToWindow(dispPtr->display, master); 
+    	TkWindow *masterPtr = (TkWindow *)TkGetTransientMaster(winPtr2);
     	if (masterPtr == winPtr) {
     	    wmPtr2 = winPtr2->wmInfoPtr;
-    	    wmPtr2->master = None;
-    	    ckfree(wmPtr2->masterWindowName);
-    	    wmPtr2->masterWindowName = NULL;
+    	    wmPtr2->master = NULL;
     	}
     }
 
@@ -1791,7 +1782,6 @@ WmDeiconifyCmd(
 {
     register WmInfo *wmPtr = winPtr->wmInfoPtr;
     NSWindow *win = TkMacOSXDrawableWindow(winPtr->window);
-    TkDisplay *dispPtr = TkGetDisplayList();
 
     if (objc != 3) {
 	Tcl_WrongNumArgs(interp, 2, objv, "window");
@@ -1830,17 +1820,16 @@ WmDeiconifyCmd(
 	 transientPtr != NULL; transientPtr = transientPtr->nextPtr) {
 	TkWindow *winPtr2 = transientPtr->winPtr;
 	WmInfo *wmPtr2 = winPtr2->wmInfoPtr;
-	Window master = TkGetTransientMaster(winPtr2);
-	TkWindow *masterPtr = (TkWindow *)Tk_IdToWindow(dispPtr->display, master); 
+	TkWindow *masterPtr = (TkWindow *)TkGetTransientMaster(winPtr2);
     	if (masterPtr == winPtr) {
-	    if (!(wmPtr2->hints.initial_state == WithdrawnState &&
-		  (transientPtr->flags & WITHDRAWN_BY_MASTER) == 0)) {
+	    if ((wmPtr2->hints.initial_state == WithdrawnState &&
+		 (transientPtr->flags & WITHDRAWN_BY_MASTER) != 0)) {
 		TkpWmSetState(winPtr2, NormalState);
 		transientPtr->flags &= ~WITHDRAWN_BY_MASTER;
 	    }
 	}
     }
-    
+
     return TCL_OK;
 }
 
@@ -2327,7 +2316,6 @@ WmIconifyCmd(
     int objc,			/* Number of arguments. */
     Tcl_Obj *const objv[])	/* Argument objects. */
 {
-    TkDisplay *dispPtr = TkGetDisplayList();
     register WmInfo *wmPtr = winPtr->wmInfoPtr;
     if (objc != 3) {
 	Tcl_WrongNumArgs(interp, 2, objv, "window");
@@ -2341,7 +2329,7 @@ WmIconifyCmd(
 	Tcl_SetErrorCode(interp, "TK", "WM", "ICONIFY", "OVERRIDE_REDIRECT",
 		NULL);
 	return TCL_ERROR;
-    } else if (wmPtr->master != None) {
+    } else if (wmPtr->master != NULL) {
 	Tcl_SetObjResult(interp, Tcl_ObjPrintf(
 		"can't iconify \"%s\": it is a transient", winPtr->pathName));
 	Tcl_SetErrorCode(interp, "TK", "WM", "ICONIFY", "TRANSIENT", NULL);
@@ -2373,8 +2361,7 @@ WmIconifyCmd(
     for (Transient *transientPtr = wmPtr->transientPtr;
 	 transientPtr != NULL; transientPtr = transientPtr->nextPtr) {
 	TkWindow *winPtr2 = transientPtr->winPtr;
-	Window master = TkGetTransientMaster(winPtr2);
-	TkWindow *masterPtr = (TkWindow *)Tk_IdToWindow(dispPtr->display, master); 
+	TkWindow *masterPtr = (TkWindow *)TkGetTransientMaster(winPtr2);
     	if (masterPtr == winPtr &&
 	    winPtr2->wmInfoPtr->hints.initial_state != WithdrawnState) {
 	    TkpWmSetState(winPtr2, WithdrawnState);
@@ -3466,7 +3453,7 @@ WmStateCmd(
 			"OVERRIDE_REDIRECT", NULL);
 		return TCL_ERROR;
 	    }
-	    if (wmPtr->master != None) {
+	    if (wmPtr->master != NULL) {
 		Tcl_SetObjResult(interp, Tcl_ObjPrintf(
 			"can't iconify \"%s\": it is a transient",
 			winPtr->pathName));
@@ -3582,17 +3569,16 @@ WmTransientCmd(
     register WmInfo *wmPtr = winPtr->wmInfoPtr;
     Tk_Window master;
     WmInfo *wmPtr2;
-    char *masterWindowName;
-    int length;
+    Transient *transient;
 
     if ((objc != 3) && (objc != 4)) {
 	Tcl_WrongNumArgs(interp, 2, objv, "window ?master?");
 	return TCL_ERROR;
     }
     if (objc == 3) {
-	if (wmPtr->master != None) {
+	if (wmPtr->master != NULL) {
 	    Tcl_SetObjResult(interp,
-		    Tcl_NewStringObj(wmPtr->masterWindowName, -1));
+		Tcl_NewStringObj(Tk_PathName(wmPtr->master), -1));
 	}
 	return TCL_OK;
     }
@@ -3640,15 +3626,20 @@ WmTransientCmd(
 	}
 
 	/*
-	 * Add the transient to the master's list.
+	 * Add the transient to the master's list, if it not already there.
 	 */
-
-	Transient *transient = ckalloc(sizeof(Transient));
-	transient->winPtr = winPtr;
-	transient->flags = 0;
-	transient->nextPtr = wmPtr->transientPtr;
-	wmPtr2->transientPtr = transient;
 	
+	for (transient = wmPtr2->transientPtr;
+	     transient != NULL && transient->winPtr != winPtr;
+	     transient = transient->nextPtr) {}
+	if (transient == NULL) {
+	    transient = ckalloc(sizeof(Transient));
+	    transient->winPtr = winPtr;
+	    transient->flags = 0;
+	    transient->nextPtr = wmPtr2->transientPtr;
+	    wmPtr2->transientPtr = transient;
+	}
+
 	/*
 	 * If the master is withdrawn or iconic then withdraw the transient.
 	 */
@@ -3660,14 +3651,7 @@ WmTransientCmd(
 	    transient->flags |= WITHDRAWN_BY_MASTER;
 	}
 
-	wmPtr->master = Tk_WindowId(masterPtr);
-	masterWindowName = masterPtr->pathName;
-	length = strlen(masterWindowName);
-	if (wmPtr->masterWindowName != NULL) {
-	    ckfree(wmPtr->masterWindowName);
-	}
-	wmPtr->masterWindowName = ckalloc(length+1);
-	strcpy(wmPtr->masterWindowName, masterWindowName);
+	wmPtr->master = (Tk_Window)masterPtr;
     }
     ApplyMasterOverrideChanges(winPtr, NULL);
     return TCL_OK;
@@ -3697,33 +3681,35 @@ RemoveTransient(
     TkWindow *winPtr)
 {
     WmInfo *wmPtr = winPtr->wmInfoPtr, *wmPtr2;
-    TkDisplay *dispPtr = TkGetDisplayList();
     TkWindow *masterPtr;
-    if (wmPtr == NULL || wmPtr->master == None) {
+    Transient *T, *temp;
+    
+    if (wmPtr == NULL || wmPtr->master == NULL) {
 	return;
     }
-    masterPtr = (TkWindow*)Tk_IdToWindow(dispPtr->display, wmPtr->master);
+    masterPtr = (TkWindow*)wmPtr->master;
     wmPtr2 = masterPtr->wmInfoPtr;
     if (wmPtr2 == NULL) {
 	return;
     }
-    wmPtr->master = None;
-    if (wmPtr->masterWindowName != NULL) {
-	ckfree(wmPtr->masterWindowName);
-    }
-    wmPtr->masterWindowName = NULL;
-    Transient *temp, *cursor = wmPtr2->transientPtr;
-    if (cursor->winPtr == winPtr) {
-	temp = cursor->nextPtr;
-	ckfree(cursor);
-	cursor = temp;
-	masterPtr->wmInfoPtr->transientPtr = cursor;
+    wmPtr->master = NULL;
+    T = wmPtr2->transientPtr;
+    while (T != NULL) {
+	if (T->winPtr != winPtr) {
+	    break;
+	}
+	temp = T->nextPtr;
+	ckfree(T);
+	T = temp;
     }
-    while (cursor != NULL) {
-	if (cursor->winPtr == winPtr) {
-	    temp = cursor->nextPtr;
-	    ckfree(cursor);
-	    cursor = temp;
+    wmPtr2->transientPtr = T;
+    while (T != NULL) {
+	if (T->nextPtr && T->nextPtr->winPtr == winPtr) {
+	    temp = T->nextPtr;
+	    T->nextPtr = temp->nextPtr;
+	    ckfree(temp);
+	} else {
+	    T = T->nextPtr;
 	}
     }
 }
@@ -3769,9 +3755,8 @@ WmWithdrawCmd(
     }
 
     TkpWmSetState(winPtr, WithdrawnState);
-    
+
     NSWindow *win = TkMacOSXDrawableWindow(winPtr->window);
-    TkDisplay *dispPtr = TkGetDisplayList();
     [win orderOut:nil];
     [win setExcludedFromWindowsMenu:YES];
 
@@ -3781,8 +3766,7 @@ WmWithdrawCmd(
     for (Transient *transientPtr = wmPtr->transientPtr;
 	 transientPtr != NULL; transientPtr = transientPtr->nextPtr) {
 	TkWindow *winPtr2 = transientPtr->winPtr;
-	Window master = TkGetTransientMaster(winPtr2);
-	TkWindow *masterPtr = (TkWindow *)Tk_IdToWindow(dispPtr->display, master); 
+	TkWindow *masterPtr = (TkWindow *)TkGetTransientMaster(winPtr2);
     	if (masterPtr == winPtr &&
 	    winPtr2->wmInfoPtr->hints.initial_state != WithdrawnState) {
 	    TkpWmSetState(winPtr2, WithdrawnState);
@@ -5335,14 +5319,14 @@ TkSetWMName(
  *----------------------------------------------------------------------
  */
 
-Window
+Tk_Window
 TkGetTransientMaster(
     TkWindow *winPtr)
 {
     if (winPtr->wmInfoPtr != NULL) {
-	return winPtr->wmInfoPtr->master;
+	return (Tk_Window)winPtr->wmInfoPtr->master;
     }
-    return None;
+    return NULL;
 }
 
 /*
@@ -6579,7 +6563,7 @@ TkMacOSXApplyWindowAttributes(
 {
     WmInfo *wmPtr = winPtr->wmInfoPtr;
     ApplyWindowAttributeFlagChanges(winPtr, macWindow, 0, 0, 0, 1);
-    if (wmPtr->master != None || winPtr->atts.override_redirect) {
+    if (wmPtr->master != NULL || winPtr->atts.override_redirect) {
 	ApplyMasterOverrideChanges(winPtr, macWindow);
     }
 }
@@ -6716,7 +6700,7 @@ ApplyWindowAttributeFlagChanges(
 		 */
 
 		if ((winPtr->atts.override_redirect) ||
-		    (wmPtr->master != None) ||
+		    (wmPtr->master != NULL) ||
 		    (winPtr->wmInfoPtr->macClass == kHelpWindowClass)) {
 		    b |= (NSWindowCollectionBehaviorCanJoinAllSpaces |
 			  NSWindowCollectionBehaviorFullScreenAuxiliary);
@@ -6793,6 +6777,7 @@ ApplyMasterOverrideChanges(
     int oldFlags = wmPtr->flags;
     unsigned long styleMask;
     NSRect structureRect;
+    NSWindow *parentWindow;
 
     if (!macWindow && winPtr->window != None &&
 	    TkMacOSXHostToplevelExists(winPtr)) {
@@ -6833,7 +6818,6 @@ ApplyMasterOverrideChanges(
 	}
     }
     if (macWindow) {
-	NSWindow *parentWindow = [macWindow parentWindow];
 	structureRect = [NSWindow frameRectForContentRect:NSZeroRect
 				  styleMask:styleMask];
 
@@ -6852,7 +6836,7 @@ ApplyMasterOverrideChanges(
 	    if (wmPtr->hints.initial_state == NormalState) {
 		[macWindow orderFront:nil];
 	    }
-	    if (wmPtr->master != None) {
+	    if (wmPtr->master != NULL) {
 		wmPtr->flags |= WM_TOPMOST;
 	    } else {
 		wmPtr->flags &= ~WM_TOPMOST;
@@ -6868,40 +6852,55 @@ ApplyMasterOverrideChanges(
 	    wmPtr->flags &= ~WM_TOPMOST;
 	}
 	if (wmPtr->master != None) {
-	    TkDisplay *dispPtr = TkGetDisplayList();
-	    TkWindow *masterWinPtr = (TkWindow *)
-		    Tk_IdToWindow(dispPtr->display, wmPtr->master);
+	    TkWindow *masterWinPtr = (TkWindow *)wmPtr->master;
 	    if (masterWinPtr && masterWinPtr->window != None &&
 		    TkMacOSXHostToplevelExists(masterWinPtr)) {
-		NSWindow *masterMacWin =
-			TkMacOSXDrawableWindow(masterWinPtr->window);
+		NSWindow *masterMacWin = TkMacOSXDrawableWindow(
+					     masterWinPtr->window);
 
-		if (masterMacWin && masterMacWin != parentWindow &&
-			(winPtr->flags & TK_MAPPED)) {
-		    if (parentWindow) {
+		/*
+		 * Try to add the transient window as a child window of the
+		 * master. A child NSWindow retains its relative position with
+		 * respect to the parent when the parent is moved.  This is
+		 * pointless if the parent is offscreen, and adding a child to
+		 * an offscreen window causes the parent to be displayed as a
+		 * zombie.  So we only do this if the parent is visible.
+		 */
+
+		if (masterMacWin &&
+		    [masterMacWin isVisible] &&
+		    (winPtr->flags & TK_MAPPED)) {
+
+		    /*
+		     * If the transient is already a child of some other window,
+		     * remove it.
+		     */
+
+		    parentWindow = [macWindow parentWindow];
+		    if (parentWindow && parentWindow != masterMacWin) {
 			[parentWindow removeChildWindow:macWindow];
 		    }
-		    
+
 		    /*
-		     * A child NSWindow retains its relative position with
-		     * respect to the parent when the parent is moved.  This is
-		     * pointless if the parent is offscreen, and adding a child
-		     * to an offscreen window causes the parent to be displayed
-		     * as a zombie.  So we should only do this if the parent is
-		     * visible.
+		     * To avoid cycles, if the master is a child of some
+		       other window, remove it.
 		     */
-
-		    if ([masterMacWin isVisible]) {
-			[masterMacWin addChildWindow:macWindow
-					     ordered:NSWindowAbove];
+		    parentWindow = [masterMacWin parentWindow];
+		    if (parentWindow) {
+			[parentWindow removeChildWindow:masterMacWin];
 		    }
-		    if (wmPtr->flags & WM_TOPMOST) {
-			[macWindow setLevel:kCGUtilityWindowLevel];
+		    [masterMacWin addChildWindow:macWindow
+					     ordered:NSWindowAbove];
 		    }
-		}
 	    }
-	} else if (parentWindow) {
-	    [parentWindow removeChildWindow:macWindow];
+	} else {
+	    parentWindow = [macWindow parentWindow];
+	    if (parentWindow) {
+		[parentWindow removeChildWindow:macWindow];
+	    }
+	}
+	if (wmPtr->flags & WM_TOPMOST) {
+	    [macWindow setLevel:kCGUtilityWindowLevel];
 	}
 	ApplyWindowAttributeFlagChanges(winPtr, macWindow, oldAttributes,
 		oldFlags, 0, 0);
diff --git a/macosx/tkMacOSXWm.h b/macosx/tkMacOSXWm.h
index a7ea92c..5f07fe6 100644
--- a/macosx/tkMacOSXWm.h
+++ b/macosx/tkMacOSXWm.h
@@ -65,7 +65,7 @@ typedef struct TkWmInfo {
     Tk_Uid titleUid;		/* Title to display in window caption. If NULL,
 				 * use name of widget. */
     char *iconName;		/* Name to display in icon. */
-    Window master;		/* Master window for TRANSIENT_FOR property, or
+    Tk_Window master;		/* Master window for TRANSIENT_FOR property, or
 				 * None. */
     XWMHints hints;		/* Various pieces of information for window
 				 * manager. */
@@ -73,10 +73,6 @@ typedef struct TkWmInfo {
 				 * (corresponds to hints.window_group).
 				 * Malloc-ed. Note: this field doesn't get
 				 * updated if leader is destroyed. */
-    char *masterWindowName;	/* Path name of window specified as master in
-				 * "wm transient" command, or NULL. Malloc-ed.
-				 * Note: this field doesn't get updated if
-				 * masterWindowName is destroyed. */
     Tk_Window icon;		/* Window to use as icon for this window, or
 				 * NULL. */
     Tk_Window iconFor;		/* Window for which this window is icon, or