From c92dc11cd94f5b00f0193cc8cf1e03644d825590 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 6 Dec 2012 11:54:45 -0800
Subject: Move DownloadManager to HttpURLConnection.

Apache HttpClient is in maintenance mode, and doesn't have support
for features like Server Name Indication (SNI).  This change moves
DownloadManager to use HttpURLConnection internally.  It also
moves redirection handling into HttpURLConnection.

Bug: 7070597
Change-Id: Ie80093eeeecd14f94e1c8b7597ff3f8f5d220691
---
 .../providers/downloads/DownloadThread.java        | 378 ++++++---------------
 src/com/android/providers/downloads/Helpers.java   |   3 +
 2 files changed, 116 insertions(+), 265 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 34bc8e34..cab1a3e6 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -16,16 +16,18 @@
 
 package com.android.providers.downloads;
 
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static java.net.HttpURLConnection.HTTP_PARTIAL;
+import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 
 import android.content.ContentValues;
 import android.content.Context;
 import android.content.Intent;
 import android.net.INetworkPolicyListener;
 import android.net.NetworkPolicyManager;
-import android.net.Proxy;
 import android.net.TrafficStats;
-import android.net.http.AndroidHttpClient;
 import android.os.FileUtils;
 import android.os.PowerManager;
 import android.os.Process;
@@ -35,25 +37,30 @@ import android.text.TextUtils;
 import android.util.Log;
 import android.util.Pair;
 
-import org.apache.http.Header;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.conn.params.ConnRouteParams;
-
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.SyncFailedException;
-import java.net.URI;
-import java.net.URISyntaxException;
+import java.net.CookieHandler;
+import java.net.CookieManager;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.URLConnection;
+
+import libcore.io.IoUtils;
 
 /**
- * Runs an actual download
+ * Thread which executes a given {@link DownloadInfo}: making network requests,
+ * persisting data to disk, and updating {@link DownloadProvider}.
  */
 public class DownloadThread extends Thread {
 
+    private static final int HTTP_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
+
+    private static final int DEFAULT_TIMEOUT = (int) MINUTE_IN_MILLIS;
+
     private final Context mContext;
     private final DownloadInfo mInfo;
     private final SystemFacade mSystemFacade;
@@ -90,8 +97,6 @@ public class DownloadThread extends Thread {
         public String mMimeType;
         public boolean mCountRetry = false;
         public int mRetryAfter = 0;
-        public int mRedirectCount = 0;
-        public String mNewUri;
         public boolean mGotData = false;
         public String mRequestUri;
         public long mTotalBytes = -1;
@@ -121,17 +126,11 @@ public class DownloadThread extends Thread {
      * State within executeDownload()
      */
     private static class InnerState {
-        public String mHeaderContentLength;
-        public String mHeaderContentDisposition;
-        public String mHeaderContentLocation;
+        public long mContentLength;
+        public String mContentDisposition;
+        public String mContentLocation;
     }
 
-    /**
-     * Raised from methods called by executeDownload() to indicate that the download should be
-     * retried immediately.
-     */
-    private class RetryDownload extends Throwable {}
-
     /**
      * Executes the download in a separate thread
      */
@@ -155,7 +154,6 @@ public class DownloadThread extends Thread {
         }
 
         State state = new State(mInfo);
-        AndroidHttpClient client = null;
         PowerManager.WakeLock wakeLock = null;
         int finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
         String errorMsg = null;
@@ -174,29 +172,24 @@ public class DownloadThread extends Thread {
                 Log.v(Constants.TAG, "initiating download for " + mInfo.mUri);
             }
 
-            client = AndroidHttpClient.newInstance(userAgent(), mContext);
-
             // network traffic on this thread should be counted against the
             // requesting uid, and is tagged with well-known value.
             TrafficStats.setThreadStatsTag(TrafficStats.TAG_SYSTEM_DOWNLOAD);
             TrafficStats.setThreadStatsUid(mInfo.mUid);
 
             boolean finished = false;
-            while(!finished) {
+            while (!finished) {
                 Log.i(Constants.TAG, "Initiating request for download " + mInfo.mId);
-                // Set or unset proxy, which may have changed since last GET request.
-                // setDefaultProxy() supports null as proxy parameter.
-                ConnRouteParams.setDefaultProxy(client.getParams(),
-                        Proxy.getPreferredHttpHost(mContext, state.mRequestUri));
-                HttpGet request = new HttpGet(state.mRequestUri);
+
+                final URL url = new URL(state.mRequestUri);
+                final HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+                conn.setConnectTimeout(DEFAULT_TIMEOUT);
+                conn.setReadTimeout(DEFAULT_TIMEOUT);
                 try {
-                    executeDownload(state, client, request);
+                    executeDownload(state, conn);
                     finished = true;
-                } catch (RetryDownload exc) {
-                    // fall through
                 } finally {
-                    request.abort();
-                    request = null;
+                    conn.disconnect();
                 }
             }
 
@@ -215,7 +208,7 @@ public class DownloadThread extends Thread {
             }
             finalStatus = error.mFinalStatus;
             // fall through to finally block
-        } catch (Throwable ex) { //sometimes the socket code throws unchecked exceptions
+        } catch (Throwable ex) {
             errorMsg = ex.getMessage();
             String msg = "Exception for id " + mInfo.mId + ": " + errorMsg;
             Log.w(Constants.TAG, msg, ex);
@@ -225,14 +218,9 @@ public class DownloadThread extends Thread {
             TrafficStats.clearThreadStatsTag();
             TrafficStats.clearThreadStatsUid();
 
-            if (client != null) {
-                client.close();
-                client = null;
-            }
             cleanupDestination(state, finalStatus);
             notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter,
-                                    state.mGotData, state.mFilename,
-                                    state.mNewUri, state.mMimeType, errorMsg);
+                    state.mGotData, state.mFilename, state.mMimeType, errorMsg);
 
             netPolicy.unregisterListener(mPolicyListener);
 
@@ -248,13 +236,13 @@ public class DownloadThread extends Thread {
      * Fully execute a single download request - setup and send the request, handle the response,
      * and transfer the data to the destination file.
      */
-    private void executeDownload(State state, AndroidHttpClient client, HttpGet request)
-            throws StopRequestException, RetryDownload {
-        InnerState innerState = new InnerState();
-        byte data[] = new byte[Constants.BUFFER_SIZE];
+    private void executeDownload(State state, HttpURLConnection conn)
+            throws IOException, StopRequestException {
+        final InnerState innerState = new InnerState();
+        final byte data[] = new byte[Constants.BUFFER_SIZE];
 
         setupDestinationFile(state, innerState);
-        addRequestHeaders(state, request);
+        addRequestHeaders(state, conn);
 
         // skip when already finished; remove after fixing race in 5217390
         if (state.mCurrentBytes == state.mTotalBytes) {
@@ -266,16 +254,14 @@ public class DownloadThread extends Thread {
         // check just before sending the request to avoid using an invalid connection at all
         checkConnectivity();
 
-        HttpResponse response = sendRequest(state, client, request);
-        handleExceptionalStatus(state, innerState, response);
+        // Asking for response code will execute the request
+        final int statusCode = conn.getResponseCode();
 
-        if (Constants.LOGV) {
-            Log.v(Constants.TAG, "received response for " + mInfo.mUri);
-        }
+        handleExceptionalStatus(state, innerState, conn, statusCode);
+        processResponseHeaders(state, innerState, conn);
 
-        processResponseHeaders(state, innerState, response);
-        InputStream entityStream = openResponseEntity(state, response);
-        transferData(state, innerState, data, entityStream);
+        final InputStream in = conn.getInputStream();
+        transferData(state, innerState, data, in);
     }
 
     /**
@@ -332,7 +318,7 @@ public class DownloadThread extends Thread {
     /**
      * Called after a successful completion to take any necessary action on the downloaded file.
      */
-    private void finalizeDestinationFile(State state) throws StopRequestException {
+    private void finalizeDestinationFile(State state) {
         if (state.mFilename != null) {
             // make sure the file is readable
             FileUtils.setPermissions(state.mFilename, 0644, -1, -1);
@@ -376,15 +362,7 @@ public class DownloadThread extends Thread {
         } catch (RuntimeException ex) {
             Log.w(Constants.TAG, "exception while syncing file: ", ex);
         } finally {
-            if(downloadedFileStream != null) {
-                try {
-                    downloadedFileStream.close();
-                } catch (IOException ex) {
-                    Log.w(Constants.TAG, "IOException while closing synced file: ", ex);
-                } catch (RuntimeException ex) {
-                    Log.w(Constants.TAG, "exception while closing file: ", ex);
-                }
-            }
+            IoUtils.closeQuietly(downloadedFileStream);
         }
     }
 
@@ -392,18 +370,8 @@ public class DownloadThread extends Thread {
      * Close the destination output stream.
      */
     private void closeDestination(State state) {
-        try {
-            // close the file
-            if (state.mStream != null) {
-                state.mStream.close();
-                state.mStream = null;
-            }
-        } catch (IOException ex) {
-            if (Constants.LOGV) {
-                Log.v(Constants.TAG, "exception when closing the file after download : " + ex);
-            }
-            // nothing can really be done if the file can't be closed
-        }
+        IoUtils.closeQuietly(state.mStream);
+        state.mStream = null;
     }
 
     /**
@@ -508,13 +476,13 @@ public class DownloadThread extends Thread {
     private void handleEndOfStream(State state, InnerState innerState) throws StopRequestException {
         ContentValues values = new ContentValues();
         values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, state.mCurrentBytes);
-        if (innerState.mHeaderContentLength == null) {
+        if (innerState.mContentLength == -1) {
             values.put(Downloads.Impl.COLUMN_TOTAL_BYTES, state.mCurrentBytes);
         }
         mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
 
-        boolean lengthMismatched = (innerState.mHeaderContentLength != null)
-                && (state.mCurrentBytes != Integer.parseInt(innerState.mHeaderContentLength));
+        boolean lengthMismatched = (innerState.mContentLength != -1)
+                && (state.mCurrentBytes != innerState.mContentLength);
         if (lengthMismatched) {
             if (cannotResume(state)) {
                 throw new StopRequestException(Downloads.Impl.STATUS_CANNOT_RESUME,
@@ -541,7 +509,6 @@ public class DownloadThread extends Thread {
         try {
             return entityStream.read(data);
         } catch (IOException ex) {
-            logNetworkState(mInfo.mUid);
             ContentValues values = new ContentValues();
             values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, state.mCurrentBytes);
             mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
@@ -557,40 +524,18 @@ public class DownloadThread extends Thread {
         }
     }
 
-    /**
-     * Open a stream for the HTTP response entity, handling I/O errors.
-     * @return an InputStream to read the response entity
-     */
-    private InputStream openResponseEntity(State state, HttpResponse response)
-            throws StopRequestException {
-        try {
-            return response.getEntity().getContent();
-        } catch (IOException ex) {
-            logNetworkState(mInfo.mUid);
-            throw new StopRequestException(getFinalStatusForHttpError(state),
-                    "while getting entity: " + ex.toString(), ex);
-        }
-    }
-
-    private void logNetworkState(int uid) {
-        if (Constants.LOGX) {
-            Log.i(Constants.TAG,
-                    "Net " + (Helpers.isNetworkAvailable(mSystemFacade, uid) ? "Up" : "Down"));
-        }
-    }
-
     /**
      * Read HTTP response headers and take appropriate action, including setting up the destination
      * file and updating the database.
      */
-    private void processResponseHeaders(State state, InnerState innerState, HttpResponse response)
+    private void processResponseHeaders(State state, InnerState innerState, HttpURLConnection conn)
             throws StopRequestException {
         if (state.mContinuingDownload) {
             // ignore response headers on resume requests
             return;
         }
 
-        readResponseHeaders(state, innerState, response);
+        readResponseHeaders(state, innerState, conn);
         if (DownloadDrmHelper.isDrmConvertNeeded(state.mMimeType)) {
             mDrmConvertSession = DrmConvertSession.open(mContext, state.mMimeType);
             if (mDrmConvertSession == null) {
@@ -603,12 +548,11 @@ public class DownloadThread extends Thread {
                 mContext,
                 mInfo.mUri,
                 mInfo.mHint,
-                innerState.mHeaderContentDisposition,
-                innerState.mHeaderContentLocation,
+                innerState.mContentDisposition,
+                innerState.mContentLocation,
                 state.mMimeType,
                 mInfo.mDestination,
-                (innerState.mHeaderContentLength != null) ?
-                        Long.parseLong(innerState.mHeaderContentLength) : 0,
+                innerState.mContentLength,
                 mInfo.mIsPublicApi, mStorageManager);
         try {
             state.mStream = new FileOutputStream(state.mFilename);
@@ -645,58 +589,30 @@ public class DownloadThread extends Thread {
     /**
      * Read headers from the HTTP response and store them into local state.
      */
-    private void readResponseHeaders(State state, InnerState innerState, HttpResponse response)
+    private void readResponseHeaders(State state, InnerState innerState, HttpURLConnection conn)
             throws StopRequestException {
-        Header header = response.getFirstHeader("Content-Disposition");
-        if (header != null) {
-            innerState.mHeaderContentDisposition = header.getValue();
-        }
-        header = response.getFirstHeader("Content-Location");
-        if (header != null) {
-            innerState.mHeaderContentLocation = header.getValue();
-        }
+        innerState.mContentDisposition = conn.getHeaderField("Content-Disposition");
+        innerState.mContentLocation = conn.getHeaderField("Content-Location");
+
         if (state.mMimeType == null) {
-            header = response.getFirstHeader("Content-Type");
-            if (header != null) {
-                state.mMimeType = Intent.normalizeMimeType(header.getValue());
-            }
+            state.mMimeType = Intent.normalizeMimeType(conn.getContentType());
         }
-        header = response.getFirstHeader("ETag");
-        if (header != null) {
-            state.mHeaderETag = header.getValue();
-        }
-        String headerTransferEncoding = null;
-        header = response.getFirstHeader("Transfer-Encoding");
-        if (header != null) {
-            headerTransferEncoding = header.getValue();
-        }
-        if (headerTransferEncoding == null) {
-            header = response.getFirstHeader("Content-Length");
-            if (header != null) {
-                innerState.mHeaderContentLength = header.getValue();
-                state.mTotalBytes = mInfo.mTotalBytes =
-                        Long.parseLong(innerState.mHeaderContentLength);
-            }
+
+        state.mHeaderETag = conn.getHeaderField("ETag");
+
+        final String transferEncoding = conn.getHeaderField("Transfer-Encoding");
+        if (transferEncoding == null) {
+            innerState.mContentLength = getHeaderFieldLong(conn, "Content-Length", -1);
         } else {
-            // Ignore content-length with transfer-encoding - 2616 4.4 3
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG,
-                        "ignoring content-length because of xfer-encoding");
-            }
-        }
-        if (Constants.LOGVV) {
-            Log.v(Constants.TAG, "Content-Disposition: " +
-                    innerState.mHeaderContentDisposition);
-            Log.v(Constants.TAG, "Content-Length: " + innerState.mHeaderContentLength);
-            Log.v(Constants.TAG, "Content-Location: " + innerState.mHeaderContentLocation);
-            Log.v(Constants.TAG, "Content-Type: " + state.mMimeType);
-            Log.v(Constants.TAG, "ETag: " + state.mHeaderETag);
-            Log.v(Constants.TAG, "Transfer-Encoding: " + headerTransferEncoding);
+            Log.i(TAG, "Ignoring Content-Length since Transfer-Encoding is also defined");
+            innerState.mContentLength = -1;
         }
 
-        boolean noSizeInfo = innerState.mHeaderContentLength == null
-                && (headerTransferEncoding == null
-                    || !headerTransferEncoding.equalsIgnoreCase("chunked"));
+        state.mTotalBytes = innerState.mContentLength;
+        mInfo.mTotalBytes = innerState.mContentLength;
+
+        final boolean noSizeInfo = innerState.mContentLength == -1
+                && (transferEncoding == null || !transferEncoding.equalsIgnoreCase("chunked"));
         if (!mInfo.mNoIntegrity && noSizeInfo) {
             throw new StopRequestException(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
                     "can't know size of download, giving up");
@@ -706,21 +622,18 @@ public class DownloadThread extends Thread {
     /**
      * Check the HTTP response status and handle anything unusual (e.g. not 200/206).
      */
-    private void handleExceptionalStatus(State state, InnerState innerState, HttpResponse response)
-            throws StopRequestException, RetryDownload {
-        int statusCode = response.getStatusLine().getStatusCode();
-        if (statusCode == 503 && mInfo.mNumFailed < Constants.MAX_RETRIES) {
-            handleServiceUnavailable(state, response);
-        }
-        if (statusCode == 301 || statusCode == 302 || statusCode == 303 || statusCode == 307) {
-            handleRedirect(state, response, statusCode);
+    private void handleExceptionalStatus(
+            State state, InnerState innerState, HttpURLConnection conn, int statusCode)
+            throws StopRequestException {
+        if (statusCode == HTTP_UNAVAILABLE && mInfo.mNumFailed < Constants.MAX_RETRIES) {
+            handleServiceUnavailable(state, conn);
         }
 
         if (Constants.LOGV) {
             Log.i(Constants.TAG, "recevd_status = " + statusCode +
                     ", mContinuingDownload = " + state.mContinuingDownload);
         }
-        int expectedStatus = state.mContinuingDownload ? 206 : Downloads.Impl.STATUS_SUCCESS;
+        int expectedStatus = state.mContinuingDownload ? HTTP_PARTIAL : HTTP_OK;
         if (statusCode != expectedStatus) {
             handleOtherStatus(state, innerState, statusCode);
         }
@@ -731,17 +644,17 @@ public class DownloadThread extends Thread {
      */
     private void handleOtherStatus(State state, InnerState innerState, int statusCode)
             throws StopRequestException {
-        if (statusCode == 416) {
+        if (statusCode == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) {
             // range request failed. it should never fail.
             throw new IllegalStateException("Http Range request failure: totalBytes = " +
                     state.mTotalBytes + ", bytes recvd so far: " + state.mCurrentBytes);
         }
         int finalStatus;
-        if (Downloads.Impl.isStatusError(statusCode)) {
+        if (statusCode >= 400 && statusCode < 600) {
             finalStatus = statusCode;
         } else if (statusCode >= 300 && statusCode < 400) {
             finalStatus = Downloads.Impl.STATUS_UNHANDLED_REDIRECT;
-        } else if (state.mContinuingDownload && statusCode == Downloads.Impl.STATUS_SUCCESS) {
+        } else if (state.mContinuingDownload && statusCode == HTTP_OK) {
             finalStatus = Downloads.Impl.STATUS_CANNOT_RESUME;
         } else {
             finalStatus = Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE;
@@ -750,98 +663,29 @@ public class DownloadThread extends Thread {
                 statusCode + ", mContinuingDownload: " + state.mContinuingDownload);
     }
 
-    /**
-     * Handle a 3xx redirect status.
-     */
-    private void handleRedirect(State state, HttpResponse response, int statusCode)
-            throws StopRequestException, RetryDownload {
-        if (Constants.LOGVV) {
-            Log.v(Constants.TAG, "got HTTP redirect " + statusCode);
-        }
-        if (state.mRedirectCount >= Constants.MAX_REDIRECTS) {
-            throw new StopRequestException(Downloads.Impl.STATUS_TOO_MANY_REDIRECTS,
-                    "too many redirects");
-        }
-        Header header = response.getFirstHeader("Location");
-        if (header == null) {
-            return;
-        }
-        if (Constants.LOGVV) {
-            Log.v(Constants.TAG, "Location :" + header.getValue());
-        }
-
-        String newUri;
-        try {
-            newUri = new URI(mInfo.mUri).resolve(new URI(header.getValue())).toString();
-        } catch(URISyntaxException ex) {
-            if (Constants.LOGV) {
-                Log.d(Constants.TAG, "Couldn't resolve redirect URI " + header.getValue()
-                        + " for " + mInfo.mUri);
-            }
-            throw new StopRequestException(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
-                    "Couldn't resolve redirect URI");
-        }
-        ++state.mRedirectCount;
-        state.mRequestUri = newUri;
-        if (statusCode == 301 || statusCode == 303) {
-            // use the new URI for all future requests (should a retry/resume be necessary)
-            state.mNewUri = newUri;
-        }
-        throw new RetryDownload();
-    }
-
     /**
      * Handle a 503 Service Unavailable status by processing the Retry-After header.
      */
-    private void handleServiceUnavailable(State state, HttpResponse response)
+    private void handleServiceUnavailable(State state, HttpURLConnection conn)
             throws StopRequestException {
-        if (Constants.LOGVV) {
-            Log.v(Constants.TAG, "got HTTP response code 503");
-        }
         state.mCountRetry = true;
-        Header header = response.getFirstHeader("Retry-After");
-        if (header != null) {
-           try {
-               if (Constants.LOGVV) {
-                   Log.v(Constants.TAG, "Retry-After :" + header.getValue());
-               }
-               state.mRetryAfter = Integer.parseInt(header.getValue());
-               if (state.mRetryAfter < 0) {
-                   state.mRetryAfter = 0;
-               } else {
-                   if (state.mRetryAfter < Constants.MIN_RETRY_AFTER) {
-                       state.mRetryAfter = Constants.MIN_RETRY_AFTER;
-                   } else if (state.mRetryAfter > Constants.MAX_RETRY_AFTER) {
-                       state.mRetryAfter = Constants.MAX_RETRY_AFTER;
-                   }
-                   state.mRetryAfter += Helpers.sRandom.nextInt(Constants.MIN_RETRY_AFTER + 1);
-                   state.mRetryAfter *= 1000;
-               }
-           } catch (NumberFormatException ex) {
-               // ignored - retryAfter stays 0 in this case.
-           }
+        state.mRetryAfter = conn.getHeaderFieldInt("Retry-After", -1);
+        if (state.mRetryAfter < 0) {
+            state.mRetryAfter = 0;
+        } else {
+            if (state.mRetryAfter < Constants.MIN_RETRY_AFTER) {
+                state.mRetryAfter = Constants.MIN_RETRY_AFTER;
+            } else if (state.mRetryAfter > Constants.MAX_RETRY_AFTER) {
+                state.mRetryAfter = Constants.MAX_RETRY_AFTER;
+            }
+            state.mRetryAfter += Helpers.sRandom.nextInt(Constants.MIN_RETRY_AFTER + 1);
+            state.mRetryAfter *= 1000;
         }
+
         throw new StopRequestException(Downloads.Impl.STATUS_WAITING_TO_RETRY,
                 "got 503 Service Unavailable, will retry later");
     }
 
-    /**
-     * Send the request to the server, handling any I/O exceptions.
-     */
-    private HttpResponse sendRequest(State state, AndroidHttpClient client, HttpGet request)
-            throws StopRequestException {
-        try {
-            return client.execute(request);
-        } catch (IllegalArgumentException ex) {
-            throw new StopRequestException(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
-                    "while trying to execute request: " + ex.toString(), ex);
-        } catch (IOException ex) {
-            logNetworkState(mInfo.mUid);
-            throw new StopRequestException(getFinalStatusForHttpError(state),
-                    "while trying to execute request: " + ex.toString(), ex);
-        }
-    }
-
     private int getFinalStatusForHttpError(State state) {
         int networkUsable = mInfo.checkCanUseNetwork();
         if (networkUsable != DownloadInfo.NETWORK_OK) {
@@ -921,7 +765,7 @@ public class DownloadThread extends Thread {
                     }
                     state.mCurrentBytes = (int) fileLength;
                     if (mInfo.mTotalBytes != -1) {
-                        innerState.mHeaderContentLength = Long.toString(mInfo.mTotalBytes);
+                        innerState.mContentLength = mInfo.mTotalBytes;
                     }
                     state.mHeaderETag = mInfo.mETag;
                     state.mContinuingDownload = true;
@@ -942,16 +786,18 @@ public class DownloadThread extends Thread {
     /**
      * Add custom headers for this download to the HTTP request.
      */
-    private void addRequestHeaders(State state, HttpGet request) {
+    private void addRequestHeaders(State state, HttpURLConnection conn) {
+        conn.addRequestProperty("User-Agent", userAgent());
+
         for (Pair<String, String> header : mInfo.getHeaders()) {
-            request.addHeader(header.first, header.second);
+            conn.addRequestProperty(header.first, header.second);
         }
 
         if (state.mContinuingDownload) {
             if (state.mHeaderETag != null) {
-                request.addHeader("If-Match", state.mHeaderETag);
+                conn.addRequestProperty("If-Match", state.mHeaderETag);
             }
-            request.addHeader("Range", "bytes=" + state.mCurrentBytes + "-");
+            conn.addRequestProperty("Range", "bytes=" + state.mCurrentBytes + "-");
             if (Constants.LOGV) {
                 Log.i(Constants.TAG, "Adding Range header: " +
                         "bytes=" + state.mCurrentBytes + "-");
@@ -963,26 +809,20 @@ public class DownloadThread extends Thread {
     /**
      * Stores information about the completed download, and notifies the initiating application.
      */
-    private void notifyDownloadCompleted(
-            int status, boolean countRetry, int retryAfter, boolean gotData,
-            String filename, String uri, String mimeType, String errorMsg) {
+    private void notifyDownloadCompleted(int status, boolean countRetry, int retryAfter,
+            boolean gotData, String filename, String mimeType, String errorMsg) {
         notifyThroughDatabase(
-                status, countRetry, retryAfter, gotData, filename, uri, mimeType,
-                errorMsg);
+                status, countRetry, retryAfter, gotData, filename, mimeType, errorMsg);
         if (Downloads.Impl.isStatusCompleted(status)) {
             mInfo.sendIntentIfRequested();
         }
     }
 
-    private void notifyThroughDatabase(
-            int status, boolean countRetry, int retryAfter, boolean gotData,
-            String filename, String uri, String mimeType, String errorMsg) {
+    private void notifyThroughDatabase(int status, boolean countRetry, int retryAfter,
+            boolean gotData, String filename, String mimeType, String errorMsg) {
         ContentValues values = new ContentValues();
         values.put(Downloads.Impl.COLUMN_STATUS, status);
         values.put(Downloads.Impl._DATA, filename);
-        if (uri != null) {
-            values.put(Downloads.Impl.COLUMN_URI, uri);
-        }
         values.put(Downloads.Impl.COLUMN_MIME_TYPE, mimeType);
         values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, mSystemFacade.currentTimeMillis());
         values.put(Constants.RETRY_AFTER_X_REDIRECT_COUNT, retryAfter);
@@ -1021,4 +861,12 @@ public class DownloadThread extends Thread {
             mPolicyDirty = true;
         }
     };
+
+    public static long getHeaderFieldLong(URLConnection conn, String field, long defaultValue) {
+        try {
+            return Long.parseLong(conn.getHeaderField(field));
+        } catch (NumberFormatException e) {
+            return defaultValue;
+        }
+    }
 }
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 359f6fa4..484c9256 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -78,6 +78,9 @@ public class Helpers {
             int destination,
             long contentLength,
             boolean isPublicApi, StorageManager storageManager) throws StopRequestException {
+        if (contentLength < 0) {
+            contentLength = 0;
+        }
         checkCanHandleDownload(context, mimeType, destination, isPublicApi);
         String path;
         File base = null;
-- 
cgit v1.2.3


From 5cff4ecb10e89e4fb39cd9e39b8753a31efbe3cc Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 6 Dec 2012 15:54:44 -0800
Subject: Cleaner I/O.

This cleans up writing of downloaded data by always writing through
OutputStream interface, which applies DRM if needed.  Hands I/O
streams along with method calls to give clearer chain of ownership.

Only retry writes once after verifying free space. Remove
checkCanHandleDownload() check, since most downloads are now using
public API.  Release DrmManagerClient sessions when finished.

Change-Id: I49e479089a8218690b556d31ec65a030930ad368
---
 .../providers/downloads/DownloadDrmHelper.java     |  58 ++-----
 .../providers/downloads/DownloadThread.java        | 156 ++++++++-----------
 .../providers/downloads/DrmConvertSession.java     | 171 ---------------------
 src/com/android/providers/downloads/Helpers.java   |  42 -----
 .../android/providers/downloads/OpenHelper.java    |  10 +-
 5 files changed, 86 insertions(+), 351 deletions(-)
 delete mode 100644 src/com/android/providers/downloads/DrmConvertSession.java

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadDrmHelper.java b/src/com/android/providers/downloads/DownloadDrmHelper.java
index 10cb792c..d1358246 100644
--- a/src/com/android/providers/downloads/DownloadDrmHelper.java
+++ b/src/com/android/providers/downloads/DownloadDrmHelper.java
@@ -19,7 +19,8 @@ package com.android.providers.downloads;
 
 import android.content.Context;
 import android.drm.DrmManagerClient;
-import android.util.Log;
+
+import java.io.File;
 
 public class DownloadDrmHelper {
 
@@ -31,31 +32,6 @@ public class DownloadDrmHelper {
 
     public static final String EXTENSION_INTERNAL_FWDL = ".fl";
 
-    /**
-     * Checks if the Media Type is a DRM Media Type
-     *
-     * @param drmManagerClient A DrmManagerClient
-     * @param mimetype Media Type to check
-     * @return True if the Media Type is DRM else false
-     */
-    public static boolean isDrmMimeType(Context context, String mimetype) {
-        boolean result = false;
-        if (context != null) {
-            try {
-                DrmManagerClient drmClient = new DrmManagerClient(context);
-                if (drmClient != null && mimetype != null && mimetype.length() > 0) {
-                    result = drmClient.canHandle("", mimetype);
-                }
-            } catch (IllegalArgumentException e) {
-                Log.w(Constants.TAG,
-                        "DrmManagerClient instance could not be created, context is Illegal.");
-            } catch (IllegalStateException e) {
-                Log.w(Constants.TAG, "DrmManagerClient didn't initialize properly.");
-            }
-        }
-        return result;
-    }
-
     /**
      * Checks if the Media Type needs to be DRM converted
      *
@@ -83,28 +59,20 @@ public class DownloadDrmHelper {
     }
 
     /**
-     * Gets the original mime type of DRM protected content.
-     *
-     * @param context The context
-     * @param path Path to the file
-     * @param containingMime The current mime type of of the file i.e. the
-     *            containing mime type
-     * @return The original mime type of the file if DRM protected else the
-     *         currentMime
+     * Return the original MIME type of the given file, using the DRM framework
+     * if the file is protected content.
      */
-    public static String getOriginalMimeType(Context context, String path, String containingMime) {
-        String result = containingMime;
-        DrmManagerClient drmClient = new DrmManagerClient(context);
+    public static String getOriginalMimeType(Context context, File file, String currentMime) {
+        final DrmManagerClient client = new DrmManagerClient(context);
         try {
-            if (drmClient.canHandle(path, null)) {
-                result = drmClient.getOriginalMimeType(path);
+            final String rawFile = file.toString();
+            if (client.canHandle(rawFile, null)) {
+                return client.getOriginalMimeType(rawFile);
+            } else {
+                return currentMime;
             }
-        } catch (IllegalArgumentException ex) {
-            Log.w(Constants.TAG,
-                    "Can't get original mime type since path is null or empty string.");
-        } catch (IllegalStateException ex) {
-            Log.w(Constants.TAG, "DrmManagerClient didn't initialize properly.");
+        } finally {
+            client.release();
         }
-        return result;
     }
 }
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index cab1a3e6..2abadb0b 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -25,6 +25,8 @@ import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 import android.content.ContentValues;
 import android.content.Context;
 import android.content.Intent;
+import android.drm.DrmManagerClient;
+import android.drm.DrmOutputStream;
 import android.net.INetworkPolicyListener;
 import android.net.NetworkPolicyManager;
 import android.net.TrafficStats;
@@ -42,9 +44,8 @@ import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.io.SyncFailedException;
-import java.net.CookieHandler;
-import java.net.CookieManager;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
@@ -65,7 +66,6 @@ public class DownloadThread extends Thread {
     private final DownloadInfo mInfo;
     private final SystemFacade mSystemFacade;
     private final StorageManager mStorageManager;
-    private DrmConvertSession mDrmConvertSession;
 
     private volatile boolean mPolicyDirty;
 
@@ -93,7 +93,6 @@ public class DownloadThread extends Thread {
      */
     static class State {
         public String mFilename;
-        public FileOutputStream mStream;
         public String mMimeType;
         public boolean mCountRetry = false;
         public int mRetryAfter = 0;
@@ -236,10 +235,8 @@ public class DownloadThread extends Thread {
      * Fully execute a single download request - setup and send the request, handle the response,
      * and transfer the data to the destination file.
      */
-    private void executeDownload(State state, HttpURLConnection conn)
-            throws IOException, StopRequestException {
+    private void executeDownload(State state, HttpURLConnection conn) throws StopRequestException {
         final InnerState innerState = new InnerState();
-        final byte data[] = new byte[Constants.BUFFER_SIZE];
 
         setupDestinationFile(state, innerState);
         addRequestHeaders(state, conn);
@@ -254,14 +251,44 @@ public class DownloadThread extends Thread {
         // check just before sending the request to avoid using an invalid connection at all
         checkConnectivity();
 
-        // Asking for response code will execute the request
-        final int statusCode = conn.getResponseCode();
+        InputStream in = null;
+        OutputStream out = null;
+        DrmManagerClient drmClient = null;
+        try {
+            try {
+                // Asking for response code will execute the request
+                final int statusCode = conn.getResponseCode();
+                in = conn.getInputStream();
+
+                handleExceptionalStatus(state, innerState, conn, statusCode);
+                processResponseHeaders(state, innerState, conn);
+            } catch (IOException e) {
+                throw new StopRequestException(
+                        Downloads.Impl.STATUS_HTTP_DATA_ERROR, "Request failed: " + e, e);
+            }
 
-        handleExceptionalStatus(state, innerState, conn, statusCode);
-        processResponseHeaders(state, innerState, conn);
+            try {
+                if (DownloadDrmHelper.isDrmConvertNeeded(state.mMimeType)) {
+                    drmClient = new DrmManagerClient(mContext);
+                    out = new DrmOutputStream(
+                            drmClient, new File(state.mFilename), state.mMimeType);
+                } else {
+                    out = new FileOutputStream(state.mFilename);
+                }
+            } catch (IOException e) {
+                throw new StopRequestException(
+                        Downloads.Impl.STATUS_FILE_ERROR, "Failed to open destination: " + e, e);
+            }
 
-        final InputStream in = conn.getInputStream();
-        transferData(state, innerState, data, in);
+            transferData(state, innerState, in, out);
+
+        } finally {
+            if (drmClient != null) {
+                drmClient.release();
+            }
+            IoUtils.closeQuietly(in);
+            IoUtils.closeQuietly(out);
+        }
     }
 
     /**
@@ -287,22 +314,21 @@ public class DownloadThread extends Thread {
     }
 
     /**
-     * Transfer as much data as possible from the HTTP response to the destination file.
-     * @param data buffer to use to read data
-     * @param entityStream stream for reading the HTTP response entity
+     * Transfer as much data as possible from the HTTP response to the
+     * destination file.
      */
-    private void transferData(
-            State state, InnerState innerState, byte[] data, InputStream entityStream)
+    private void transferData(State state, InnerState innerState, InputStream in, OutputStream out)
             throws StopRequestException {
+        final byte data[] = new byte[Constants.BUFFER_SIZE];
         for (;;) {
-            int bytesRead = readFromResponse(state, innerState, data, entityStream);
+            int bytesRead = readFromResponse(state, innerState, data, in);
             if (bytesRead == -1) { // success, end of stream already reached
                 handleEndOfStream(state, innerState);
                 return;
             }
 
             state.mGotData = true;
-            writeDataToDestination(state, data, bytesRead);
+            writeDataToDestination(state, data, bytesRead, out);
             state.mCurrentBytes += bytesRead;
             reportProgress(state, innerState);
 
@@ -331,11 +357,6 @@ public class DownloadThread extends Thread {
      * the downloaded file.
      */
     private void cleanupDestination(State state, int finalStatus) {
-        if (mDrmConvertSession != null) {
-            finalStatus = mDrmConvertSession.close(state.mFilename);
-        }
-
-        closeDestination(state);
         if (state.mFilename != null && Downloads.Impl.isStatusError(finalStatus)) {
             if (Constants.LOGVV) {
                 Log.d(TAG, "cleanupDestination() deleting " + state.mFilename);
@@ -366,14 +387,6 @@ public class DownloadThread extends Thread {
         }
     }
 
-    /**
-     * Close the destination output stream.
-     */
-    private void closeDestination(State state) {
-        IoUtils.closeQuietly(state.mStream);
-        state.mStream = null;
-    }
-
     /**
      * Check if the download has been paused or canceled, stopping the request appropriately if it
      * has been.
@@ -433,37 +446,25 @@ public class DownloadThread extends Thread {
      * @param data buffer containing the data to write
      * @param bytesRead how many bytes to write from the buffer
      */
-    private void writeDataToDestination(State state, byte[] data, int bytesRead)
+    private void writeDataToDestination(State state, byte[] data, int bytesRead, OutputStream out)
             throws StopRequestException {
-        for (;;) {
+        mStorageManager.verifySpaceBeforeWritingToFile(
+                mInfo.mDestination, state.mFilename, bytesRead);
+
+        boolean forceVerified = false;
+        while (true) {
             try {
-                if (state.mStream == null) {
-                    state.mStream = new FileOutputStream(state.mFilename, true);
-                }
-                mStorageManager.verifySpaceBeforeWritingToFile(mInfo.mDestination, state.mFilename,
-                        bytesRead);
-                if (!DownloadDrmHelper.isDrmConvertNeeded(mInfo.mMimeType)) {
-                    state.mStream.write(data, 0, bytesRead);
-                } else {
-                    byte[] convertedData = mDrmConvertSession.convert(data, bytesRead);
-                    if (convertedData != null) {
-                        state.mStream.write(convertedData, 0, convertedData.length);
-                    } else {
-                        throw new StopRequestException(Downloads.Impl.STATUS_FILE_ERROR,
-                                "Error converting drm data.");
-                    }
-                }
+                out.write(data, 0, bytesRead);
                 return;
             } catch (IOException ex) {
-                // couldn't write to file. are we out of space? check.
-                // TODO this check should only be done once. why is this being done
-                // in a while(true) loop (see the enclosing statement: for(;;)
-                if (state.mStream != null) {
+                // TODO: better differentiate between DRM and disk failures
+                if (!forceVerified) {
+                    // couldn't write to file. are we out of space? check.
                     mStorageManager.verifySpace(mInfo.mDestination, state.mFilename, bytesRead);
-                }
-            } finally {
-                if (mInfo.mDestination == Downloads.Impl.DESTINATION_EXTERNAL) {
-                    closeDestination(state);
+                    forceVerified = true;
+                } else {
+                    throw new StopRequestException(Downloads.Impl.STATUS_FILE_ERROR,
+                            "Failed to write data: " + ex);
                 }
             }
         }
@@ -486,7 +487,7 @@ public class DownloadThread extends Thread {
         if (lengthMismatched) {
             if (cannotResume(state)) {
                 throw new StopRequestException(Downloads.Impl.STATUS_CANNOT_RESUME,
-                        "mismatched content length");
+                        "mismatched content length; unable to resume");
             } else {
                 throw new StopRequestException(getFinalStatusForHttpError(state),
                         "closed socket before end of file");
@@ -495,7 +496,8 @@ public class DownloadThread extends Thread {
     }
 
     private boolean cannotResume(State state) {
-        return state.mCurrentBytes > 0 && !mInfo.mNoIntegrity && state.mHeaderETag == null;
+        return (state.mCurrentBytes > 0 && !mInfo.mNoIntegrity && state.mHeaderETag == null)
+                || DownloadDrmHelper.isDrmConvertNeeded(state.mMimeType);
     }
 
     /**
@@ -513,13 +515,11 @@ public class DownloadThread extends Thread {
             values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, state.mCurrentBytes);
             mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
             if (cannotResume(state)) {
-                String message = "while reading response: " + ex.toString()
-                + ", can't resume interrupted download with no ETag";
                 throw new StopRequestException(Downloads.Impl.STATUS_CANNOT_RESUME,
-                        message, ex);
+                        "Failed reading response: " + ex + "; unable to resume", ex);
             } else {
                 throw new StopRequestException(getFinalStatusForHttpError(state),
-                        "while reading response: " + ex.toString(), ex);
+                        "Failed reading response: " + ex, ex);
             }
         }
     }
@@ -536,13 +536,6 @@ public class DownloadThread extends Thread {
         }
 
         readResponseHeaders(state, innerState, conn);
-        if (DownloadDrmHelper.isDrmConvertNeeded(state.mMimeType)) {
-            mDrmConvertSession = DrmConvertSession.open(mContext, state.mMimeType);
-            if (mDrmConvertSession == null) {
-                throw new StopRequestException(Downloads.Impl.STATUS_NOT_ACCEPTABLE, "Mimetype "
-                        + state.mMimeType + " can not be converted.");
-            }
-        }
 
         state.mFilename = Helpers.generateSaveFile(
                 mContext,
@@ -554,15 +547,6 @@ public class DownloadThread extends Thread {
                 mInfo.mDestination,
                 innerState.mContentLength,
                 mInfo.mIsPublicApi, mStorageManager);
-        try {
-            state.mStream = new FileOutputStream(state.mFilename);
-        } catch (FileNotFoundException exc) {
-            throw new StopRequestException(Downloads.Impl.STATUS_FILE_ERROR,
-                    "while opening destination file: " + exc.toString(), exc);
-        }
-        if (Constants.LOGV) {
-            Log.v(Constants.TAG, "writing " + mInfo.mUri + " to " + state.mFilename);
-        }
 
         updateDatabaseFromHeaders(state, innerState);
         // check connectivity again now that we know the total size
@@ -757,12 +741,6 @@ public class DownloadThread extends Thread {
                         Log.i(Constants.TAG, "resuming download for id: " + mInfo.mId +
                                 ", and starting with file of length: " + fileLength);
                     }
-                    try {
-                        state.mStream = new FileOutputStream(state.mFilename, true);
-                    } catch (FileNotFoundException exc) {
-                        throw new StopRequestException(Downloads.Impl.STATUS_FILE_ERROR,
-                                "while opening destination for resuming: " + exc.toString(), exc);
-                    }
                     state.mCurrentBytes = (int) fileLength;
                     if (mInfo.mTotalBytes != -1) {
                         innerState.mContentLength = mInfo.mTotalBytes;
@@ -777,10 +755,6 @@ public class DownloadThread extends Thread {
                 }
             }
         }
-
-        if (state.mStream != null && mInfo.mDestination == Downloads.Impl.DESTINATION_EXTERNAL) {
-            closeDestination(state);
-        }
     }
 
     /**
diff --git a/src/com/android/providers/downloads/DrmConvertSession.java b/src/com/android/providers/downloads/DrmConvertSession.java
deleted file mode 100644
index d10edf14..00000000
--- a/src/com/android/providers/downloads/DrmConvertSession.java
+++ /dev/null
@@ -1,171 +0,0 @@
-/*
- * Copyright (C) 2011 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-package com.android.providers.downloads;
-
-import android.content.Context;
-import android.drm.DrmConvertedStatus;
-import android.drm.DrmManagerClient;
-import android.util.Log;
-import android.provider.Downloads;
-
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.RandomAccessFile;
-
-
-public class DrmConvertSession {
-    private DrmManagerClient mDrmClient;
-    private int mConvertSessionId;
-
-    private DrmConvertSession(DrmManagerClient drmClient, int convertSessionId) {
-        mDrmClient = drmClient;
-        mConvertSessionId = convertSessionId;
-    }
-
-    /**
-     * Start of converting a file.
-     *
-     * @param context The context of the application running the convert session.
-     * @param mimeType Mimetype of content that shall be converted.
-     * @return A convert session or null in case an error occurs.
-     */
-    public static DrmConvertSession open(Context context, String mimeType) {
-        DrmManagerClient drmClient = null;
-        int convertSessionId = -1;
-        if (context != null && mimeType != null && !mimeType.equals("")) {
-            try {
-                drmClient = new DrmManagerClient(context);
-                try {
-                    convertSessionId = drmClient.openConvertSession(mimeType);
-                } catch (IllegalArgumentException e) {
-                    Log.w(Constants.TAG, "Conversion of Mimetype: " + mimeType
-                            + " is not supported.", e);
-                } catch (IllegalStateException e) {
-                    Log.w(Constants.TAG, "Could not access Open DrmFramework.", e);
-                }
-            } catch (IllegalArgumentException e) {
-                Log.w(Constants.TAG,
-                        "DrmManagerClient instance could not be created, context is Illegal.");
-            } catch (IllegalStateException e) {
-                Log.w(Constants.TAG, "DrmManagerClient didn't initialize properly.");
-            }
-        }
-
-        if (drmClient == null || convertSessionId < 0) {
-            return null;
-        } else {
-            return new DrmConvertSession(drmClient, convertSessionId);
-        }
-    }
-    /**
-     * Convert a buffer of data to protected format.
-     *
-     * @param buffer Buffer filled with data to convert.
-     * @param size The number of bytes that shall be converted.
-     * @return A Buffer filled with converted data, if execution is ok, in all
-     *         other case null.
-     */
-    public byte [] convert(byte[] inBuffer, int size) {
-        byte[] result = null;
-        if (inBuffer != null) {
-            DrmConvertedStatus convertedStatus = null;
-            try {
-                if (size != inBuffer.length) {
-                    byte[] buf = new byte[size];
-                    System.arraycopy(inBuffer, 0, buf, 0, size);
-                    convertedStatus = mDrmClient.convertData(mConvertSessionId, buf);
-                } else {
-                    convertedStatus = mDrmClient.convertData(mConvertSessionId, inBuffer);
-                }
-
-                if (convertedStatus != null &&
-                        convertedStatus.statusCode == DrmConvertedStatus.STATUS_OK &&
-                        convertedStatus.convertedData != null) {
-                    result = convertedStatus.convertedData;
-                }
-            } catch (IllegalArgumentException e) {
-                Log.w(Constants.TAG, "Buffer with data to convert is illegal. Convertsession: "
-                        + mConvertSessionId, e);
-            } catch (IllegalStateException e) {
-                Log.w(Constants.TAG, "Could not convert data. Convertsession: " +
-                        mConvertSessionId, e);
-            }
-        } else {
-            throw new IllegalArgumentException("Parameter inBuffer is null");
-        }
-        return result;
-    }
-
-    /**
-     * Ends a conversion session of a file.
-     *
-     * @param fileName The filename of the converted file.
-     * @return Downloads.Impl.STATUS_SUCCESS if execution is ok.
-     *         Downloads.Impl.STATUS_FILE_ERROR in case converted file can not
-     *         be accessed. Downloads.Impl.STATUS_NOT_ACCEPTABLE if a problem
-     *         occurs when accessing drm framework.
-     *         Downloads.Impl.STATUS_UNKNOWN_ERROR if a general error occurred.
-     */
-    public int close(String filename) {
-        DrmConvertedStatus convertedStatus = null;
-        int result = Downloads.Impl.STATUS_UNKNOWN_ERROR;
-        if (mDrmClient != null && mConvertSessionId >= 0) {
-            try {
-                convertedStatus = mDrmClient.closeConvertSession(mConvertSessionId);
-                if (convertedStatus == null ||
-                        convertedStatus.statusCode != DrmConvertedStatus.STATUS_OK ||
-                        convertedStatus.convertedData == null) {
-                    result = Downloads.Impl.STATUS_NOT_ACCEPTABLE;
-                } else {
-                    RandomAccessFile rndAccessFile = null;
-                    try {
-                        rndAccessFile = new RandomAccessFile(filename, "rw");
-                        rndAccessFile.seek(convertedStatus.offset);
-                        rndAccessFile.write(convertedStatus.convertedData);
-                        result = Downloads.Impl.STATUS_SUCCESS;
-                    } catch (FileNotFoundException e) {
-                        result = Downloads.Impl.STATUS_FILE_ERROR;
-                        Log.w(Constants.TAG, "File: " + filename + " could not be found.", e);
-                    } catch (IOException e) {
-                        result = Downloads.Impl.STATUS_FILE_ERROR;
-                        Log.w(Constants.TAG, "Could not access File: " + filename + " .", e);
-                    } catch (IllegalArgumentException e) {
-                        result = Downloads.Impl.STATUS_FILE_ERROR;
-                        Log.w(Constants.TAG, "Could not open file in mode: rw", e);
-                    } catch (SecurityException e) {
-                        Log.w(Constants.TAG, "Access to File: " + filename +
-                                " was denied denied by SecurityManager.", e);
-                    } finally {
-                        if (rndAccessFile != null) {
-                            try {
-                                rndAccessFile.close();
-                            } catch (IOException e) {
-                                result = Downloads.Impl.STATUS_FILE_ERROR;
-                                Log.w(Constants.TAG, "Failed to close File:" + filename
-                                        + ".", e);
-                            }
-                        }
-                    }
-                }
-            } catch (IllegalStateException e) {
-                Log.w(Constants.TAG, "Could not close convertsession. Convertsession: " +
-                        mConvertSessionId, e);
-            }
-        }
-        return result;
-    }
-}
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 484c9256..225b8d49 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -81,7 +81,6 @@ public class Helpers {
         if (contentLength < 0) {
             contentLength = 0;
         }
-        checkCanHandleDownload(context, mimeType, destination, isPublicApi);
         String path;
         File base = null;
         if (destination == Downloads.Impl.DESTINATION_FILE_URI) {
@@ -136,47 +135,6 @@ public class Helpers {
         return chooseUniqueFilename(destination, filename, extension, recoveryDir);
     }
 
-    private static void checkCanHandleDownload(Context context, String mimeType, int destination,
-            boolean isPublicApi) throws StopRequestException {
-        if (isPublicApi) {
-            return;
-        }
-
-        if (destination == Downloads.Impl.DESTINATION_EXTERNAL
-                || destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) {
-            if (mimeType == null) {
-                throw new StopRequestException(Downloads.Impl.STATUS_NOT_ACCEPTABLE,
-                        "external download with no mime type not allowed");
-            }
-            if (!DownloadDrmHelper.isDrmMimeType(context, mimeType)) {
-                // Check to see if we are allowed to download this file. Only files
-                // that can be handled by the platform can be downloaded.
-                // special case DRM files, which we should always allow downloading.
-                Intent intent = new Intent(Intent.ACTION_VIEW);
-
-                // We can provide data as either content: or file: URIs,
-                // so allow both.  (I think it would be nice if we just did
-                // everything as content: URIs)
-                // Actually, right now the download manager's UId restrictions
-                // prevent use from using content: so it's got to be file: or
-                // nothing
-
-                PackageManager pm = context.getPackageManager();
-                intent.setDataAndType(Uri.fromParts("file", "", null), mimeType);
-                ResolveInfo ri = pm.resolveActivity(intent, PackageManager.MATCH_DEFAULT_ONLY);
-                //Log.i(Constants.TAG, "*** FILENAME QUERY " + intent + ": " + list);
-
-                if (ri == null) {
-                    if (Constants.LOGV) {
-                        Log.v(Constants.TAG, "no handler found for type " + mimeType);
-                    }
-                    throw new StopRequestException(Downloads.Impl.STATUS_NOT_ACCEPTABLE,
-                            "no handler found for this download type");
-                }
-            }
-        }
-    }
-
     private static String chooseFilename(String url, String hint, String contentDisposition,
             String contentLocation, int destination) {
         String filename = null;
diff --git a/src/com/android/providers/downloads/OpenHelper.java b/src/com/android/providers/downloads/OpenHelper.java
index 7eca95c9..0d5f5e92 100644
--- a/src/com/android/providers/downloads/OpenHelper.java
+++ b/src/com/android/providers/downloads/OpenHelper.java
@@ -30,6 +30,8 @@ import android.database.Cursor;
 import android.net.Uri;
 import android.provider.Downloads.Impl.RequestHeaders;
 
+import java.io.File;
+
 public class OpenHelper {
     /**
      * Build an {@link Intent} to view the download at current {@link Cursor}
@@ -47,9 +49,9 @@ public class OpenHelper {
             }
 
             final Uri localUri = getCursorUri(cursor, COLUMN_LOCAL_URI);
-            final String filename = getCursorString(cursor, COLUMN_LOCAL_FILENAME);
+            final File file = getCursorFile(cursor, COLUMN_LOCAL_FILENAME);
             String mimeType = getCursorString(cursor, COLUMN_MEDIA_TYPE);
-            mimeType = DownloadDrmHelper.getOriginalMimeType(context, filename, mimeType);
+            mimeType = DownloadDrmHelper.getOriginalMimeType(context, file, mimeType);
 
             final Intent intent = new Intent(Intent.ACTION_VIEW);
             intent.setFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
@@ -122,4 +124,8 @@ public class OpenHelper {
     private static long getCursorLong(Cursor cursor, String column) {
         return cursor.getLong(cursor.getColumnIndexOrThrow(column));
     }
+
+    private static File getCursorFile(Cursor cursor, String column) {
+        return new File(cursor.getString(cursor.getColumnIndexOrThrow(column)));
+    }
 }
-- 
cgit v1.2.3


From a85832b4772626852142b60c4806ff5384a76478 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Mon, 17 Dec 2012 17:05:03 -0800
Subject: Always append to files, handle end of stream.

Fix bug where resumed downloads wouldn't open in append mode. Handle
end of stream exceptions from URLConnection as special-case for now
to keep tests passing.

Move stream creation outside of DrmOutputStream, and always fsync()
before closing files. Treat HTTP header errors as retryable. Add
explicit state checks to redirection tests.

Change-Id: I19d007284f6bfbffaac93859fe47cd98b79a59c4
---
 .../providers/downloads/DownloadThread.java        | 64 ++++++++++++----------
 1 file changed, 34 insertions(+), 30 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 2abadb0b..0f1d5f10 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -40,12 +40,12 @@ import android.util.Log;
 import android.util.Pair;
 
 import java.io.File;
-import java.io.FileNotFoundException;
+import java.io.FileDescriptor;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.io.SyncFailedException;
+import java.io.RandomAccessFile;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
@@ -251,9 +251,10 @@ public class DownloadThread extends Thread {
         // check just before sending the request to avoid using an invalid connection at all
         checkConnectivity();
 
+        DrmManagerClient drmClient = null;
         InputStream in = null;
         OutputStream out = null;
-        DrmManagerClient drmClient = null;
+        FileDescriptor outFd = null;
         try {
             try {
                 // Asking for response code will execute the request
@@ -264,16 +265,19 @@ public class DownloadThread extends Thread {
                 processResponseHeaders(state, innerState, conn);
             } catch (IOException e) {
                 throw new StopRequestException(
-                        Downloads.Impl.STATUS_HTTP_DATA_ERROR, "Request failed: " + e, e);
+                        getFinalStatusForHttpError(state), "Request failed: " + e, e);
             }
 
             try {
                 if (DownloadDrmHelper.isDrmConvertNeeded(state.mMimeType)) {
                     drmClient = new DrmManagerClient(mContext);
-                    out = new DrmOutputStream(
-                            drmClient, new File(state.mFilename), state.mMimeType);
+                    final RandomAccessFile file = new RandomAccessFile(
+                            new File(state.mFilename), "rw");
+                    out = new DrmOutputStream(drmClient, file, state.mMimeType);
+                    outFd = file.getFD();
                 } else {
-                    out = new FileOutputStream(state.mFilename);
+                    out = new FileOutputStream(state.mFilename, true);
+                    outFd = ((FileOutputStream) out).getFD();
                 }
             } catch (IOException e) {
                 throw new StopRequestException(
@@ -282,12 +286,29 @@ public class DownloadThread extends Thread {
 
             transferData(state, innerState, in, out);
 
+            try {
+                if (out instanceof DrmOutputStream) {
+                    ((DrmOutputStream) out).finish();
+                }
+            } catch (IOException e) {
+                throw new StopRequestException(
+                        Downloads.Impl.STATUS_FILE_ERROR, "Failed to finish: " + e, e);
+            }
+
         } finally {
             if (drmClient != null) {
                 drmClient.release();
             }
+
             IoUtils.closeQuietly(in);
-            IoUtils.closeQuietly(out);
+
+            try {
+                if (out != null) out.flush();
+                if (outFd != null) outFd.sync();
+            } catch (IOException e) {
+            } finally {
+                IoUtils.closeQuietly(out);
+            }
         }
     }
 
@@ -348,7 +369,6 @@ public class DownloadThread extends Thread {
         if (state.mFilename != null) {
             // make sure the file is readable
             FileUtils.setPermissions(state.mFilename, 0644, -1, -1);
-            syncDestination(state);
         }
     }
 
@@ -366,27 +386,6 @@ public class DownloadThread extends Thread {
         }
     }
 
-    /**
-     * Sync the destination file to storage.
-     */
-    private void syncDestination(State state) {
-        FileOutputStream downloadedFileStream = null;
-        try {
-            downloadedFileStream = new FileOutputStream(state.mFilename, true);
-            downloadedFileStream.getFD().sync();
-        } catch (FileNotFoundException ex) {
-            Log.w(Constants.TAG, "file " + state.mFilename + " not found: " + ex);
-        } catch (SyncFailedException ex) {
-            Log.w(Constants.TAG, "file " + state.mFilename + " sync failed: " + ex);
-        } catch (IOException ex) {
-            Log.w(Constants.TAG, "IOException trying to sync " + state.mFilename + ": " + ex);
-        } catch (RuntimeException ex) {
-            Log.w(Constants.TAG, "exception while syncing file: ", ex);
-        } finally {
-            IoUtils.closeQuietly(downloadedFileStream);
-        }
-    }
-
     /**
      * Check if the download has been paused or canceled, stopping the request appropriately if it
      * has been.
@@ -511,6 +510,11 @@ public class DownloadThread extends Thread {
         try {
             return entityStream.read(data);
         } catch (IOException ex) {
+            // TODO: handle stream errors the same as other retries
+            if ("unexpected end of stream".equals(ex.getMessage())) {
+                return -1;
+            }
+
             ContentValues values = new ContentValues();
             values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, state.mCurrentBytes);
             mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
-- 
cgit v1.2.3


From b62364ce70074dbdea03aafdc0f64c7ba39c3a69 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 20 Dec 2012 15:10:34 -0800
Subject: Fold InnerState into State.

It was cluttering up method signatures, and can easily be reset
before starting each download pass.

Change-Id: I7dee9d2160c3b5f737e7db86baa826d5d0b04b2d
---
 .../providers/downloads/DownloadThread.java        | 93 +++++++++++-----------
 1 file changed, 46 insertions(+), 47 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 0f1d5f10..eb59d3f7 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -112,6 +112,10 @@ public class DownloadThread extends Thread {
         /** Bytes transferred since current sample started. */
         public long mSpeedSampleBytes;
 
+        public long mContentLength = -1;
+        public String mContentDisposition;
+        public String mContentLocation;
+
         public State(DownloadInfo info) {
             mMimeType = Intent.normalizeMimeType(info.mMimeType);
             mRequestUri = info.mUri;
@@ -119,15 +123,13 @@ public class DownloadThread extends Thread {
             mTotalBytes = info.mTotalBytes;
             mCurrentBytes = info.mCurrentBytes;
         }
-    }
 
-    /**
-     * State within executeDownload()
-     */
-    private static class InnerState {
-        public long mContentLength;
-        public String mContentDisposition;
-        public String mContentLocation;
+        public void resetBeforeExecute() {
+            // Reset any state from previous execution
+            mContentLength = -1;
+            mContentDisposition = null;
+            mContentLocation = null;
+        }
     }
 
     /**
@@ -236,9 +238,9 @@ public class DownloadThread extends Thread {
      * and transfer the data to the destination file.
      */
     private void executeDownload(State state, HttpURLConnection conn) throws StopRequestException {
-        final InnerState innerState = new InnerState();
+        state.resetBeforeExecute();
 
-        setupDestinationFile(state, innerState);
+        setupDestinationFile(state);
         addRequestHeaders(state, conn);
 
         // skip when already finished; remove after fixing race in 5217390
@@ -261,8 +263,8 @@ public class DownloadThread extends Thread {
                 final int statusCode = conn.getResponseCode();
                 in = conn.getInputStream();
 
-                handleExceptionalStatus(state, innerState, conn, statusCode);
-                processResponseHeaders(state, innerState, conn);
+                handleExceptionalStatus(state, conn, statusCode);
+                processResponseHeaders(state, conn);
             } catch (IOException e) {
                 throw new StopRequestException(
                         getFinalStatusForHttpError(state), "Request failed: " + e, e);
@@ -284,7 +286,7 @@ public class DownloadThread extends Thread {
                         Downloads.Impl.STATUS_FILE_ERROR, "Failed to open destination: " + e, e);
             }
 
-            transferData(state, innerState, in, out);
+            transferData(state, in, out);
 
             try {
                 if (out instanceof DrmOutputStream) {
@@ -338,20 +340,20 @@ public class DownloadThread extends Thread {
      * Transfer as much data as possible from the HTTP response to the
      * destination file.
      */
-    private void transferData(State state, InnerState innerState, InputStream in, OutputStream out)
+    private void transferData(State state, InputStream in, OutputStream out)
             throws StopRequestException {
         final byte data[] = new byte[Constants.BUFFER_SIZE];
         for (;;) {
-            int bytesRead = readFromResponse(state, innerState, data, in);
+            int bytesRead = readFromResponse(state, data, in);
             if (bytesRead == -1) { // success, end of stream already reached
-                handleEndOfStream(state, innerState);
+                handleEndOfStream(state);
                 return;
             }
 
             state.mGotData = true;
             writeDataToDestination(state, data, bytesRead, out);
             state.mCurrentBytes += bytesRead;
-            reportProgress(state, innerState);
+            reportProgress(state);
 
             if (Constants.LOGVV) {
                 Log.v(Constants.TAG, "downloaded " + state.mCurrentBytes + " for "
@@ -410,7 +412,7 @@ public class DownloadThread extends Thread {
     /**
      * Report download progress through the database if necessary.
      */
-    private void reportProgress(State state, InnerState innerState) {
+    private void reportProgress(State state) {
         final long now = SystemClock.elapsedRealtime();
 
         final long sampleDelta = now - state.mSpeedSampleStart;
@@ -473,16 +475,16 @@ public class DownloadThread extends Thread {
      * Called when we've reached the end of the HTTP response stream, to update the database and
      * check for consistency.
      */
-    private void handleEndOfStream(State state, InnerState innerState) throws StopRequestException {
+    private void handleEndOfStream(State state) throws StopRequestException {
         ContentValues values = new ContentValues();
         values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, state.mCurrentBytes);
-        if (innerState.mContentLength == -1) {
+        if (state.mContentLength == -1) {
             values.put(Downloads.Impl.COLUMN_TOTAL_BYTES, state.mCurrentBytes);
         }
         mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
 
-        boolean lengthMismatched = (innerState.mContentLength != -1)
-                && (state.mCurrentBytes != innerState.mContentLength);
+        final boolean lengthMismatched = (state.mContentLength != -1)
+                && (state.mCurrentBytes != state.mContentLength);
         if (lengthMismatched) {
             if (cannotResume(state)) {
                 throw new StopRequestException(Downloads.Impl.STATUS_CANNOT_RESUME,
@@ -505,8 +507,8 @@ public class DownloadThread extends Thread {
      * @param entityStream stream for reading the HTTP response entity
      * @return the number of bytes actually read or -1 if the end of the stream has been reached
      */
-    private int readFromResponse(State state, InnerState innerState, byte[] data,
-                                 InputStream entityStream) throws StopRequestException {
+    private int readFromResponse(State state, byte[] data, InputStream entityStream)
+            throws StopRequestException {
         try {
             return entityStream.read(data);
         } catch (IOException ex) {
@@ -532,27 +534,27 @@ public class DownloadThread extends Thread {
      * Read HTTP response headers and take appropriate action, including setting up the destination
      * file and updating the database.
      */
-    private void processResponseHeaders(State state, InnerState innerState, HttpURLConnection conn)
+    private void processResponseHeaders(State state, HttpURLConnection conn)
             throws StopRequestException {
         if (state.mContinuingDownload) {
             // ignore response headers on resume requests
             return;
         }
 
-        readResponseHeaders(state, innerState, conn);
+        readResponseHeaders(state, conn);
 
         state.mFilename = Helpers.generateSaveFile(
                 mContext,
                 mInfo.mUri,
                 mInfo.mHint,
-                innerState.mContentDisposition,
-                innerState.mContentLocation,
+                state.mContentDisposition,
+                state.mContentLocation,
                 state.mMimeType,
                 mInfo.mDestination,
-                innerState.mContentLength,
+                state.mContentLength,
                 mInfo.mIsPublicApi, mStorageManager);
 
-        updateDatabaseFromHeaders(state, innerState);
+        updateDatabaseFromHeaders(state);
         // check connectivity again now that we know the total size
         checkConnectivity();
     }
@@ -561,7 +563,7 @@ public class DownloadThread extends Thread {
      * Update necessary database fields based on values of HTTP response headers that have been
      * read.
      */
-    private void updateDatabaseFromHeaders(State state, InnerState innerState) {
+    private void updateDatabaseFromHeaders(State state) {
         ContentValues values = new ContentValues();
         values.put(Downloads.Impl._DATA, state.mFilename);
         if (state.mHeaderETag != null) {
@@ -577,10 +579,10 @@ public class DownloadThread extends Thread {
     /**
      * Read headers from the HTTP response and store them into local state.
      */
-    private void readResponseHeaders(State state, InnerState innerState, HttpURLConnection conn)
+    private void readResponseHeaders(State state, HttpURLConnection conn)
             throws StopRequestException {
-        innerState.mContentDisposition = conn.getHeaderField("Content-Disposition");
-        innerState.mContentLocation = conn.getHeaderField("Content-Location");
+        state.mContentDisposition = conn.getHeaderField("Content-Disposition");
+        state.mContentLocation = conn.getHeaderField("Content-Location");
 
         if (state.mMimeType == null) {
             state.mMimeType = Intent.normalizeMimeType(conn.getContentType());
@@ -590,16 +592,16 @@ public class DownloadThread extends Thread {
 
         final String transferEncoding = conn.getHeaderField("Transfer-Encoding");
         if (transferEncoding == null) {
-            innerState.mContentLength = getHeaderFieldLong(conn, "Content-Length", -1);
+            state.mContentLength = getHeaderFieldLong(conn, "Content-Length", -1);
         } else {
             Log.i(TAG, "Ignoring Content-Length since Transfer-Encoding is also defined");
-            innerState.mContentLength = -1;
+            state.mContentLength = -1;
         }
 
-        state.mTotalBytes = innerState.mContentLength;
-        mInfo.mTotalBytes = innerState.mContentLength;
+        state.mTotalBytes = state.mContentLength;
+        mInfo.mTotalBytes = state.mContentLength;
 
-        final boolean noSizeInfo = innerState.mContentLength == -1
+        final boolean noSizeInfo = state.mContentLength == -1
                 && (transferEncoding == null || !transferEncoding.equalsIgnoreCase("chunked"));
         if (!mInfo.mNoIntegrity && noSizeInfo) {
             throw new StopRequestException(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
@@ -610,8 +612,7 @@ public class DownloadThread extends Thread {
     /**
      * Check the HTTP response status and handle anything unusual (e.g. not 200/206).
      */
-    private void handleExceptionalStatus(
-            State state, InnerState innerState, HttpURLConnection conn, int statusCode)
+    private void handleExceptionalStatus(State state, HttpURLConnection conn, int statusCode)
             throws StopRequestException {
         if (statusCode == HTTP_UNAVAILABLE && mInfo.mNumFailed < Constants.MAX_RETRIES) {
             handleServiceUnavailable(state, conn);
@@ -623,15 +624,14 @@ public class DownloadThread extends Thread {
         }
         int expectedStatus = state.mContinuingDownload ? HTTP_PARTIAL : HTTP_OK;
         if (statusCode != expectedStatus) {
-            handleOtherStatus(state, innerState, statusCode);
+            handleOtherStatus(state, statusCode);
         }
     }
 
     /**
      * Handle a status that we don't know how to deal with properly.
      */
-    private void handleOtherStatus(State state, InnerState innerState, int statusCode)
-            throws StopRequestException {
+    private void handleOtherStatus(State state, int statusCode) throws StopRequestException {
         if (statusCode == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) {
             // range request failed. it should never fail.
             throw new IllegalStateException("Http Range request failure: totalBytes = " +
@@ -697,8 +697,7 @@ public class DownloadThread extends Thread {
      * Prepare the destination file to receive data.  If the file already exists, we'll set up
      * appropriately for resumption.
      */
-    private void setupDestinationFile(State state, InnerState innerState)
-            throws StopRequestException {
+    private void setupDestinationFile(State state) throws StopRequestException {
         if (!TextUtils.isEmpty(state.mFilename)) { // only true if we've already run a thread for this download
             if (Constants.LOGV) {
                 Log.i(Constants.TAG, "have run thread before for id: " + mInfo.mId +
@@ -747,7 +746,7 @@ public class DownloadThread extends Thread {
                     }
                     state.mCurrentBytes = (int) fileLength;
                     if (mInfo.mTotalBytes != -1) {
-                        innerState.mContentLength = mInfo.mTotalBytes;
+                        state.mContentLength = mInfo.mTotalBytes;
                     }
                     state.mHeaderETag = mInfo.mETag;
                     state.mContinuingDownload = true;
-- 
cgit v1.2.3


From 701d66efeff513a7509eeaafab6e47f4f6edb857 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Sat, 5 Jan 2013 09:47:36 -0800
Subject: Remove singleton StorageManager.

Now DownloadService creates and owns the lifecycle of its own
StorageManager instance.

Change-Id: I8f6bedc02f1dbe610a8e6a25d55383a12716d344
---
 .../android/providers/downloads/DownloadInfo.java  | 16 +++++++++------
 .../providers/downloads/DownloadProvider.java      |  2 +-
 .../providers/downloads/DownloadService.java       |  4 ++--
 .../providers/downloads/StorageManager.java        | 24 ++++++----------------
 4 files changed, 19 insertions(+), 27 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 5172b696..2ea7d84d 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -54,8 +54,9 @@ public class DownloadInfo {
             mCursor = cursor;
         }
 
-        public DownloadInfo newDownloadInfo(Context context, SystemFacade systemFacade) {
-            DownloadInfo info = new DownloadInfo(context, systemFacade);
+        public DownloadInfo newDownloadInfo(Context context, SystemFacade systemFacade,
+                StorageManager storageManager) {
+            DownloadInfo info = new DownloadInfo(context, systemFacade, storageManager);
             updateFromDatabase(info);
             readRequestHeaders(info);
             return info;
@@ -229,12 +230,15 @@ public class DownloadInfo {
     public int mFuzz;
 
     private List<Pair<String, String>> mRequestHeaders = new ArrayList<Pair<String, String>>();
-    private SystemFacade mSystemFacade;
-    private Context mContext;
 
-    private DownloadInfo(Context context, SystemFacade systemFacade) {
+    private final Context mContext;
+    private final SystemFacade mSystemFacade;
+    private final StorageManager mStorageManager;
+
+    private DownloadInfo(Context context, SystemFacade systemFacade, StorageManager storageManager) {
         mContext = context;
         mSystemFacade = systemFacade;
+        mStorageManager = storageManager;
         mFuzz = Helpers.sRandom.nextInt(1001);
     }
 
@@ -572,7 +576,7 @@ public class DownloadInfo {
 
     void startDownloadThread() {
         DownloadThread downloader = new DownloadThread(mContext, mSystemFacade, this,
-                StorageManager.getInstance(mContext));
+                mStorageManager);
         mSystemFacade.startThread(downloader);
     }
 
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index c554e41d..7af8173b 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -446,7 +446,7 @@ public final class DownloadProvider extends ContentProvider {
         // saves us by getting some initialization code in DownloadService out of the way.
         Context context = getContext();
         context.startService(new Intent(context, DownloadService.class));
-        mDownloadsDataDir = StorageManager.getInstance(getContext()).getDownloadDataDirectory();
+        mDownloadsDataDir = StorageManager.getDownloadDataDirectory(getContext());
         return true;
     }
 
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index b97346b2..e0fe4c55 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -225,7 +225,7 @@ public class DownloadService extends Service {
         mNotifier = new DownloadNotifier(this);
         mNotifier.cancelAll();
 
-        mStorageManager = StorageManager.getInstance(getApplicationContext());
+        mStorageManager = new StorageManager(this);
         updateFromProvider();
     }
 
@@ -435,7 +435,7 @@ public class DownloadService extends Service {
      * download if appropriate.
      */
     private DownloadInfo insertDownloadLocked(DownloadInfo.Reader reader, long now) {
-        DownloadInfo info = reader.newDownloadInfo(this, mSystemFacade);
+        DownloadInfo info = reader.newDownloadInfo(this, mSystemFacade, mStorageManager);
         mDownloads.put(info.mId, info);
 
         if (Constants.LOGVV) {
diff --git a/src/com/android/providers/downloads/StorageManager.java b/src/com/android/providers/downloads/StorageManager.java
index 915d141b..8ca17300 100644
--- a/src/com/android/providers/downloads/StorageManager.java
+++ b/src/com/android/providers/downloads/StorageManager.java
@@ -71,12 +71,6 @@ class StorageManager {
      */
     private final File mDownloadDataDir;
 
-    /** the Singleton instance of this class.
-     * TODO: once DownloadService is refactored into a long-living object, there is no need
-     * for this Singleton'ing.
-     */
-    private static StorageManager sSingleton = null;
-
     /** how often do we need to perform checks on space to make sure space is available */
     private static final int FREQUENCY_OF_CHECKS_ON_SPACE_AVAILABILITY = 1024 * 1024; // 1MB
     private int mBytesDownloadedSinceLastCheckOnSpace = 0;
@@ -84,19 +78,9 @@ class StorageManager {
     /** misc members */
     private final Context mContext;
 
-    /**
-     * maintains Singleton instance of this class
-     */
-    synchronized static StorageManager getInstance(Context context) {
-        if (sSingleton == null) {
-            sSingleton = new StorageManager(context);
-        }
-        return sSingleton;
-    }
-
-    private StorageManager(Context context) { // constructor is private
+    public StorageManager(Context context) {
         mContext = context;
-        mDownloadDataDir = context.getCacheDir();
+        mDownloadDataDir = getDownloadDataDirectory(context);
         mExternalStorageDir = Environment.getExternalStorageDirectory();
         mSystemCacheDir = Environment.getDownloadCacheDirectory();
         startThreadToCleanupDatabaseAndPurgeFileSystem();
@@ -308,6 +292,10 @@ class StorageManager {
         return mDownloadDataDir;
     }
 
+    public static File getDownloadDataDirectory(Context context) {
+        return context.getCacheDir();
+    }
+
     /**
      * Deletes purgeable files from the cache partition. This also deletes
      * the matching database entries. Files are deleted in LRU order until
-- 
cgit v1.2.3


From 8ac10e0e0667a4fe35191deebb5fa9786bf4226c Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 3 Jan 2013 22:59:50 -0800
Subject: Clean up DownloadManager threading tests.

Change runUntilStatus() methods to polling with timeout instead of
requiring internal knowledge about threading.

Fix notification tests, and move opening of InputStream until after
handling headers to avoid FNFE.  Always reset facade to defaults
before each test.

Change-Id: I6b2d6cfc4e685d2090c1133b1b2e89ae12760f8b
---
 .../providers/downloads/DownloadHandler.java       | 24 ----------------------
 .../android/providers/downloads/DownloadInfo.java  |  5 ++---
 .../providers/downloads/DownloadService.java       |  2 +-
 .../providers/downloads/DownloadThread.java        |  3 +--
 .../providers/downloads/RealSystemFacade.java      |  9 ++++----
 .../android/providers/downloads/SystemFacade.java  |  5 -----
 6 files changed, 8 insertions(+), 40 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadHandler.java b/src/com/android/providers/downloads/DownloadHandler.java
index 2f02864e..c376ff1e 100644
--- a/src/com/android/providers/downloads/DownloadHandler.java
+++ b/src/com/android/providers/downloads/DownloadHandler.java
@@ -96,28 +96,4 @@ public class DownloadHandler {
     public synchronized long getCurrentSpeed(long id) {
         return mCurrentSpeed.get(id, -1L);
     }
-
-    // right now this is only used by tests. but there is no reason why it can't be used
-    // by any module using DownloadManager (TODO add API to DownloadManager.java)
-    public synchronized void waitUntilDownloadsTerminate() throws InterruptedException {
-        if (mDownloadsInProgress.size() == 0 && mDownloadsQueue.size() == 0) {
-            if (Constants.LOGVV) {
-                Log.i(TAG, "nothing to wait on");
-            }
-            return;
-        }
-        if (Constants.LOGVV) {
-            for (DownloadInfo info : mDownloadsInProgress.values()) {
-                Log.i(TAG, "** progress: " + info.mId + ", " + info.mUri);
-            }
-            for (DownloadInfo info : mDownloadsQueue.values()) {
-                Log.i(TAG, "** in Q: " + info.mId + ", " + info.mUri);
-            }
-        }
-        if (Constants.LOGVV) {
-            Log.i(TAG, "waiting for 5 sec");
-        }
-        // wait upto 5 sec
-        wait(5 * 1000);
-    }
 }
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 2ea7d84d..e64db289 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -575,9 +575,8 @@ public class DownloadInfo {
     }
 
     void startDownloadThread() {
-        DownloadThread downloader = new DownloadThread(mContext, mSystemFacade, this,
-                mStorageManager);
-        mSystemFacade.startThread(downloader);
+        // TODO: keep this thread strongly referenced
+        new DownloadThread(mContext, mSystemFacade, this, mStorageManager).start();
     }
 
     /**
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index e0fe4c55..4a1b40d5 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -259,7 +259,7 @@ public class DownloadService extends Service {
             mPendingUpdate = true;
             if (mUpdateThread == null) {
                 mUpdateThread = new UpdateThread();
-                mSystemFacade.startThread(mUpdateThread);
+                mUpdateThread.start();
             }
         }
     }
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index eb59d3f7..ae279260 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -261,10 +261,9 @@ public class DownloadThread extends Thread {
             try {
                 // Asking for response code will execute the request
                 final int statusCode = conn.getResponseCode();
-                in = conn.getInputStream();
-
                 handleExceptionalStatus(state, conn, statusCode);
                 processResponseHeaders(state, conn);
+                in = conn.getInputStream();
             } catch (IOException e) {
                 throw new StopRequestException(
                         getFinalStatusForHttpError(state), "Request failed: " + e, e);
diff --git a/src/com/android/providers/downloads/RealSystemFacade.java b/src/com/android/providers/downloads/RealSystemFacade.java
index 228c7165..fa4f3488 100644
--- a/src/com/android/providers/downloads/RealSystemFacade.java
+++ b/src/com/android/providers/downloads/RealSystemFacade.java
@@ -32,10 +32,12 @@ class RealSystemFacade implements SystemFacade {
         mContext = context;
     }
 
+    @Override
     public long currentTimeMillis() {
         return System.currentTimeMillis();
     }
 
+    @Override
     public NetworkInfo getActiveNetworkInfo(int uid) {
         ConnectivityManager connectivity =
                 (ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE);
@@ -57,6 +59,7 @@ class RealSystemFacade implements SystemFacade {
         return conn.isActiveNetworkMetered();
     }
 
+    @Override
     public boolean isNetworkRoaming() {
         ConnectivityManager connectivity =
             (ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE);
@@ -74,6 +77,7 @@ class RealSystemFacade implements SystemFacade {
         return isRoaming;
     }
 
+    @Override
     public Long getMaxBytesOverMobile() {
         return DownloadManager.getMaxBytesOverMobile(mContext);
     }
@@ -92,9 +96,4 @@ class RealSystemFacade implements SystemFacade {
     public boolean userOwnsPackage(int uid, String packageName) throws NameNotFoundException {
         return mContext.getPackageManager().getApplicationInfo(packageName, 0).uid == uid;
     }
-
-    @Override
-    public void startThread(Thread thread) {
-        thread.start();
-    }
 }
diff --git a/src/com/android/providers/downloads/SystemFacade.java b/src/com/android/providers/downloads/SystemFacade.java
index fda97e08..15fc31f9 100644
--- a/src/com/android/providers/downloads/SystemFacade.java
+++ b/src/com/android/providers/downloads/SystemFacade.java
@@ -61,9 +61,4 @@ interface SystemFacade {
      * Returns true if the specified UID owns the specified package name.
      */
     public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException;
-
-    /**
-     * Start a thread.
-     */
-    public void startThread(Thread thread);
 }
-- 
cgit v1.2.3


From 0de55602ec6d350548248feddc68c91b29326eff Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Sun, 23 Dec 2012 19:28:09 -0800
Subject: Simplify download flow control, handle redirects.

Move redirection handling into a single loop, and handle each HTTP
response code inline to make flow control easier to reason about.

Fix race condition in tests by waiting for first status update.

Bug: 7887226
Change-Id: Id4bfd182941baad4cd0bb702376c4beeb7275bb2
---
 .../android/providers/downloads/DownloadInfo.java  |   3 +
 .../providers/downloads/DownloadThread.java        | 219 ++++++++++++---------
 src/com/android/providers/downloads/Helpers.java   |   3 +
 .../providers/downloads/StopRequestException.java  |  29 ++-
 4 files changed, 153 insertions(+), 101 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index e64db289..eb4ef0a5 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -45,6 +45,9 @@ import java.util.List;
  * Stores information about an individual download.
  */
 public class DownloadInfo {
+    // TODO: move towards these in-memory objects being sources of truth, and
+    // periodically pushing to provider.
+
     public static class Reader {
         private ContentResolver mResolver;
         private Cursor mCursor;
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index ae279260..dc2ef571 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -16,10 +16,21 @@
 
 package com.android.providers.downloads;
 
+import static android.provider.Downloads.Impl.STATUS_BAD_REQUEST;
+import static android.provider.Downloads.Impl.STATUS_CANNOT_RESUME;
+import static android.provider.Downloads.Impl.STATUS_FILE_ERROR;
+import static android.provider.Downloads.Impl.STATUS_HTTP_DATA_ERROR;
+import static android.provider.Downloads.Impl.STATUS_TOO_MANY_REDIRECTS;
+import static android.provider.Downloads.Impl.STATUS_WAITING_TO_RETRY;
 import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
+import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
+import static java.net.HttpURLConnection.HTTP_MOVED_PERM;
+import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.net.HttpURLConnection.HTTP_PARTIAL;
+import static java.net.HttpURLConnection.HTTP_PRECON_FAILED;
+import static java.net.HttpURLConnection.HTTP_SEE_OTHER;
 import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 
 import android.content.ContentValues;
@@ -47,10 +58,12 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.RandomAccessFile;
 import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
 
 import libcore.io.IoUtils;
+import libcore.net.http.HttpEngine;
 
 /**
  * Thread which executes a given {@link DownloadInfo}: making network requests,
@@ -59,6 +72,7 @@ import libcore.io.IoUtils;
 public class DownloadThread extends Thread {
 
     private static final int HTTP_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
+    private static final int HTTP_TEMP_REDIRECT = 307;
 
     private static final int DEFAULT_TIMEOUT = (int) MINUTE_IN_MILLIS;
 
@@ -116,6 +130,9 @@ public class DownloadThread extends Thread {
         public String mContentDisposition;
         public String mContentLocation;
 
+        public int mRedirectionCount;
+        public URL mUrl;
+
         public State(DownloadInfo info) {
             mMimeType = Intent.normalizeMimeType(info.mMimeType);
             mRequestUri = info.mUri;
@@ -129,6 +146,7 @@ public class DownloadThread extends Thread {
             mContentLength = -1;
             mContentDisposition = null;
             mContentLocation = null;
+            mRedirectionCount = 0;
         }
     }
 
@@ -169,31 +187,22 @@ public class DownloadThread extends Thread {
             // while performing download, register for rules updates
             netPolicy.registerListener(mPolicyListener);
 
-            if (Constants.LOGV) {
-                Log.v(Constants.TAG, "initiating download for " + mInfo.mUri);
-            }
+            Log.i(Constants.TAG, "Initiating download " + mInfo.mId);
 
-            // network traffic on this thread should be counted against the
-            // requesting uid, and is tagged with well-known value.
+            // Network traffic on this thread should be counted against the
+            // requesting UID, and is tagged with well-known value.
             TrafficStats.setThreadStatsTag(TrafficStats.TAG_SYSTEM_DOWNLOAD);
             TrafficStats.setThreadStatsUid(mInfo.mUid);
 
-            boolean finished = false;
-            while (!finished) {
-                Log.i(Constants.TAG, "Initiating request for download " + mInfo.mId);
-
-                final URL url = new URL(state.mRequestUri);
-                final HttpURLConnection conn = (HttpURLConnection) url.openConnection();
-                conn.setConnectTimeout(DEFAULT_TIMEOUT);
-                conn.setReadTimeout(DEFAULT_TIMEOUT);
-                try {
-                    executeDownload(state, conn);
-                    finished = true;
-                } finally {
-                    conn.disconnect();
-                }
+            try {
+                // TODO: migrate URL sanity checking into client side of API
+                state.mUrl = new URL(state.mRequestUri);
+            } catch (MalformedURLException e) {
+                throw new StopRequestException(STATUS_BAD_REQUEST, e);
             }
 
+            executeDownload(state);
+
             if (Constants.LOGV) {
                 Log.v(Constants.TAG, "download completed for " + mInfo.mUri);
             }
@@ -207,7 +216,7 @@ public class DownloadThread extends Thread {
             if (Constants.LOGV) {
                 Log.w(Constants.TAG, msg, error);
             }
-            finalStatus = error.mFinalStatus;
+            finalStatus = error.getFinalStatus();
             // fall through to finally block
         } catch (Throwable ex) {
             errorMsg = ex.getMessage();
@@ -234,14 +243,12 @@ public class DownloadThread extends Thread {
     }
 
     /**
-     * Fully execute a single download request - setup and send the request, handle the response,
-     * and transfer the data to the destination file.
+     * Fully execute a single download request. Setup and send the request,
+     * handle the response, and transfer the data to the destination file.
      */
-    private void executeDownload(State state, HttpURLConnection conn) throws StopRequestException {
+    private void executeDownload(State state) throws StopRequestException {
         state.resetBeforeExecute();
-
         setupDestinationFile(state);
-        addRequestHeaders(state, conn);
 
         // skip when already finished; remove after fixing race in 5217390
         if (state.mCurrentBytes == state.mTotalBytes) {
@@ -250,23 +257,96 @@ public class DownloadThread extends Thread {
             return;
         }
 
-        // check just before sending the request to avoid using an invalid connection at all
-        checkConnectivity();
+        // TODO: compare mInfo.mNumFailed against Constants.MAX_RETRIES
 
+        while (state.mRedirectionCount++ < HttpEngine.MAX_REDIRECTS) {
+            // Open connection and follow any redirects until we have a useful
+            // response with body.
+            HttpURLConnection conn = null;
+            try {
+                checkConnectivity();
+                conn = (HttpURLConnection) state.mUrl.openConnection();
+                conn.setInstanceFollowRedirects(false);
+                conn.setConnectTimeout(DEFAULT_TIMEOUT);
+                conn.setReadTimeout(DEFAULT_TIMEOUT);
+
+                addRequestHeaders(state, conn);
+
+                final int responseCode = conn.getResponseCode();
+                switch (responseCode) {
+                    case HTTP_OK:
+                        if (state.mContinuingDownload) {
+                            throw new StopRequestException(
+                                    STATUS_CANNOT_RESUME, "Expected partial, but received OK");
+                        }
+                        processResponseHeaders(state, conn);
+                        transferData(state, conn);
+                        return;
+
+                    case HTTP_PARTIAL:
+                        if (!state.mContinuingDownload) {
+                            throw new StopRequestException(
+                                    STATUS_CANNOT_RESUME, "Expected OK, but received partial");
+                        }
+                        transferData(state, conn);
+                        return;
+
+                    case HTTP_MOVED_PERM:
+                    case HTTP_MOVED_TEMP:
+                    case HTTP_SEE_OTHER:
+                    case HTTP_TEMP_REDIRECT:
+                        final String location = conn.getHeaderField("Location");
+                        state.mUrl = new URL(state.mUrl, location);
+                        continue;
+
+                    case HTTP_REQUESTED_RANGE_NOT_SATISFIABLE:
+                        throw new StopRequestException(
+                                STATUS_CANNOT_RESUME, "Requested range not satisfiable");
+
+                    case HTTP_PRECON_FAILED:
+                        // TODO: probably means our etag precondition was
+                        // changed; flush and retry again
+                        StopRequestException.throwUnhandledHttpError(responseCode);
+
+                    case HTTP_UNAVAILABLE:
+                        parseRetryAfterHeaders(state, conn);
+                        if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
+                            throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Unavailable");
+                        } else {
+                            throw new StopRequestException(STATUS_CANNOT_RESUME, "Unavailable");
+                        }
+
+                    case HTTP_INTERNAL_ERROR:
+                        throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Internal error");
+
+                    default:
+                        StopRequestException.throwUnhandledHttpError(responseCode);
+                }
+            } catch (IOException e) {
+                // Trouble with low-level sockets
+                throw new StopRequestException(STATUS_WAITING_TO_RETRY, e);
+
+            } finally {
+                if (conn != null) conn.disconnect();
+            }
+        }
+
+        throw new StopRequestException(STATUS_TOO_MANY_REDIRECTS, "Too many redirects");
+    }
+
+    /**
+     * Transfer data from the given connection to the destination file.
+     */
+    private void transferData(State state, HttpURLConnection conn) throws StopRequestException {
         DrmManagerClient drmClient = null;
         InputStream in = null;
         OutputStream out = null;
         FileDescriptor outFd = null;
         try {
             try {
-                // Asking for response code will execute the request
-                final int statusCode = conn.getResponseCode();
-                handleExceptionalStatus(state, conn, statusCode);
-                processResponseHeaders(state, conn);
                 in = conn.getInputStream();
             } catch (IOException e) {
-                throw new StopRequestException(
-                        getFinalStatusForHttpError(state), "Request failed: " + e, e);
+                throw new StopRequestException(STATUS_HTTP_DATA_ERROR, e);
             }
 
             try {
@@ -281,10 +361,11 @@ public class DownloadThread extends Thread {
                     outFd = ((FileOutputStream) out).getFD();
                 }
             } catch (IOException e) {
-                throw new StopRequestException(
-                        Downloads.Impl.STATUS_FILE_ERROR, "Failed to open destination: " + e, e);
+                throw new StopRequestException(STATUS_FILE_ERROR, e);
             }
 
+            // Start streaming data, periodically watch for pause/cancel
+            // commands and checking disk space as needed.
             transferData(state, in, out);
 
             try {
@@ -292,8 +373,7 @@ public class DownloadThread extends Thread {
                     ((DrmOutputStream) out).finish();
                 }
             } catch (IOException e) {
-                throw new StopRequestException(
-                        Downloads.Impl.STATUS_FILE_ERROR, "Failed to finish: " + e, e);
+                throw new StopRequestException(STATUS_FILE_ERROR, e);
             }
 
         } finally {
@@ -530,15 +610,12 @@ public class DownloadThread extends Thread {
     }
 
     /**
-     * Read HTTP response headers and take appropriate action, including setting up the destination
-     * file and updating the database.
+     * Prepare target file based on given network response. Derives filename and
+     * target size as needed.
      */
     private void processResponseHeaders(State state, HttpURLConnection conn)
             throws StopRequestException {
-        if (state.mContinuingDownload) {
-            // ignore response headers on resume requests
-            return;
-        }
+        // TODO: fallocate the entire file if header gave us specific length
 
         readResponseHeaders(state, conn);
 
@@ -608,53 +685,7 @@ public class DownloadThread extends Thread {
         }
     }
 
-    /**
-     * Check the HTTP response status and handle anything unusual (e.g. not 200/206).
-     */
-    private void handleExceptionalStatus(State state, HttpURLConnection conn, int statusCode)
-            throws StopRequestException {
-        if (statusCode == HTTP_UNAVAILABLE && mInfo.mNumFailed < Constants.MAX_RETRIES) {
-            handleServiceUnavailable(state, conn);
-        }
-
-        if (Constants.LOGV) {
-            Log.i(Constants.TAG, "recevd_status = " + statusCode +
-                    ", mContinuingDownload = " + state.mContinuingDownload);
-        }
-        int expectedStatus = state.mContinuingDownload ? HTTP_PARTIAL : HTTP_OK;
-        if (statusCode != expectedStatus) {
-            handleOtherStatus(state, statusCode);
-        }
-    }
-
-    /**
-     * Handle a status that we don't know how to deal with properly.
-     */
-    private void handleOtherStatus(State state, int statusCode) throws StopRequestException {
-        if (statusCode == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) {
-            // range request failed. it should never fail.
-            throw new IllegalStateException("Http Range request failure: totalBytes = " +
-                    state.mTotalBytes + ", bytes recvd so far: " + state.mCurrentBytes);
-        }
-        int finalStatus;
-        if (statusCode >= 400 && statusCode < 600) {
-            finalStatus = statusCode;
-        } else if (statusCode >= 300 && statusCode < 400) {
-            finalStatus = Downloads.Impl.STATUS_UNHANDLED_REDIRECT;
-        } else if (state.mContinuingDownload && statusCode == HTTP_OK) {
-            finalStatus = Downloads.Impl.STATUS_CANNOT_RESUME;
-        } else {
-            finalStatus = Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE;
-        }
-        throw new StopRequestException(finalStatus, "http error " +
-                statusCode + ", mContinuingDownload: " + state.mContinuingDownload);
-    }
-
-    /**
-     * Handle a 503 Service Unavailable status by processing the Retry-After header.
-     */
-    private void handleServiceUnavailable(State state, HttpURLConnection conn)
-            throws StopRequestException {
+    private void parseRetryAfterHeaders(State state, HttpURLConnection conn) {
         state.mCountRetry = true;
         state.mRetryAfter = conn.getHeaderFieldInt("Retry-After", -1);
         if (state.mRetryAfter < 0) {
@@ -668,9 +699,6 @@ public class DownloadThread extends Thread {
             state.mRetryAfter += Helpers.sRandom.nextInt(Constants.MIN_RETRY_AFTER + 1);
             state.mRetryAfter *= 1000;
         }
-
-        throw new StopRequestException(Downloads.Impl.STATUS_WAITING_TO_RETRY,
-                "got 503 Service Unavailable, will retry later");
     }
 
     private int getFinalStatusForHttpError(State state) {
@@ -774,11 +802,6 @@ public class DownloadThread extends Thread {
                 conn.addRequestProperty("If-Match", state.mHeaderETag);
             }
             conn.addRequestProperty("Range", "bytes=" + state.mCurrentBytes + "-");
-            if (Constants.LOGV) {
-                Log.i(Constants.TAG, "Adding Range header: " +
-                        "bytes=" + state.mCurrentBytes + "-");
-                Log.i(Constants.TAG, "  totalBytes = " + state.mTotalBytes);
-            }
         }
     }
 
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 225b8d49..059e9703 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -284,6 +284,9 @@ public class Helpers {
 
     private static String chooseUniqueFilename(int destination, String filename,
             String extension, boolean recoveryDir) throws StopRequestException {
+        // TODO: switch to actually creating the file here, otherwise we expose
+        // ourselves to race conditions.
+
         String fullFilename = filename + extension;
         if (!new File(fullFilename).exists()
                 && (!recoveryDir ||
diff --git a/src/com/android/providers/downloads/StopRequestException.java b/src/com/android/providers/downloads/StopRequestException.java
index 0ccf53cb..6df61dce 100644
--- a/src/com/android/providers/downloads/StopRequestException.java
+++ b/src/com/android/providers/downloads/StopRequestException.java
@@ -15,6 +15,9 @@
  */
 package com.android.providers.downloads;
 
+import static android.provider.Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE;
+import static android.provider.Downloads.Impl.STATUS_UNHANDLED_REDIRECT;
+
 /**
  * Raised to indicate that the current request should be stopped immediately.
  *
@@ -23,15 +26,35 @@ package com.android.providers.downloads;
  * URI, headers, or destination filename.
  */
 class StopRequestException extends Exception {
-    public int mFinalStatus;
+    private final int mFinalStatus;
 
     public StopRequestException(int finalStatus, String message) {
         super(message);
         mFinalStatus = finalStatus;
     }
 
-    public StopRequestException(int finalStatus, String message, Throwable throwable) {
-        super(message, throwable);
+    public StopRequestException(int finalStatus, Throwable t) {
+        super(t);
+        mFinalStatus = finalStatus;
+    }
+
+    public StopRequestException(int finalStatus, String message, Throwable t) {
+        super(message, t);
         mFinalStatus = finalStatus;
     }
+
+    public int getFinalStatus() {
+        return mFinalStatus;
+    }
+
+    public static StopRequestException throwUnhandledHttpError(int responseCode)
+            throws StopRequestException {
+        if (responseCode >= 400 && responseCode < 600) {
+            throw new StopRequestException(responseCode, "Unhandled HTTP response");
+        } else if (responseCode >= 300 && responseCode < 400) {
+            throw new StopRequestException(STATUS_UNHANDLED_REDIRECT, "Unhandled HTTP response");
+        } else {
+            throw new StopRequestException(STATUS_UNHANDLED_HTTP_CODE, "Unhandled HTTP response");
+        }
+    }
 }
-- 
cgit v1.2.3


From 89afc754d46a8574a9e014c7670746668de9f9b3 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 10 Jan 2013 11:12:52 -0800
Subject: Only add one User-Agent header.

Also include more details when reporting HTTP error codes.

Bug: 7966393
Change-Id: I251b1ec7c827693817391b6e9fb8b0cab995395e
---
 src/com/android/providers/downloads/DownloadThread.java     | 13 +++++++++----
 .../android/providers/downloads/StopRequestException.java   | 13 +++++++------
 2 files changed, 16 insertions(+), 10 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index dc2ef571..c77224a7 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -306,7 +306,8 @@ public class DownloadThread extends Thread {
                     case HTTP_PRECON_FAILED:
                         // TODO: probably means our etag precondition was
                         // changed; flush and retry again
-                        StopRequestException.throwUnhandledHttpError(responseCode);
+                        StopRequestException.throwUnhandledHttpError(
+                                responseCode, conn.getResponseMessage());
 
                     case HTTP_UNAVAILABLE:
                         parseRetryAfterHeaders(state, conn);
@@ -320,7 +321,8 @@ public class DownloadThread extends Thread {
                         throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Internal error");
 
                     default:
-                        StopRequestException.throwUnhandledHttpError(responseCode);
+                        StopRequestException.throwUnhandledHttpError(
+                                responseCode, conn.getResponseMessage());
                 }
             } catch (IOException e) {
                 // Trouble with low-level sockets
@@ -791,12 +793,15 @@ public class DownloadThread extends Thread {
      * Add custom headers for this download to the HTTP request.
      */
     private void addRequestHeaders(State state, HttpURLConnection conn) {
-        conn.addRequestProperty("User-Agent", userAgent());
-
         for (Pair<String, String> header : mInfo.getHeaders()) {
             conn.addRequestProperty(header.first, header.second);
         }
 
+        // Only splice in user agent when not already defined
+        if (conn.getRequestProperty("User-Agent") == null) {
+            conn.addRequestProperty("User-Agent", userAgent());
+        }
+
         if (state.mContinuingDownload) {
             if (state.mHeaderETag != null) {
                 conn.addRequestProperty("If-Match", state.mHeaderETag);
diff --git a/src/com/android/providers/downloads/StopRequestException.java b/src/com/android/providers/downloads/StopRequestException.java
index 6df61dce..a2b642d8 100644
--- a/src/com/android/providers/downloads/StopRequestException.java
+++ b/src/com/android/providers/downloads/StopRequestException.java
@@ -47,14 +47,15 @@ class StopRequestException extends Exception {
         return mFinalStatus;
     }
 
-    public static StopRequestException throwUnhandledHttpError(int responseCode)
+    public static StopRequestException throwUnhandledHttpError(int code, String message)
             throws StopRequestException {
-        if (responseCode >= 400 && responseCode < 600) {
-            throw new StopRequestException(responseCode, "Unhandled HTTP response");
-        } else if (responseCode >= 300 && responseCode < 400) {
-            throw new StopRequestException(STATUS_UNHANDLED_REDIRECT, "Unhandled HTTP response");
+        final String error = "Unhandled HTTP response: " + code + " " + message;
+        if (code >= 400 && code < 600) {
+            throw new StopRequestException(code, error);
+        } else if (code >= 300 && code < 400) {
+            throw new StopRequestException(STATUS_UNHANDLED_REDIRECT, error);
         } else {
-            throw new StopRequestException(STATUS_UNHANDLED_HTTP_CODE, "Unhandled HTTP response");
+            throw new StopRequestException(STATUS_UNHANDLED_HTTP_CODE, error);
         }
     }
 }
-- 
cgit v1.2.3


From 97862429de71477b5c4488faa911a2256b90089b Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Sat, 12 Jan 2013 15:01:04 -0800
Subject: Move network state to enums for type safety.

Change-Id: Ib8ea24fc58a866f8a5626cdd20e5891eb0a2bbeb
---
 .../android/providers/downloads/DownloadInfo.java  | 138 +++++++++------------
 .../providers/downloads/DownloadThread.java        |  21 ++--
 2 files changed, 68 insertions(+), 91 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index eb4ef0a5..10e27a71 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -150,44 +150,49 @@ public class DownloadInfo {
         }
     }
 
-    // the following NETWORK_* constants are used to indicates specfic reasons for disallowing a
-    // download from using a network, since specific causes can require special handling
-
-    /**
-     * The network is usable for the given download.
-     */
-    public static final int NETWORK_OK = 1;
-
-    /**
-     * There is no network connectivity.
-     */
-    public static final int NETWORK_NO_CONNECTION = 2;
-
-    /**
-     * The download exceeds the maximum size for this network.
-     */
-    public static final int NETWORK_UNUSABLE_DUE_TO_SIZE = 3;
-
-    /**
-     * The download exceeds the recommended maximum size for this network, the user must confirm for
-     * this download to proceed without WiFi.
-     */
-    public static final int NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE = 4;
-
-    /**
-     * The current connection is roaming, and the download can't proceed over a roaming connection.
-     */
-    public static final int NETWORK_CANNOT_USE_ROAMING = 5;
-
     /**
-     * The app requesting the download specific that it can't use the current network connection.
+     * Constants used to indicate network state for a specific download, after
+     * applying any requested constraints.
      */
-    public static final int NETWORK_TYPE_DISALLOWED_BY_REQUESTOR = 6;
-
-    /**
-     * Current network is blocked for requesting application.
-     */
-    public static final int NETWORK_BLOCKED = 7;
+    public enum NetworkState {
+        /**
+         * The network is usable for the given download.
+         */
+        OK,
+
+        /**
+         * There is no network connectivity.
+         */
+        NO_CONNECTION,
+
+        /**
+         * The download exceeds the maximum size for this network.
+         */
+        UNUSABLE_DUE_TO_SIZE,
+
+        /**
+         * The download exceeds the recommended maximum size for this network,
+         * the user must confirm for this download to proceed without WiFi.
+         */
+        RECOMMENDED_UNUSABLE_DUE_TO_SIZE,
+
+        /**
+         * The current connection is roaming, and the download can't proceed
+         * over a roaming connection.
+         */
+        CANNOT_USE_ROAMING,
+
+        /**
+         * The app requesting the download specific that it can't use the
+         * current network connection.
+         */
+        TYPE_DISALLOWED_BY_REQUESTOR,
+
+        /**
+         * Current network is blocked for requesting application.
+         */
+        BLOCKED;
+    }
 
     /**
      * For intents used to notify the user that a download exceeds a size threshold, if this extra
@@ -313,7 +318,7 @@ public class DownloadInfo {
 
             case Downloads.Impl.STATUS_WAITING_FOR_NETWORK:
             case Downloads.Impl.STATUS_QUEUED_FOR_WIFI:
-                return checkCanUseNetwork() == NETWORK_OK;
+                return checkCanUseNetwork() == NetworkState.OK;
 
             case Downloads.Impl.STATUS_WAITING_TO_RETRY:
                 // download was waiting for a delayed restart
@@ -346,19 +351,19 @@ public class DownloadInfo {
      * Returns whether this download is allowed to use the network.
      * @return one of the NETWORK_* constants
      */
-    public int checkCanUseNetwork() {
+    public NetworkState checkCanUseNetwork() {
         final NetworkInfo info = mSystemFacade.getActiveNetworkInfo(mUid);
         if (info == null || !info.isConnected()) {
-            return NETWORK_NO_CONNECTION;
+            return NetworkState.NO_CONNECTION;
         }
         if (DetailedState.BLOCKED.equals(info.getDetailedState())) {
-            return NETWORK_BLOCKED;
+            return NetworkState.BLOCKED;
         }
-        if (!isRoamingAllowed() && mSystemFacade.isNetworkRoaming()) {
-            return NETWORK_CANNOT_USE_ROAMING;
+        if (mSystemFacade.isNetworkRoaming() && !isRoamingAllowed()) {
+            return NetworkState.CANNOT_USE_ROAMING;
         }
-        if (!mAllowMetered && mSystemFacade.isActiveNetworkMetered()) {
-            return NETWORK_TYPE_DISALLOWED_BY_REQUESTOR;
+        if (mSystemFacade.isActiveNetworkMetered() && !mAllowMetered) {
+            return NetworkState.TYPE_DISALLOWED_BY_REQUESTOR;
         }
         return checkIsNetworkTypeAllowed(info.getType());
     }
@@ -371,46 +376,17 @@ public class DownloadInfo {
         }
     }
 
-    /**
-     * @return a non-localized string appropriate for logging corresponding to one of the
-     * NETWORK_* constants.
-     */
-    public String getLogMessageForNetworkError(int networkError) {
-        switch (networkError) {
-            case NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
-                return "download size exceeds recommended limit for mobile network";
-
-            case NETWORK_UNUSABLE_DUE_TO_SIZE:
-                return "download size exceeds limit for mobile network";
-
-            case NETWORK_NO_CONNECTION:
-                return "no network connection available";
-
-            case NETWORK_CANNOT_USE_ROAMING:
-                return "download cannot use the current network connection because it is roaming";
-
-            case NETWORK_TYPE_DISALLOWED_BY_REQUESTOR:
-                return "download was requested to not use the current network type";
-
-            case NETWORK_BLOCKED:
-                return "network is blocked for requesting application";
-
-            default:
-                return "unknown error with network connectivity";
-        }
-    }
-
     /**
      * Check if this download can proceed over the given network type.
      * @param networkType a constant from ConnectivityManager.TYPE_*.
      * @return one of the NETWORK_* constants
      */
-    private int checkIsNetworkTypeAllowed(int networkType) {
+    private NetworkState checkIsNetworkTypeAllowed(int networkType) {
         if (mIsPublicApi) {
             final int flag = translateNetworkTypeToApiFlag(networkType);
             final boolean allowAllNetworkTypes = mAllowedNetworkTypes == ~0;
             if (!allowAllNetworkTypes && (flag & mAllowedNetworkTypes) == 0) {
-                return NETWORK_TYPE_DISALLOWED_BY_REQUESTOR;
+                return NetworkState.TYPE_DISALLOWED_BY_REQUESTOR;
             }
         }
         return checkSizeAllowedForNetwork(networkType);
@@ -440,25 +416,25 @@ public class DownloadInfo {
      * Check if the download's size prohibits it from running over the current network.
      * @return one of the NETWORK_* constants
      */
-    private int checkSizeAllowedForNetwork(int networkType) {
+    private NetworkState checkSizeAllowedForNetwork(int networkType) {
         if (mTotalBytes <= 0) {
-            return NETWORK_OK; // we don't know the size yet
+            return NetworkState.OK; // we don't know the size yet
         }
         if (networkType == ConnectivityManager.TYPE_WIFI) {
-            return NETWORK_OK; // anything goes over wifi
+            return NetworkState.OK; // anything goes over wifi
         }
         Long maxBytesOverMobile = mSystemFacade.getMaxBytesOverMobile();
         if (maxBytesOverMobile != null && mTotalBytes > maxBytesOverMobile) {
-            return NETWORK_UNUSABLE_DUE_TO_SIZE;
+            return NetworkState.UNUSABLE_DUE_TO_SIZE;
         }
         if (mBypassRecommendedSizeLimit == 0) {
             Long recommendedMaxBytesOverMobile = mSystemFacade.getRecommendedMaxBytesOverMobile();
             if (recommendedMaxBytesOverMobile != null
                     && mTotalBytes > recommendedMaxBytesOverMobile) {
-                return NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE;
+                return NetworkState.RECOMMENDED_UNUSABLE_DUE_TO_SIZE;
             }
         }
-        return NETWORK_OK;
+        return NetworkState.OK;
     }
 
     void startIfReady(long now, StorageManager storageManager) {
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index c77224a7..15183117 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -50,6 +50,8 @@ import android.text.TextUtils;
 import android.util.Log;
 import android.util.Pair;
 
+import com.android.providers.downloads.DownloadInfo.NetworkState;
+
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileOutputStream;
@@ -402,18 +404,17 @@ public class DownloadThread extends Thread {
         // checking connectivity will apply current policy
         mPolicyDirty = false;
 
-        int networkUsable = mInfo.checkCanUseNetwork();
-        if (networkUsable != DownloadInfo.NETWORK_OK) {
+        final NetworkState networkUsable = mInfo.checkCanUseNetwork();
+        if (networkUsable != NetworkState.OK) {
             int status = Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
-            if (networkUsable == DownloadInfo.NETWORK_UNUSABLE_DUE_TO_SIZE) {
+            if (networkUsable == NetworkState.UNUSABLE_DUE_TO_SIZE) {
                 status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
                 mInfo.notifyPauseDueToSize(true);
-            } else if (networkUsable == DownloadInfo.NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE) {
+            } else if (networkUsable == NetworkState.RECOMMENDED_UNUSABLE_DUE_TO_SIZE) {
                 status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
                 mInfo.notifyPauseDueToSize(false);
             }
-            throw new StopRequestException(status,
-                    mInfo.getLogMessageForNetworkError(networkUsable));
+            throw new StopRequestException(status, networkUsable.name());
         }
     }
 
@@ -704,11 +705,11 @@ public class DownloadThread extends Thread {
     }
 
     private int getFinalStatusForHttpError(State state) {
-        int networkUsable = mInfo.checkCanUseNetwork();
-        if (networkUsable != DownloadInfo.NETWORK_OK) {
+        final NetworkState networkUsable = mInfo.checkCanUseNetwork();
+        if (networkUsable != NetworkState.OK) {
             switch (networkUsable) {
-                case DownloadInfo.NETWORK_UNUSABLE_DUE_TO_SIZE:
-                case DownloadInfo.NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
+                case UNUSABLE_DUE_TO_SIZE:
+                case RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
                     return Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
                 default:
                     return Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
-- 
cgit v1.2.3


From 12f5dc46aaa8e28cabfbe25d55f0af68f24ab306 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 17 Jan 2013 17:26:51 -0800
Subject: Better handling of retryable errors.

Now the final errors are always thrown, and the outer code decides
how to handle them as retries.  Also clean up method signatures.

Bug: 8022478
Change-Id: I4e7e43be793294ab837370df521e7c381e0bb6c3
---
 src/com/android/providers/downloads/Constants.java |   3 -
 .../android/providers/downloads/DownloadInfo.java  |   2 +-
 .../providers/downloads/DownloadProvider.java      |   2 +-
 .../providers/downloads/DownloadThread.java        | 110 +++++++++++----------
 4 files changed, 59 insertions(+), 58 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/Constants.java b/src/com/android/providers/downloads/Constants.java
index 8d806182..08ef4665 100644
--- a/src/com/android/providers/downloads/Constants.java
+++ b/src/com/android/providers/downloads/Constants.java
@@ -48,9 +48,6 @@ public class Constants {
     /** The column that is used to remember whether the media scanner was invoked */
     public static final String MEDIA_SCANNED = "scanned";
 
-    /** The column that is used to count retries */
-    public static final String FAILED_CONNECTIONS = "numfailed";
-
     /** The intent that gets sent when the service must wake up for a retry */
     public static final String ACTION_RETRY = "android.intent.action.DOWNLOAD_WAKEUP";
 
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 10e27a71..b984bdef 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -75,7 +75,7 @@ public class DownloadInfo {
             info.mDestination = getInt(Downloads.Impl.COLUMN_DESTINATION);
             info.mVisibility = getInt(Downloads.Impl.COLUMN_VISIBILITY);
             info.mStatus = getInt(Downloads.Impl.COLUMN_STATUS);
-            info.mNumFailed = getInt(Constants.FAILED_CONNECTIONS);
+            info.mNumFailed = getInt(Downloads.Impl.COLUMN_FAILED_CONNECTIONS);
             int retryRedirect = getInt(Constants.RETRY_AFTER_X_REDIRECT_COUNT);
             info.mRetryAfter = retryRedirect & 0xfffffff;
             info.mLastMod = getLong(Downloads.Impl.COLUMN_LAST_MODIFICATION);
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 7af8173b..19a47631 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -384,7 +384,7 @@ public final class DownloadProvider extends ContentProvider {
                         Downloads.Impl.COLUMN_VISIBILITY + " INTEGER, " +
                         Downloads.Impl.COLUMN_CONTROL + " INTEGER, " +
                         Downloads.Impl.COLUMN_STATUS + " INTEGER, " +
-                        Constants.FAILED_CONNECTIONS + " INTEGER, " +
+                        Downloads.Impl.COLUMN_FAILED_CONNECTIONS + " INTEGER, " +
                         Downloads.Impl.COLUMN_LAST_MODIFICATION + " BIGINT, " +
                         Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE + " TEXT, " +
                         Downloads.Impl.COLUMN_NOTIFICATION_CLASS + " TEXT, " +
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 15183117..5bb1e9bd 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -110,7 +110,6 @@ public class DownloadThread extends Thread {
     static class State {
         public String mFilename;
         public String mMimeType;
-        public boolean mCountRetry = false;
         public int mRetryAfter = 0;
         public boolean mGotData = false;
         public String mRequestUri;
@@ -177,6 +176,7 @@ public class DownloadThread extends Thread {
         State state = new State(mInfo);
         PowerManager.WakeLock wakeLock = null;
         int finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
+        int numFailed = mInfo.mNumFailed;
         String errorMsg = null;
 
         final NetworkPolicyManager netPolicy = NetworkPolicyManager.from(mContext);
@@ -219,6 +219,24 @@ public class DownloadThread extends Thread {
                 Log.w(Constants.TAG, msg, error);
             }
             finalStatus = error.getFinalStatus();
+
+            if (finalStatus == STATUS_WAITING_TO_RETRY) {
+                throw new IllegalStateException("Execution should always throw final error codes");
+            }
+
+            // Some errors should be retryable later, unless we fail too many times.
+            if (isStatusRetryable(finalStatus)) {
+                if (state.mGotData) {
+                    numFailed = 1;
+                } else {
+                    numFailed += 1;
+                }
+
+                if (numFailed < Constants.MAX_RETRIES) {
+                    finalStatus = STATUS_WAITING_TO_RETRY;
+                }
+            }
+
             // fall through to finally block
         } catch (Throwable ex) {
             errorMsg = ex.getMessage();
@@ -231,8 +249,7 @@ public class DownloadThread extends Thread {
             TrafficStats.clearThreadStatsUid();
 
             cleanupDestination(state, finalStatus);
-            notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter,
-                    state.mGotData, state.mFilename, state.mMimeType, errorMsg);
+            notifyDownloadCompleted(state, finalStatus, errorMsg, numFailed);
 
             netPolicy.unregisterListener(mPolicyListener);
 
@@ -313,14 +330,12 @@ public class DownloadThread extends Thread {
 
                     case HTTP_UNAVAILABLE:
                         parseRetryAfterHeaders(state, conn);
-                        if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
-                            throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Unavailable");
-                        } else {
-                            throw new StopRequestException(STATUS_CANNOT_RESUME, "Unavailable");
-                        }
+                        throw new StopRequestException(
+                                HTTP_UNAVAILABLE, conn.getResponseMessage());
 
                     case HTTP_INTERNAL_ERROR:
-                        throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Internal error");
+                        throw new StopRequestException(
+                                HTTP_INTERNAL_ERROR, conn.getResponseMessage());
 
                     default:
                         StopRequestException.throwUnhandledHttpError(
@@ -328,7 +343,7 @@ public class DownloadThread extends Thread {
                 }
             } catch (IOException e) {
                 // Trouble with low-level sockets
-                throw new StopRequestException(STATUS_WAITING_TO_RETRY, e);
+                throw new StopRequestException(STATUS_HTTP_DATA_ERROR, e);
 
             } finally {
                 if (conn != null) conn.disconnect();
@@ -569,10 +584,10 @@ public class DownloadThread extends Thread {
                 && (state.mCurrentBytes != state.mContentLength);
         if (lengthMismatched) {
             if (cannotResume(state)) {
-                throw new StopRequestException(Downloads.Impl.STATUS_CANNOT_RESUME,
+                throw new StopRequestException(STATUS_CANNOT_RESUME,
                         "mismatched content length; unable to resume");
             } else {
-                throw new StopRequestException(getFinalStatusForHttpError(state),
+                throw new StopRequestException(STATUS_HTTP_DATA_ERROR,
                         "closed socket before end of file");
             }
         }
@@ -603,10 +618,10 @@ public class DownloadThread extends Thread {
             values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, state.mCurrentBytes);
             mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
             if (cannotResume(state)) {
-                throw new StopRequestException(Downloads.Impl.STATUS_CANNOT_RESUME,
+                throw new StopRequestException(STATUS_CANNOT_RESUME,
                         "Failed reading response: " + ex + "; unable to resume", ex);
             } else {
-                throw new StopRequestException(getFinalStatusForHttpError(state),
+                throw new StopRequestException(STATUS_HTTP_DATA_ERROR,
                         "Failed reading response: " + ex, ex);
             }
         }
@@ -683,13 +698,12 @@ public class DownloadThread extends Thread {
         final boolean noSizeInfo = state.mContentLength == -1
                 && (transferEncoding == null || !transferEncoding.equalsIgnoreCase("chunked"));
         if (!mInfo.mNoIntegrity && noSizeInfo) {
-            throw new StopRequestException(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
+            throw new StopRequestException(STATUS_CANNOT_RESUME,
                     "can't know size of download, giving up");
         }
     }
 
     private void parseRetryAfterHeaders(State state, HttpURLConnection conn) {
-        state.mCountRetry = true;
         state.mRetryAfter = conn.getHeaderFieldInt("Retry-After", -1);
         if (state.mRetryAfter < 0) {
             state.mRetryAfter = 0;
@@ -704,25 +718,6 @@ public class DownloadThread extends Thread {
         }
     }
 
-    private int getFinalStatusForHttpError(State state) {
-        final NetworkState networkUsable = mInfo.checkCanUseNetwork();
-        if (networkUsable != NetworkState.OK) {
-            switch (networkUsable) {
-                case UNUSABLE_DUE_TO_SIZE:
-                case RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
-                    return Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
-                default:
-                    return Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
-            }
-        } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
-            state.mCountRetry = true;
-            return Downloads.Impl.STATUS_WAITING_TO_RETRY;
-        } else {
-            Log.w(Constants.TAG, "reached max retries for " + mInfo.mId);
-            return Downloads.Impl.STATUS_HTTP_DATA_ERROR;
-        }
-    }
-
     /**
      * Prepare the destination file to receive data.  If the file already exists, we'll set up
      * appropriately for resumption.
@@ -814,30 +809,24 @@ public class DownloadThread extends Thread {
     /**
      * Stores information about the completed download, and notifies the initiating application.
      */
-    private void notifyDownloadCompleted(int status, boolean countRetry, int retryAfter,
-            boolean gotData, String filename, String mimeType, String errorMsg) {
-        notifyThroughDatabase(
-                status, countRetry, retryAfter, gotData, filename, mimeType, errorMsg);
-        if (Downloads.Impl.isStatusCompleted(status)) {
+    private void notifyDownloadCompleted(
+            State state, int finalStatus, String errorMsg, int numFailed) {
+        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
+        if (Downloads.Impl.isStatusCompleted(finalStatus)) {
             mInfo.sendIntentIfRequested();
         }
     }
 
-    private void notifyThroughDatabase(int status, boolean countRetry, int retryAfter,
-            boolean gotData, String filename, String mimeType, String errorMsg) {
+    private void notifyThroughDatabase(
+            State state, int finalStatus, String errorMsg, int numFailed) {
         ContentValues values = new ContentValues();
-        values.put(Downloads.Impl.COLUMN_STATUS, status);
-        values.put(Downloads.Impl._DATA, filename);
-        values.put(Downloads.Impl.COLUMN_MIME_TYPE, mimeType);
+        values.put(Downloads.Impl.COLUMN_STATUS, finalStatus);
+        values.put(Downloads.Impl._DATA, state.mFilename);
+        values.put(Downloads.Impl.COLUMN_MIME_TYPE, state.mMimeType);
         values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, mSystemFacade.currentTimeMillis());
-        values.put(Constants.RETRY_AFTER_X_REDIRECT_COUNT, retryAfter);
-        if (!countRetry) {
-            values.put(Constants.FAILED_CONNECTIONS, 0);
-        } else if (gotData) {
-            values.put(Constants.FAILED_CONNECTIONS, 1);
-        } else {
-            values.put(Constants.FAILED_CONNECTIONS, mInfo.mNumFailed + 1);
-        }
+        values.put(Downloads.Impl.COLUMN_FAILED_CONNECTIONS, numFailed);
+        values.put(Constants.RETRY_AFTER_X_REDIRECT_COUNT, state.mRetryAfter);
+
         // save the error message. could be useful to developers.
         if (!TextUtils.isEmpty(errorMsg)) {
             values.put(Downloads.Impl.COLUMN_ERROR_MSG, errorMsg);
@@ -874,4 +863,19 @@ public class DownloadThread extends Thread {
             return defaultValue;
         }
     }
+
+    /**
+     * Return if given status is eligible to be treated as
+     * {@link android.provider.Downloads.Impl#STATUS_WAITING_TO_RETRY}.
+     */
+    public static boolean isStatusRetryable(int status) {
+        switch (status) {
+            case STATUS_HTTP_DATA_ERROR:
+            case HTTP_UNAVAILABLE:
+            case HTTP_INTERNAL_ERROR:
+                return true;
+            default:
+                return false;
+        }
+    }
 }
-- 
cgit v1.2.3


From 38648831a92295e9a11831e19e5a9dab4cbd939e Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Sat, 12 Jan 2013 15:58:51 -0800
Subject: Cleaner thread management, less global state.

Switch to using a ThreadPoolExecutor for handling downloads, which
gives us parallelism logic that is easier to reason about.  Also
open the door to eventually waiting until the executor is drained
to stopSelf().

Removes DownloadHandler singleton, and gives explicit path for
publishing active download speeds to notifications.

Change-Id: I1836e7742bb8a84861d1ca6bd1e59b2040bd12f8
---
 .../providers/downloads/DownloadHandler.java       | 99 ----------------------
 .../android/providers/downloads/DownloadInfo.java  | 70 ++++++++-------
 .../providers/downloads/DownloadNotifier.java      | 36 ++++++--
 .../providers/downloads/DownloadService.java       | 30 +++++--
 .../providers/downloads/DownloadThread.java        | 15 ++--
 5 files changed, 101 insertions(+), 149 deletions(-)
 delete mode 100644 src/com/android/providers/downloads/DownloadHandler.java

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadHandler.java b/src/com/android/providers/downloads/DownloadHandler.java
deleted file mode 100644
index c376ff1e..00000000
--- a/src/com/android/providers/downloads/DownloadHandler.java
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * Copyright (C) 2011 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.providers.downloads;
-
-import android.content.res.Resources;
-import android.util.Log;
-import android.util.LongSparseArray;
-
-import com.android.internal.annotations.GuardedBy;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-
-public class DownloadHandler {
-    private static final String TAG = "DownloadHandler";
-
-    @GuardedBy("this")
-    private final LinkedHashMap<Long, DownloadInfo> mDownloadsQueue =
-            new LinkedHashMap<Long, DownloadInfo>();
-    @GuardedBy("this")
-    private final HashMap<Long, DownloadInfo> mDownloadsInProgress =
-            new HashMap<Long, DownloadInfo>();
-    @GuardedBy("this")
-    private final LongSparseArray<Long> mCurrentSpeed = new LongSparseArray<Long>();
-
-    private final int mMaxConcurrentDownloadsAllowed = Resources.getSystem().getInteger(
-            com.android.internal.R.integer.config_MaxConcurrentDownloadsAllowed);
-
-    private static final DownloadHandler sDownloadHandler = new DownloadHandler();
-
-    public static DownloadHandler getInstance() {
-        return sDownloadHandler;
-    }
-
-    public synchronized void enqueueDownload(DownloadInfo info) {
-        if (!mDownloadsQueue.containsKey(info.mId)) {
-            if (Constants.LOGV) {
-                Log.i(TAG, "enqueued download. id: " + info.mId + ", uri: " + info.mUri);
-            }
-            mDownloadsQueue.put(info.mId, info);
-            startDownloadThreadLocked();
-        }
-    }
-
-    private void startDownloadThreadLocked() {
-        Iterator<Long> keys = mDownloadsQueue.keySet().iterator();
-        ArrayList<Long> ids = new ArrayList<Long>();
-        while (mDownloadsInProgress.size() < mMaxConcurrentDownloadsAllowed && keys.hasNext()) {
-            Long id = keys.next();
-            DownloadInfo info = mDownloadsQueue.get(id);
-            info.startDownloadThread();
-            ids.add(id);
-            mDownloadsInProgress.put(id, mDownloadsQueue.get(id));
-            if (Constants.LOGV) {
-                Log.i(TAG, "started download for : " + id);
-            }
-        }
-        for (Long id : ids) {
-            mDownloadsQueue.remove(id);
-        }
-    }
-
-    public synchronized boolean hasDownloadInQueue(long id) {
-        return mDownloadsQueue.containsKey(id) || mDownloadsInProgress.containsKey(id);
-    }
-
-    public synchronized void dequeueDownload(long id) {
-        mDownloadsInProgress.remove(id);
-        mCurrentSpeed.remove(id);
-        startDownloadThreadLocked();
-        if (mDownloadsInProgress.size() == 0 && mDownloadsQueue.size() == 0) {
-            notifyAll();
-        }
-    }
-
-    public synchronized void setCurrentSpeed(long id, long speed) {
-        mCurrentSpeed.put(id, speed);
-    }
-
-    public synchronized long getCurrentSpeed(long id) {
-        return mCurrentSpeed.get(id, -1L);
-    }
-}
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index b984bdef..74b52d48 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -31,15 +31,18 @@ import android.os.Environment;
 import android.provider.Downloads;
 import android.provider.Downloads.Impl;
 import android.text.TextUtils;
-import android.util.Log;
 import android.util.Pair;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.IndentingPrintWriter;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
 
 /**
  * Stores information about an individual download.
@@ -58,8 +61,9 @@ public class DownloadInfo {
         }
 
         public DownloadInfo newDownloadInfo(Context context, SystemFacade systemFacade,
-                StorageManager storageManager) {
-            DownloadInfo info = new DownloadInfo(context, systemFacade, storageManager);
+                StorageManager storageManager, DownloadNotifier notifier) {
+            final DownloadInfo info = new DownloadInfo(
+                    context, systemFacade, storageManager, notifier);
             updateFromDatabase(info);
             readRequestHeaders(info);
             return info;
@@ -200,7 +204,6 @@ public class DownloadInfo {
      */
     public static final String EXTRA_IS_WIFI_REQUIRED = "isWifiRequired";
 
-
     public long mId;
     public String mUri;
     public boolean mNoIntegrity;
@@ -239,14 +242,24 @@ public class DownloadInfo {
 
     private List<Pair<String, String>> mRequestHeaders = new ArrayList<Pair<String, String>>();
 
+    /**
+     * Result of last {@link DownloadThread} started by
+     * {@link #startIfReady(ExecutorService)}.
+     */
+    @GuardedBy("this")
+    private Future<?> mActiveTask;
+
     private final Context mContext;
     private final SystemFacade mSystemFacade;
     private final StorageManager mStorageManager;
+    private final DownloadNotifier mNotifier;
 
-    private DownloadInfo(Context context, SystemFacade systemFacade, StorageManager storageManager) {
+    private DownloadInfo(Context context, SystemFacade systemFacade, StorageManager storageManager,
+            DownloadNotifier notifier) {
         mContext = context;
         mSystemFacade = systemFacade;
         mStorageManager = storageManager;
+        mNotifier = notifier;
         mFuzz = Helpers.sRandom.nextInt(1001);
     }
 
@@ -297,14 +310,9 @@ public class DownloadInfo {
     }
 
     /**
-     * Returns whether this download (which the download manager hasn't seen yet)
-     * should be started.
+     * Returns whether this download should be enqueued.
      */
-    private boolean isReadyToStart(long now) {
-        if (DownloadHandler.getInstance().hasDownloadInQueue(mId)) {
-            // already running
-            return false;
-        }
+    private boolean isReadyToStart() {
         if (mControl == Downloads.Impl.CONTROL_PAUSED) {
             // the download is paused, so it's not going to start
             return false;
@@ -322,6 +330,7 @@ public class DownloadInfo {
 
             case Downloads.Impl.STATUS_WAITING_TO_RETRY:
                 // download was waiting for a delayed restart
+                final long now = mSystemFacade.currentTimeMillis();
                 return restartTime(now) <= now;
             case Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR:
                 // is the media mounted?
@@ -437,21 +446,27 @@ public class DownloadInfo {
         return NetworkState.OK;
     }
 
-    void startIfReady(long now, StorageManager storageManager) {
-        if (!isReadyToStart(now)) {
-            return;
-        }
+    /**
+     * If download is ready to start, and isn't already pending or executing,
+     * create a {@link DownloadThread} and enqueue it into given
+     * {@link Executor}.
+     */
+    public void startIfReady(ExecutorService executor) {
+        synchronized (this) {
+            final boolean isActive = mActiveTask != null && !mActiveTask.isDone();
+            if (isReadyToStart() && !isActive) {
+                if (mStatus != Impl.STATUS_RUNNING) {
+                    mStatus = Impl.STATUS_RUNNING;
+                    ContentValues values = new ContentValues();
+                    values.put(Impl.COLUMN_STATUS, mStatus);
+                    mContext.getContentResolver().update(getAllDownloadsUri(), values, null, null);
+                }
 
-        if (Constants.LOGV) {
-            Log.v(Constants.TAG, "Service spawning thread to handle download " + mId);
-        }
-        if (mStatus != Impl.STATUS_RUNNING) {
-            mStatus = Impl.STATUS_RUNNING;
-            ContentValues values = new ContentValues();
-            values.put(Impl.COLUMN_STATUS, mStatus);
-            mContext.getContentResolver().update(getAllDownloadsUri(), values, null, null);
+                final DownloadThread task = new DownloadThread(
+                        mContext, mSystemFacade, this, mStorageManager, mNotifier);
+                mActiveTask = executor.submit(task);
+            }
         }
-        DownloadHandler.getInstance().enqueueDownload(this);
     }
 
     public boolean isOnCache() {
@@ -553,11 +568,6 @@ public class DownloadInfo {
         mContext.startActivity(intent);
     }
 
-    void startDownloadThread() {
-        // TODO: keep this thread strongly referenced
-        new DownloadThread(mContext, mSystemFacade, this, mStorageManager).start();
-    }
-
     /**
      * Query and return status of requested download.
      */
diff --git a/src/com/android/providers/downloads/DownloadNotifier.java b/src/com/android/providers/downloads/DownloadNotifier.java
index daae7831..2dd9805a 100644
--- a/src/com/android/providers/downloads/DownloadNotifier.java
+++ b/src/com/android/providers/downloads/DownloadNotifier.java
@@ -32,6 +32,7 @@ import android.net.Uri;
 import android.provider.Downloads;
 import android.text.TextUtils;
 import android.text.format.DateUtils;
+import android.util.LongSparseLongArray;
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Maps;
@@ -66,6 +67,13 @@ public class DownloadNotifier {
     @GuardedBy("mActiveNotifs")
     private final HashMap<String, Long> mActiveNotifs = Maps.newHashMap();
 
+    /**
+     * Current speed of active downloads, mapped from {@link DownloadInfo#mId}
+     * to speed in bytes per second.
+     */
+    @GuardedBy("mDownloadSpeed")
+    private final LongSparseLongArray mDownloadSpeed = new LongSparseLongArray();
+
     public DownloadNotifier(Context context) {
         mContext = context;
         mNotifManager = (NotificationManager) context.getSystemService(
@@ -76,6 +84,20 @@ public class DownloadNotifier {
         mNotifManager.cancelAll();
     }
 
+    /**
+     * Notify the current speed of an active download, used for calcuating
+     * estimated remaining time.
+     */
+    public void notifyDownloadSpeed(long id, long bytesPerSecond) {
+        synchronized (mDownloadSpeed) {
+            if (bytesPerSecond != 0) {
+                mDownloadSpeed.put(id, bytesPerSecond);
+            } else {
+                mDownloadSpeed.delete(id);
+            }
+        }
+    }
+
     /**
      * Update {@link NotificationManager} to reflect the given set of
      * {@link DownloadInfo}, adding, collapsing, and removing as needed.
@@ -163,16 +185,16 @@ public class DownloadNotifier {
             String remainingText = null;
             String percentText = null;
             if (type == TYPE_ACTIVE) {
-                final DownloadHandler handler = DownloadHandler.getInstance();
-
                 long current = 0;
                 long total = 0;
                 long speed = 0;
-                for (DownloadInfo info : cluster) {
-                    if (info.mTotalBytes != -1) {
-                        current += info.mCurrentBytes;
-                        total += info.mTotalBytes;
-                        speed += handler.getCurrentSpeed(info.mId);
+                synchronized (mDownloadSpeed) {
+                    for (DownloadInfo info : cluster) {
+                        if (info.mTotalBytes != -1) {
+                            current += info.mCurrentBytes;
+                            total += info.mTotalBytes;
+                            speed += mDownloadSpeed.get(info.mId);
+                        }
                     }
                 }
 
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 4a1b40d5..039f12cd 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -26,6 +26,7 @@ import android.content.ContentValues;
 import android.content.Context;
 import android.content.Intent;
 import android.content.ServiceConnection;
+import android.content.res.Resources;
 import android.database.ContentObserver;
 import android.database.Cursor;
 import android.media.IMediaScannerListener;
@@ -53,6 +54,10 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Performs the background downloads requested by applications that use the Downloads provider.
@@ -76,12 +81,26 @@ public class DownloadService extends Service {
     @GuardedBy("mDownloads")
     private Map<Long, DownloadInfo> mDownloads = Maps.newHashMap();
 
+    private final ExecutorService mExecutor = buildDownloadExecutor();
+
+    private static ExecutorService buildDownloadExecutor() {
+        final int maxConcurrent = Resources.getSystem().getInteger(
+                com.android.internal.R.integer.config_MaxConcurrentDownloadsAllowed);
+
+        // Create a bounded thread pool for executing downloads; it creates
+        // threads as needed (up to maximum) and reclaims them when finished.
+        final ThreadPoolExecutor executor = new ThreadPoolExecutor(
+                maxConcurrent, maxConcurrent, 10, TimeUnit.SECONDS,
+                new LinkedBlockingQueue<Runnable>());
+        executor.allowCoreThreadTimeOut(true);
+        return executor;
+    }
+
     /**
      * The thread that updates the internal download list from the content
      * provider.
      */
-    @VisibleForTesting
-    UpdateThread mUpdateThread;
+    private UpdateThread mUpdateThread;
 
     /**
      * Whether the internal download list should be updated from the content
@@ -435,14 +454,15 @@ public class DownloadService extends Service {
      * download if appropriate.
      */
     private DownloadInfo insertDownloadLocked(DownloadInfo.Reader reader, long now) {
-        DownloadInfo info = reader.newDownloadInfo(this, mSystemFacade, mStorageManager);
+        final DownloadInfo info = reader.newDownloadInfo(
+                this, mSystemFacade, mStorageManager, mNotifier);
         mDownloads.put(info.mId, info);
 
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "processing inserted download " + info.mId);
         }
 
-        info.startIfReady(now, mStorageManager);
+        info.startIfReady(mExecutor);
         return info;
     }
 
@@ -458,7 +478,7 @@ public class DownloadService extends Service {
             Log.v(Constants.TAG, "processing updated download " + info.mId +
                     ", status: " + info.mStatus);
         }
-        info.startIfReady(now, mStorageManager);
+        info.startIfReady(mExecutor);
     }
 
     /**
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 5bb1e9bd..bd347e42 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -68,10 +68,10 @@ import libcore.io.IoUtils;
 import libcore.net.http.HttpEngine;
 
 /**
- * Thread which executes a given {@link DownloadInfo}: making network requests,
+ * Task which executes a given {@link DownloadInfo}: making network requests,
  * persisting data to disk, and updating {@link DownloadProvider}.
  */
-public class DownloadThread extends Thread {
+public class DownloadThread implements Runnable {
 
     private static final int HTTP_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
     private static final int HTTP_TEMP_REDIRECT = 307;
@@ -82,15 +82,17 @@ public class DownloadThread extends Thread {
     private final DownloadInfo mInfo;
     private final SystemFacade mSystemFacade;
     private final StorageManager mStorageManager;
+    private final DownloadNotifier mNotifier;
 
     private volatile boolean mPolicyDirty;
 
     public DownloadThread(Context context, SystemFacade systemFacade, DownloadInfo info,
-            StorageManager storageManager) {
+            StorageManager storageManager, DownloadNotifier notifier) {
         mContext = context;
         mSystemFacade = systemFacade;
         mInfo = info;
         mStorageManager = storageManager;
+        mNotifier = notifier;
     }
 
     /**
@@ -151,16 +153,13 @@ public class DownloadThread extends Thread {
         }
     }
 
-    /**
-     * Executes the download in a separate thread
-     */
     @Override
     public void run() {
         Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
         try {
             runInternal();
         } finally {
-            DownloadHandler.getInstance().dequeueDownload(mInfo.mId);
+            mNotifier.notifyDownloadSpeed(mInfo.mId, 0);
         }
     }
 
@@ -526,7 +525,7 @@ public class DownloadThread extends Thread {
             state.mSpeedSampleStart = now;
             state.mSpeedSampleBytes = state.mCurrentBytes;
 
-            DownloadHandler.getInstance().setCurrentSpeed(mInfo.mId, state.mSpeed);
+            mNotifier.notifyDownloadSpeed(mInfo.mId, state.mSpeed);
         }
 
         if (state.mCurrentBytes - state.mBytesNotified > Constants.MIN_PROGRESS_STEP &&
-- 
cgit v1.2.3


From 58eee87b70862a7ced85eabc3c225fad24664065 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 29 Jan 2013 14:48:46 -0800
Subject: Tests for max retries/redirects, ETag switches.

Verify that servers responding with many retries or redirects result
in failed download, instead of spinning out of control.  Test to
verify that changed ETag results in download failing.

Also fix handling of HTTP 301 to update Uri in database.

Change-Id: Iff2948d79961a245b7900117d107edaa356618c9
---
 .../android/providers/downloads/DownloadThread.java | 21 ++++++++++-----------
 src/com/android/providers/downloads/Helpers.java    |  2 +-
 2 files changed, 11 insertions(+), 12 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index bd347e42..0d427fdd 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -65,7 +65,6 @@ import java.net.URL;
 import java.net.URLConnection;
 
 import libcore.io.IoUtils;
-import libcore.net.http.HttpEngine;
 
 /**
  * Task which executes a given {@link DownloadInfo}: making network requests,
@@ -275,9 +274,7 @@ public class DownloadThread implements Runnable {
             return;
         }
 
-        // TODO: compare mInfo.mNumFailed against Constants.MAX_RETRIES
-
-        while (state.mRedirectionCount++ < HttpEngine.MAX_REDIRECTS) {
+        while (state.mRedirectionCount++ < Constants.MAX_REDIRECTS) {
             // Open connection and follow any redirects until we have a useful
             // response with body.
             HttpURLConnection conn = null;
@@ -315,18 +312,16 @@ public class DownloadThread implements Runnable {
                     case HTTP_TEMP_REDIRECT:
                         final String location = conn.getHeaderField("Location");
                         state.mUrl = new URL(state.mUrl, location);
+                        if (responseCode == HTTP_MOVED_PERM) {
+                            // Push updated URL back to database
+                            state.mRequestUri = state.mUrl.toString();
+                        }
                         continue;
 
                     case HTTP_REQUESTED_RANGE_NOT_SATISFIABLE:
                         throw new StopRequestException(
                                 STATUS_CANNOT_RESUME, "Requested range not satisfiable");
 
-                    case HTTP_PRECON_FAILED:
-                        // TODO: probably means our etag precondition was
-                        // changed; flush and retry again
-                        StopRequestException.throwUnhandledHttpError(
-                                responseCode, conn.getResponseMessage());
-
                     case HTTP_UNAVAILABLE:
                         parseRetryAfterHeaders(state, conn);
                         throw new StopRequestException(
@@ -645,7 +640,7 @@ public class DownloadThread implements Runnable {
                 state.mMimeType,
                 mInfo.mDestination,
                 state.mContentLength,
-                mInfo.mIsPublicApi, mStorageManager);
+                mStorageManager);
 
         updateDatabaseFromHeaders(state);
         // check connectivity again now that we know the total size
@@ -826,6 +821,10 @@ public class DownloadThread implements Runnable {
         values.put(Downloads.Impl.COLUMN_FAILED_CONNECTIONS, numFailed);
         values.put(Constants.RETRY_AFTER_X_REDIRECT_COUNT, state.mRetryAfter);
 
+        if (!TextUtils.equals(mInfo.mUri, state.mRequestUri)) {
+            values.put(Downloads.Impl.COLUMN_URI, state.mRequestUri);
+        }
+
         // save the error message. could be useful to developers.
         if (!TextUtils.isEmpty(errorMsg)) {
             values.put(Downloads.Impl.COLUMN_ERROR_MSG, errorMsg);
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 059e9703..5c34ebe1 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -77,7 +77,7 @@ public class Helpers {
             String mimeType,
             int destination,
             long contentLength,
-            boolean isPublicApi, StorageManager storageManager) throws StopRequestException {
+            StorageManager storageManager) throws StopRequestException {
         if (contentLength < 0) {
             contentLength = 0;
         }
-- 
cgit v1.2.3


From 1d0a0aa2cc5bfed8107aa70f7e890fde9a7ea2b4 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Wed, 30 Jan 2013 11:26:46 -0800
Subject: Dump recent downloads from provider.

The lifetime of DownloadService can be limited, and it's often
missing from bugreports.  The provider has a much longer lifetime,
so have it dump raw data about recent downloads.

Bug: 7350685
Change-Id: I55c9d602d77014ea27820936f1cf5c8ad24f286a
---
 .../providers/downloads/DownloadProvider.java      | 42 +++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 19a47631..2d0c807a 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -37,17 +37,22 @@ import android.os.Binder;
 import android.os.Environment;
 import android.os.ParcelFileDescriptor;
 import android.os.Process;
+import android.provider.BaseColumns;
 import android.provider.Downloads;
 import android.provider.OpenableColumns;
 import android.text.TextUtils;
+import android.text.format.DateUtils;
 import android.util.Log;
 
+import com.android.internal.util.IndentingPrintWriter;
 import com.google.android.collect.Maps;
 import com.google.common.annotations.VisibleForTesting;
 
 import java.io.File;
+import java.io.FileDescriptor;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -1199,6 +1204,41 @@ public final class DownloadProvider extends ContentProvider {
         return ret;
     }
 
+    @Override
+    public void dump(FileDescriptor fd, PrintWriter writer, String[] args) {
+        final IndentingPrintWriter pw = new IndentingPrintWriter(writer, "  ", 120);
+
+        pw.println("Downloads updated in last hour:");
+        pw.increaseIndent();
+
+        final SQLiteDatabase db = mOpenHelper.getReadableDatabase();
+        final long modifiedAfter = mSystemFacade.currentTimeMillis() - DateUtils.HOUR_IN_MILLIS;
+        final Cursor cursor = db.query(DB_TABLE, null,
+                Downloads.Impl.COLUMN_LAST_MODIFICATION + ">" + modifiedAfter, null, null, null,
+                Downloads.Impl._ID + " ASC");
+        try {
+            final String[] cols = cursor.getColumnNames();
+            final int idCol = cursor.getColumnIndex(BaseColumns._ID);
+            while (cursor.moveToNext()) {
+                pw.println("Download #" + cursor.getInt(idCol) + ":");
+                pw.increaseIndent();
+                for (int i = 0; i < cols.length; i++) {
+                    // Omit sensitive data when dumping
+                    if (Downloads.Impl.COLUMN_COOKIE_DATA.equals(cols[i])) {
+                        continue;
+                    }
+                    pw.printPair(cols[i], cursor.getString(i));
+                }
+                pw.println();
+                pw.decreaseIndent();
+            }
+        } finally {
+            cursor.close();
+        }
+
+        pw.decreaseIndent();
+    }
+
     private void logVerboseOpenFileInfo(Uri uri, String mode) {
         Log.v(Constants.TAG, "openFile uri: " + uri + ", mode: " + mode
                 + ", uid: " + Binder.getCallingUid());
@@ -1229,7 +1269,7 @@ public final class DownloadProvider extends ContentProvider {
                     Log.v(Constants.TAG, "file exists in openFile");
                 }
             }
-           cursor.close();
+            cursor.close();
         }
     }
 
-- 
cgit v1.2.3


From 1ad10ce731d1b54692d7d5ee32601e965f503fa4 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 7 Feb 2013 16:13:20 -0800
Subject: Active notifications only for running downloads.

Bug: 8145142
Change-Id: I9119796f809aa967f7ec6bb2a3d2b815e86eaf1b
---
 src/com/android/providers/downloads/DownloadNotifier.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadNotifier.java b/src/com/android/providers/downloads/DownloadNotifier.java
index 2dd9805a..0af9cb86 100644
--- a/src/com/android/providers/downloads/DownloadNotifier.java
+++ b/src/com/android/providers/downloads/DownloadNotifier.java
@@ -19,6 +19,7 @@ package com.android.providers.downloads;
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE;
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED;
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_ONLY_COMPLETION;
+import static android.provider.Downloads.Impl.STATUS_RUNNING;
 
 import android.app.DownloadManager;
 import android.app.Notification;
@@ -327,7 +328,7 @@ public class DownloadNotifier {
     }
 
     private static boolean isActiveAndVisible(DownloadInfo download) {
-        return Downloads.Impl.isStatusInformational(download.mStatus) &&
+        return download.mStatus == STATUS_RUNNING &&
                 (download.mVisibility == VISIBILITY_VISIBLE
                 || download.mVisibility == VISIBILITY_VISIBLE_NOTIFY_COMPLETED);
     }
-- 
cgit v1.2.3


From 925976230936a5177365dc24b50da8607a9af8d4 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Mon, 11 Feb 2013 16:19:39 -0800
Subject: Redesign of DownloadManager update loop.

Previously, the service lifecycle was managed through a large for()
loop which was extremely tricky to reason about.  This resulted in
several race conditions that could leave the service running
indefinitely, or terminate it early before tasks had finished.

This change redesigns the update loop to be event driven based on
database updates, and to collapse mutiple pending update passes.  It
is much easier to reason about service termination conditions, and
it correctly uses startId to handle races during command delivery.

Also moves scanner into isolated class, and switches to using public
API instead of binding to private interface.

Bug: 7638470, 7455406, 7162341
Change-Id: I380e77f5432223b2acb4e819e37f29f98ee4782b
---
 .../android/providers/downloads/DownloadInfo.java  |  50 +-
 .../providers/downloads/DownloadScanner.java       | 157 ++++++
 .../providers/downloads/DownloadService.java       | 539 +++++++--------------
 .../providers/downloads/DownloadThread.java        |  26 +-
 4 files changed, 401 insertions(+), 371 deletions(-)
 create mode 100644 src/com/android/providers/downloads/DownloadScanner.java

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 74b52d48..65242227 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -244,10 +244,10 @@ public class DownloadInfo {
 
     /**
      * Result of last {@link DownloadThread} started by
-     * {@link #startIfReady(ExecutorService)}.
+     * {@link #startDownloadIfReady(ExecutorService)}.
      */
     @GuardedBy("this")
-    private Future<?> mActiveTask;
+    private Future<?> mActiveDownload;
 
     private final Context mContext;
     private final SystemFacade mSystemFacade;
@@ -312,7 +312,7 @@ public class DownloadInfo {
     /**
      * Returns whether this download should be enqueued.
      */
-    private boolean isReadyToStart() {
+    private boolean isReadyToDownload() {
         if (mControl == Downloads.Impl.CONTROL_PAUSED) {
             // the download is paused, so it's not going to start
             return false;
@@ -450,11 +450,14 @@ public class DownloadInfo {
      * If download is ready to start, and isn't already pending or executing,
      * create a {@link DownloadThread} and enqueue it into given
      * {@link Executor}.
+     *
+     * @return If actively downloading.
      */
-    public void startIfReady(ExecutorService executor) {
+    public boolean startDownloadIfReady(ExecutorService executor) {
         synchronized (this) {
-            final boolean isActive = mActiveTask != null && !mActiveTask.isDone();
-            if (isReadyToStart() && !isActive) {
+            final boolean isReady = isReadyToDownload();
+            final boolean isActive = mActiveDownload != null && !mActiveDownload.isDone();
+            if (isReady && !isActive) {
                 if (mStatus != Impl.STATUS_RUNNING) {
                     mStatus = Impl.STATUS_RUNNING;
                     ContentValues values = new ContentValues();
@@ -464,8 +467,25 @@ public class DownloadInfo {
 
                 final DownloadThread task = new DownloadThread(
                         mContext, mSystemFacade, this, mStorageManager, mNotifier);
-                mActiveTask = executor.submit(task);
+                mActiveDownload = executor.submit(task);
             }
+            return isReady;
+        }
+    }
+
+    /**
+     * If download is ready to be scanned, enqueue it into the given
+     * {@link DownloadScanner}.
+     *
+     * @return If actively scanning.
+     */
+    public boolean startScanIfReady(DownloadScanner scanner) {
+        synchronized (this) {
+            final boolean isReady = shouldScanFile();
+            if (isReady) {
+                scanner.requestScan(this);
+            }
+            return isReady;
         }
     }
 
@@ -527,15 +547,15 @@ public class DownloadInfo {
     }
 
     /**
-     * Returns the amount of time (as measured from the "now" parameter)
-     * at which a download will be active.
-     * 0 = immediately - service should stick around to handle this download.
-     * -1 = never - service can go away without ever waking up.
-     * positive value - service must wake up in the future, as specified in ms from "now"
+     * Return time when this download will be ready for its next action, in
+     * milliseconds after given time.
+     *
+     * @return If {@code 0}, download is ready to proceed immediately. If
+     *         {@link Long#MAX_VALUE}, then download has no future actions.
      */
-    long nextAction(long now) {
+    public long nextActionMillis(long now) {
         if (Downloads.Impl.isStatusCompleted(mStatus)) {
-            return -1;
+            return Long.MAX_VALUE;
         }
         if (mStatus != Downloads.Impl.STATUS_WAITING_TO_RETRY) {
             return 0;
@@ -550,7 +570,7 @@ public class DownloadInfo {
     /**
      * Returns whether a file should be scanned
      */
-    boolean shouldScanFile() {
+    public boolean shouldScanFile() {
         return (mMediaScanned == 0)
                 && (mDestination == Downloads.Impl.DESTINATION_EXTERNAL ||
                         mDestination == Downloads.Impl.DESTINATION_FILE_URI ||
diff --git a/src/com/android/providers/downloads/DownloadScanner.java b/src/com/android/providers/downloads/DownloadScanner.java
new file mode 100644
index 00000000..ca795062
--- /dev/null
+++ b/src/com/android/providers/downloads/DownloadScanner.java
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2013 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.downloads;
+
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
+import static com.android.providers.downloads.Constants.LOGV;
+import static com.android.providers.downloads.Constants.TAG;
+
+import android.content.ContentResolver;
+import android.content.ContentUris;
+import android.content.ContentValues;
+import android.content.Context;
+import android.media.MediaScannerConnection;
+import android.media.MediaScannerConnection.MediaScannerConnectionClient;
+import android.net.Uri;
+import android.os.SystemClock;
+import android.provider.Downloads;
+import android.util.Log;
+
+import com.android.internal.annotations.GuardedBy;
+import com.google.common.collect.Maps;
+
+import java.util.HashMap;
+
+/**
+ * Manages asynchronous scanning of completed downloads.
+ */
+public class DownloadScanner implements MediaScannerConnectionClient {
+    private static final long SCAN_TIMEOUT = MINUTE_IN_MILLIS;
+
+    private final Context mContext;
+    private final MediaScannerConnection mConnection;
+
+    private static class ScanRequest {
+        public final long id;
+        public final String path;
+        public final String mimeType;
+        public final long requestRealtime;
+
+        public ScanRequest(long id, String path, String mimeType) {
+            this.id = id;
+            this.path = path;
+            this.mimeType = mimeType;
+            this.requestRealtime = SystemClock.elapsedRealtime();
+        }
+
+        public void exec(MediaScannerConnection conn) {
+            conn.scanFile(path, mimeType);
+        }
+    }
+
+    @GuardedBy("mConnection")
+    private HashMap<String, ScanRequest> mPending = Maps.newHashMap();
+
+    public DownloadScanner(Context context) {
+        mContext = context;
+        mConnection = new MediaScannerConnection(context, this);
+    }
+
+    /**
+     * Check if requested scans are still pending. Scans may timeout after an
+     * internal duration.
+     */
+    public boolean hasPendingScans() {
+        synchronized (mConnection) {
+            if (mPending.isEmpty()) {
+                return false;
+            } else {
+                // Check if pending scans have timed out
+                final long nowRealtime = SystemClock.elapsedRealtime();
+                for (ScanRequest req : mPending.values()) {
+                    if (nowRealtime < req.requestRealtime + SCAN_TIMEOUT) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        }
+    }
+
+    /**
+     * Request that given {@link DownloadInfo} be scanned at some point in
+     * future. Enqueues the request to be scanned asynchronously.
+     *
+     * @see #hasPendingScans()
+     */
+    public void requestScan(DownloadInfo info) {
+        if (LOGV) Log.v(TAG, "requestScan() for " + info.mFileName);
+        synchronized (mConnection) {
+            final ScanRequest req = new ScanRequest(info.mId, info.mFileName, info.mMimeType);
+            mPending.put(req.path, req);
+
+            if (mConnection.isConnected()) {
+                req.exec(mConnection);
+            } else {
+                mConnection.connect();
+            }
+        }
+    }
+
+    public void shutdown() {
+        mConnection.disconnect();
+    }
+
+    @Override
+    public void onMediaScannerConnected() {
+        synchronized (mConnection) {
+            for (ScanRequest req : mPending.values()) {
+                req.exec(mConnection);
+            }
+        }
+    }
+
+    @Override
+    public void onScanCompleted(String path, Uri uri) {
+        final ScanRequest req;
+        synchronized (mConnection) {
+            req = mPending.remove(path);
+        }
+        if (req == null) {
+            Log.w(TAG, "Missing request for path " + path);
+            return;
+        }
+
+        // Update scanned column, which will kick off a database update pass,
+        // eventually deciding if overall service is ready for teardown.
+        final ContentValues values = new ContentValues();
+        values.put(Downloads.Impl.COLUMN_MEDIA_SCANNED, 1);
+        if (uri != null) {
+            values.put(Downloads.Impl.COLUMN_MEDIAPROVIDER_URI, uri.toString());
+        }
+
+        final ContentResolver resolver = mContext.getContentResolver();
+        final Uri downloadUri = ContentUris.withAppendedId(
+                Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, req.id);
+        final int rows = resolver.update(downloadUri, values, null, null);
+        if (rows == 0) {
+            // Local row disappeared during scan; download was probably deleted
+            // so clean up now-orphaned media entry.
+            resolver.delete(uri, null, null);
+        }
+    }
+}
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 039f12cd..34b1b495 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -16,26 +16,25 @@
 
 package com.android.providers.downloads;
 
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
 
 import android.app.AlarmManager;
+import android.app.DownloadManager;
 import android.app.PendingIntent;
 import android.app.Service;
-import android.content.ComponentName;
-import android.content.ContentValues;
+import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
-import android.content.ServiceConnection;
 import android.content.res.Resources;
 import android.database.ContentObserver;
 import android.database.Cursor;
-import android.media.IMediaScannerListener;
-import android.media.IMediaScannerService;
 import android.net.Uri;
 import android.os.Handler;
+import android.os.HandlerThread;
 import android.os.IBinder;
+import android.os.Message;
 import android.os.Process;
-import android.os.RemoteException;
 import android.provider.Downloads;
 import android.text.TextUtils;
 import android.util.Log;
@@ -45,12 +44,12 @@ import com.android.internal.util.IndentingPrintWriter;
 import com.google.android.collect.Maps;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -60,11 +59,25 @@ import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
 /**
- * Performs the background downloads requested by applications that use the Downloads provider.
+ * Performs background downloads as requested by applications that use
+ * {@link DownloadManager}. Multiple start commands can be issued at this
+ * service, and it will continue running until no downloads are being actively
+ * processed. It may schedule alarms to resume downloads in future.
+ * <p>
+ * Any database updates important enough to initiate tasks should always be
+ * delivered through {@link Context#startService(Intent)}.
  */
 public class DownloadService extends Service {
-    /** amount of time to wait to connect to MediaScannerService before timing out */
-    private static final long WAIT_TIMEOUT = 10 * 1000;
+    // TODO: migrate WakeLock from individual DownloadThreads out into
+    // DownloadReceiver to protect our entire workflow.
+
+    private static final boolean DEBUG_LIFECYCLE = true;
+
+    @VisibleForTesting
+    SystemFacade mSystemFacade;
+
+    private AlarmManager mAlarmManager;
+    private StorageManager mStorageManager;
 
     /** Observer to get notified when the content observer's data changes */
     private DownloadManagerContentObserver mObserver;
@@ -79,7 +92,7 @@ public class DownloadService extends Service {
      * content provider changes or disappears.
      */
     @GuardedBy("mDownloads")
-    private Map<Long, DownloadInfo> mDownloads = Maps.newHashMap();
+    private final Map<Long, DownloadInfo> mDownloads = Maps.newHashMap();
 
     private final ExecutorService mExecutor = buildDownloadExecutor();
 
@@ -96,115 +109,24 @@ public class DownloadService extends Service {
         return executor;
     }
 
-    /**
-     * The thread that updates the internal download list from the content
-     * provider.
-     */
-    private UpdateThread mUpdateThread;
+    private DownloadScanner mScanner;
 
-    /**
-     * Whether the internal download list should be updated from the content
-     * provider.
-     */
-    private boolean mPendingUpdate;
+    private HandlerThread mUpdateThread;
+    private Handler mUpdateHandler;
 
-    /**
-     * The ServiceConnection object that tells us when we're connected to and disconnected from
-     * the Media Scanner
-     */
-    private MediaScannerConnection mMediaScannerConnection;
-
-    private boolean mMediaScannerConnecting;
-
-    /**
-     * The IPC interface to the Media Scanner
-     */
-    private IMediaScannerService mMediaScannerService;
-
-    @VisibleForTesting
-    SystemFacade mSystemFacade;
-
-    private StorageManager mStorageManager;
+    private volatile int mLastStartId;
 
     /**
      * Receives notifications when the data in the content provider changes
      */
     private class DownloadManagerContentObserver extends ContentObserver {
-
         public DownloadManagerContentObserver() {
             super(new Handler());
         }
 
-        /**
-         * Receives notification when the data in the observed content
-         * provider changes.
-         */
         @Override
         public void onChange(final boolean selfChange) {
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG, "Service ContentObserver received notification");
-            }
-            updateFromProvider();
-        }
-
-    }
-
-    /**
-     * Gets called back when the connection to the media
-     * scanner is established or lost.
-     */
-    public class MediaScannerConnection implements ServiceConnection {
-        public void onServiceConnected(ComponentName className, IBinder service) {
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG, "Connected to Media Scanner");
-            }
-            synchronized (DownloadService.this) {
-                try {
-                    mMediaScannerConnecting = false;
-                    mMediaScannerService = IMediaScannerService.Stub.asInterface(service);
-                    if (mMediaScannerService != null) {
-                        updateFromProvider();
-                    }
-                } finally {
-                    // notify anyone waiting on successful connection to MediaService
-                    DownloadService.this.notifyAll();
-                }
-            }
-        }
-
-        public void disconnectMediaScanner() {
-            synchronized (DownloadService.this) {
-                mMediaScannerConnecting = false;
-                if (mMediaScannerService != null) {
-                    mMediaScannerService = null;
-                    if (Constants.LOGVV) {
-                        Log.v(Constants.TAG, "Disconnecting from Media Scanner");
-                    }
-                    try {
-                        unbindService(this);
-                    } catch (IllegalArgumentException ex) {
-                        Log.w(Constants.TAG, "unbindService failed: " + ex);
-                    } finally {
-                        // notify anyone waiting on unsuccessful connection to MediaService
-                        DownloadService.this.notifyAll();
-                    }
-                }
-            }
-        }
-
-        public void onServiceDisconnected(ComponentName className) {
-            try {
-                if (Constants.LOGVV) {
-                    Log.v(Constants.TAG, "Disconnected from Media Scanner");
-                }
-            } finally {
-                synchronized (DownloadService.this) {
-                    mMediaScannerService = null;
-                    mMediaScannerConnecting = false;
-                    // notify anyone waiting on disconnect from MediaService
-                    DownloadService.this.notifyAll();
-                }
-            }
+            enqueueUpdate();
         }
     }
 
@@ -233,19 +155,21 @@ public class DownloadService extends Service {
             mSystemFacade = new RealSystemFacade(this);
         }
 
-        mObserver = new DownloadManagerContentObserver();
-        getContentResolver().registerContentObserver(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                true, mObserver);
+        mAlarmManager = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
+        mStorageManager = new StorageManager(this);
+
+        mUpdateThread = new HandlerThread(TAG + "-UpdateThread");
+        mUpdateThread.start();
+        mUpdateHandler = new Handler(mUpdateThread.getLooper(), mUpdateCallback);
 
-        mMediaScannerService = null;
-        mMediaScannerConnecting = false;
-        mMediaScannerConnection = new MediaScannerConnection();
+        mScanner = new DownloadScanner(this);
 
         mNotifier = new DownloadNotifier(this);
         mNotifier.cancelAll();
 
-        mStorageManager = new StorageManager(this);
-        updateFromProvider();
+        mObserver = new DownloadManagerContentObserver();
+        getContentResolver().registerContentObserver(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+                true, mObserver);
     }
 
     @Override
@@ -254,15 +178,14 @@ public class DownloadService extends Service {
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onStart");
         }
-        updateFromProvider();
+        mLastStartId = startId;
+        enqueueUpdate();
         return returnValue;
     }
 
-    /**
-     * Cleans up when the service is destroyed
-     */
     @Override
     public void onDestroy() {
+        mScanner.shutdown();
         getContentResolver().unregisterContentObserver(mObserver);
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onDestroy");
@@ -271,182 +194,167 @@ public class DownloadService extends Service {
     }
 
     /**
-     * Parses data from the content provider into private array
+     * Enqueue an {@link #updateLocked()} pass to occur in future.
      */
-    private void updateFromProvider() {
-        synchronized (this) {
-            mPendingUpdate = true;
-            if (mUpdateThread == null) {
-                mUpdateThread = new UpdateThread();
-                mUpdateThread.start();
-            }
-        }
+    private void enqueueUpdate() {
+        mUpdateHandler.removeMessages(MSG_UPDATE);
+        mUpdateHandler.obtainMessage(MSG_UPDATE, mLastStartId, -1).sendToTarget();
     }
 
-    private class UpdateThread extends Thread {
-        public UpdateThread() {
-            super("Download Service");
-        }
+    /**
+     * Enqueue an {@link #updateLocked()} pass to occur after delay, usually to
+     * catch any finished operations that didn't trigger an update pass.
+     */
+    private void enqueueFinalUpdate() {
+        mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
+        mUpdateHandler.sendMessageDelayed(
+                mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
+                MINUTE_IN_MILLIS);
+    }
+
+    private static final int MSG_UPDATE = 1;
+    private static final int MSG_FINAL_UPDATE = 2;
 
+    private Handler.Callback mUpdateCallback = new Handler.Callback() {
         @Override
-        public void run() {
+        public boolean handleMessage(Message msg) {
             Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
-            boolean keepService = false;
-            // for each update from the database, remember which download is
-            // supposed to get restarted soonest in the future
-            long wakeUp = Long.MAX_VALUE;
-            for (;;) {
-                synchronized (DownloadService.this) {
-                    if (mUpdateThread != this) {
-                        throw new IllegalStateException(
-                                "multiple UpdateThreads in DownloadService");
-                    }
-                    if (!mPendingUpdate) {
-                        mUpdateThread = null;
-                        if (!keepService) {
-                            stopSelf();
-                        }
-                        if (wakeUp != Long.MAX_VALUE) {
-                            scheduleAlarm(wakeUp);
-                        }
-                        return;
-                    }
-                    mPendingUpdate = false;
+
+            final int startId = msg.arg1;
+            if (DEBUG_LIFECYCLE) Log.v(TAG, "Updating for startId " + startId);
+
+            // Since database is current source of truth, our "active" status
+            // depends on database state. We always get one final update pass
+            // once the real actions have finished and persisted their state.
+
+            // TODO: switch to asking real tasks to derive active state
+            // TODO: handle media scanner timeouts
+
+            final boolean isActive;
+            synchronized (mDownloads) {
+                isActive = updateLocked();
+            }
+
+            if (msg.what == MSG_FINAL_UPDATE) {
+                Log.wtf(TAG, "Final update pass triggered, isActive=" + isActive
+                        + "; someone didn't update correctly.");
+            }
+
+            if (isActive) {
+                // Still doing useful work, keep service alive. These active
+                // tasks will trigger another update pass when they're finished.
+
+                // Enqueue delayed update pass to catch finished operations that
+                // didn't trigger an update pass; these are bugs.
+                enqueueFinalUpdate();
+
+            } else {
+                // No active tasks, and any pending update messages can be
+                // ignored, since any updates important enough to initiate tasks
+                // will always be delivered with a new startId.
+
+                if (stopSelfResult(startId)) {
+                    if (DEBUG_LIFECYCLE) Log.v(TAG, "Nothing left; stopped");
+                    mUpdateHandler.removeMessages(MSG_UPDATE);
+                    mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
                 }
+            }
 
-                synchronized (mDownloads) {
-                    long now = mSystemFacade.currentTimeMillis();
-                    boolean mustScan = false;
-                    keepService = false;
-                    wakeUp = Long.MAX_VALUE;
-                    Set<Long> idsNoLongerInDatabase = new HashSet<Long>(mDownloads.keySet());
-
-                    Cursor cursor = getContentResolver().query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                            null, null, null, null);
-                    if (cursor == null) {
-                        continue;
-                    }
-                    try {
-                        DownloadInfo.Reader reader =
-                                new DownloadInfo.Reader(getContentResolver(), cursor);
-                        int idColumn = cursor.getColumnIndexOrThrow(Downloads.Impl._ID);
-                        if (Constants.LOGVV) {
-                            Log.i(Constants.TAG, "number of rows from downloads-db: " +
-                                    cursor.getCount());
-                        }
-                        for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) {
-                            long id = cursor.getLong(idColumn);
-                            idsNoLongerInDatabase.remove(id);
-                            DownloadInfo info = mDownloads.get(id);
-                            if (info != null) {
-                                updateDownload(reader, info, now);
-                            } else {
-                                info = insertDownloadLocked(reader, now);
-                            }
-
-                            if (info.shouldScanFile() && !scanFile(info, true, false)) {
-                                mustScan = true;
-                                keepService = true;
-                            }
-                            if (info.hasCompletionNotification()) {
-                                keepService = true;
-                            }
-                            long next = info.nextAction(now);
-                            if (next == 0) {
-                                keepService = true;
-                            } else if (next > 0 && next < wakeUp) {
-                                wakeUp = next;
-                            }
-                        }
-                    } finally {
-                        cursor.close();
-                    }
+            return true;
+        }
+    };
 
-                    for (Long id : idsNoLongerInDatabase) {
-                        deleteDownloadLocked(id);
-                    }
+    /**
+     * Update {@link #mDownloads} to match {@link DownloadProvider} state.
+     * Depending on current download state it may enqueue {@link DownloadThread}
+     * instances, request {@link DownloadScanner} scans, update user-visible
+     * notifications, and/or schedule future actions with {@link AlarmManager}.
+     * <p>
+     * Should only be called from {@link #mUpdateThread} as after being
+     * requested through {@link #enqueueUpdate()}.
+     *
+     * @return If there are active tasks being processed, as of the database
+     *         snapshot taken in this update.
+     */
+    private boolean updateLocked() {
+        final long now = mSystemFacade.currentTimeMillis();
 
-                    // is there a need to start the DownloadService? yes, if there are rows to be
-                    // deleted.
-                    if (!mustScan) {
-                        for (DownloadInfo info : mDownloads.values()) {
-                            if (info.mDeleted && TextUtils.isEmpty(info.mMediaProviderUri)) {
-                                mustScan = true;
-                                keepService = true;
-                                break;
-                            }
-                        }
-                    }
-                    mNotifier.updateWith(mDownloads.values());
-                    if (mustScan) {
-                        bindMediaScanner();
-                    } else {
-                        mMediaScannerConnection.disconnectMediaScanner();
+        boolean isActive = false;
+        long nextActionMillis = Long.MAX_VALUE;
+
+        final Set<Long> staleIds = Sets.newHashSet(mDownloads.keySet());
+
+        final ContentResolver resolver = getContentResolver();
+        final Cursor cursor = resolver.query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+                null, null, null, null);
+        try {
+            final DownloadInfo.Reader reader = new DownloadInfo.Reader(resolver, cursor);
+            final int idColumn = cursor.getColumnIndexOrThrow(Downloads.Impl._ID);
+            while (cursor.moveToNext()) {
+                final long id = cursor.getLong(idColumn);
+                staleIds.remove(id);
+
+                DownloadInfo info = mDownloads.get(id);
+                if (info != null) {
+                    updateDownload(reader, info, now);
+                } else {
+                    info = insertDownloadLocked(reader, now);
+                }
+
+                if (info.mDeleted) {
+                    // Delete download if requested, but only after cleaning up
+                    if (!TextUtils.isEmpty(info.mMediaProviderUri)) {
+                        resolver.delete(Uri.parse(info.mMediaProviderUri), null, null);
                     }
 
-                    // look for all rows with deleted flag set and delete the rows from the database
-                    // permanently
-                    for (DownloadInfo info : mDownloads.values()) {
-                        if (info.mDeleted) {
-                            // this row is to be deleted from the database. but does it have
-                            // mediaProviderUri?
-                            if (TextUtils.isEmpty(info.mMediaProviderUri)) {
-                                if (info.shouldScanFile()) {
-                                    // initiate rescan of the file to - which will populate
-                                    // mediaProviderUri column in this row
-                                    if (!scanFile(info, false, true)) {
-                                        throw new IllegalStateException("scanFile failed!");
-                                    }
-                                    continue;
-                                }
-                            } else {
-                                // yes it has mediaProviderUri column already filled in.
-                                // delete it from MediaProvider database.
-                                getContentResolver().delete(Uri.parse(info.mMediaProviderUri), null,
-                                        null);
-                            }
-                            // delete the file
-                            deleteFileIfExists(info.mFileName);
-                            // delete from the downloads db
-                            getContentResolver().delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                                    Downloads.Impl._ID + " = ? ",
-                                    new String[]{String.valueOf(info.mId)});
-                        }
+                    deleteFileIfExists(info.mFileName);
+                    resolver.delete(info.getAllDownloadsUri(), null, null);
+
+                } else {
+                    // Kick off download task if ready
+                    final boolean activeDownload = info.startDownloadIfReady(mExecutor);
+
+                    // Kick off media scan if completed
+                    final boolean activeScan = info.startScanIfReady(mScanner);
+
+                    if (DEBUG_LIFECYCLE && (activeDownload || activeScan)) {
+                        Log.v(TAG, "Download " + info.mId + ": activeDownload=" + activeDownload
+                                + ", activeScan=" + activeScan);
                     }
+
+                    isActive |= activeDownload;
+                    isActive |= activeScan;
                 }
+
+                // Keep track of nearest next action
+                nextActionMillis = Math.min(info.nextActionMillis(now), nextActionMillis);
             }
+        } finally {
+            cursor.close();
         }
 
-        private void bindMediaScanner() {
-            if (!mMediaScannerConnecting) {
-                Intent intent = new Intent();
-                intent.setClassName("com.android.providers.media",
-                        "com.android.providers.media.MediaScannerService");
-                mMediaScannerConnecting = true;
-                bindService(intent, mMediaScannerConnection, BIND_AUTO_CREATE);
-            }
+        // Clean up stale downloads that disappeared
+        for (Long id : staleIds) {
+            deleteDownloadLocked(id);
         }
 
-        private void scheduleAlarm(long wakeUp) {
-            AlarmManager alarms = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
-            if (alarms == null) {
-                Log.e(Constants.TAG, "couldn't get alarm manager");
-                return;
-            }
+        // Update notifications visible to user
+        mNotifier.updateWith(mDownloads.values());
 
+        // Set alarm when next action is in future. It's okay if the service
+        // continues to run in meantime, since it will kick off an update pass.
+        if (nextActionMillis > 0 && nextActionMillis < Long.MAX_VALUE) {
             if (Constants.LOGV) {
-                Log.v(Constants.TAG, "scheduling retry in " + wakeUp + "ms");
+                Log.v(TAG, "scheduling start in " + nextActionMillis + "ms");
             }
 
-            Intent intent = new Intent(Constants.ACTION_RETRY);
-            intent.setClassName("com.android.providers.downloads",
-                    DownloadReceiver.class.getName());
-            alarms.set(
-                    AlarmManager.RTC_WAKEUP,
-                    mSystemFacade.currentTimeMillis() + wakeUp,
-                    PendingIntent.getBroadcast(DownloadService.this, 0, intent,
-                            PendingIntent.FLAG_ONE_SHOT));
+            final Intent intent = new Intent(Constants.ACTION_RETRY);
+            intent.setClass(this, DownloadReceiver.class);
+            mAlarmManager.set(AlarmManager.RTC_WAKEUP, now + nextActionMillis,
+                    PendingIntent.getBroadcast(this, 0, intent, PendingIntent.FLAG_ONE_SHOT));
         }
+
+        return isActive;
     }
 
     /**
@@ -462,7 +370,6 @@ public class DownloadService extends Service {
             Log.v(Constants.TAG, "processing inserted download " + info.mId);
         }
 
-        info.startIfReady(mExecutor);
         return info;
     }
 
@@ -470,15 +377,11 @@ public class DownloadService extends Service {
      * Updates the local copy of the info about a download.
      */
     private void updateDownload(DownloadInfo.Reader reader, DownloadInfo info, long now) {
-        int oldVisibility = info.mVisibility;
-        int oldStatus = info.mStatus;
-
         reader.updateFromDatabase(info);
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "processing updated download " + info.mId +
                     ", status: " + info.mStatus);
         }
-        info.startIfReady(mExecutor);
     }
 
     /**
@@ -493,88 +396,20 @@ public class DownloadService extends Service {
             if (Constants.LOGVV) {
                 Log.d(TAG, "deleteDownloadLocked() deleting " + info.mFileName);
             }
-            new File(info.mFileName).delete();
+            deleteFileIfExists(info.mFileName);
         }
         mDownloads.remove(info.mId);
     }
 
-    /**
-     * Attempts to scan the file if necessary.
-     * @return true if the file has been properly scanned.
-     */
-    private boolean scanFile(DownloadInfo info, final boolean updateDatabase,
-            final boolean deleteFile) {
-        synchronized (this) {
-            if (mMediaScannerService == null) {
-                // not bound to mediaservice. but if in the process of connecting to it, wait until
-                // connection is resolved
-                while (mMediaScannerConnecting) {
-                    Log.d(Constants.TAG, "waiting for mMediaScannerService service: ");
-                    try {
-                        this.wait(WAIT_TIMEOUT);
-                    } catch (InterruptedException e1) {
-                        throw new IllegalStateException("wait interrupted");
-                    }
-                }
-            }
-            // do we have mediaservice?
-            if (mMediaScannerService == null) {
-                // no available MediaService And not even in the process of connecting to it
-                return false;
-            }
-            if (Constants.LOGV) {
-                Log.v(Constants.TAG, "Scanning file " + info.mFileName);
-            }
-            try {
-                final Uri key = info.getAllDownloadsUri();
-                final long id = info.mId;
-                mMediaScannerService.requestScanFile(info.mFileName, info.mMimeType,
-                        new IMediaScannerListener.Stub() {
-                            public void scanCompleted(String path, Uri uri) {
-                                if (updateDatabase) {
-                                    // Mark this as 'scanned' in the database
-                                    // so that it is NOT subject to re-scanning by MediaScanner
-                                    // next time this database row row is encountered
-                                    ContentValues values = new ContentValues();
-                                    values.put(Constants.MEDIA_SCANNED, 1);
-                                    if (uri != null) {
-                                        values.put(Downloads.Impl.COLUMN_MEDIAPROVIDER_URI,
-                                                uri.toString());
-                                    }
-                                    getContentResolver().update(key, values, null, null);
-                                } else if (deleteFile) {
-                                    if (uri != null) {
-                                        // use the Uri returned to delete it from the MediaProvider
-                                        getContentResolver().delete(uri, null, null);
-                                    }
-                                    // delete the file and delete its row from the downloads db
-                                    deleteFileIfExists(path);
-                                    getContentResolver().delete(
-                                            Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                                            Downloads.Impl._ID + " = ? ",
-                                            new String[]{String.valueOf(id)});
-                                }
-                            }
-                        });
-                return true;
-            } catch (RemoteException e) {
-                Log.w(Constants.TAG, "Failed to scan file " + info.mFileName);
-                return false;
-            }
-        }
-    }
-
     private void deleteFileIfExists(String path) {
-        try {
-            if (!TextUtils.isEmpty(path)) {
-                if (Constants.LOGVV) {
-                    Log.d(TAG, "deleteFileIfExists() deleting " + path);
-                }
-                File file = new File(path);
-                file.delete();
+        if (!TextUtils.isEmpty(path)) {
+            if (Constants.LOGVV) {
+                Log.d(TAG, "deleteFileIfExists() deleting " + path);
+            }
+            final File file = new File(path);
+            if (file.exists() && !file.delete()) {
+                Log.w(TAG, "file: '" + path + "' couldn't be deleted");
             }
-        } catch (Exception e) {
-            Log.w(Constants.TAG, "file: '" + path + "' couldn't be deleted", e);
         }
     }
 
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 0d427fdd..c60b02a0 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -20,7 +20,9 @@ import static android.provider.Downloads.Impl.STATUS_BAD_REQUEST;
 import static android.provider.Downloads.Impl.STATUS_CANNOT_RESUME;
 import static android.provider.Downloads.Impl.STATUS_FILE_ERROR;
 import static android.provider.Downloads.Impl.STATUS_HTTP_DATA_ERROR;
+import static android.provider.Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
 import static android.provider.Downloads.Impl.STATUS_TOO_MANY_REDIRECTS;
+import static android.provider.Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
 import static android.provider.Downloads.Impl.STATUS_WAITING_TO_RETRY;
 import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
@@ -29,7 +31,6 @@ import static java.net.HttpURLConnection.HTTP_MOVED_PERM;
 import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.net.HttpURLConnection.HTTP_PARTIAL;
-import static java.net.HttpURLConnection.HTTP_PRECON_FAILED;
 import static java.net.HttpURLConnection.HTTP_SEE_OTHER;
 import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 
@@ -218,11 +219,13 @@ public class DownloadThread implements Runnable {
             }
             finalStatus = error.getFinalStatus();
 
+            // Nobody below our level should request retries, since we handle
+            // failure counts at this level.
             if (finalStatus == STATUS_WAITING_TO_RETRY) {
                 throw new IllegalStateException("Execution should always throw final error codes");
             }
 
-            // Some errors should be retryable later, unless we fail too many times.
+            // Some errors should be retryable, unless we fail too many times.
             if (isStatusRetryable(finalStatus)) {
                 if (state.mGotData) {
                     numFailed = 1;
@@ -231,7 +234,7 @@ public class DownloadThread implements Runnable {
                 }
 
                 if (numFailed < Constants.MAX_RETRIES) {
-                    finalStatus = STATUS_WAITING_TO_RETRY;
+                    finalStatus = getFinalRetryStatus();
                 }
             }
 
@@ -427,6 +430,21 @@ public class DownloadThread implements Runnable {
         }
     }
 
+    /**
+     * Return retry status appropriate for current network conditions.
+     */
+    private int getFinalRetryStatus() {
+        switch (mInfo.checkCanUseNetwork()) {
+            case OK:
+                return STATUS_WAITING_TO_RETRY;
+            case UNUSABLE_DUE_TO_SIZE:
+            case RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
+                return STATUS_QUEUED_FOR_WIFI;
+            default:
+                return STATUS_WAITING_FOR_NETWORK;
+        }
+    }
+
     /**
      * Transfer as much data as possible from the HTTP response to the
      * destination file.
@@ -805,10 +823,10 @@ public class DownloadThread implements Runnable {
      */
     private void notifyDownloadCompleted(
             State state, int finalStatus, String errorMsg, int numFailed) {
-        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
         if (Downloads.Impl.isStatusCompleted(finalStatus)) {
             mInfo.sendIntentIfRequested();
         }
+        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
     }
 
     private void notifyThroughDatabase(
-- 
cgit v1.2.3


From 292f9bffb4d4055db57b6e6419591f14e00bfc74 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 14 Feb 2013 10:07:16 -0800
Subject: Update database before sending broadcast.

This was moved to to solve a race condition around service shutdown,
but ended up causing another race with remote apps.

Bug: 8200919
Change-Id: Ief470e9454e9be8ec43ca3ec11e3b3440fa5852d
---
 src/com/android/providers/downloads/DownloadThread.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index c60b02a0..d19f71be 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -823,10 +823,10 @@ public class DownloadThread implements Runnable {
      */
     private void notifyDownloadCompleted(
             State state, int finalStatus, String errorMsg, int numFailed) {
+        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
         if (Downloads.Impl.isStatusCompleted(finalStatus)) {
             mInfo.sendIntentIfRequested();
         }
-        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
     }
 
     private void notifyThroughDatabase(
-- 
cgit v1.2.3


From 2eb144d8effed2dbb067957c5b25e735233bca89 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 19 Feb 2013 12:48:08 -0800
Subject: Retries shouldn't backoff when network changes.

When a download fails due to a network change, treat it as waiting
for network, instead of subjecting it to full retry backoff.

Change-Id: Ifdae62fd7c2baad7422f68e09da94740b5f513d0
---
 .../android/providers/downloads/DownloadInfo.java  |  1 -
 .../providers/downloads/DownloadThread.java        | 39 ++++++++++++----------
 src/com/android/providers/downloads/Helpers.java   |  8 -----
 3 files changed, 22 insertions(+), 26 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 65242227..e6ed059b 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -358,7 +358,6 @@ public class DownloadInfo {
 
     /**
      * Returns whether this download is allowed to use the network.
-     * @return one of the NETWORK_* constants
      */
     public NetworkState checkCanUseNetwork() {
         final NetworkInfo info = mSystemFacade.getActiveNetworkInfo(mUid);
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index d19f71be..95754a03 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -20,7 +20,6 @@ import static android.provider.Downloads.Impl.STATUS_BAD_REQUEST;
 import static android.provider.Downloads.Impl.STATUS_CANNOT_RESUME;
 import static android.provider.Downloads.Impl.STATUS_FILE_ERROR;
 import static android.provider.Downloads.Impl.STATUS_HTTP_DATA_ERROR;
-import static android.provider.Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
 import static android.provider.Downloads.Impl.STATUS_TOO_MANY_REDIRECTS;
 import static android.provider.Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
 import static android.provider.Downloads.Impl.STATUS_WAITING_TO_RETRY;
@@ -39,7 +38,9 @@ import android.content.Context;
 import android.content.Intent;
 import android.drm.DrmManagerClient;
 import android.drm.DrmOutputStream;
+import android.net.ConnectivityManager;
 import android.net.INetworkPolicyListener;
+import android.net.NetworkInfo;
 import android.net.NetworkPolicyManager;
 import android.net.TrafficStats;
 import android.os.FileUtils;
@@ -73,6 +74,9 @@ import libcore.io.IoUtils;
  */
 public class DownloadThread implements Runnable {
 
+    // TODO: bind each download to a specific network interface to avoid state
+    // checking races once we have ConnectivityManager API
+
     private static final int HTTP_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
     private static final int HTTP_TEMP_REDIRECT = 307;
 
@@ -121,6 +125,7 @@ public class DownloadThread implements Runnable {
         public boolean mContinuingDownload = false;
         public long mBytesNotified = 0;
         public long mTimeLastNotification = 0;
+        public int mNetworkType = ConnectivityManager.TYPE_NONE;
 
         /** Historical bytes/second speed of this download. */
         public long mSpeed;
@@ -190,6 +195,13 @@ public class DownloadThread implements Runnable {
 
             Log.i(Constants.TAG, "Initiating download " + mInfo.mId);
 
+            // Remember which network this download started on; used to
+            // determine if errors were due to network changes.
+            final NetworkInfo info = mSystemFacade.getActiveNetworkInfo(mInfo.mUid);
+            if (info != null) {
+                state.mNetworkType = info.getType();
+            }
+
             // Network traffic on this thread should be counted against the
             // requesting UID, and is tagged with well-known value.
             TrafficStats.setThreadStatsTag(TrafficStats.TAG_SYSTEM_DOWNLOAD);
@@ -234,7 +246,15 @@ public class DownloadThread implements Runnable {
                 }
 
                 if (numFailed < Constants.MAX_RETRIES) {
-                    finalStatus = getFinalRetryStatus();
+                    final NetworkInfo info = mSystemFacade.getActiveNetworkInfo(mInfo.mUid);
+                    if (info != null && info.getType() == state.mNetworkType
+                            && info.isConnected()) {
+                        // Underlying network is still intact, use normal backoff
+                        finalStatus = STATUS_WAITING_TO_RETRY;
+                    } else {
+                        // Network changed, retry on any next available
+                        finalStatus = STATUS_WAITING_FOR_NETWORK;
+                    }
                 }
             }
 
@@ -430,21 +450,6 @@ public class DownloadThread implements Runnable {
         }
     }
 
-    /**
-     * Return retry status appropriate for current network conditions.
-     */
-    private int getFinalRetryStatus() {
-        switch (mInfo.checkCanUseNetwork()) {
-            case OK:
-                return STATUS_WAITING_TO_RETRY;
-            case UNUSABLE_DUE_TO_SIZE:
-            case RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
-                return STATUS_QUEUED_FOR_WIFI;
-            default:
-                return STATUS_WAITING_FOR_NETWORK;
-        }
-    }
-
     /**
      * Transfer as much data as possible from the HTTP response to the
      * destination file.
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 5c34ebe1..593e28b8 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -328,14 +328,6 @@ public class Helpers {
                 "failed to generate an unused filename on internal download storage");
     }
 
-    /**
-     * Returns whether the network is available
-     */
-    public static boolean isNetworkAvailable(SystemFacade system, int uid) {
-        final NetworkInfo info = system.getActiveNetworkInfo(uid);
-        return info != null && info.isConnected();
-    }
-
     /**
      * Checks whether the filename looks legitimate
      */
-- 
cgit v1.2.3


From d1214c9c92b6a4a44cfc46125c33d071a0f3d880 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 19 Feb 2013 17:11:41 -0800
Subject: Only report speeds from full samples windows.

Wait until we've passed a full sample window (500ms) before reporting
an estimated speed.  This avoid showing skewed times like "900 hours
remaining."

Also remember to clean up the UpdateThread.

Bug: 8176417
Change-Id: I851e0abcbb443114abe9c22f4650fee7a9bc3aaa
---
 src/com/android/providers/downloads/DownloadService.java | 1 +
 src/com/android/providers/downloads/DownloadThread.java  | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 34b1b495..66d3be96 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -185,6 +185,7 @@ public class DownloadService extends Service {
 
     @Override
     public void onDestroy() {
+        mUpdateThread.quit();
         mScanner.shutdown();
         getContentResolver().unregisterContentObserver(mObserver);
         if (Constants.LOGVV) {
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 95754a03..48eebfc0 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -540,10 +540,13 @@ public class DownloadThread implements Runnable {
                 state.mSpeed = ((state.mSpeed * 3) + sampleSpeed) / 4;
             }
 
+            // Only notify once we have a full sample window
+            if (state.mSpeedSampleStart != 0) {
+                mNotifier.notifyDownloadSpeed(mInfo.mId, state.mSpeed);
+            }
+
             state.mSpeedSampleStart = now;
             state.mSpeedSampleBytes = state.mCurrentBytes;
-
-            mNotifier.notifyDownloadSpeed(mInfo.mId, state.mSpeed);
         }
 
         if (state.mCurrentBytes - state.mBytesNotified > Constants.MIN_PROGRESS_STEP &&
-- 
cgit v1.2.3


From 71a53040699e713bcae839a01b7f8abd9e13bab6 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Wed, 20 Feb 2013 15:54:26 -0800
Subject: Only use single UpdateThread.

Since DownloadService starts and stops frequently, recycle a single
UpdateThread across invocations.

Bug: 8233041
Change-Id: I07756fb6bfdbad811cbd58e628fdfdbf63d71bf8
---
 src/com/android/providers/downloads/DownloadService.java | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 66d3be96..20cf6294 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -158,9 +158,11 @@ public class DownloadService extends Service {
         mAlarmManager = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
         mStorageManager = new StorageManager(this);
 
-        mUpdateThread = new HandlerThread(TAG + "-UpdateThread");
-        mUpdateThread.start();
-        mUpdateHandler = new Handler(mUpdateThread.getLooper(), mUpdateCallback);
+        if (mUpdateThread == null) {
+            mUpdateThread = new HandlerThread(TAG + "-UpdateThread");
+            mUpdateThread.start();
+            mUpdateHandler = new Handler(mUpdateThread.getLooper(), mUpdateCallback);
+        }
 
         mScanner = new DownloadScanner(this);
 
@@ -185,7 +187,6 @@ public class DownloadService extends Service {
 
     @Override
     public void onDestroy() {
-        mUpdateThread.quit();
         mScanner.shutdown();
         getContentResolver().unregisterContentObserver(mObserver);
         if (Constants.LOGVV) {
-- 
cgit v1.2.3


From dd1179c09fb6ac2420292e52ea3ced395f76a7be Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Wed, 20 Feb 2013 16:10:39 -0800
Subject: Ack, we actually need to UpdateThread.quit().

Otherwise they end up leaking.  There is a race around UpdateThread
continuing to process messages before onDestroy() has been invoked,
so explicitly UpdateThread.quit() in both places.

Bug: 8233041
Change-Id: I73f1b70aedace19e23a61a3dddf4986d82f2c7d6
---
 src/com/android/providers/downloads/DownloadService.java | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 20cf6294..c8e55d7d 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -158,11 +158,9 @@ public class DownloadService extends Service {
         mAlarmManager = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
         mStorageManager = new StorageManager(this);
 
-        if (mUpdateThread == null) {
-            mUpdateThread = new HandlerThread(TAG + "-UpdateThread");
-            mUpdateThread.start();
-            mUpdateHandler = new Handler(mUpdateThread.getLooper(), mUpdateCallback);
-        }
+        mUpdateThread = new HandlerThread(TAG + "-UpdateThread");
+        mUpdateThread.start();
+        mUpdateHandler = new Handler(mUpdateThread.getLooper(), mUpdateCallback);
 
         mScanner = new DownloadScanner(this);
 
@@ -187,8 +185,9 @@ public class DownloadService extends Service {
 
     @Override
     public void onDestroy() {
-        mScanner.shutdown();
         getContentResolver().unregisterContentObserver(mObserver);
+        mScanner.shutdown();
+        mUpdateThread.quit();
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onDestroy");
         }
@@ -257,8 +256,7 @@ public class DownloadService extends Service {
 
                 if (stopSelfResult(startId)) {
                     if (DEBUG_LIFECYCLE) Log.v(TAG, "Nothing left; stopped");
-                    mUpdateHandler.removeMessages(MSG_UPDATE);
-                    mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
+                    mUpdateThread.quit();
                 }
             }
 
-- 
cgit v1.2.3


From 80a535d83ea9ed21f443fdc701c743569ae53eec Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 26 Feb 2013 12:43:56 -0800
Subject: Adjust timeouts to reduce false-positive bugs.

Otherwise we end up triggering MSG_FINAL_UPDATE while still waiting
for socket timeouts.  Using 20 seconds for timeout is more sane, and
matches Volley.

Bug: 8233041
Change-Id: Ia7220033a5942c46ca1d79a88e2b3f530cb3edac
---
 src/com/android/providers/downloads/DownloadService.java | 4 ++--
 src/com/android/providers/downloads/DownloadThread.java  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index c8e55d7d..d6ed9d6d 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -16,7 +16,7 @@
 
 package com.android.providers.downloads;
 
-import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
+import static android.text.format.DateUtils.SECOND_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
 
 import android.app.AlarmManager;
@@ -210,7 +210,7 @@ public class DownloadService extends Service {
         mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
         mUpdateHandler.sendMessageDelayed(
                 mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
-                MINUTE_IN_MILLIS);
+                90 * SECOND_IN_MILLIS);
     }
 
     private static final int MSG_UPDATE = 1;
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 48eebfc0..a0b3e54a 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -23,7 +23,7 @@ import static android.provider.Downloads.Impl.STATUS_HTTP_DATA_ERROR;
 import static android.provider.Downloads.Impl.STATUS_TOO_MANY_REDIRECTS;
 import static android.provider.Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
 import static android.provider.Downloads.Impl.STATUS_WAITING_TO_RETRY;
-import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
+import static android.text.format.DateUtils.SECOND_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
 import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
 import static java.net.HttpURLConnection.HTTP_MOVED_PERM;
@@ -80,7 +80,7 @@ public class DownloadThread implements Runnable {
     private static final int HTTP_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
     private static final int HTTP_TEMP_REDIRECT = 307;
 
-    private static final int DEFAULT_TIMEOUT = (int) MINUTE_IN_MILLIS;
+    private static final int DEFAULT_TIMEOUT = (int) (20 * SECOND_IN_MILLIS);
 
     private final Context mContext;
     private final DownloadInfo mInfo;
-- 
cgit v1.2.3


From 5ba69740a47857fcebd36866e07963ba798269d5 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Fri, 1 Mar 2013 11:18:38 -0800
Subject: Fix race conditions around filename claiming.

When multiple downloads are running in parallel, they can end up
claiming the same filename and clobber over each other.  This change
introduces locking around filename generation, and touches the
claimed filename so other threads fail the File.exists() check and
keep looking.

Tests to verify.

Bug: 8255596
Change-Id: Ie75ed047c199cf679832c75159056ca036eac18d
---
 src/com/android/providers/downloads/Helpers.java | 30 ++++++++++++++++--------
 1 file changed, 20 insertions(+), 10 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 593e28b8..33205557 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -17,10 +17,6 @@
 package com.android.providers.downloads;
 
 import android.content.Context;
-import android.content.Intent;
-import android.content.pm.PackageManager;
-import android.content.pm.ResolveInfo;
-import android.net.NetworkInfo;
 import android.net.Uri;
 import android.os.Environment;
 import android.os.SystemClock;
@@ -29,6 +25,7 @@ import android.util.Log;
 import android.webkit.MimeTypeMap;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.Random;
 import java.util.Set;
 import java.util.regex.Matcher;
@@ -44,6 +41,8 @@ public class Helpers {
     private static final Pattern CONTENT_DISPOSITION_PATTERN =
             Pattern.compile("attachment;\\s*filename\\s*=\\s*\"([^\"]*)\"");
 
+    private static final Object sUniqueLock = new Object();
+
     private Helpers() {
     }
 
@@ -92,10 +91,10 @@ public class Helpers {
                                              destination);
         }
         storageManager.verifySpace(destination, path, contentLength);
-        path = getFullPath(path, mimeType, destination, base);
         if (DownloadDrmHelper.isDrmConvertNeeded(mimeType)) {
             path = DownloadDrmHelper.modifyDrmFwLockFileExtension(path);
         }
+        path = getFullPath(path, mimeType, destination, base);
         return path;
     }
 
@@ -132,7 +131,21 @@ public class Helpers {
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "target file: " + filename + extension);
         }
-        return chooseUniqueFilename(destination, filename, extension, recoveryDir);
+
+        synchronized (sUniqueLock) {
+            final String path = chooseUniqueFilenameLocked(
+                    destination, filename, extension, recoveryDir);
+
+            // Claim this filename inside lock to prevent other threads from
+            // clobbering us. We're not paranoid enough to use O_EXCL.
+            try {
+                new File(path).createNewFile();
+            } catch (IOException e) {
+                throw new StopRequestException(Downloads.Impl.STATUS_FILE_ERROR,
+                        "Failed to create target file " + path, e);
+            }
+            return path;
+        }
     }
 
     private static String chooseFilename(String url, String hint, String contentDisposition,
@@ -282,11 +295,8 @@ public class Helpers {
         return extension;
     }
 
-    private static String chooseUniqueFilename(int destination, String filename,
+    private static String chooseUniqueFilenameLocked(int destination, String filename,
             String extension, boolean recoveryDir) throws StopRequestException {
-        // TODO: switch to actually creating the file here, otherwise we expose
-        // ourselves to race conditions.
-
         String fullFilename = filename + extension;
         if (!new File(fullFilename).exists()
                 && (!recoveryDir ||
-- 
cgit v1.2.3


From 844782d81e12d8bd13c9b26a5f7aab3f4be81b5a Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Mon, 18 Mar 2013 18:30:02 -0700
Subject: Defeat transparent Accept-Encoding: gzip.

Transparent gzip encoding doesn't allow us to easily resume partial
requests, so defeat it for now.

Bug: 8409417
Change-Id: I1172709c09d1153fff1ba8df072a9bef896e244d
---
 src/com/android/providers/downloads/DownloadThread.java | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index a0b3e54a..f1add241 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -818,6 +818,10 @@ public class DownloadThread implements Runnable {
             conn.addRequestProperty("User-Agent", userAgent());
         }
 
+        // Defeat transparent gzip compression, since it doesn't allow us to
+        // easily resume partial downloads.
+        conn.setRequestProperty("Accept-Encoding", "identity");
+
         if (state.mContinuingDownload) {
             if (state.mHeaderETag != null) {
                 conn.addRequestProperty("If-Match", state.mHeaderETag);
-- 
cgit v1.2.3


From a9917b41429e3e18074d8a8a709894e488883686 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Wed, 20 Mar 2013 17:03:48 -0700
Subject: Increase timeout for domains with many DNS entries.

Bug: 8233041
Change-Id: Ifb70273474c391ef687ba018d9ef809a359c7149
---
 src/com/android/providers/downloads/DownloadService.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index d6ed9d6d..07641097 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -16,7 +16,7 @@
 
 package com.android.providers.downloads;
 
-import static android.text.format.DateUtils.SECOND_IN_MILLIS;
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
 
 import android.app.AlarmManager;
@@ -210,7 +210,7 @@ public class DownloadService extends Service {
         mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
         mUpdateHandler.sendMessageDelayed(
                 mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
-                90 * SECOND_IN_MILLIS);
+                5 * MINUTE_IN_MILLIS);
     }
 
     private static final int MSG_UPDATE = 1;
-- 
cgit v1.2.3


From 703bc3a83056a878a83e263b992fb5331b84535f Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Mon, 25 Mar 2013 13:54:29 -0700
Subject: Reduce logging, dump stacks before wtf().

Most wtf() are looking like network timeouts, not threading bugs, so
disable verbose debugging and add more targeted thread logging before
calling wtf().

Bug: 8233041
Change-Id: I8e276bffd7880cfe13b65e7e81f5507cab627692
---
 .../android/providers/downloads/DownloadInfo.java  | 11 ++++++----
 .../providers/downloads/DownloadNotifier.java      | 25 +++++++++++++++++++++-
 .../providers/downloads/DownloadService.java       | 14 +++++++++++-
 3 files changed, 44 insertions(+), 6 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index e6ed059b..7a912d5a 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -247,7 +247,10 @@ public class DownloadInfo {
      * {@link #startDownloadIfReady(ExecutorService)}.
      */
     @GuardedBy("this")
-    private Future<?> mActiveDownload;
+    private Future<?> mSubmittedTask;
+
+    @GuardedBy("this")
+    private DownloadThread mTask;
 
     private final Context mContext;
     private final SystemFacade mSystemFacade;
@@ -455,7 +458,7 @@ public class DownloadInfo {
     public boolean startDownloadIfReady(ExecutorService executor) {
         synchronized (this) {
             final boolean isReady = isReadyToDownload();
-            final boolean isActive = mActiveDownload != null && !mActiveDownload.isDone();
+            final boolean isActive = mSubmittedTask != null && !mSubmittedTask.isDone();
             if (isReady && !isActive) {
                 if (mStatus != Impl.STATUS_RUNNING) {
                     mStatus = Impl.STATUS_RUNNING;
@@ -464,9 +467,9 @@ public class DownloadInfo {
                     mContext.getContentResolver().update(getAllDownloadsUri(), values, null, null);
                 }
 
-                final DownloadThread task = new DownloadThread(
+                mTask = new DownloadThread(
                         mContext, mSystemFacade, this, mStorageManager, mNotifier);
-                mActiveDownload = executor.submit(task);
+                mSubmittedTask = executor.submit(mTask);
             }
             return isReady;
         }
diff --git a/src/com/android/providers/downloads/DownloadNotifier.java b/src/com/android/providers/downloads/DownloadNotifier.java
index 0af9cb86..f832eae3 100644
--- a/src/com/android/providers/downloads/DownloadNotifier.java
+++ b/src/com/android/providers/downloads/DownloadNotifier.java
@@ -20,6 +20,7 @@ import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE;
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED;
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_ONLY_COMPLETION;
 import static android.provider.Downloads.Impl.STATUS_RUNNING;
+import static com.android.providers.downloads.Constants.TAG;
 
 import android.app.DownloadManager;
 import android.app.Notification;
@@ -30,9 +31,11 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.res.Resources;
 import android.net.Uri;
+import android.os.SystemClock;
 import android.provider.Downloads;
 import android.text.TextUtils;
 import android.text.format.DateUtils;
+import android.util.Log;
 import android.util.LongSparseLongArray;
 
 import com.google.common.collect.ArrayListMultimap;
@@ -75,6 +78,13 @@ public class DownloadNotifier {
     @GuardedBy("mDownloadSpeed")
     private final LongSparseLongArray mDownloadSpeed = new LongSparseLongArray();
 
+    /**
+     * Last time speed was reproted, mapped from {@link DownloadInfo#mId} to
+     * {@link SystemClock#elapsedRealtime()}.
+     */
+    @GuardedBy("mDownloadSpeed")
+    private final LongSparseLongArray mDownloadTouch = new LongSparseLongArray();
+
     public DownloadNotifier(Context context) {
         mContext = context;
         mNotifManager = (NotificationManager) context.getSystemService(
@@ -86,15 +96,17 @@ public class DownloadNotifier {
     }
 
     /**
-     * Notify the current speed of an active download, used for calcuating
+     * Notify the current speed of an active download, used for calculating
      * estimated remaining time.
      */
     public void notifyDownloadSpeed(long id, long bytesPerSecond) {
         synchronized (mDownloadSpeed) {
             if (bytesPerSecond != 0) {
                 mDownloadSpeed.put(id, bytesPerSecond);
+                mDownloadTouch.put(id, SystemClock.elapsedRealtime());
             } else {
                 mDownloadSpeed.delete(id);
+                mDownloadTouch.delete(id);
             }
         }
     }
@@ -302,6 +314,17 @@ public class DownloadNotifier {
         return ids;
     }
 
+    public void dumpSpeeds() {
+        synchronized (mDownloadSpeed) {
+            for (int i = 0; i < mDownloadSpeed.size(); i++) {
+                final long id = mDownloadSpeed.keyAt(i);
+                final long delta = SystemClock.elapsedRealtime() - mDownloadTouch.get(id);
+                Log.d(TAG, "Download " + id + " speed " + mDownloadSpeed.valueAt(i) + "bps, "
+                        + delta + "ms ago");
+            }
+        }
+    }
+
     /**
      * Build tag used for collapsing several {@link DownloadInfo} into a single
      * {@link Notification}.
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 07641097..6c61193c 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -49,6 +49,7 @@ import com.google.common.collect.Sets;
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -71,7 +72,7 @@ public class DownloadService extends Service {
     // TODO: migrate WakeLock from individual DownloadThreads out into
     // DownloadReceiver to protect our entire workflow.
 
-    private static final boolean DEBUG_LIFECYCLE = true;
+    private static final boolean DEBUG_LIFECYCLE = false;
 
     @VisibleForTesting
     SystemFacade mSystemFacade;
@@ -237,6 +238,17 @@ public class DownloadService extends Service {
             }
 
             if (msg.what == MSG_FINAL_UPDATE) {
+                // Dump thread stacks belonging to pool
+                for (Map.Entry<Thread, StackTraceElement[]> entry :
+                        Thread.getAllStackTraces().entrySet()) {
+                    if (entry.getKey().getName().startsWith("pool")) {
+                        Log.d(TAG, entry.getKey() + ": " + Arrays.toString(entry.getValue()));
+                    }
+                }
+
+                // Dump speed and update details
+                mNotifier.dumpSpeeds();
+
                 Log.wtf(TAG, "Final update pass triggered, isActive=" + isActive
                         + "; someone didn't update correctly.");
             }
-- 
cgit v1.2.3


From 97d507d95f9885ceb12f2ce2483361b5ed265f9f Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Mon, 25 Mar 2013 17:45:19 -0700
Subject: Avoid sending messages after HandlerThread.quit().

Bug: 8470658
Change-Id: I4cfd6a01c2c2d845a72d3f58c29eec8b44176537
---
 .../android/providers/downloads/DownloadService.java   | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 6c61193c..c519faaf 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -189,6 +189,7 @@ public class DownloadService extends Service {
         getContentResolver().unregisterContentObserver(mObserver);
         mScanner.shutdown();
         mUpdateThread.quit();
+        mUpdateThread = null;
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onDestroy");
         }
@@ -199,8 +200,10 @@ public class DownloadService extends Service {
      * Enqueue an {@link #updateLocked()} pass to occur in future.
      */
     private void enqueueUpdate() {
-        mUpdateHandler.removeMessages(MSG_UPDATE);
-        mUpdateHandler.obtainMessage(MSG_UPDATE, mLastStartId, -1).sendToTarget();
+        if (mUpdateThread != null) {
+            mUpdateHandler.removeMessages(MSG_UPDATE);
+            mUpdateHandler.obtainMessage(MSG_UPDATE, mLastStartId, -1).sendToTarget();
+        }
     }
 
     /**
@@ -208,10 +211,12 @@ public class DownloadService extends Service {
      * catch any finished operations that didn't trigger an update pass.
      */
     private void enqueueFinalUpdate() {
-        mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
-        mUpdateHandler.sendMessageDelayed(
-                mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
-                5 * MINUTE_IN_MILLIS);
+        if (mUpdateThread != null) {
+            mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
+            mUpdateHandler.sendMessageDelayed(
+                    mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
+                    5 * MINUTE_IN_MILLIS);
+        }
     }
 
     private static final int MSG_UPDATE = 1;
@@ -269,6 +274,7 @@ public class DownloadService extends Service {
                 if (stopSelfResult(startId)) {
                     if (DEBUG_LIFECYCLE) Log.v(TAG, "Nothing left; stopped");
                     mUpdateThread.quit();
+                    mUpdateThread = null;
                 }
             }
 
-- 
cgit v1.2.3


From 0f2f63932bd8d963764d37527b3237d219fdaa4d Mon Sep 17 00:00:00 2001
From: Fabrice Di Meglio <fdimeglio@google.com>
Date: Tue, 26 Mar 2013 03:12:57 +0000
Subject: Revert "Avoid sending messages after HandlerThread.quit()."

This reverts commit 97d507d95f9885ceb12f2ce2483361b5ed265f9f

Change-Id: I20374301561b3a1c79c2d986292af50049de2aac
---
 .../android/providers/downloads/DownloadService.java   | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index c519faaf..6c61193c 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -189,7 +189,6 @@ public class DownloadService extends Service {
         getContentResolver().unregisterContentObserver(mObserver);
         mScanner.shutdown();
         mUpdateThread.quit();
-        mUpdateThread = null;
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onDestroy");
         }
@@ -200,10 +199,8 @@ public class DownloadService extends Service {
      * Enqueue an {@link #updateLocked()} pass to occur in future.
      */
     private void enqueueUpdate() {
-        if (mUpdateThread != null) {
-            mUpdateHandler.removeMessages(MSG_UPDATE);
-            mUpdateHandler.obtainMessage(MSG_UPDATE, mLastStartId, -1).sendToTarget();
-        }
+        mUpdateHandler.removeMessages(MSG_UPDATE);
+        mUpdateHandler.obtainMessage(MSG_UPDATE, mLastStartId, -1).sendToTarget();
     }
 
     /**
@@ -211,12 +208,10 @@ public class DownloadService extends Service {
      * catch any finished operations that didn't trigger an update pass.
      */
     private void enqueueFinalUpdate() {
-        if (mUpdateThread != null) {
-            mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
-            mUpdateHandler.sendMessageDelayed(
-                    mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
-                    5 * MINUTE_IN_MILLIS);
-        }
+        mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
+        mUpdateHandler.sendMessageDelayed(
+                mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
+                5 * MINUTE_IN_MILLIS);
     }
 
     private static final int MSG_UPDATE = 1;
@@ -274,7 +269,6 @@ public class DownloadService extends Service {
                 if (stopSelfResult(startId)) {
                     if (DEBUG_LIFECYCLE) Log.v(TAG, "Nothing left; stopped");
                     mUpdateThread.quit();
-                    mUpdateThread = null;
                 }
             }
 
-- 
cgit v1.2.3


From ff0220f5b4624049a1052bd868d7706eee5a0daf Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 26 Mar 2013 11:18:16 -0700
Subject: Unregister observer when tearing down service.

Also reduce and adjust some logging.

Bug: 8470658
Change-Id: Ia1f1cbd315ded04edd2113506e5c5a1db5ec85b4
---
 src/com/android/providers/downloads/DownloadService.java | 2 ++
 src/com/android/providers/downloads/DownloadThread.java  | 8 ++++----
 src/com/android/providers/downloads/StorageManager.java  | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 6c61193c..7d746cca 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -268,6 +268,8 @@ public class DownloadService extends Service {
 
                 if (stopSelfResult(startId)) {
                     if (DEBUG_LIFECYCLE) Log.v(TAG, "Nothing left; stopped");
+                    getContentResolver().unregisterContentObserver(mObserver);
+                    mScanner.shutdown();
                     mUpdateThread.quit();
                 }
             }
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index f1add241..6a0eb47e 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -193,7 +193,7 @@ public class DownloadThread implements Runnable {
             // while performing download, register for rules updates
             netPolicy.registerListener(mPolicyListener);
 
-            Log.i(Constants.TAG, "Initiating download " + mInfo.mId);
+            Log.i(Constants.TAG, "Download " + mInfo.mId + " starting");
 
             // Remember which network this download started on; used to
             // determine if errors were due to network changes.
@@ -216,9 +216,6 @@ public class DownloadThread implements Runnable {
 
             executeDownload(state);
 
-            if (Constants.LOGV) {
-                Log.v(Constants.TAG, "download completed for " + mInfo.mUri);
-            }
             finalizeDestinationFile(state);
             finalStatus = Downloads.Impl.STATUS_SUCCESS;
         } catch (StopRequestException error) {
@@ -272,6 +269,9 @@ public class DownloadThread implements Runnable {
             cleanupDestination(state, finalStatus);
             notifyDownloadCompleted(state, finalStatus, errorMsg, numFailed);
 
+            Log.i(Constants.TAG, "Download " + mInfo.mId + " finished with status "
+                    + Downloads.Impl.statusToString(finalStatus));
+
             netPolicy.unregisterListener(mPolicyListener);
 
             if (wakeLock != null) {
diff --git a/src/com/android/providers/downloads/StorageManager.java b/src/com/android/providers/downloads/StorageManager.java
index 8ca17300..deb412e7 100644
--- a/src/com/android/providers/downloads/StorageManager.java
+++ b/src/com/android/providers/downloads/StorageManager.java
@@ -358,7 +358,7 @@ class StorageManager {
      * This is not a very common occurrence. So, do this only once in a while.
      */
     private void removeSpuriousFiles() {
-        if (true || Constants.LOGV) {
+        if (Constants.LOGV) {
             Log.i(Constants.TAG, "in removeSpuriousFiles");
         }
         // get a list of all files in system cache dir and downloads data dir
-- 
cgit v1.2.3


From 5048492e352be8134d92178d757a60117491e292 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 26 Mar 2013 13:15:43 -0700
Subject: New PendingIntents when extras change.

Otherwise notifications end up launching with stale download IDs.

Bug: 8417220
Change-Id: Ie72a2f4ac7b72213678ac6001af45709034492dd
---
 src/com/android/providers/downloads/DownloadNotifier.java | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadNotifier.java b/src/com/android/providers/downloads/DownloadNotifier.java
index f832eae3..df0bf840 100644
--- a/src/com/android/providers/downloads/DownloadNotifier.java
+++ b/src/com/android/providers/downloads/DownloadNotifier.java
@@ -165,7 +165,8 @@ public class DownloadNotifier {
                         null, mContext, DownloadReceiver.class);
                 intent.putExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS,
                         getDownloadIds(cluster));
-                builder.setContentIntent(PendingIntent.getBroadcast(mContext, 0, intent, 0));
+                builder.setContentIntent(PendingIntent.getBroadcast(
+                        mContext, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT));
                 builder.setOngoing(true);
 
             } else if (type == TYPE_COMPLETE) {
@@ -187,7 +188,8 @@ public class DownloadNotifier {
                 final Intent intent = new Intent(action, uri, mContext, DownloadReceiver.class);
                 intent.putExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS,
                         getDownloadIds(cluster));
-                builder.setContentIntent(PendingIntent.getBroadcast(mContext, 0, intent, 0));
+                builder.setContentIntent(PendingIntent.getBroadcast(
+                        mContext, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT));
 
                 final Intent hideIntent = new Intent(Constants.ACTION_HIDE,
                         uri, mContext, DownloadReceiver.class);
-- 
cgit v1.2.3


From 498f62e2da65df9e11e8da6feb959ed0d513f792 Mon Sep 17 00:00:00 2001
From: Geremy Condra <gcondra@google.com>
Date: Wed, 3 Apr 2013 19:07:45 -0700
Subject: Restore the appropriate SELinux context to the downloads dir.

Change-Id: I4839fd07abdd1c6b866f1d94dc36567df047e30c
---
 src/com/android/providers/downloads/DownloadProvider.java | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 2d0c807a..e0b5842d 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -37,6 +37,7 @@ import android.os.Binder;
 import android.os.Environment;
 import android.os.ParcelFileDescriptor;
 import android.os.Process;
+import android.os.SELinux;
 import android.provider.BaseColumns;
 import android.provider.Downloads;
 import android.provider.OpenableColumns;
@@ -441,8 +442,7 @@ public final class DownloadProvider extends ContentProvider {
             appInfo = getContext().getPackageManager().
                     getApplicationInfo("com.android.defcontainer", 0);
         } catch (NameNotFoundException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
+            Log.wtf(Constants.TAG, "Could not get ApplicationInfo for com.android.defconatiner", e);
         }
         if (appInfo != null) {
             mDefContainerUid = appInfo.uid;
@@ -452,6 +452,11 @@ public final class DownloadProvider extends ContentProvider {
         Context context = getContext();
         context.startService(new Intent(context, DownloadService.class));
         mDownloadsDataDir = StorageManager.getDownloadDataDirectory(getContext());
+        try {
+            SELinux.restorecon(mDownloadsDataDir.getCanonicalPath());
+        } catch (IOException e) {
+            Log.wtf(Constants.TAG, "Could not get canonical path for download directory", e);
+        }
         return true;
     }
 
-- 
cgit v1.2.3


From af49093b1554fdc55977e9281505e05865f33813 Mon Sep 17 00:00:00 2001
From: Danny Baumann <dannybaumann@web.de>
Date: Wed, 3 Apr 2013 14:36:21 +0200
Subject: Fix download notification click behaviour.

PendingIntent.getBroadcast() doesn't update the intent extras if not
explicitly given something to distinguish the intents. This caused the
notification on-click to do nothing on everything but the first
download.

Change-Id: I29544ae5b04f8304cbbe720066a26ff90e422107
---
 src/com/android/providers/downloads/DownloadNotifier.java | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadNotifier.java b/src/com/android/providers/downloads/DownloadNotifier.java
index df0bf840..04054783 100644
--- a/src/com/android/providers/downloads/DownloadNotifier.java
+++ b/src/com/android/providers/downloads/DownloadNotifier.java
@@ -161,12 +161,14 @@ public class DownloadNotifier {
 
             // Build action intents
             if (type == TYPE_ACTIVE || type == TYPE_WAITING) {
+                // build a synthetic uri for intent identification purposes
+                final Uri uri = new Uri.Builder().scheme("active-dl").appendPath(tag).build();
                 final Intent intent = new Intent(Constants.ACTION_LIST,
-                        null, mContext, DownloadReceiver.class);
+                        uri, mContext, DownloadReceiver.class);
                 intent.putExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS,
                         getDownloadIds(cluster));
-                builder.setContentIntent(PendingIntent.getBroadcast(
-                        mContext, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT));
+                builder.setContentIntent(PendingIntent.getBroadcast(mContext,
+                        0, intent, PendingIntent.FLAG_UPDATE_CURRENT));
                 builder.setOngoing(true);
 
             } else if (type == TYPE_COMPLETE) {
@@ -188,8 +190,8 @@ public class DownloadNotifier {
                 final Intent intent = new Intent(action, uri, mContext, DownloadReceiver.class);
                 intent.putExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS,
                         getDownloadIds(cluster));
-                builder.setContentIntent(PendingIntent.getBroadcast(
-                        mContext, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT));
+                builder.setContentIntent(PendingIntent.getBroadcast(mContext,
+                        0, intent, PendingIntent.FLAG_UPDATE_CURRENT));
 
                 final Intent hideIntent = new Intent(Constants.ACTION_HIDE,
                         uri, mContext, DownloadReceiver.class);
-- 
cgit v1.2.3


From 64a9e6cb3628db724e04492cf207757348960eb7 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Sun, 28 Apr 2013 15:55:53 -0700
Subject: Completed downloads should clear when touched.

Bug: 8744610
Change-Id: I135a3acbc819fd725f00ba57461e21db1fe24850
---
 src/com/android/providers/downloads/DownloadNotifier.java | 1 +
 1 file changed, 1 insertion(+)

(limited to 'src/com/android/providers')

diff --git a/src/com/android/providers/downloads/DownloadNotifier.java b/src/com/android/providers/downloads/DownloadNotifier.java
index 04054783..ac52eba2 100644
--- a/src/com/android/providers/downloads/DownloadNotifier.java
+++ b/src/com/android/providers/downloads/DownloadNotifier.java
@@ -175,6 +175,7 @@ public class DownloadNotifier {
                 final DownloadInfo info = cluster.iterator().next();
                 final Uri uri = ContentUris.withAppendedId(
                         Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, info.mId);
+                builder.setAutoCancel(true);
 
                 final String action;
                 if (Downloads.Impl.isStatusError(info.mStatus)) {
-- 
cgit v1.2.3