<AWT Dev> Request for review: x11 - ensure non zero heights and widths

[Omair Majid]

Hi Anthony,

On 09/24/2010 04:27 PM, Anthony Petrov wrote:
> Hi Omair,
>
> On 9/24/2010 6:12 PM, Omair Majid wrote:
>> While trying out a Java program, I noticed that some BadValue X errors
>> were being reported (I had to set -Dsun.awt.noisyerrorhandler=True
>> first). The webrev at
>> http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.00/
>>
>> attempts to fix the issue.

Updated webrev at 
http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.01/

>
> Please make sure you put the true-branches of the if statements into {}
> blocks.
>

I noticed that XBaseWindow is already doing a check like this using the 
constant MIN_SIZE and the math library. So I switched to that instead. I 
will keep this style guideline in mind for any future patches.

> Do I understand correctly that we're fixing an issue with top-level
> windows only? If so, then I suggest to not touch the code in the
> XBasWindow, but rather tweak the bounds in the XWindowPeer where we add
> them to the create params map. (and perhaps in the XDecoratedPeer - need
> to check if their logic of initializing the params is separate or not.)
>

Yes, I can only create this issue with top-level windows. The issue does 
not happen on initialization (checkParams in XBaseWindow takes care of 
that). It only happens on resizing after initialization.

checkShellRectSize() in XDecoratedPeer is supposed to correct the size, 
but it misses out the corner case where widths and heights are exactly 
zero. XWindowPeer (and other X peers) call xSetBounds which already 
takes care of the zero case.

For the sake of consistency, I have made sure that all the places which 
check for the widht or height <= 0 case use Math.max(MIN_SIZE, width) 
just like XBaseWindow.checkParams().

>
>> The problem is that there are some cases where the jdk is creating or
>> resizing windows and setting widths and/or heights to 0. The man page
>
> What are the exact cases?
>

The attached program, when run with -Dsun.awt.noisyerrorhandler=True 
prints out a BadValue error caused by X_ConfigureWindow. Before 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6678385, this would 
have caused the process to crash. Currently the resize in the test 
program does not do anything; with the fix applied it would set it to 
the smallest size possible.

Thanks,
Omair

> --
> best regards,
> Anthony
>
>> for XResizeWindow states "If either width or height is zero, a BadValue
>> error results". I looked at the source code for xorg-xserver and it
>> appears that specifying a height or width of 0 for pretty much any X
>> call will cause a BadValue error.
>>
>> The patch takes an approach similar to what is currently being done in
>> XBaseWindow.xSetBounds(). It checks heights and widths, setting them to
>> 1 if needed.
>>
>> Any comments?
>>
>> Cheers,
>> Omair

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: X11ZeroValuesFrameTest.java
Url: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20100927/4ddde0ce/X11ZeroValuesFrameTest.java 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openjdk7-awt-x11-windowsize-badvalue.patch
Type: text/x-patch
Size: 1527 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20100927/4ddde0ce/openjdk7-awt-x11-windowsize-badvalue.patch