diff options
9 files changed, 105 insertions, 127 deletions
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java index a2048958..b6f478c8 100644 --- a/src/com/android/providers/downloads/DownloadInfo.java +++ b/src/com/android/providers/downloads/DownloadInfo.java @@ -227,7 +227,6 @@ public class DownloadInfo { public String mTitle; public String mDescription; public int mBypassRecommendedSizeLimit; - public String mPausedReason; public int mFuzz; @@ -275,10 +274,12 @@ public class DownloadInfo { } /** - * Returns the time when a download should be restarted. Must only - * be called when numFailed > 0. + * Returns the time when a download should be restarted. */ - public long restartTime() { + public long restartTime(long now) { + if (mNumFailed == 0) { + return now; + } if (mRetryAfter > 0) { return mLastMod + mRetryAfter; } @@ -291,67 +292,29 @@ public class DownloadInfo { * Returns whether this download (which the download manager hasn't seen yet) * should be started. */ - public boolean isReadyToStart(long now) { + private boolean isReadyToStart(long now) { + if (mHasActiveThread) { + // already running + return false; + } if (mControl == Downloads.Impl.CONTROL_PAUSED) { // the download is paused, so it's not going to start return false; } - if (mStatus == 0) { - // status hasn't been initialized yet, this is a new download - return true; - } - if (mStatus == Downloads.Impl.STATUS_PENDING) { - // download is explicit marked as ready to start - return true; - } - if (mStatus == Downloads.Impl.STATUS_RUNNING) { - // download was interrupted (process killed, loss of power) while it was running, - // without a chance to update the database - return true; - } - if (mStatus == Downloads.Impl.STATUS_RUNNING_PAUSED) { - if (mNumFailed == 0) { - // download is waiting for network connectivity to return before it can resume + switch (mStatus) { + case 0: // status hasn't been initialized yet, this is a new download + case Downloads.Impl.STATUS_PENDING: // download is explicit marked as ready to start + case Downloads.Impl.STATUS_RUNNING: // download interrupted (process killed etc) while + // running, without a chance to update the database return true; - } - if (restartTime() < now) { - // download was waiting for a delayed restart, and the delay has expired - return true; - } - } - return false; - } - /** - * Returns whether this download (which the download manager has already seen - * and therefore potentially started) should be restarted. - * - * In a nutshell, this returns true if the download isn't already running - * but should be, and it can know whether the download is already running - * by checking the status. - */ - public boolean isReadyToRestart(long now) { - if (mControl == Downloads.Impl.CONTROL_PAUSED) { - // the download is paused, so it's not going to restart - return false; - } - if (mStatus == 0) { - // download hadn't been initialized yet - return true; - } - if (mStatus == Downloads.Impl.STATUS_PENDING) { - // download is explicit marked as ready to start - return true; - } - if (mStatus == Downloads.Impl.STATUS_RUNNING_PAUSED) { - if (mNumFailed == 0) { - // download is waiting for network connectivity to return before it can resume + case Downloads.Impl.STATUS_WAITING_FOR_NETWORK: + case Downloads.Impl.STATUS_QUEUED_FOR_WIFI: return checkCanUseNetwork() == NETWORK_OK; - } - if (restartTime() < now) { - // download was waiting for a delayed restart, and the delay has expired - return true; - } + + case Downloads.Impl.STATUS_WAITING_TO_RETRY: + // download was waiting for a delayed restart + return restartTime(now) <= now; } return false; } @@ -450,7 +413,11 @@ public class DownloadInfo { return NETWORK_OK; } - void start(long now) { + void startIfReady(long now) { + if (!isReadyToStart(now)) { + return; + } + if (Constants.LOGV) { Log.v(Constants.TAG, "Service spawning thread to handle download " + mId); } @@ -521,13 +488,10 @@ public class DownloadInfo { if (Downloads.Impl.isStatusCompleted(mStatus)) { return -1; } - if (mStatus != Downloads.Impl.STATUS_RUNNING_PAUSED) { - return 0; - } - if (mNumFailed == 0) { + if (mStatus != Downloads.Impl.STATUS_WAITING_TO_RETRY) { return 0; } - long when = restartTime(); + long when = restartTime(now); if (when <= now) { return 0; } @@ -545,8 +509,6 @@ public class DownloadInfo { } void notifyPauseDueToSize(boolean isWifiRequired) { - mPausedReason = mContext.getResources().getString( - R.string.notification_need_wifi_for_size); Intent intent = new Intent(Intent.ACTION_VIEW); intent.setData(getAllDownloadsUri()); intent.setClassName(SizeLimitActivity.class.getPackage().getName(), diff --git a/src/com/android/providers/downloads/DownloadNotification.java b/src/com/android/providers/downloads/DownloadNotification.java index 90c8693b..4d615df7 100644 --- a/src/com/android/providers/downloads/DownloadNotification.java +++ b/src/com/android/providers/downloads/DownloadNotification.java @@ -138,8 +138,10 @@ class DownloadNotification { item.addItem(title, progress, max); mNotifications.put(packageName, item); } - if (hasPausedReason(download) && item.mPausedText == null) { - item.mPausedText = download.mPausedReason; + if (download.mStatus == Downloads.Impl.STATUS_QUEUED_FOR_WIFI + && item.mPausedText == null) { + item.mPausedText = mContext.getResources().getString( + R.string.notification_need_wifi_for_size); } } @@ -205,10 +207,6 @@ class DownloadNotification { } } - private boolean hasPausedReason(DownloadInfo download) { - return download.mStatus == Downloads.STATUS_RUNNING_PAUSED && download.mPausedReason != null; - } - private void updateCompletedNotification(Collection<DownloadInfo> downloads) { for (DownloadInfo download : downloads) { if (!isCompleteAndVisible(download)) { diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index c79ecc44..8fbcf4b4 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -439,10 +439,7 @@ public class DownloadService extends Service { info.logVerboseInfo(); } - if (info.isReadyToStart(now)) { - info.start(now); - } - + info.startIfReady(now); return info; } @@ -466,9 +463,7 @@ public class DownloadService extends Service { mSystemFacade.cancelNotification(info.mId); } - if (info.isReadyToRestart(now)) { - info.start(now); - } + info.startIfReady(now); } /** diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java index 12ddfa60..b3847715 100644 --- a/src/com/android/providers/downloads/DownloadThread.java +++ b/src/com/android/providers/downloads/DownloadThread.java @@ -141,7 +141,6 @@ public class DownloadThread extends Thread { AndroidHttpClient client = null; PowerManager.WakeLock wakeLock = null; int finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR; - mInfo.mPausedReason = null; try { PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE); @@ -157,6 +156,7 @@ public class DownloadThread extends Thread { boolean finished = false; while(!finished) { + Log.i(Constants.TAG, "Initiating request for download " + mInfo.mId); HttpGet request = new HttpGet(state.mRequestUri); try { executeDownload(state, client, request); @@ -175,25 +175,19 @@ public class DownloadThread extends Thread { finalizeDestinationFile(state); finalStatus = Downloads.Impl.STATUS_SUCCESS; } catch (StopRequest error) { - if (Constants.LOGV) { - Log.v(Constants.TAG, "Aborting request for " + mInfo.mUri, error); - } + // remove the cause before printing, in case it contains PII + Log.w(Constants.TAG, "Aborting request for download " + mInfo.mId, removeCause(error)); finalStatus = error.mFinalStatus; // fall through to finally block } catch (FileNotFoundException ex) { - Log.d(Constants.TAG, "FileNotFoundException for " + state.mFilename + " : " + ex); + Log.w(Constants.TAG, "FileNotFoundException for " + state.mFilename, ex); finalStatus = Downloads.Impl.STATUS_FILE_ERROR; // falls through to the code that reports an error - } catch (RuntimeException ex) { //sometimes the socket code throws unchecked exceptions - if (Constants.LOGV) { - Log.d(Constants.TAG, "Exception for " + mInfo.mUri, ex); - } else if (Config.LOGD) { - Log.d(Constants.TAG, "Exception for id " + mInfo.mId, ex); - } + } catch (Throwable ex) { //sometimes the socket code throws unchecked exceptions + Log.w(Constants.TAG, "Exception for id " + mInfo.mId, ex); finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR; // falls through to the code that reports an error } finally { - mInfo.mHasActiveThread = false; if (wakeLock != null) { wakeLock.release(); wakeLock = null; @@ -206,10 +200,20 @@ public class DownloadThread extends Thread { notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter, state.mRedirectCount, state.mGotData, state.mFilename, state.mNewUri, state.mMimeType); + mInfo.mHasActiveThread = false; } } /** + * @return an identical StopRequest but with the cause removed. + */ + private StopRequest removeCause(StopRequest error) { + StopRequest newException = new StopRequest(error.mFinalStatus); + newException.setStackTrace(error.getStackTrace()); + return newException; + } + + /** * Fully execute a single download request - setup and send the request, handle the response, * and transfer the data to the destination file. */ @@ -242,12 +246,15 @@ public class DownloadThread extends Thread { private void checkConnectivity(State state) throws StopRequest { int networkUsable = mInfo.checkCanUseNetwork(); if (networkUsable != DownloadInfo.NETWORK_OK) { + int status = Downloads.Impl.STATUS_WAITING_FOR_NETWORK; if (networkUsable == DownloadInfo.NETWORK_UNUSABLE_DUE_TO_SIZE) { + status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI; mInfo.notifyPauseDueToSize(true); } else if (networkUsable == DownloadInfo.NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE) { + status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI; mInfo.notifyPauseDueToSize(false); } - throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED); + throw new StopRequest(status); } } @@ -385,7 +392,7 @@ public class DownloadThread extends Thread { if (Constants.LOGV) { Log.v(Constants.TAG, "paused " + mInfo.mUri); } - throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED); + throw new StopRequest(Downloads.Impl.STATUS_PAUSED_BY_APP); } } if (mInfo.mStatus == Downloads.Impl.STATUS_CANCELED) { @@ -766,7 +773,7 @@ public class DownloadThread extends Thread { // ignored - retryAfter stays 0 in this case. } } - throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED); + throw new StopRequest(Downloads.Impl.STATUS_WAITING_TO_RETRY); } /** @@ -801,10 +808,10 @@ public class DownloadThread extends Thread { } if (!Helpers.isNetworkAvailable(mSystemFacade)) { - return Downloads.Impl.STATUS_RUNNING_PAUSED; + return Downloads.Impl.STATUS_WAITING_FOR_NETWORK; } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) { state.mCountRetry = true; - return Downloads.Impl.STATUS_RUNNING_PAUSED; + return Downloads.Impl.STATUS_WAITING_TO_RETRY; } else { Log.d(Constants.TAG, "reached max retries: " + message + " for " + mInfo.mId); return Downloads.Impl.STATUS_HTTP_DATA_ERROR; diff --git a/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java b/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java index 350c63d4..0cb63e0f 100644 --- a/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java @@ -82,7 +82,7 @@ public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFuncti downloadUri = requestDownload("/path"); updateDownload(downloadUri, Downloads.COLUMN_DESTINATION, Integer.toString(Downloads.DESTINATION_CACHE_PARTITION_NOROAMING)); - runUntilStatus(downloadUri, Downloads.STATUS_RUNNING_PAUSED); + runUntilStatus(downloadUri, Downloads.Impl.STATUS_WAITING_FOR_NETWORK); // ...and pick up when we're off roaming enqueueResponse(HTTP_OK, FILE_CONTENT); diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index ab7d0e4e..cad01df6 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -531,7 +531,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { Download download = enqueueRequest(getRequest().setDestinationUri(destination)); download.runUntilStatus(DownloadManager.STATUS_FAILED); assertEquals(DownloadManager.ERROR_FILE_ALREADY_EXISTS, - download.getLongField(DownloadManager.COLUMN_ERROR_CODE)); + download.getLongField(DownloadManager.COLUMN_REASON)); } public void testEmptyFields() throws Exception { @@ -542,7 +542,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { assertEquals(0, download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR)); assertEquals(-1, download.getLongField(DownloadManager.COLUMN_TOTAL_SIZE_BYTES)); // just ensure no exception is thrown - download.getLongField(DownloadManager.COLUMN_ERROR_CODE); + download.getLongField(DownloadManager.COLUMN_REASON); } public void testRestart() throws Exception { @@ -567,7 +567,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { Download download = enqueueRequest(getRequest()); download.runUntilStatus(DownloadManager.STATUS_FAILED); assertEquals(expectedErrorCode, - download.getLongField(DownloadManager.COLUMN_ERROR_CODE)); + download.getLongField(DownloadManager.COLUMN_REASON)); } /** diff --git a/ui/res/values/strings.xml b/ui/res/values/strings.xml index bb3654af..6bd9f2c9 100644 --- a/ui/res/values/strings.xml +++ b/ui/res/values/strings.xml @@ -38,9 +38,9 @@ <!-- Status messages --> - <!-- Status indicating that the download has not yet begun. Appears for an individual item in - the download list. [CHAR LIMIT=11] --> - <string name="download_pending">Queued</string> + <!-- Status indicating that the download has been queued to start in the future. Appears for an + individual item in the download list. [CHAR LIMIT=11] --> + <string name="download_queued">Queued</string> <!-- Status indicating that the system is currently downloading the file. Appears for an individual item in the download list. [CHAR LIMIT=11] --> <string name="download_running">In progress</string> diff --git a/ui/src/com/android/providers/downloads/ui/DownloadAdapter.java b/ui/src/com/android/providers/downloads/ui/DownloadAdapter.java index c7d4c2cb..9c572538 100644 --- a/ui/src/com/android/providers/downloads/ui/DownloadAdapter.java +++ b/ui/src/com/android/providers/downloads/ui/DownloadAdapter.java @@ -57,6 +57,7 @@ public class DownloadAdapter extends CursorAdapter { private int mTitleColumnId; private int mDescriptionColumnId; private int mStatusColumnId; + private int mReasonColumnId; private int mTotalBytesColumnId; private int mMediaTypeColumnId; private int mDateColumnId; @@ -76,6 +77,7 @@ public class DownloadAdapter extends CursorAdapter { mTitleColumnId = cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TITLE); mDescriptionColumnId = cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_DESCRIPTION); mStatusColumnId = cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS); + mReasonColumnId = cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON); mTotalBytesColumnId = cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TOTAL_SIZE_BYTES); mMediaTypeColumnId = cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_MEDIA_TYPE); mDateColumnId = @@ -150,11 +152,15 @@ public class DownloadAdapter extends CursorAdapter { return R.string.download_success; case DownloadManager.STATUS_PENDING: - return R.string.download_pending; - case DownloadManager.STATUS_RUNNING: - case DownloadManager.STATUS_PAUSED: return R.string.download_running; + + case DownloadManager.STATUS_PAUSED: + if (mCursor.getInt(mReasonColumnId) == DownloadManager.PAUSED_QUEUED_FOR_WIFI) { + return R.string.download_queued; + } else { + return R.string.download_running; + } } throw new IllegalStateException("Unknown status: " + mCursor.getInt(mStatusColumnId)); } diff --git a/ui/src/com/android/providers/downloads/ui/DownloadList.java b/ui/src/com/android/providers/downloads/ui/DownloadList.java index 4b3547c2..bb331534 100644 --- a/ui/src/com/android/providers/downloads/ui/DownloadList.java +++ b/ui/src/com/android/providers/downloads/ui/DownloadList.java @@ -85,7 +85,7 @@ public class DownloadList extends Activity private int mIdColumnId; private int mLocalUriColumnId; private int mMediaTypeColumnId; - private int mErrorCodeColumndId; + private int mReasonColumndId; private boolean mIsSortedBySize = false; private Set<Long> mSelectedIds = new HashSet<Long>(); @@ -94,8 +94,9 @@ public class DownloadList extends Activity * We keep track of when a dialog is being displayed for a pending download, because if that * download starts running, we want to immediately hide the dialog. */ - private Long mPendingDownloadId = null; - private AlertDialog mPendingDialog; + private Long mQueuedDownloadId = null; + private AlertDialog mQueuedDialog; + private class MyContentObserver extends ContentObserver { public MyContentObserver() { @@ -145,8 +146,8 @@ public class DownloadList extends Activity mDateSortedCursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LOCAL_URI); mMediaTypeColumnId = mDateSortedCursor.getColumnIndexOrThrow(DownloadManager.COLUMN_MEDIA_TYPE); - mErrorCodeColumndId = - mDateSortedCursor.getColumnIndexOrThrow(DownloadManager.COLUMN_ERROR_CODE); + mReasonColumndId = + mDateSortedCursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON); mDateSortedAdapter = new DateSortedDownloadAdapter(this, mDateSortedCursor, this); mDateOrderedListView.setAdapter(mDateSortedAdapter); @@ -359,19 +360,23 @@ public class DownloadList extends Activity long id = cursor.getInt(mIdColumnId); switch (cursor.getInt(mStatusColumnId)) { case DownloadManager.STATUS_PENDING: - mPendingDownloadId = id; - mPendingDialog = new AlertDialog.Builder(this) - .setTitle(R.string.dialog_title_not_available) - .setMessage(R.string.dialog_queued_body) - .setPositiveButton(R.string.keep_queued_download, null) - .setNegativeButton(R.string.remove_download, getDeleteClickHandler(id)) - .setOnCancelListener(this) - .show(); + case DownloadManager.STATUS_RUNNING: + sendRunningDownloadClickedBroadcast(id); break; - case DownloadManager.STATUS_RUNNING: case DownloadManager.STATUS_PAUSED: - sendRunningDownloadClickedBroadcast(id); + if (isPausedForWifi(cursor)) { + mQueuedDownloadId = id; + mQueuedDialog = new AlertDialog.Builder(this) + .setTitle(R.string.dialog_title_not_available) + .setMessage(R.string.dialog_queued_body) + .setPositiveButton(R.string.keep_queued_download, null) + .setNegativeButton(R.string.remove_download, getDeleteClickHandler(id)) + .setOnCancelListener(this) + .show(); + } else { + sendRunningDownloadClickedBroadcast(id); + } break; case DownloadManager.STATUS_SUCCESSFUL: @@ -388,7 +393,7 @@ public class DownloadList extends Activity * @return the appropriate error message for the failed download pointed to by cursor */ private String getErrorMessage(Cursor cursor) { - switch (cursor.getInt(mErrorCodeColumndId)) { + switch (cursor.getInt(mReasonColumndId)) { case DownloadManager.ERROR_FILE_ALREADY_EXISTS: if (isOnExternalStorage(cursor)) { return getString(R.string.dialog_file_already_exists); @@ -616,13 +621,18 @@ public class DownloadList extends Activity void handleDownloadsChanged() { checkSelectionForDeletedEntries(); - if (mPendingDownloadId != null && moveToDownload(mPendingDownloadId)) { - if (mDateSortedCursor.getInt(mStatusColumnId) != DownloadManager.STATUS_PENDING) { - mPendingDialog.cancel(); + if (mQueuedDownloadId != null && moveToDownload(mQueuedDownloadId)) { + if (mDateSortedCursor.getInt(mStatusColumnId) != DownloadManager.STATUS_PAUSED + || !isPausedForWifi(mDateSortedCursor)) { + mQueuedDialog.cancel(); } } } + private boolean isPausedForWifi(Cursor cursor) { + return cursor.getInt(mReasonColumndId) == DownloadManager.PAUSED_QUEUED_FOR_WIFI; + } + /** * Check if any of the selected downloads have been deleted from the downloads database, and * remove such downloads from the selection. @@ -662,7 +672,7 @@ public class DownloadList extends Activity */ @Override public void onCancel(DialogInterface dialog) { - mPendingDownloadId = null; - mPendingDialog = null; + mQueuedDownloadId = null; + mQueuedDialog = null; } } |