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.bylok...@oracle.com>; Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com> Cc: swing-dev@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.veerabhadra...@oracle.com] Sent: Thursday, July 6, 2017 12:48 PM To: Sergey Bylokhov <sergey.bylok...@oracle.com>; Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com> Cc: 'swing-dev@openjdk.java.net' <swing-dev@openjdk.java.net>; Ajit Ghaisas <ajit.ghai...@oracle.com>; Philip Race <philip.r...@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.sadhuk...@oracle.com> Cc: swing-dev@openjdk.java.net; Shashidhara Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>; Ajit Ghaisas <ajit.ghai...@oracle.com>; Philip Race <philip.r...@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.sadhuk...@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@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.veerabhadra...@oracle.com>; > swing-dev@openjdk.java.net; > >> Philip Race <philip.r...@oracle.com>; Sergey Bylokhov > >> <sergey.bylok...@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@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 > >
