Refactoring of resource download

core/src/main/java/net/adoptopenjdk/icedteaweb/client/parts/downloadindicator/DefaultDownloadIndicator.java

@@ -103,22 +103,6 @@ public class DefaultDownloadIndicator implements DownloadIndicator {
         verticalIndent.insets = new Insets(0, 10, 3, 0);
     }
 
-    /**
-     * @return the update rate.
-     */
-    @Override
-    public int getUpdateRate() {
-        return 150; //ms
-    }
-
-    /**
-     * @return the initial delay before obtaining a listener.
-     */
-    @Override
-    public int getInitialDelay() {
-        return 300; //ms
-    }
-
     /**
      * Return a download service listener that displays the progress
      * in a shared download info window.

core/src/main/java/net/adoptopenjdk/icedteaweb/client/parts/downloadindicator/DownloadIndicator.java

@@ -16,8 +16,8 @@
 
 package net.adoptopenjdk.icedteaweb.client.parts.downloadindicator;
 
-import java.net.URL;
 import javax.jnlp.DownloadServiceListener;
+import java.net.URL;
 
 /**
  * A DownloadIndicator creates DownloadServiceListeners that are
@@ -54,27 +54,4 @@ public interface DownloadIndicator {
      * @param listener the listener that is no longer in use
      */
     void disposeListener(DownloadServiceListener listener);
-
-    /**
-     * Return the desired time in milliseconds between updates.
-     * Updates are not guaranteed to occur based on this value; for
-     * example, they may occur based on the download percent or some
-     * other factor.
-     *
-     * @return rate in milliseconds, must be >= 0
-     */
-    int getUpdateRate();
-
-    /**
-     * Return a time in milliseconds to wait for a download to
-     * complete before obtaining a listener for the download.  This
-     * value can be used to skip lengthy operations, such as
-     * initializing a GUI, for downloads that complete quickly.  The
-     * getListener method is not called if the download completes
-     * in less time than the returned delay.
-     *
-     * @return delay in milliseconds, must be >= 0
-     */
-    int getInitialDelay();
-
 }

core/src/main/java/net/adoptopenjdk/icedteaweb/client/parts/downloadindicator/DummyDownloadIndicator.java

@@ -0,0 +1,27 @@
+package net.adoptopenjdk.icedteaweb.client.parts.downloadindicator;
+
+import javax.jnlp.DownloadServiceListener;
+import java.net.URL;
+
+public class DummyDownloadIndicator implements DownloadIndicator {
+
+    @Override
+    public DownloadServiceListener getListener(final String downloadName, final URL[] resources) {
+        return new DownloadServiceListener() {
+            @Override
+            public void progress(final URL url, final String s, final long l, final long l1, final int i) {}
+
+            @Override
+            public void validating(final URL url, final String s, final long l, final long l1, final int i) {}
+
+            @Override
+            public void upgradingArchive(final URL url, final String s, final int i, final int i1) {}
+
+            @Override
+            public void downloadFailed(final URL url, final String s) {}
+        };
+    }
+
+    @Override
+    public void disposeListener(final DownloadServiceListener listener) {}
+}

core/src/main/java/net/adoptopenjdk/icedteaweb/resources/CachedDaemonThreadPoolProvider.java

@@ -59,8 +59,11 @@ public static ExecutorService createDaemonThreadPool() {
     public static class DaemonThreadFactory implements ThreadFactory {
 
         private static final AtomicInteger poolNumber = new AtomicInteger(1);
+
         private final ThreadGroup group;
+
         private final AtomicInteger threadNumber = new AtomicInteger(1);
+
         private final String namePrefix;
 
         public DaemonThreadFactory() {

core/src/main/java/net/adoptopenjdk/icedteaweb/resources/Resource.java

@@ -21,9 +21,12 @@
 import net.sourceforge.jnlp.util.UrlUtils;
 import net.sourceforge.jnlp.util.WeakList;
 
+import java.beans.PropertyChangeListener;
+import java.beans.PropertyChangeSupport;
 import java.io.File;
 import java.net.URL;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.concurrent.Future;
 
 /**
@@ -49,6 +52,10 @@ public class Resource {
 
     /** list of weak references of resources currently in use */
     private static final WeakList<Resource> resources = new WeakList<>();
+    public static final String SIZE_PROPERTY = "size";
+    public static final String TRANSFERRED_PROPERTY = "transferred";
+
+    private final PropertyChangeSupport propertyChangeSupport;
 
     public enum Status {
         INCOMPLETE,
@@ -91,6 +98,15 @@ private Resource(final URL location, final VersionString requestVersion, final D
         this.requestVersion = requestVersion;
         this.downloadOptions = downloadOptions;
         this.updatePolicy = updatePolicy;
+        this.propertyChangeSupport = new PropertyChangeSupport(this);
+    }
+
+    public void addPropertyChangeListener(String propertyName, PropertyChangeListener listener) {
+        this.propertyChangeSupport.addPropertyChangeListener(propertyName, listener);
+    }
+
+    public void addPropertyChangeListener(PropertyChangeListener listener) {
+        this.propertyChangeSupport.addPropertyChangeListener(listener);
     }
 
     /**
@@ -169,7 +185,9 @@ long getTransferred() {
      * @param transferred set the whole transferred amount to this value
      */
     public void setTransferred(long transferred) {
+        final long oldTransferred = this.transferred;
         this.transferred = transferred;
+        this.propertyChangeSupport.firePropertyChange(TRANSFERRED_PROPERTY, oldTransferred, this.transferred);
     }
 
     /**
@@ -187,7 +205,9 @@ public long getSize() {
      * @param size desired size of resource
      */
     public void setSize(long size) {
+        final long oldSize = this.size;
         this.size = size;
+        this.propertyChangeSupport.firePropertyChange(SIZE_PROPERTY, oldSize, this.size);
     }
 
     boolean isBeingProcessed() {
@@ -263,4 +283,12 @@ public boolean equals(Object other) {
     public String toString() {
         return "location=" + location.toString() + " version=" + requestVersion + " state=" + status;
     }
+
+    public String getSimpleName() {
+        return Optional.ofNullable(getLocation())
+                .map(l -> l.getPath())
+                .map(p -> p.split("/"))
+                .map(a -> a[a.length - 1])
+                .orElse("UNKNOWN");
+    }
 }

core/src/main/java/net/adoptopenjdk/icedteaweb/resources/ResourceHandler.java

@@ -20,9 +20,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Future;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 import static net.adoptopenjdk.icedteaweb.resources.Resource.Status.DOWNLOADED;
@@ -34,43 +31,52 @@
 class ResourceHandler {
 
     private static final Logger LOG = LoggerFactory.getLogger(ResourceHandler.class);
-    private static final Executor localExecutor = new ThreadPoolExecutor(0, Runtime.getRuntime().availableProcessors() * 2,
-            10L, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
 
     private final Resource resource;
 
     ResourceHandler(Resource resource) {
         this.resource = Assert.requireNonNull(resource, "resource");
     }
 
-    Future<Resource> putIntoCache() {
+    Future<Resource> putIntoCache(final Executor downloadExecutor) {
+        LOG.debug("Will check and maybe put into cache: {}", resource.getSimpleName());
         validateWithWhitelist();
-        final CompletableFuture<Resource> result = new CompletableFuture<>();
 
         // the thread which is processing this resource will set its future onto the resource all other
         // threads will return this future and ensure a resource is only processed by a single thread
         synchronized (resource) {
-            final Future<Resource> futureResource = resource.getFutureForDownloaded();
-            if (futureResource != null) {
+            final Future<Resource> future = resource.getFutureForDownloaded();
+            if(future == null) {
+                LOG.debug("Download for {} has not been started until now", resource.getSimpleName());
+                final Future<Resource> futureResource = getDownloadStateAndStartUnstartedDownload(downloadExecutor);
+                resource.startProcessing(futureResource);
                 return futureResource;
+            } else {
+                LOG.debug("Download for {} has already been started.", resource.getSimpleName());
+                return future;
             }
-            resource.startProcessing(result);
         }
+    }
 
+    private Future<Resource> getDownloadStateAndStartUnstartedDownload(final Executor downloadExecutor) {
+        LOG.debug("Checking download state of {}", resource.getSimpleName());
+        final CompletableFuture<Resource> result = new CompletableFuture<>();
         if (resource.isComplete()) {
+            LOG.debug("Resource is already downloaded: {} ", resource.getSimpleName());
             result.complete(resource);
         } else if (isNotCacheable()) {
+            LOG.debug("Resource is not cacheable: {}", resource.getSimpleName());
             result.complete(initNoneCacheableResources());
         } else {
-            localExecutor.execute(() -> {
+            LOG.debug("Download has not been started yet: {}", resource.getSimpleName());
+            downloadExecutor.execute(() -> {
                 try {
                     result.complete(download());
                 } catch (Exception e) {
                     result.completeExceptionally(e);
                 }
             });
         }
-
         return result;
     }
 
@@ -96,8 +102,7 @@ private Resource download() {
                 return downloadResource();
             } catch (Exception e) {
                 if (--triesLeft < 0) {
-                    LOG.info("giving up while downloading '{}' because of {}", resource.getLocation(), e.getMessage());
-                    LOG.debug("Exception while downloading", e);
+                    LOG.debug("Exception while downloading '{}'", resource.getSimpleName(), e);
                     resource.setStatus(ERROR);
                     throw e;
                 }
@@ -106,6 +111,7 @@ private Resource download() {
     }
 
     private Resource downloadResource() {
+        LOG.debug("Download of resource {} will start now!", resource.getSimpleName());
         final ResourceInitializer initializer = ResourceInitializer.of(resource);
         final InitializationResult initResult = initializer.init();
         if (initResult.needsDownload()) {