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

[Shashidhara Veerabhadraiah]

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