<Swing Dev> [9] Review fix for JDK-8020039 : SynthTableHeaderUI refers to possibly null parameter in cell renderer

[Sergey Bylokhov]

Hi, Hello Ajit.
I am not sure that exclusion of the code is a correct fix here. For 
example can you clarify should we call setText(or other setXXX) when the 
table is null or not? Another question is: should we skip the code in 
SynthTableHeaderUI.java or we can assume table==null as enabled table?

On 17.02.16 14:29, Rajeev Chamyal wrote:
> Looks good to me.
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Ajit Ghaisas
> *Sent:* 17 February 2016 16:51
> *To:* Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy;
> swing-dev at openjdk.java.net
> *Subject:* RE: <Swing Dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hi,
>
>      I have corrected formatting of test code and removed the additional
> System.out.printlns.
>
>      Please review :
> http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.02/
>
> Regards,
>
> Ajit
>
> *From:*Ajit Ghaisas
> *Sent:* Tuesday, February 16, 2016 3:12 PM
> *To:* Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy;
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> <Swing-dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hi,
>
>      Thanks Rajeev for review comments.
>
>       I have checked – windows LAF WindowsTableHeaderUI handles it
> correctly.
>
>       I have added null check for MacOs Aqua.
>
>       Also, I have added a test checking for this null pointer access.
>
>      Please review --
> http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.01/
>
> Regards,
>
> Ajit
>
> *From:*Rajeev Chamyal
> *Sent:* Monday, February 15, 2016 5:39 PM
> *To:* Ajit Ghaisas; Sergey Bylokhov; Alexander Scherbatiy;
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* RE: <Swing-dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hello Ajit,
>
> Can you please if similar fix is required for other LAF windows ,Aqua etc.
>
> Please add a regression test case also.
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Ajit Ghaisas
> *Sent:* 15 February 2016 17:30
> *To:* Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy;
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* <Swing-dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hi,
>
> Please review the fix for jdk9,
>
>                  Bug: https://bugs.openjdk.java.net/browse/JDK-8020039
>
> Webrev: http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.00/
>
> Issue :
>
>      If null is passed as ‘table’ parameter to
> SynthTableHeaderUI::getTableCellRendererComponent() method in
>   src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTableHeaderUI.java,
>
>      there is Null Pointer Exception.
>
> Analysis :
>
>     This method already has a null check for ‘table’ parameter for
> second access of this parameter in method.
>
>     Whereas the first access of the ‘table’ parameter lacks this check.
>
> Fix :
>
>     Added null check for the first access of ‘table’ parameter in
> SynthTableHeaderUI::getTableCellRendererComponent().
>
>     There is no else block added as the flow continues and passes table
> to base class method using super. getTableCellRendererComponent().
>
>     The passed parameter is already checked in base class method
> correctly. Hence, no change is needed in base class.
>
> Test :
>
>     The fix is pretty straight forward.
>
>     Executed the code snippet given in the bug description. There is no
> NPE after the fix.
>
> Regards,
>
> Ajit
>


-- 
Best regards, Sergey.