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

[Anthony Petrov]

Thanks Omair. Now the problem is clear. And your last fix looks great. 
I've just filed

    6987945: XDecoratedPeer shouldn't allow to resize a frame to zero size

so that you could push your fix. Thanks!

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