[rfc][icedtea-web] Cache viewer cleanup

[Jiri Vanek]

On 10/01/2013 10:51 PM, Jacob Wisor wrote:
> Hello,
>
>> 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.
>  >>>> [...]
>>>>> 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.
>
> After throughly testing this patch I think it is ready to be pushed.
> Please review the NEWS and ChangeLog sections. I have also decided to include the new NonEditableTableModel class with this patch, since this patch can also make use of it. It is also going to be ready for the upcoming certificate viewer patch.

I'm happy with the patch code, however the delete button have not worked for me :(
Also I have noted  two empty catch blocs. Please log the exceptions in Debug mode on.

Also  - that is nit probably for another change set (if changeset at all) - is multiple selection or "delete all" button in your queue?

Once delete button working/not working cleared and catch block logged, ok to go.

Thank you very much!

J.