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

[Omair Majid]

Hi Anthony,

On 09/28/2010 10:00 AM, Anthony Petrov wrote:
> Thanks Omair. Now the problem is clear. And your last fix looks great.
> I've just filed

Thanks for reviewing the patch and pointing out the problems.

>
> 6987945: XDecoratedPeer shouldn't allow to resize a frame to zero size
>
> so that you could push your fix. Thanks!
>

Thanks for taking care of the bug id. I have pushed the changeset as 
6994facc6a8b.

Cheers,
Omair

> --
> best regards,
> Anthony
>
>
> On 09/27/2010 08:36 PM, Omair Majid wrote:
>> 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
>>