Improved cancellation of downloads: fast abort of operation and fixed thread synchron...

diff --git a/src/com/owncloud/android/files/services/FileDownloader.java b/src/com/owncloud/android/files/services/FileDownloader.java
index 2e0c686..4969c90 100644 (file)
--- a/src/com/owncloud/android/files/services/FileDownloader.java
+++ b/src/com/owncloud/android/files/services/FileDownloader.java
@@ -52,36 +52,15 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
     private WebdavClient mDownloadClient = null;\r
     private Account mLastAccount = null;\r
     \r
     private WebdavClient mDownloadClient = null;\r
     private Account mLastAccount = null;\r
     \r

-    //private AbstractList<Account> mAccounts = new Vector<Account>();\r

     private ConcurrentMap<String, DownloadFileOperation> mPendingDownloads = new ConcurrentHashMap<String, DownloadFileOperation>();\r
     private DownloadFileOperation mCurrentDownload = null;\r
     \r
     private ConcurrentMap<String, DownloadFileOperation> mPendingDownloads = new ConcurrentHashMap<String, DownloadFileOperation>();\r
     private DownloadFileOperation mCurrentDownload = null;\r
     \r

-    /*\r
-    private Account mAccount;\r
-    private String mFilePath;\r
-    private String mRemotePath;\r
-    private long mTotalDownloadSize;\r
-    private long mCurrentDownloadSize;\r
-    */\r
-    \r

     private NotificationManager mNotificationMngr;\r
     private Notification mNotification;\r
     private int mLastPercent;\r
     \r
     \r
     /**\r
     private NotificationManager mNotificationMngr;\r
     private Notification mNotification;\r
     private int mLastPercent;\r
     \r
     \r
     /**\r

-     * Static map with the files being download and the path to the temporal file were are download\r
-     */\r
-    //private static Set<String> mDownloadsInProgress = Collections.synchronizedSet(new HashSet<String>());\r
-    \r
-    /**\r
-     * Returns True when the file referred by 'remotePath' in the ownCloud account 'account' is downloading\r
-     */\r
-    /*public static boolean isDownloading(Account account, String remotePath) {\r
-        return (mDownloadsInProgress.contains(buildRemoteName(account.name, remotePath)));\r
-    }*/\r
-    \r
-    /**\r

      * Builds a key for mDownloadsInProgress from the accountName and remotePath\r
      */\r
     private static String buildRemoteName(String accountName, String remotePath) {\r
      * Builds a key for mDownloadsInProgress from the accountName and remotePath\r
      */\r
     private static String buildRemoteName(String accountName, String remotePath) {\r


@@ -186,11 +165,12 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
          * @param remotePath    URL to the remote file in the queue of downloads.\r
          */\r
         public void cancel(Account account, String remotePath) {\r
          * @param remotePath    URL to the remote file in the queue of downloads.\r
          */\r
         public void cancel(Account account, String remotePath) {\r

+            DownloadFileOperation download = null;\r

             synchronized (mPendingDownloads) {\r
             synchronized (mPendingDownloads) {\r

-                DownloadFileOperation download = mPendingDownloads.remove(buildRemoteName(account.name, remotePath));\r
-                if (download != null) {\r
-                    download.cancel();\r
-                }\r
+                download = mPendingDownloads.remove(buildRemoteName(account.name, remotePath));\r
+            }\r
+            if (download != null) {\r
+                download.cancel();\r

             }\r
         }\r
         \r
             }\r
         }\r
         \r

diff --git a/src/com/owncloud/android/operations/DownloadFileOperation.java b/src/com/owncloud/android/operations/DownloadFileOperation.java
index fbeedd8..c41db78 100644 (file)
--- a/src/com/owncloud/android/operations/DownloadFileOperation.java
+++ b/src/com/owncloud/android/operations/DownloadFileOperation.java
@@ -25,6 +25,7 @@ import java.io.IOException;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;

+import java.util.concurrent.atomic.AtomicBoolean;

 
 import org.apache.commons.httpclient.HttpException;
 import org.apache.commons.httpclient.methods.GetMethod;
 
 import org.apache.commons.httpclient.HttpException;
 import org.apache.commons.httpclient.methods.GetMethod;


@@ -55,7 +56,7 @@ public class DownloadFileOperation extends RemoteOperation {
     private String mRemotePath = null;
     private String mMimeType = null;
     private long mSize = -1;
     private String mRemotePath = null;
     private String mMimeType = null;
     private long mSize = -1;

-    private Boolean mCancellationRequested = false;
+    private final AtomicBoolean mCancellationRequested = new AtomicBoolean(false);

     
     private Set<OnDatatransferProgressListener> mDataTransferListeners = new HashSet<OnDatatransferProgressListener>();
 
     
     private Set<OnDatatransferProgressListener> mDataTransferListeners = new HashSet<OnDatatransferProgressListener>();
 


@@ -164,19 +165,21 @@ public class DownloadFileOperation extends RemoteOperation {
         GetMethod get = new GetMethod(client.getBaseUri() + WebdavUtils.encodePath(mRemotePath));
         Iterator<OnDatatransferProgressListener> it = null;
         
         GetMethod get = new GetMethod(client.getBaseUri() + WebdavUtils.encodePath(mRemotePath));
         Iterator<OnDatatransferProgressListener> it = null;
         

+        FileOutputStream fos = null;

         try {
             status = client.executeMethod(get);
             if (isSuccess(status)) {
                 targetFile.createNewFile();
                 BufferedInputStream bis = new BufferedInputStream(get.getResponseBodyAsStream());
         try {
             status = client.executeMethod(get);
             if (isSuccess(status)) {
                 targetFile.createNewFile();
                 BufferedInputStream bis = new BufferedInputStream(get.getResponseBodyAsStream());

-                FileOutputStream fos = new FileOutputStream(targetFile);
+                fos = new FileOutputStream(targetFile);

                 long transferred = 0;
 
                 byte[] bytes = new byte[4096];
                 int readResult = 0;
                 while ((readResult = bis.read(bytes)) != -1) {
                     synchronized(mCancellationRequested) {
                 long transferred = 0;
 
                 byte[] bytes = new byte[4096];
                 int readResult = 0;
                 while ((readResult = bis.read(bytes)) != -1) {
                     synchronized(mCancellationRequested) {

-                        if (mCancellationRequested) {
+                        if (mCancellationRequested.get()) {
+                            get.abort();

                             throw new OperationCancelledException();
                         }
                     }
                             throw new OperationCancelledException();
                         }
                     }


@@ -187,7 +190,6 @@ public class DownloadFileOperation extends RemoteOperation {
                         it.next().onTransferProgress(readResult, transferred, mSize, targetFile.getName());
                     }
                 }
                         it.next().onTransferProgress(readResult, transferred, mSize, targetFile.getName());
                     }
                 }

-                fos.close();

                 savedFile = true;
                 
             } else {
                 savedFile = true;
                 
             } else {


@@ -195,6 +197,7 @@ public class DownloadFileOperation extends RemoteOperation {
             }
                 
         } finally {
             }
                 
         } finally {

+            if (fos != null) fos.close();

             if (!savedFile && targetFile.exists()) {
                 targetFile.delete();
             }
             if (!savedFile && targetFile.exists()) {
                 targetFile.delete();
             }


@@ -205,9 +208,7 @@ public class DownloadFileOperation extends RemoteOperation {
 
     
     public void cancel() {
 
     
     public void cancel() {

-        synchronized(mCancellationRequested) {
-            mCancellationRequested = true;
-        }
+        mCancellationRequested.set(true);   // atomic set; there is no need of synchronizing it

     }
     
 }
     }
     
 }

diff --git a/src/com/owncloud/android/ui/fragment/FileDetailFragment.java b/src/com/owncloud/android/ui/fragment/FileDetailFragment.java
index d0c2c74..91980e5 100644 (file)
--- a/src/com/owncloud/android/ui/fragment/FileDetailFragment.java
+++ b/src/com/owncloud/android/ui/fragment/FileDetailFragment.java
@@ -84,7 +84,6 @@ import com.owncloud.android.network.OwnCloudClientUtils;
 import com.owncloud.android.ui.activity.FileDetailActivity;\r
 import com.owncloud.android.ui.activity.FileDisplayActivity;\r
 import com.owncloud.android.ui.activity.TransferServiceGetter;\r
 import com.owncloud.android.ui.activity.FileDetailActivity;\r
 import com.owncloud.android.ui.activity.FileDisplayActivity;\r
 import com.owncloud.android.ui.activity.TransferServiceGetter;\r

-import com.owncloud.android.ui.fragment.OCFileListFragment.ContainerActivity;\r

 import com.owncloud.android.utils.OwnCloudVersion;\r
 \r
 import com.owncloud.android.R;\r
 import com.owncloud.android.utils.OwnCloudVersion;\r
 \r
 import com.owncloud.android.R;\r

diff --git a/src/com/owncloud/android/ui/fragment/OCFileListFragment.java b/src/com/owncloud/android/ui/fragment/OCFileListFragment.java
index 740f66e..a65e2d7 100644 (file)
--- a/src/com/owncloud/android/ui/fragment/OCFileListFragment.java
+++ b/src/com/owncloud/android/ui/fragment/OCFileListFragment.java
@@ -19,7 +19,6 @@ package com.owncloud.android.ui.fragment;
 
 import com.owncloud.android.datamodel.DataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
 
 import com.owncloud.android.datamodel.DataStorageManager;
 import com.owncloud.android.datamodel.OCFile;

-import com.owncloud.android.files.services.FileDownloader.FileDownloaderBinder;

 import com.owncloud.android.ui.FragmentListView;
 import com.owncloud.android.ui.activity.TransferServiceGetter;
 import com.owncloud.android.ui.adapter.FileListListAdapter;
 import com.owncloud.android.ui.FragmentListView;
 import com.owncloud.android.ui.activity.TransferServiceGetter;
 import com.owncloud.android.ui.adapter.FileListListAdapter;