Improve how the download manager reports paused statuses.

This change makes the download manager report more detail when a
download is paused.  Rather than always reporting status
RUNNING_PAUSED, there are now four different statuses:

 * paused by the app
 * waiting to retry after a network error
 * waiting for network connectivity
 * queued for wifi due to size limits

This allows a few improvements:

 * code deciding when to run a download can be improved and cleaned up
   (I've taken some extra steps in cleaning up this particular code)
 * notification code no longer has to rely on the in-memory-only
   "mPausedReason" member of DownloadInfo; instead, it knows from the
   status that the download is queued for wifi, and can display the
   appropriate string.  This moves the string fetching out into the
   UI-specific logic and is a sign that this is really the right way
   to do things.

And finally, the real motivation for this change: I've changed the
meaning of "Queued" in the downloads UI so it now means "Queued for
WiFi'.  This is what was originally intended, I'd misunderstood.  What
was formerly known as "Queued", a download that hadn't started, is now
displayed as "In progress" (it's always a transient state so it's
basically meaningless anyway).  Otherwise it remains the same (in
particular, downloads paused for other reasons are still reported as
"In progress").

I've also increased some of the logging in DownloadThread a bit, as
this change initally introduced some bugs that were impossible to
track down without that logging.  There have been other bug reports
that were impossible to diagnose and these few extra log statements
should really help, without cluttering logs too much.  I've taken care
to avoid potentially introducing any PII into the logs.

Change-Id: Id0b8d65fc8e4406ad7ffa1439ffc22a0281b051f

4 files changed, 58 insertions, 96 deletions

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index a2048958..b6f478c8 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -227,7 +227,6 @@ public class DownloadInfo {
     public String mTitle;
     public String mDescription;
     public int mBypassRecommendedSizeLimit;
-    public String mPausedReason;
 
     public int mFuzz;
 
@@ -275,10 +274,12 @@ public class DownloadInfo {
     }
 
     /**
-     * Returns the time when a download should be restarted. Must only
-     * be called when numFailed > 0.
+     * Returns the time when a download should be restarted.
      */
-    public long restartTime() {
+    public long restartTime(long now) {
+        if (mNumFailed == 0) {
+            return now;
+        }
         if (mRetryAfter > 0) {
             return mLastMod + mRetryAfter;
         }
@@ -291,67 +292,29 @@ public class DownloadInfo {
      * Returns whether this download (which the download manager hasn't seen yet)
      * should be started.
      */
-    public boolean isReadyToStart(long now) {
+    private boolean isReadyToStart(long now) {
+        if (mHasActiveThread) {
+            // already running
+            return false;
+        }
         if (mControl == Downloads.Impl.CONTROL_PAUSED) {
             // the download is paused, so it's not going to start
             return false;
         }
-        if (mStatus == 0) {
-            // status hasn't been initialized yet, this is a new download
-            return true;
-        }
-        if (mStatus == Downloads.Impl.STATUS_PENDING) {
-            // download is explicit marked as ready to start
-            return true;
-        }
-        if (mStatus == Downloads.Impl.STATUS_RUNNING) {
-            // download was interrupted (process killed, loss of power) while it was running,
-            //     without a chance to update the database
-            return true;
-        }
-        if (mStatus == Downloads.Impl.STATUS_RUNNING_PAUSED) {
-            if (mNumFailed == 0) {
-                // download is waiting for network connectivity to return before it can resume
+        switch (mStatus) {
+            case 0: // status hasn't been initialized yet, this is a new download
+            case Downloads.Impl.STATUS_PENDING: // download is explicit marked as ready to start
+            case Downloads.Impl.STATUS_RUNNING: // download interrupted (process killed etc) while
+                                                // running, without a chance to update the database
                 return true;
-            }
-            if (restartTime() < now) {
-                // download was waiting for a delayed restart, and the delay has expired
-                return true;
-            }
-        }
-        return false;
-    }
 
-    /**
-     * Returns whether this download (which the download manager has already seen
-     * and therefore potentially started) should be restarted.
-     *
-     * In a nutshell, this returns true if the download isn't already running
-     * but should be, and it can know whether the download is already running
-     * by checking the status.
-     */
-    public boolean isReadyToRestart(long now) {
-        if (mControl == Downloads.Impl.CONTROL_PAUSED) {
-            // the download is paused, so it's not going to restart
-            return false;
-        }
-        if (mStatus == 0) {
-            // download hadn't been initialized yet
-            return true;
-        }
-        if (mStatus == Downloads.Impl.STATUS_PENDING) {
-            // download is explicit marked as ready to start
-            return true;
-        }
-        if (mStatus == Downloads.Impl.STATUS_RUNNING_PAUSED) {
-            if (mNumFailed == 0) {
-                // download is waiting for network connectivity to return before it can resume
+            case Downloads.Impl.STATUS_WAITING_FOR_NETWORK:
+            case Downloads.Impl.STATUS_QUEUED_FOR_WIFI:
                 return checkCanUseNetwork() == NETWORK_OK;
-            }
-            if (restartTime() < now) {
-                // download was waiting for a delayed restart, and the delay has expired
-                return true;
-            }
+
+            case Downloads.Impl.STATUS_WAITING_TO_RETRY:
+                // download was waiting for a delayed restart
+                return restartTime(now) <= now;
         }
         return false;
     }
@@ -450,7 +413,11 @@ public class DownloadInfo {
         return NETWORK_OK;
     }
 
-    void start(long now) {
+    void startIfReady(long now) {
+        if (!isReadyToStart(now)) {
+            return;
+        }
+
         if (Constants.LOGV) {
             Log.v(Constants.TAG, "Service spawning thread to handle download " + mId);
         }
@@ -521,13 +488,10 @@ public class DownloadInfo {
         if (Downloads.Impl.isStatusCompleted(mStatus)) {
             return -1;
         }
-        if (mStatus != Downloads.Impl.STATUS_RUNNING_PAUSED) {
-            return 0;
-        }
-        if (mNumFailed == 0) {
+        if (mStatus != Downloads.Impl.STATUS_WAITING_TO_RETRY) {
             return 0;
         }
-        long when = restartTime();
+        long when = restartTime(now);
         if (when <= now) {
             return 0;
         }
@@ -545,8 +509,6 @@ public class DownloadInfo {
     }
 
     void notifyPauseDueToSize(boolean isWifiRequired) {
-        mPausedReason = mContext.getResources().getString(
-                R.string.notification_need_wifi_for_size);
         Intent intent = new Intent(Intent.ACTION_VIEW);
         intent.setData(getAllDownloadsUri());
         intent.setClassName(SizeLimitActivity.class.getPackage().getName(),
diff --git a/src/com/android/providers/downloads/DownloadNotification.java b/src/com/android/providers/downloads/DownloadNotification.java
index 90c8693b..4d615df7 100644
--- a/src/com/android/providers/downloads/DownloadNotification.java
+++ b/src/com/android/providers/downloads/DownloadNotification.java
@@ -138,8 +138,10 @@ class DownloadNotification {
                 item.addItem(title, progress, max);
                 mNotifications.put(packageName, item);
             }
-            if (hasPausedReason(download) && item.mPausedText == null) {
-                item.mPausedText = download.mPausedReason;
+            if (download.mStatus == Downloads.Impl.STATUS_QUEUED_FOR_WIFI
+                    && item.mPausedText == null) {
+                item.mPausedText = mContext.getResources().getString(
+                        R.string.notification_need_wifi_for_size);
             }
         }
 
@@ -205,10 +207,6 @@ class DownloadNotification {
         }
     }
 
-    private boolean hasPausedReason(DownloadInfo download) {
-        return download.mStatus == Downloads.STATUS_RUNNING_PAUSED && download.mPausedReason != null;
-    }
-
     private void updateCompletedNotification(Collection<DownloadInfo> downloads) {
         for (DownloadInfo download : downloads) {
             if (!isCompleteAndVisible(download)) {
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index c79ecc44..8fbcf4b4 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -439,10 +439,7 @@ public class DownloadService extends Service {
             info.logVerboseInfo();
         }
 
-        if (info.isReadyToStart(now)) {
-            info.start(now);
-        }
-
+        info.startIfReady(now);
         return info;
     }
 
@@ -466,9 +463,7 @@ public class DownloadService extends Service {
             mSystemFacade.cancelNotification(info.mId);
         }
 
-        if (info.isReadyToRestart(now)) {
-            info.start(now);
-        }
+        info.startIfReady(now);
     }
 
     /**
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 12ddfa60..b3847715 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -141,7 +141,6 @@ public class DownloadThread extends Thread {
         AndroidHttpClient client = null;
         PowerManager.WakeLock wakeLock = null;
         int finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
-        mInfo.mPausedReason = null;
 
         try {
             PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);
@@ -157,6 +156,7 @@ public class DownloadThread extends Thread {
 
             boolean finished = false;
             while(!finished) {
+                Log.i(Constants.TAG, "Initiating request for download " + mInfo.mId);
                 HttpGet request = new HttpGet(state.mRequestUri);
                 try {
                     executeDownload(state, client, request);
@@ -175,25 +175,19 @@ public class DownloadThread extends Thread {
             finalizeDestinationFile(state);
             finalStatus = Downloads.Impl.STATUS_SUCCESS;
         } catch (StopRequest error) {
-            if (Constants.LOGV) {
-                Log.v(Constants.TAG, "Aborting request for " + mInfo.mUri, error);
-            }
+            // remove the cause before printing, in case it contains PII
+            Log.w(Constants.TAG, "Aborting request for download " + mInfo.mId, removeCause(error));
             finalStatus = error.mFinalStatus;
             // fall through to finally block
         } catch (FileNotFoundException ex) {
-            Log.d(Constants.TAG, "FileNotFoundException for " + state.mFilename + " : " +  ex);
+            Log.w(Constants.TAG, "FileNotFoundException for " + state.mFilename, ex);
             finalStatus = Downloads.Impl.STATUS_FILE_ERROR;
             // falls through to the code that reports an error
-        } catch (RuntimeException ex) { //sometimes the socket code throws unchecked exceptions
-            if (Constants.LOGV) {
-                Log.d(Constants.TAG, "Exception for " + mInfo.mUri, ex);
-            } else if (Config.LOGD) {
-                Log.d(Constants.TAG, "Exception for id " + mInfo.mId, ex);
-            }
+        } catch (Throwable ex) { //sometimes the socket code throws unchecked exceptions
+            Log.w(Constants.TAG, "Exception for id " + mInfo.mId, ex);
             finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
             // falls through to the code that reports an error
         } finally {
-            mInfo.mHasActiveThread = false;
             if (wakeLock != null) {
                 wakeLock.release();
                 wakeLock = null;
@@ -206,10 +200,20 @@ public class DownloadThread extends Thread {
             notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter,
                                     state.mRedirectCount, state.mGotData, state.mFilename,
                                     state.mNewUri, state.mMimeType);
+            mInfo.mHasActiveThread = false;
         }
     }
 
     /**
+     * @return an identical StopRequest but with the cause removed.
+     */
+    private StopRequest removeCause(StopRequest error) {
+        StopRequest newException = new StopRequest(error.mFinalStatus);
+        newException.setStackTrace(error.getStackTrace());
+        return newException;
+    }
+
+    /**
      * Fully execute a single download request - setup and send the request, handle the response,
      * and transfer the data to the destination file.
      */
@@ -242,12 +246,15 @@ public class DownloadThread extends Thread {
     private void checkConnectivity(State state) throws StopRequest {
         int networkUsable = mInfo.checkCanUseNetwork();
         if (networkUsable != DownloadInfo.NETWORK_OK) {
+            int status = Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
             if (networkUsable == DownloadInfo.NETWORK_UNUSABLE_DUE_TO_SIZE) {
+                status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
                 mInfo.notifyPauseDueToSize(true);
             } else if (networkUsable == DownloadInfo.NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE) {
+                status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
                 mInfo.notifyPauseDueToSize(false);
             }
-            throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED);
+            throw new StopRequest(status);
         }
     }
 
@@ -385,7 +392,7 @@ public class DownloadThread extends Thread {
                 if (Constants.LOGV) {
                     Log.v(Constants.TAG, "paused " + mInfo.mUri);
                 }
-                throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED);
+                throw new StopRequest(Downloads.Impl.STATUS_PAUSED_BY_APP);
             }
         }
         if (mInfo.mStatus == Downloads.Impl.STATUS_CANCELED) {
@@ -766,7 +773,7 @@ public class DownloadThread extends Thread {
                // ignored - retryAfter stays 0 in this case.
            }
         }
-        throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED);
+        throw new StopRequest(Downloads.Impl.STATUS_WAITING_TO_RETRY);
     }
 
     /**
@@ -801,10 +808,10 @@ public class DownloadThread extends Thread {
         }
 
         if (!Helpers.isNetworkAvailable(mSystemFacade)) {
-            return Downloads.Impl.STATUS_RUNNING_PAUSED;
+            return Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
         } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
             state.mCountRetry = true;
-            return Downloads.Impl.STATUS_RUNNING_PAUSED;
+            return Downloads.Impl.STATUS_WAITING_TO_RETRY;
         } else {
             Log.d(Constants.TAG, "reached max retries: " + message + " for " + mInfo.mId);
             return Downloads.Impl.STATUS_HTTP_DATA_ERROR;