[RFC][Icedtea-web]: Changed how cache works.

[Omair Majid]

Hey,

While the patch looks fine overall, there are a few changes that I think 
would be good to have. It's really your call if you want to commit the 
patch first and then fix them or fix them first.

> +    /* back-end of how LRU is implemented */
> +    private PropertiesFile cacheOrder = new PropertiesFile(new File(cacheDir + File.separator + "RecentlyUsed"));
> +

Andrew Hughes posted that he would have preferred a name like 
recently_used instead of RecentlyUsed. Can you please fix this?

> +    public synchronized boolean addEntry(String identifier, String path) {

> +    public synchronized boolean removeEntry(String key) {

> +    public synchronized boolean updateEntry(String oldKey) {

I would have been happier with all these methods taking identifiers or 
keys. As such, there is on way to add an entry and then remove it (as 
you wouldnt know the key for it).

> +    @SuppressWarnings("unchecked")
> +    public synchronized List<Entry<String, String>>  getLRUSortedEntries() {

Is there a reason to suppress these warnings?

> +    /*
> +     * This file is to keep track of the most recently used items.
> +     * The items are to be kept with
> +     * key = (current time accessed) followed by folder of item.
> +     * value = path to file.
> +     */
> +
> +    private static final CacheLRUWrapper lruHandler = CacheLRUWrapper.getInstance();

The comment describes an internal representation which is now 
encapsulated in CacheLRUWrapper. I am also not sure if it is accurate. 
Please (re)move it.

>
> +    public static void cleanCache() {
> +        if (okToClearCache()) {
> +            // First we want to figure out which stuff we need to delete.
> +            // There should not be any duplicate entries in cacheOrder, since we always clear old when setting new.
> +            // Thus we don't need to search through that.
> +            // We now go through all the entries and pick out the ones that aren't marked for delete starting from the back.
> +            HashSet<String>  keep = new HashSet<String>();

is 'keep' used right now?

> @@ -754,6 +779,17 @@
>
>               int size = connection.getContentLength();
>               boolean current = CacheUtil.isCurrent(resource.location, resource.requestVersion, connection)&&  resource.getUpdatePolicy() != UpdatePolicy.FORCE;
> +            if (!current) {
> +                if (entry.isCached()) {
> +                    entry.markForDelete();
> +                    entry.store();
> +                    localFile = CacheUtil.makeNewCacheFile(resource.location, resource.downloadVersion);
> +                    CacheEntry newEntry = new CacheEntry(resource.location, resource.requestVersion);
> +                    newEntry.lock();
> +                    entry.unlock();
> +                    entry = newEntry;
> +                }
> +            }

Have you tested this while offline? I am afraid that this might break 
running jnlp applications offline.

Everything else looks fine to me.

Cheers,
Omair