From: David A. Velasco <dvelasco@solidgear.es>
Date: Wed, 10 Oct 2012 09:30:47 +0000 (+0200)
Subject: Improved cancellation of downloads: fast abort of operation and fixed thread synchron... 
X-Git-Tag: oc-android-1.4.3~160
X-Git-Url: http://git.linex4red.de/pub/Android/ownCloud.git/commitdiff_plain/b246cf7075c829a7662047e89a927c0b6cc18ac5?ds=inline;hp=-c

Improved cancellation of downloads: fast abort of operation and fixed thread synchronization
---

b246cf7075c829a7662047e89a927c0b6cc18ac5
diff --git a/src/com/owncloud/android/files/services/FileDownloader.java b/src/com/owncloud/android/files/services/FileDownloader.java
index 2e0c6863..4969c908 100644
--- 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;
     private Account mLastAccount = null;
     
-    //private AbstractList<Account> mAccounts = new Vector<Account>();
     private ConcurrentMap<String, DownloadFileOperation> mPendingDownloads = new ConcurrentHashMap<String, DownloadFileOperation>();
     private DownloadFileOperation mCurrentDownload = null;
     
-    /*
-    private Account mAccount;
-    private String mFilePath;
-    private String mRemotePath;
-    private long mTotalDownloadSize;
-    private long mCurrentDownloadSize;
-    */
-    
     private NotificationManager mNotificationMngr;
     private Notification mNotification;
     private int mLastPercent;
     
     
     /**
-     * Static map with the files being download and the path to the temporal file were are download
-     */
-    //private static Set<String> mDownloadsInProgress = Collections.synchronizedSet(new HashSet<String>());
-    
-    /**
-     * Returns True when the file referred by 'remotePath' in the ownCloud account 'account' is downloading
-     */
-    /*public static boolean isDownloading(Account account, String remotePath) {
-        return (mDownloadsInProgress.contains(buildRemoteName(account.name, remotePath)));
-    }*/
-    
-    /**
      * Builds a key for mDownloadsInProgress from the accountName and remotePath
      */
     private static String buildRemoteName(String accountName, String remotePath) {
@@ -186,11 +165,12 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
          * @param remotePath    URL to the remote file in the queue of downloads.
          */
         public void cancel(Account account, String remotePath) {
+            DownloadFileOperation download = null;
             synchronized (mPendingDownloads) {
-                DownloadFileOperation download = mPendingDownloads.remove(buildRemoteName(account.name, remotePath));
-                if (download != null) {
-                    download.cancel();
-                }
+                download = mPendingDownloads.remove(buildRemoteName(account.name, remotePath));
+            }
+            if (download != null) {
+                download.cancel();
             }
         }
         
diff --git a/src/com/owncloud/android/operations/DownloadFileOperation.java b/src/com/owncloud/android/operations/DownloadFileOperation.java
index fbeedd8a..c41db782 100644
--- 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.concurrent.atomic.AtomicBoolean;
 
 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 Boolean mCancellationRequested = false;
+    private final AtomicBoolean mCancellationRequested = new AtomicBoolean(false);
     
     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;
         
+        FileOutputStream fos = null;
         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) {
-                        if (mCancellationRequested) {
+                        if (mCancellationRequested.get()) {
+                            get.abort();
                             throw new OperationCancelledException();
                         }
                     }
@@ -187,7 +190,6 @@ public class DownloadFileOperation extends RemoteOperation {
                         it.next().onTransferProgress(readResult, transferred, mSize, targetFile.getName());
                     }
                 }
-                fos.close();
                 savedFile = true;
                 
             } else {
@@ -195,6 +197,7 @@ public class DownloadFileOperation extends RemoteOperation {
             }
                 
         } finally {
+            if (fos != null) fos.close();
             if (!savedFile && targetFile.exists()) {
                 targetFile.delete();
             }
@@ -205,9 +208,7 @@ public class DownloadFileOperation extends RemoteOperation {
 
     
     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 d0c2c747..91980e5f 100644
--- 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;
 import com.owncloud.android.ui.activity.FileDisplayActivity;
 import com.owncloud.android.ui.activity.TransferServiceGetter;
-import com.owncloud.android.ui.fragment.OCFileListFragment.ContainerActivity;
 import com.owncloud.android.utils.OwnCloudVersion;
 
 import com.owncloud.android.R;
diff --git a/src/com/owncloud/android/ui/fragment/OCFileListFragment.java b/src/com/owncloud/android/ui/fragment/OCFileListFragment.java
index 740f66eb..a65e2d7c 100644
--- 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.files.services.FileDownloader.FileDownloaderBinder;
 import com.owncloud.android.ui.FragmentListView;
 import com.owncloud.android.ui.activity.TransferServiceGetter;
 import com.owncloud.android.ui.adapter.FileListListAdapter;