From 272b317f3d021e2ce385820ac2660909ff939ab5 Mon Sep 17 00:00:00 2001
From: Martin Hibdon <mhibdon@google.com>
Date: Fri, 8 Nov 2013 13:57:51 -0800
Subject: [PATCH] Fix an infinitely retrying download problem

b/11436795
If an attachment download fails due to a timeout, or
an exception being thrown from startDownload(), we'd call
cancelDownload() on it. But this didn't actually cancel,
it would remove it from the inProgres list, but leave it
in the list of all downloads, so we'd immediately retry it.
This is bad for two reasons:
1. It can starve out other attachment downloads that could
have been successful.
2. It will keep attempting to do network work, even if it's
hopeless, forever, draining battery.
Now, if an attachment download fails in this way, for the first
few times, we'll move it to the tail end of the list of
downloads we'd like to perform. If it fails more than 10 times,
we'll give up completely. Giving up is not permanent, if we
have a reason to attempt a download again (such as the user
tapping on it), then it will get added back to the download
service and retried.

Change-Id: I5364a7d8b4b25ce299b8dcf061db6e9ce12daf75
---
 .../service/AttachmentDownloadService.java    | 36 +++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/com/android/email/service/AttachmentDownloadService.java b/src/com/android/email/service/AttachmentDownloadService.java
index 87b9c30b0..76bd11650 100644
--- a/src/com/android/email/service/AttachmentDownloadService.java
+++ b/src/com/android/email/service/AttachmentDownloadService.java
@@ -196,9 +196,25 @@ public class AttachmentDownloadService extends Service implements Runnable {
                 accountId = messageId = -1;
             }
             priority = getPriority(attachment);
-            time = System.currentTimeMillis();
+            time = SystemClock.elapsedRealtime();
         }
 
+        private DownloadRequest(DownloadRequest orig, long newTime) {
+            priority = orig.priority;
+            attachmentId = orig.attachmentId;
+            messageId = orig.messageId;
+            accountId = orig.accountId;
+            time = newTime;
+            inProgress = orig.inProgress;
+            lastStatusCode = orig.lastStatusCode;
+            lastProgress = orig.lastProgress;
+            lastCallbackTime = orig.lastCallbackTime;
+            startTime = orig.startTime;
+            retryCount = orig.retryCount;
+            retryStartTime = orig.retryStartTime;
+        }
+
+
         @Override
         public int hashCode() {
             return (int)attachmentId;
@@ -527,8 +543,24 @@ public class AttachmentDownloadService extends Service implements Runnable {
         }
 
         private void cancelDownload(DownloadRequest req) {
-            mDownloadsInProgress.remove(req.attachmentId);
+            LogUtils.d(TAG, "cancelDownload #%d", req.attachmentId);
             req.inProgress = false;
+            mDownloadsInProgress.remove(req.attachmentId);
+            // Remove the download from our queue, and then decide whether or not to add it back.
+            remove(req);
+            req.retryCount++;
+            if (req.retryCount > CONNECTION_ERROR_MAX_RETRIES) {
+                LogUtils.d(TAG, "too many failures, giving up");
+            } else {
+                LogUtils.d(TAG, "moving to end of queue, will retry");
+                // The time field of DownloadRequest is final, because it's unsafe to change it
+                // as long as the DownloadRequest is in the DownloadSet. It's needed for the
+                // comparator, so changing time would make the request unfindable.
+                // Instead, we'll create a new DownloadRequest with an updated time.
+                // This will sort at the end of the set.
+                req = new DownloadRequest(req, SystemClock.elapsedRealtime());
+                add(req);
+            }
         }
 
         /**