RFR (XS): 6358357: Division by zero in Threads tab when shrinking jconsole window

[Erik Gahlin]

Thanks for the input

The graph will now disappear when the area for the plot becomes too 
small, which is probably the best way to handle a small window size.

Here is an updated review with Leif suggestion

http://cr.openjdk.java.net/~egahlin/6358357_2/

Erik

Leif Samuelsson skrev 2013-08-15 20:20:
> Hi,
>
> It's been so long since I worked on either Swing or JConsole that I 
> barely
> recognize the code anymore. :)
>
> I think the best practice here would be move the test w <= 0 || h <= 0 
> to to the
> top of the paintComponent() method. There is a potential big problem 
> with having
> the return in the middle, because the color in the GrahicsContext has 
> already
> been modified and should be restored before returning. If code is 
> later moved
> around in the method, then other things like font etc might be 
> affected too.
>
> I see other returns later on in the method. I'm not sure when they 
> crept in, but
> they should probably be dealt with at some point too, by adding the 
> restore lines
> or even by using a goto (gasp!).
>
> Anyway, for this fix, I suggest the following.
>
> 337     public void paintComponent(Graphics g) {
> 338         super.paintComponent(g);
> +
> +           int w = getWidth()-rightMargin-leftMargin-10;
> +           int h = getHeight()-topMargin-bottomMargin;
> +           if (w <= 0 || h <= 0) {
> +               // not enough room to paint anything
> +               return;
> +           }
> 339
> 340         Color oldColor = g.getColor();
> 341         Font  oldFont  = g.getFont();
> ---
> 383-         int w = getWidth()-rightMargin-leftMargin-10;
> 384-         if (w <= 0) {
> 385-             // width too small, abort to avoid division by zero
> 386-             return;
> 387-         }
> 388-         int h = getHeight()-topMargin-bottomMargin;
>
> Adding a guard to calculateTickInterval() as Mandy suggests would 
> probably be
> good, but not necessary as it's currently only called after w has been 
> tested.
>
> Leif
>
>
>
> On 2013-08-15 10:31, Mandy Chung wrote:
>>
>> On 8/15/2013 5:35 AM, Erik Gahlin wrote:
>>> Mandy Chung skrev 2013-08-12 20:49:
>>>> Should L358, 359, 388 need to be guarded to validate the result value?
>>>
>>> paintBorder and fillRect accepts negative and zero values so there 
>>> will not be an exception.
>>>
>>
>> What happens if the variable h evaluated at L358 is non-zero? If the 
>> Swing components handle invalid values, would you think fixing the 
>> calculateTickInterval method to handle invalid parameter (namely 
>> non-zero w value) would be appropriate?
>>
>> I'm including Leif who may have advice on how to handle resizing.  I 
>> think there would be some best practices that you can follow.
>>
>> Mandy
>>
>>> Erik
>>>
>>>> Mandy
>>>>
>>>> On 7/29/2013 1:02 PM, Erik Grahlin wrote:
>>>>> Please review a small change:
>>>>>
>>>>> Description:
>>>>> Switch to the Threads tab and try shrinking the window. When the 
>>>>> window
>>>>> width becomes smaller than some particular size, an exception is 
>>>>> printed
>>>>> to the terminal on every window update.
>>>>>
>>>>> Bug:
>>>>> http://monaco.us.oracle.com/detail.jsf?cr=6358357
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~egahlin/6358357/ 
>>>>> <http://cr.openjdk.java.net/%7Eegahlin/6358357/>
>>>>>
>>>>> Testing:
>>>>> Verified that no exceptions occur.
>>>>>
>>>>> A sponsor is needed.
>>>>>
>>>>> Thanks
>>>>> Erik
>>>
>>
>>