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

[Artem Ananiev]

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
check_if_event_predicate().

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
'isPacked'.

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
XBaseWindow?

Will take a look at XDecoratedPeer and XWM shortly.

Thanks,

Artem

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