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

[Jacob Wisor]

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