[rfc][icedtea-web][itweb-settings] Improve Icedtea-Web cache disk space

[helpcrypto helpcrypto]

My two cents:KISS

Place a textbox and let the user define "maximum cache size" using text. 0
for no cache. IMHO: Initial value to 100MB will be fair enough.

Remove the spinner, any other references, values and checks.





On Tue, Jul 8, 2014 at 6:49 PM, Jacob Wisor <gitne at gmx.de> wrote:

> On 07/08/2014 04:20 PM, Lukasz Dracz wrote:
>
>> Hello,
>>
>> This patch addresses the bug PR1857. Removed JSlider and improved the
>> JSpinner using the suggestions from bugzilla. The JSpinner was changed to
>> go from 0 to 1024, with a step size of 32. A checkbox was added to
>> implement unlimited cache size or limit to the JSpinner value. When limit
>> is unchecked the JSpinner is disabled. Andrew Azores helped in the making
>> of this patch.
>>
>> Thank you,
>> Lukasz Dracz
>>
>
> Thank you for addressing this issue.
>
> To be honest, I am not fully satisfied with this patch although it bears
> good intentions. My main concern is that automation is always key and
> preferred.
>
> 1. IcedTea-Web should not impose any (arbitrary) hard limit on the cache
> size. If the user is supposed to be able to set a limit then there should
> not be a limit on the limit, or rather only a physical limit. ;-) Of
> course, there will always be some physical limit but we should not care
> about that. So, please eliminate TemporaryInternetFilesPanel.CACHE_MAX_SIZE.
> Furthermore, for that matter, the value representing the user defined cache
> size limit should be a long (in MB), since this is the largest primitive
> integer data type that should be sufficient for this purpose and is the
> "least problematic" large numeric data type. Please note that Java itself
> can only handle single files and file systems no larger than Long.MAX_VALUE
> in bytes. So using long for this purpose should be OK.
>
> 2. Any hard limit should not be hard coded.
>
> 3. Automation; When checking the TemporaryInternetFilesPanel.limitCacheSizeCheckBox
> the preset value in the TemporaryInternetFilesPanel.cacheSizeSpinner
> should either be the available disk space in MB at the user's discretion or
> his quota in the cache directory (which in fact is the limit), or 0. But,
> in any case the limit should be determined at run time and be no larger
> than the amount of available disk space (not free space).
>
> 4. An arbitrary step size of 32 is rather annoying to the user and may
> lead the user onto the wrong assumption that only cache sizes in steps of
> 32 MB are valid or accepted.
> So, TemporaryInternetFilesPanel.cacheSizeSpinner should rather accelerate
> - increment the step size - by a magnitude when hitting the next magnitude.
> E.g. while the initial step size is 1 and hitting 10, the next step size
> would become 10, and when hitting 100 the next step size should be 100, and
> so on.
> Yes, I am aware this is a detail but this is what makes great software. ;-)
>
> 5. TemporaryInternetFilesPanel.cacheSizeSpinner's SpinnerNumberModel is a
> *very* *good* candidate for a nested class due to its complexity. If the
> JSpinner is to check for the available disk space then it is even more of a
> candidate for a nested class. Due to its complexity, it is even advisable
> to create an extended JSpinner with its SpinnerNumberModel nested. This
> extended JSpinner should also hold the new private final static constants.
>
> All the other changes look good.
>
> Jacob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140709/6879762b/attachment.html>