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

[Anthony Petrov]

Hi Oleg,

Thank you very much for reviewing that!

On 06/21/2009 08:11 AM, Oleg Sukhodolsky wrote:
> src/share/classes/java/awt/Component.java
> 
> I suspect that you are disallowed to remove non-transient field from
> serializable component :( this is backward incompatible change)
That makes me very sad... I moved it back to the Component class.

> src/share/classes/sun/awt/image/VolatileSurfaceManager.java
> 
> are these changes related to insets-code?
Not at all. I've just filed 6853984 to track this issue.

> src/solaris/classes/sun/awt/X11/XlibUtil.java
> 
> since new restriction for getWindowGeometry(long window, long
> rootWindowBuffer) is rather synthetic, I'd suggest to
> add appropriate parameter check to it (someone have to enforce
> "unnatural" restrictions ;)
Good suggestion!

> src/solaris/classes/sun/awt/X11/XContentWindow.java
> 
>> @@ -128,11 +128,10 @@
>>                handleResize(newBounds);
>>              }
>>          } finally {
>>              XToolkit.awtUnlock();
>>          }
>> -        validateSurface();
>>      }
> 
> why we do not need validateSurface() here?  Is this related to insets changes?
The reshape() method that is invoked just before the handleResize() 
already calls the validateSurface(). I don't see why we need to call it 
twice.

> src/solaris/classes/sun/awt/X11/XWindow.java
> 
> why we need SubstructureNotifyMask?  I see that because of it event
> processing becomes a little bit more
> error prone, so it would be better to understand why we need this.
Well, as the comment above the XWindow.handleReparentNotifyEvent() 
states, this is needed to guarantee receiving the ReparentNotify events 
just after mapping the window. Currently commenting out the 
SubstructureNotifyMask seems to not make things generally worse, and I 
don't actually remember how exactly I reproduced the issue.
Guess I need to remove it because theoretically it should all work w/o 
this event mask. I'm just a bit scared to re-run all the tests after 
that change though...

> src/solaris/classes/sun/awt/X11/XWM.java
> 
> why waitForExtentsUpdateEvent() has to be so complicated?  Why we can
> not user just one attempt with bigger delay?
Because we can receive the desired event much earlier than the "full" 
delay. Making five tries reduces the unneeded CPU time wasting.

--
best regards,
Anthony