<Swing Dev> [10] JDK-6919529: NPE from MultiUIDefaults.getUIError

[Shashidhara Veerabhadraiah]

Sergey, Here is the clarification for the "catch Error()" usage:

After we change the logic in the MultiUIDefaults.getUIError() method by adding protection for the tables, the "else" path would be executed rather than the "IF" path which was the earlier behavior. As can be seen from this logic, the else part would call the super getUIError() method and which throws the error to the user because the component UI is null.

Thanks and regards,
Shashi

-----Original Message-----
From: Shashidhara Veerabhadraiah 
Sent: Thursday, July 6, 2017 12:49 PM
To: Sergey Bylokhov <sergey.bylokhov at oracle.com>; Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
Cc: swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [10] JDK-6919529: NPE from MultiUIDefaults.getUIError

Hi,
Here is the Webrev link for the updates:
http://cr.openjdk.java.net/~pkbalakr/shashi/6919529/webrev_02/

Thanks and regards,
Shashi

-----Original Message-----
From: Shashidhara Veerabhadraiah [mailto:shashidhara.veerabhadraiah at oracle.com]
Sent: Thursday, July 6, 2017 12:48 PM
To: Sergey Bylokhov <sergey.bylokhov at oracle.com>; Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
Cc: 'swing-dev at openjdk.java.net' <swing-dev at openjdk.java.net>; Ajit Ghaisas <ajit.ghaisas at oracle.com>; Philip Race <philip.race at oracle.com>
Subject: RE: <Swing Dev> [10] JDK-6919529: NPE from MultiUIDefaults.getUIError

Hi Sergey, I have modified the test to move the swing GUI to EDT now and also with other comments as passed by other reviewers.

I found these nice links on EDT and a little bit of history of the event modeling in java:
http://www.javaworld.com/article/2077754/core-java/swing-threading-and-the-event-dispatch-thread.html
https://www.leepoint.net/JavaBasics/gui/gui-commentary/guicom-main-thread.html

Thanks and regards,
Shashi

-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, July 5, 2017 8:38 PM
To: Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
Cc: swing-dev at openjdk.java.net; Shashidhara Veerabhadraiah <shashidhara.veerabhadraiah at oracle.com>; Ajit Ghaisas <ajit.ghaisas at oracle.com>; Philip Race <philip.race at oracle.com>
Subject: Re: <Swing Dev> [10] JDK-6919529: NPE from MultiUIDefaults.getUIError

Hi,
Can somebody clarify when the "catch Error()" in the test is executed? Note that the test operates on Swing components on non-EDT.

----- prasanta.sadhukhan at oracle.com wrote:

> one thing. In the test, it is missing
> 
> @run main <testname>
> 
> so add that when you check in.
> 
> Regards
> Prasanta
> On 7/5/2017 2:35 PM, Prasanta Sadhukhan wrote:
> > +1
> >
> > Regards
> > Prasanta
> > On 7/5/2017 1:44 PM, Ajit Ghaisas wrote:
> >> +1
> >>
> >> There is an additional space on line 28 in test.
> >> Please correct it before checking in. You do not need a new webrev
> 
> >> just for that.
> >>
> >> Regards,
> >> Ajit
> >>
> >> -----Original Message-----
> >> From: Shashidhara Veerabhadraiah
> >> Sent: Wednesday, July 05, 2017 11:04 AM
> >> To: Ajit Ghaisas; swing-dev at openjdk.java.net; Philip Race; Sergey 
> >> Bylokhov
> >> Subject: RE: <Swing Dev> [10] JDK-6919529: NPE from 
> >> MultiUIDefaults.getUIError
> >>
> >> Hi, Here is the updated Webrev with fixes and a new test to test
> out
> >> the fix:
> >>
> >> http://cr.openjdk.java.net/~pkbalakr/shashi/6919529/webrev_01/
> >>
> >> Thanks and regards,
> >> Shashi
> >>
> >> -----Original Message-----
> >> From: Ajit Ghaisas
> >> Sent: Tuesday, June 27, 2017 3:22 PM
> >> To: Shashidhara Veerabhadraiah
> >> <shashidhara.veerabhadraiah at oracle.com>;
> swing-dev at openjdk.java.net;
> >> Philip Race <philip.race at oracle.com>; Sergey Bylokhov 
> >> <sergey.bylokhov at oracle.com>
> >> Subject: RE: <Swing Dev> [10] JDK-6919529: NPE from 
> >> MultiUIDefaults.getUIError
> >>
> >> 1. You are not checking whether 'tables' is null.
> >>
> >> I think the condition check should be in following order :
> >> if (tables != null && tables.length > 0 && tables[0] != null)
> >>
> >> 2.  Can you please add a regression test for this fix?
> >>
> >> Regards,
> >> Ajit
> >>
> >> From: Shashidhara Veerabhadraiah
> >> Sent: Tuesday, June 27, 2017 2:56 PM
> >> To: swing-dev at openjdk.java.net; Philip Race; Sergey Bylokhov
> >> Subject: <Swing Dev> [10] JDK-6919529: NPE from 
> >> MultiUIDefaults.getUIError
> >>
> >> Hi All,
> >>  Please review the fix for the JDK-6919529 where there is a
> NPE
> >> at MultiUIDefaults.
> >>
> >> Issue: NPE at MultiUIDefaults overridden method of
> getUIError(String)
> >> which does not initializes the tables[0] object. There are 2 ways
> to
> >> initialize the tables[0] object either via the default constructor
> or
> >> parametric constructor. Since the method that is being overridden
> has
> >> the signature of getUIError(String), there is no way it can accept
> 
> >> the UIDefault instance as an input but the code assumes that the 
> >> default 'UIDefaults' instance be the one that is local to the 
> >> MultiUIDefaults class and assumes that the object is fully 
> >> constructed and hence defaults to the tables[] usage as the default
> 
> >> behavior to get the UI error.
> >>
> >> Resolution: The default assumption that the class instance is fully
> 
> >> constructed is checked for it's completeness before it's usage. All
> 
> >> the other methods including overridden methods checks the tables[]
> is
> >> null or not whereas only the getUIError() methods does not. This is
> 
> >> fixed in this fix.
> >>
> >> Test outputs: No NPE at the end of this fix.
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-6919529
> >> Webrev:
> http://cr.openjdk.java.net/~pkbalakr/shashi/6919529/webrev_00/
> >>
> >> Thanks and regards,
> >> Shashi
> >