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

[Leif Samuelsson]

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