Re: Review Request JDK-8152419 JColorChooser throws Exception

Wed, 15 Jun 2016 04:14:54 -0700 

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@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@openjdk.java.net <mailto:swing-dev@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@openjdk.java.net <mailto:swing-dev@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@openjdk.java.net <mailto:swing-dev@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@openjdk.java.net <mailto:swing-dev@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

Reply via email to