This is the mail archive of the mailing list for the Java project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch: Window and Dialog fixes

On Thu, 2003-09-04 at 14:02, Tom Tromey wrote:
> >>>>> "Tom" == Thomas Fitzsimmons <> writes:
> Tom> Please review and comment.
> This is pretty good.  I think there are a few problems though.
> Tom> 	* java/awt/ Move owned windows logic	to ...
> Tom> 	* java/awt/ (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

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

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,

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]