Re: Patch: Window and Dialog fixes

[Thomas Fitzsimmons <fitzsim at redhat dot com>]

On Thu, 2003-09-04 at 14:02, Tom Tromey wrote:
> >>>>> "Tom" == Thomas Fitzsimmons <fitzsim@redhat.com> writes:
> 
> Tom> Please review and comment.
> 
> This is pretty good.  I think there are a few problems though.
> 
> Tom> 	* java/awt/Frame.java: Move owned windows logic	to ...
> Tom> 	* java/awt/Window.java (ownedWindows, owner, weakThis):	New
> Tom> 	fields.
> 
> I'm curious about this.  In the current Frame code, ownedWindows is
> marked with @serial.  If this field is specified by serialization,
> then we either need to keep it in Frame or add readObject/writeObject
> methods.

Yup, you're right.  ownedWindows has to stay in Frame.

> 
> Likewise, fields added to a serializable class have to be marked
> transient if they aren't mentioned in the serialization spec.
> 
> There's a "serialized form" link from each page of Sun's javadoc.
> 

> I'd prefer a different implementation of the owned windows code.  It
> would be better if we could get rid of Window.finalize().  Also we
> shouldn't need the weakThis field, I think.
> 

I took a hint from Sun's javadocs for this.  From the Frame
documentation:

protected  void finalize()
          We have to remove the (hard) reference to weakThis in the
Vector, otherwise the WeakReference instance will never get garbage
collected.

There's a similar comment in Window's documentation.  I guess I don't
have to follow their implementation though.

> I think Window.hide() will fail during the window between when the
> `weakThis' is cleared and when the actual window is finalized -- this
> isn't an atomic operation.  In this situation, I'd expect a
> NullPointerException.  

Hmm, I don't understand.  By cleared you mean removed from the
ownedWindows vector?

> Also, it seems like the number of owned windows
> could change during the loops in dispose(), hide(), getOwnedWindows(),
> etc, as there is no synchronization.
> 

Yes, good point.  I'll declare these methods synchronized.

> 
> +  /* Apparently a jboolean can have a value greater than 1.  gboolean
> +     variables may only contain the value TRUE or FALSE. */
> 
> Hmm, when can this happen?

I think I must have been using the wrong compiler or something when I
saw this.  When the jboolean argument was true in the java code, it was
being passed to the JNI code with a value of 112.  I was seeing some
other weirdness at that time so I guess I'll attribute it to a problem
with my build environment.

Thanks for the feedback,
Tom