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