<AWT Dev> Review request: 6689983 (reevaluate our inset-related code in XAWT)

Artem Ananiev Artem.Ananiev at Sun.COM
Tue Jun 30 08:03:11 PDT 2009

Hi, Anthony,

here are some random comments from my side:

1. Should XlibWrapper.CheckIfEvent be XCheckIfEvent?

2. XlibWrapper.CheckIfEvent: I might be wrong, but it seems 'event'
parameter can be eliminated. It's enough to make use of 'event' param in

3. I'd vote for removing 'isPacked' field from Component class if it's
possible. I'm not an expert in serialization, though, to help you with
this :)

4. Window.reshape(): is there any help from having the tree lock here?
Access to 'width' and 'height' is not synchronized, the same is true for

5. XWindow.getRootWindow(): the code is pretty ugly, although I
understand it saves another call to Xlib comparing to what we have now
(XRootWindow + XGetWindowAttributes). Is it worth moving this method to

Will take a look at XDecoratedPeer and XWM shortly.



Anthony Petrov wrote:
> Hello.
> The insets-related code in the XToolkit has been a nightmare to maintain 
> for quite a long time. This fix is a try to rewrite the code making it 
> more understandable and maintainable.
> Testing: all more-or-less related automatic regression tests have been 
> run. Categories include but not limited to: top-level tests, layouts 
> tests, embedded frame tests, mouse events tests, focus tests, menu/popup 
> menu tests, and some other. Nearly 60 tests were found failing with a 
> clean build, so I filed the corresponding CRs. The rest pass with this 
> fix on:
> linux-i586: Gnome/Metacity 2.24, Gnome/Compiz 2.24/0.7.8, KDE/KWin 
> 4.1.3/3.0
> solaris-sparc: Gnome/Metacity 2.8, CDE/DTWM Solaris 10
> Please review the code at: 
> http://cr.openjdk.java.net/~anthony/7-16-insets-6689983.0/
> Suggestions are welcome. Thanks!
> -- 
> best regards,
> Anthony

More information about the awt-dev mailing list