<Swing Dev> Review Request JDK-8152419 JColorChooser throws Exception

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Wed Jun 15 11:13:44 UTC 2016


The fix looks good to me.

Thanks,
Alexandr.

On 6/15/2016 12:25 PM, Rajeev Chamyal wrote:
>
> Looks fine to me.
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Prem Balakrishnan
> *Sent:* 15 June 2016 14:51
> *To:* Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov; 
> swing-dev at openjdk.java.net
> *Subject:* RE: <Swing Dev> Review Request JDK-8152419 JColorChooser 
> throws Exception
>
> Hi Rajeev,
>
> Thank you for the review.
>
> Updated patch as per review comment.
>
> http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.03/ 
> <http://cr.openjdk.java.net/%7Epkbalakr/8152419/webrev.03/>
>
> Regards,
> Prem
>
> *From:*Rajeev Chamyal
> *Sent:* Wednesday, June 15, 2016 2:38 PM
> *To:* Alexander Scherbatiy; Prem Balakrishnan; Sergey Bylokhov; 
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* RE: <Swing Dev> Review Request JDK-8152419 JColorChooser 
> throws Exception
>
> Hello Prem,
>
> testResult variable is accessed in 2 different threads. It should be 
> declared volatile.
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Alexandr Scherbatiy
> *Sent:* 10 June 2016 19:53
> *To:* Prem Balakrishnan; Sergey Bylokhov; swing-dev at openjdk.java.net 
> <mailto:swing-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> Review Request JDK-8152419 JColorChooser 
> throws Exception
>
>
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 6/10/2016 12:39 PM, Prem Balakrishnan wrote:
>
>     Hi Alexander,
>
>     Please review updated patch as per review comments.
>
>     http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.02/
>     <http://cr.openjdk.java.net/%7Epkbalakr/8152419/webrev.02/>
>
>     Regards,
>     Prem
>
>     *From:*Alexander Scherbatiy
>     *Sent:* Tuesday, May 31, 2016 4:04 PM
>     *To:* Prem Balakrishnan; Sergey Bylokhov;
>     swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>     *Subject:* Re: Review Request JDK-8152419 JColorChooser throws
>     Exception
>
>     On 31/05/16 14:03, Prem Balakrishnan wrote:
>
>         Hi Alexander,
>
>         Please review the updated patch.
>
>         http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.01/
>         <http://cr.openjdk.java.net/%7Epkbalakr/8152419/webrev.01/>
>
>
>     Math.max(getWidth() - this.insets.left - this.insets.right,
>     getWidth()) can give incorrect result for the case where a
>     component size is 50x50 and insets are [10, 10, 10 , 10].
>     max(50-10-10, 50) = 50 but the expected results is 30.
>
>     The correct formula should be max(width-insets.left-insets.right,
>     minWidthValue) where minWidthValue is zero or some specified
>     minimal value.
>
>     The DiagramComponent.paintComponent() code tries to create an
>     array of size width*height and BufferedImage with component size.
>     In this case it may be better just to check that the component
>     size minus insets is
>     greater than zero. If it is less or equal to zero we can just
>     return from the paintComponent() method.
>
>     Thanks,
>     Alexandr.
>
>     Regards,
>
>     Prem
>
>     *From:*Alexander Scherbatiy
>     *Sent:* Monday, May 30, 2016 9:42 PM
>     *To:* Prem Balakrishnan; Sergey Bylokhov;
>     swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>     *Subject:* Re: Review Request JDK-8152419 JColorChooser throws
>     Exception
>
>     On 30/05/16 12:39, Prem Balakrishnan wrote:
>
>         Hi*,*
>
>         Please review fix for JDK9,
>
>         *Bug:* https://bugs.openjdk.java.net/browse/JDK-8152419
>
>         *Webrev:*
>         http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.00/
>         <http://cr.openjdk.java.net/%7Epkbalakr/8152419/webrev.00/>
>
>         *Issue:*
>
>         JColorChooser throws Exception(NegativeArraySizeException)
>
>         *Fix:*
>
>         Absolute value is passed while creating array.
>
>        If component size is 10x10 and insets are [30, 30, 30, 30] the
>     absolute value of the difference will be  abs(10 - 30 - 30)=50.
>        It seems that the right component size should be zero or some
>     minimal values which is max(width-insets.left-insets.right,
>     minWidthValue).
>
>       Thanks,
>       Alexandr.
>
>     Regards,
>     Prem
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160615/96fba288/attachment.html>


More information about the swing-dev mailing list