[rfc][icedtea-web] do not dye if jnlp's icons points to nothing (eg 404)

[Jie Kang]

----- Original Message -----
> On 02/02/2015 01:57 PM, Jiri Vanek wrote:
> > This patch is fixing state, when icon is expected to be downlaoded, but its
> > location do not exists.
> > Then when this code expect it to be found, NPE is thrown.
> >
> > Hmm.. Looking to this after break,
> >
> > - location = CacheUtil.getCachedResource(uiconLocation, null,
> > UpdatePolicy.SESSION)
> > -                     .toString();
> > + locationURL = CacheUtil.getCachedResource(uiconLocation, null,
> > UpdatePolicy.SESSION);
> > + if (locationURL == null) {return null;}
> > + location = locationURL.toString();
> >
> > Is probably much more suitable (need to test it)
> >
> >
> > J.
> Or maybe this third attempt?
> 
> Note - this is not refactoring, it changes behaviou, but I have not found
> more usages of
> getCachedResource ...

Hello,

Ignore my previous e-mail;

This looks much better!



+    public static File  getCachedResourceFile(URL location, Version version, UpdatePolicy policy) {

Can we add comments to this function?


     public static URL getCachedResource(URL location, Version version, UpdatePolicy policy) {

Since we have getCachedResourceFile, maybe rename this to getCachedResourceURL?


+            //this throws npe, if url (specified in jnl) points to 404

s/jnl/jnlp


+                    cantCache();
                 }
             } catch (IOException ex) {
                 //favicon 404 or similar

With these changes, is this IOException catch still relevant? I am wondering, we choose to throw the NonFileProtocolException up to the caller, and the function says:

    private void cacheIcon() throws IOException, NonFileProtocolException {

How come we don't throw this IOException up too?

So in:
    public void createDesktopShortcuts(AccessWarningPaneComplexReturn.ShortcutResult menu, AccessWarningPaneComplexReturn.ShortcutResult desktop, boolean isSigned) {
                try {
                    cacheIcon();
                } catch (NonFileProtocolException ex) {
                    OutputController.getLogger().log(ex);
                    //default icon will be used later
                }

Maybe we should catch the IOException here as well? I am not too familiar with this code so any explanations would be very helpful!



Regards,


> 
> J.
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern