Re: Patch: Implement Dialog modality

[Thomas Fitzsimmons <fitzsim at redhat dot com>]

On Wed, 2003-10-08 at 16:02, Tom Tromey wrote:
> >>>>> "Tom" == Thomas Fitzsimmons <fitzsim@redhat.com> writes:
> 
> Tom> This patch implements Dialog modality and fixes the following problems
> Tom> with windows:
> 
> Looks great to me.  Just a couple questions.
> 
> Tom> +    insets = new Insets (latestInsets.top,
> Tom> +			 latestInsets.left,
> Tom> +			 latestInsets.bottom,
> Tom> +			 latestInsets.right);
> 
> There's no synchronization around these uses of latestInsets.  That's
> fine if the code is robust in the face of race conditions, which I
> think it is.  It could use a comment about this somewhere, though.
> 

Yeah, I was considering adding synchronization here;  I don't think it
would ever happen, but not synchronizing could theoretically produce
wrong inset values.  I've added synchronization and checked in the
patch.

> Tom>    public void show()
> Tom> [ ... ]
> Tom> +    // Show visible owned windows.
> Tom> +    synchronized (ownedWindows)
> Tom> +      {
> 
> I wonder if this code should synchronize on the tree lock instead.
> (It's fine to check it in as-is and then go back and change it later,
> as we're already using ownedWindows as a lock elsewhere.  If it does
> need to be changed then we can change it everywhere at once.)
> 

I'll look into this tomorrow.

Thanks for the comments,
Tom