<Swing Dev> Bug Report (with fix): NPE in CompoundBorder.getInsets()

[Jon VanAlten]

Whoops, replied to sender instead of list.


----- "jon vanalten" <jon.vanalten at redhat.com> wrote:

> Hello Alex,
> 
> Thanks for the response.
> 
> ----- "Alexander Potochkin" <Alexander.Potochkin at sun.com> wrote:
> 
> > Hello Jon
> >
> > in the description of this bug
> > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=477
> >
> > we can read that
> > "It functions correctly when using the JDK from Sun."
> >
> > Do you have an idea why it happens?
> >
> 
> Yes, and I have confirmed that the the bug does not happen using
> proprietary JDK (tested with b20).  Upon closer inspection, it is
> com.sun.java.swing.plaf.gtk.GTKPainter.ListTableFocusBorder that
> returns the null insets in this case.  If the
> getBorderInsets(Component) version of the method is called, it passes
> null Insets reference to the getBorderInsets(Component, Insets)
> method.  In this method, unless the component is a JLabel subclass
> which also implements SynthUI, this null insets reference will never
> be assigned to anything before it is returned.
> 
> In other examples I found in the jdk where either of these methods are
> implemented, the single argument version of the method creates an
> Insets object if it needs to delegate to the two argument version. 
> The only reason I can think of for not doing so in this case is to
> save some cycles on the object creation, which does not seem worth a
> possible uncaught NPE.  Many of the two argument methods in other
> Border objects assume that they receive a non-null Insets object, and
> would throw NPEs themselves if called with null.
> 
> With this in mind, to me it makes the most sense to change the
> behavior of this ListTableFocusBorder object so that it does not have
> a possibility of returning null.  The simplest way to do so would be
> to take out the overrided single argument method, letting it default
> back to the AbstractBorder method which creates an Insets(0, 0, 0, 0)
> when delegating to the two-argument method.  There don't seem to be
> any other more appropriate values for default insets of this class. 
> This takes care of the NPE, and as far as I can tell the appearance of
> the program that reported the bug is the same as in proprietary JDK. 
> This simple webrev shows the change:
> 
> http://icedtea.classpath.org/~vanaltj/webrevs/swing/bordernpe/webrev2/
> 
> Thoughts?
> 
> 
> cheers,
> 
> jon
> 
> 
> > I checked that we don't have the null check in that method
> > either in the Sun JDK
> >
> > Thanks
> > alexp
> >
> > > Hi,
> > >
> > > Downstream in IcedTea there was a report of this NPE:
> > >
> > > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=477
> > >
> > > The problem being that, while it checks that each of insideBorder
> > and outsideBorder are non-null before trying to call their
> > getBorderInsets() methods, there is no check that a non-null Insets
> > object is returned before attempting to access its members.  See CR
> > for simple fix:
> > >
> > >
> >
> http://icedtea.classpath.org/~vanaltj/webrevs/swing/bordernpe/webrev/
> > >
> > > Comments?
> > >
> > > Thanks!
> > >
> > > jon