Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-18 Thread Alexander Scherbatiy
On 18/04/16 20:55, Sergey Bylokhov wrote: On 18.04.16 18:36, Alexander Scherbatiy wrote: On 4/15/2016 10:55 PM, Sergey Bylokhov wrote: On 15.04.16 21:49, Phil Race wrote: I think that even if no exception is thrown we ought to document what happens. Which of course means that we need to be

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-18 Thread Sergey Bylokhov
On 18.04.16 18:36, Alexander Scherbatiy wrote: On 4/15/2016 10:55 PM, Sergey Bylokhov wrote: On 15.04.16 21:49, Phil Race wrote: I think that even if no exception is thrown we ought to document what happens. Which of course means that we need to be sure that it does happen :-) And the

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-18 Thread Alexander Scherbatiy
On 4/15/2016 10:55 PM, Sergey Bylokhov wrote: On 15.04.16 21:49, Phil Race wrote: I think that even if no exception is thrown we ought to document what happens. Which of course means that we need to be sure that it does happen :-) And the difference depending on SM may be hard to defend. And I

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Semyon Sadetsky
On 4/15/2016 10:26 PM, Phil Race wrote: I touched on this several emails ago. An unresponsive URL may simply never produce an Image. If we wanted to handle it like that we could, but it would have to be explicit. And I don't know if you could justify the same for the method that takes a File.

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Sergey Bylokhov
On 15.04.16 21:49, Phil Race wrote: I think that even if no exception is thrown we ought to document what happens. Which of course means that we need to be sure that it does happen :-) And the difference depending on SM may be hard to defend. And I am referring to both methods, not just the URL

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Phil Race
I touched on this several emails ago. An unresponsive URL may simply never produce an Image. If we wanted to handle it like that we could, but it would have to be explicit. And I don't know if you could justify the same for the method that takes a File. -phil. On 04/15/2016 12:17 PM, Semyon

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Semyon Sadetsky
URL addresses are often taken from resources which may be unavailable for some reason. The question is what should we do with the app in this case: let it survive without showing the image (in the same way if the URL is unavailable) or let it die gracefully according to the specs. The second

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Alexander Scherbatiy
On 15/04/16 22:54, Sergey Bylokhov wrote: On 15.04.16 21:47, Alexander Scherbatiy wrote: If the second option is acceptable probably the Toolkit.getImage() javadoc can be updated. As far as I understand, we always throw NPE in case of SecManager, and throw NPE w/o secManager since

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Sergey Bylokhov
On 15.04.16 21:54, Sergey Bylokhov wrote: Also please take a look to the similar bug https://bugs.openjdk.java.net/browse/JDK-4358053 As usual I suggest to fail fast, and throw NPE. in Toolkit class and in ImageIcon, probably in some other places. The reason is that we already do that and I

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Sergey Bylokhov
On 15.04.16 21:47, Alexander Scherbatiy wrote: If the second option is acceptable probably the Toolkit.getImage() javadoc can be updated. As far as I understand, we always throw NPE in case of SecManager, and throw NPE w/o secManager since 7u40/8u25? Also please take a look to the similar

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Phil Race
I think that even if no exception is thrown we ought to document what happens. Which of course means that we need to be sure that it does happen :-) And the difference depending on SM may be hard to defend. And I am referring to both methods, not just the URL one. -phil. On 04/15/2016 11:47

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Alexander Scherbatiy
On 15/04/16 22:11, Phil Race wrote: And it prints the stack trace ?? This gets even messier. Is there anything we can do to have sensible specification of what happens with null that isn't likely to cause (much) harm ? I see only two options. Do not throw the NPE as it was before and throw

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Phil Race
And it prints the stack trace ?? This gets even messier. Is there anything we can do to have sensible specification of what happens with null that isn't likely to cause (much) harm ? -phil. On 04/15/2016 11:10 AM, Alexander Scherbatiy wrote: On 15/04/16 20:30, Sergey Bylokhov wrote: How the

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Sergey Bylokhov
How the object of URLImageSource(null) will be useful? I guess we will get some unspecified NPE exceptions if will try to call its methods? On 15.04.16 18:02, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8132706 webrev:

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Semyon Sadetsky
Looks good. --Semyon On 4/15/2016 6:02 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8132706 webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00 The fix makes the Toolkit.getDefaultToolkit().getImage(URL)

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Alexander Scherbatiy
On 15/04/16 19:26, prasanta sadhukhan wrote: Hi Alex, Just one observation. If url is null in getImageFromHash() then wouldn't we be getting NPE from checkPermissions(url) as it calls URLUtil.getConnectPermission(url); which calls url.toString().toLowerCase() so it will not come to your

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread prasanta sadhukhan
Hi Alex, Just one observation. If url is null in getImageFromHash() then wouldn't we be getting NPE from checkPermissions(url) as it calls URLUtil.getConnectPermission(url); which calls url.toString().toLowerCase() so it will not come to your check , right? String key = (url == null) ?

[9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

2016-04-15 Thread Alexander Scherbatiy
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8132706 webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00 The fix makes the Toolkit.getDefaultToolkit().getImage(URL) return a ToolkitImage based on URLImageSource with null url as it was before