<Swing Dev> Webrev for NPE at BasicTreeUI$Actions.page:4470

[Jaroslav Tulach]

Hello again.

Dne Pá 16. listopadu 2012 18:00:45, Alexander Scherbatiy napsal(a):
> >> The ui.getPathBounds(tree, newPath) method definitely can return
> >> null so it needs to have this check.
> > 
> > Should I start working on a webrev, so (in case you don't find any
> > violation of good Swing practices in TreeView) we can eliminate this NPE
> > once and forever?
>      Yes, please.

Here is my proposed fix:
http://xelfi.cz/webrev/NB222081/

>      The fix should check the NPE and may be do some possible default
> actions in this case.

Done. As this is just about navigation and selection, I think doing nothing is 
a reasonable default.

>      It is a good idea to add an automated test but it can be possible
> if there are known steps which reproduce the issue.

I created an automated test, but it is just artificial one - it returns null 
for no reason and only checks the caller is ready for such situation. That is 
the best I was able to come up.

>      At least you can run the test/javax/swing/JTree  automated test to
> check possible regressions.

As I wrote yesterday. The results before my fix were:
> Test results: passed: 3; failed: 2
and after my fix 
> Test results: passed: 4; failed: 2
- e.g. I am almost sure that my patch causes no regressions.

Btw. I included artificial bug number 9999999 in the test right now (as there 
is no bugtraq/bugdb report for the NPE afaik). I also tried to reference the 
NetBeans bug (by name of the directory and test). I can of course change that 
according to your suggestions.

Thanks in advance for your review.
-jt
 
> > Dne Pá 16. listopadu 2012 15:30:00, Alexander Scherbatiy napsal(a):
> >> On 11/15/2012 7:58 PM, Jaroslav Tulach wrote:
> >>> Dear Swing maintainers,
> >>> my name is Jaroslav Tulach and I am maintaining NetBeans explorer - a
> >>> component that is using JTree heavily.
> >>> 
> >>>>  From time to time I receive a user report with a NPE from Swing where
> >>>> 
> >>>> little>
> >>> 
> >>> or even no NetBeans code involved. Just today I got
> >>> http://netbeans.org/bugzilla/show_bug.cgi?id=222081
> >>> We have about 35 other ones (which is not that much given the fact we
> >>> have
> >>> million of users), but still...
> >>> 
> >>> According to
> >>> http://statistics.netbeans.org/exceptions/exception.do?id=628832
> >>> the report comes from jdk7u9-b05. The source code is here
> >>> http://hg.openjdk.java.net/jdk7u/jdk7u/jdk/file/jdk7u9-
> >>> b05/src/share/classes/javax/swing/plaf/basic/BasicTreeUI.java
> >>> and thus it looks like the call on line 4468 to
> >>> 
> >>> ui.getPathBounds(tree, newPath);
> >>> 
> >>> can return null (under some rare and unknown circumstances).
> >>> 
> >>> I can close the bug #222081 as "worksforme", but it is clear that such
> >>> error happens from time to time and we don't want our users to face
> >>> errors. A simple:
> >>> 
> >>> 4469 if (newRect == null) return;
> >>> 
> >>> would do the trick. One question remains: if I try to donate such patch,
> >>> will you accept it?
> >>> 
> >>       The ui.getPathBounds(tree, newPath) method definitely can return
> >> 
> >> null so it needs to have this check.
> >> 
> >>       However, such fix can mask the real issue, for example, in the
> >> 
> >> treeState.getBounds() method where the  treeState can be instance of
> >> FixedHeightLayoutCache or VariableHeightLayoutCache class.
> >> 
> >>       If it is possible, could you send a code snippet that shows how
> >> 
> >> NetBeans uses JTree? May be it can give a hint what can be wrong in this
> >> case.
> >> 
> >>       Thanks,
> >>       Alexandr.
> >>> 
> >>> -jt