[rfc][icedtea-web] Cache viewer cleanup

[Jacob Wisor]

Jiri Vanek wrote:
> On 09/16/2013 02:17 AM, Jakob Wisor wrote:
>> Hello there!
>>
>> I have checked and corrected the code's formatting, and followed 
>> Jiri's reasonable advice.
>>
>> Jiri Vanek wrote:
>>> On 07/31/2013 01:47 AM, Jacob Wisor wrote:
>>>> Hello,
>>>>
>>>> * Added closing of cache viewer by ESC key
>>>> * Added proper dis-/enabling of cache viewer's buttons
>>>> * Added busy mouse cursor indicator when populating the cache viewer
>>>>
>>>> Everything is done on purpose on the AWT thread as before. Although 
>>>> I am personally not in favor
>>>> of anonymous classes, I have tried to stick to the prevalent coding 
>>>> style.
>>>>
>>>> Unfortunately, I was unable to test the cache viewer's behavior 
>>>> because I do not know how to put
>>>> synthetic cache entries into the cache. Can anyone help me with 
>>>> that? What does a the cache look
>>>> like when filled with resources? Is there any index file that keeps 
>>>> track of the cache?
> 
> As this is gui change only, it shouldbe ok without test.
> However cache is set of jars and jnlp files saved in url-like-filestrucutre
> 
> eg http:\\my.domain\myApp.jnlp with myJAr in "." codebase will be saved 
> like
> cache/http/my/domain/myApp.jnlp
> cache/http/my/domain/myJar.jnlp
> 
> and yes, there is index file in tom cache directory
> 
> I have some memorry that there are already tests for it - most simple 
> case how to import jar into cache is to lunch javaws ;)
> 
>>>>
> ...,snip...
>>> Those should be in finally block
>>>> +        parent.getContentPane().setCursor(Cursor.getDefaultCursor());
>>
>> Done. Please check whether I have done it correctly or the way you 
>> have meant.
>>
>>>>       }
> 
> Thank you.
>>>>       /**
>>>> @@ -305,4 +363,4 @@
>>>>               defaultFocusComponent.requestFocusInWindow();
>>>>           }
>>>>       }
>>>> -}
>>>> +}
>>>> \ No newline at end of file
>>>> diff --git a/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>>>> b/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>>>> --- a/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>>>> +++ b/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>>>> @@ -22,7 +22,10 @@
>>>>   import java.awt.Frame;
>>>>   import java.awt.GridBagConstraints;
>>>>   import java.awt.GridBagLayout;
>>>> +import java.awt.KeyEventDispatcher;
>>>> +import java.awt.KeyboardFocusManager;
>>>>   import java.awt.Toolkit;
>>>> +import java.awt.event.KeyEvent;
>>>>   import java.awt.event.WindowAdapter;
>>>>   import java.awt.event.WindowEvent;
>>>> @@ -53,8 +56,9 @@
>>>>        */
>>>>       public CacheViewer(DeploymentConfiguration config) {
>>>>           super((Frame) null, dialogTitle, true); // Don't need a 
>>>> parent.
>>>> +        if ((this.config = config) == null)
>>>> +            throw new IllegalArgumentException("config: " + config);
>>>>           
>>>> setIconImages(ImageResources.INSTANCE.getApplicationImages());
> 
> Please for head - split this to onemore line
> 
> +            this.config = config;
> +        if (this.config  == null)
> +            throw new IllegalArgumentException("config: " + config);
> 
> for possible backport as as you mentioned) please dont add this change 
> of exception. 1.4 expect NPE later or some error.
> 
>>>> -        this.config
>>>> +        });
>>>> +
>>>>           initialized = true;
>>>>       }
>>>> @@ -116,4 +145,4 @@
>>>>       private void centerDialog() {
>>>>           ScreenFinder.centerWindowsToCurrentScreen(this);
>>>>       }
>>>> -}
>>>> +}
>>>> \ No newline at end of file
>>>
>>> Please mention both Cache viewer cleanup and  CerViewr viewer cleanup 
>>> in news.
>>
>> Will do but they are separate patches.
>>
>> Happy reviewing!
> 
> For _this_ patch, the NEWS line should be attached explicitly.
> For CerViewr the separate patch is ok.
> 
> 
> No more review needed. With above fixed, ok to head (and 1.4 if wonted)

Okay! :-) Thank you for reviewing. But, I want to test it at least a little bit. 
Now, that I have written a few JNLP application and applet tests I have kinda 
also found the time and leisure to really do it. ;-) It's better do it once 
before feeling sorry afterwards.

Jacob