diff options
27 files changed, 752 insertions, 512 deletions
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java index 65aa770..79c4a63 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java @@ -58,8 +58,8 @@ public class ChromeDownloadDelegate implements ContentViewDownloadDelegate { public void onInfoBarButtonClicked(boolean confirm) { assert mTab != null; if (mPendingRequest == null) return; - if (mPendingRequest.hasDownloadId()) { - nativeDangerousDownloadValidated(mTab, mPendingRequest.getDownloadId(), confirm); + if (mPendingRequest.getDownloadGuid() != null) { + nativeDangerousDownloadValidated(mTab, mPendingRequest.getDownloadGuid(), confirm); if (confirm) { showDownloadStartNotification(); } @@ -107,9 +107,10 @@ public class ChromeDownloadDelegate implements ContentViewDownloadDelegate { @Override public void onInfoBarDismissed() { if (mPendingRequest != null) { - if (mPendingRequest.hasDownloadId()) { + if (mPendingRequest.getDownloadGuid() != null) { assert mTab != null; - nativeDangerousDownloadValidated(mTab, mPendingRequest.getDownloadId(), false); + nativeDangerousDownloadValidated( + mTab, mPendingRequest.getDownloadGuid(), false); } else if (!mPendingRequest.isGETRequest()) { // Infobar was dismissed, discard the file if a POST download is pending. discardFile(mPendingRequest.getFilePath()); @@ -269,15 +270,14 @@ public class ChromeDownloadDelegate implements ContentViewDownloadDelegate { * Called when a danagerous download is about to start. * * @param filename File name of the download item. - * @param downloadId ID of the download. + * @param downloadGuid GUID of the download. */ @Override - public void onDangerousDownload(String filename, int downloadId) { + public void onDangerousDownload(String filename, String downloadGuid) { DownloadInfo downloadInfo = new DownloadInfo.Builder() .setFileName(filename) .setDescription(filename) - .setHasDownloadId(true) - .setDownloadId(downloadId).build(); + .setDownloadGuid(downloadGuid).build(); confirmDangerousDownload(downloadInfo); } @@ -416,7 +416,8 @@ public class ChromeDownloadDelegate implements ContentViewDownloadDelegate { */ private void enqueueDownloadManagerRequest(final DownloadInfo info) { DownloadManagerService.getDownloadManagerService( - mContext.getApplicationContext()).enqueueDownloadManagerRequest(info, true); + mContext.getApplicationContext()).enqueueDownloadManagerRequest( + new DownloadItem(true, info), true); closeBlankTab(); } @@ -630,7 +631,7 @@ public class ChromeDownloadDelegate implements ContentViewDownloadDelegate { private static native String nativeGetDownloadWarningText(String filename); private static native boolean nativeIsDownloadDangerous(String filename); private static native void nativeDangerousDownloadValidated( - Object tab, int downloadId, boolean accept); + Object tab, String downloadGuid, boolean accept); private static native void nativeLaunchDownloadOverwriteInfoBar(ChromeDownloadDelegate delegate, Tab tab, DownloadInfo downloadInfo, String fileName, String dirName, String dirFullPath); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadItem.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadItem.java new file mode 100644 index 0000000..1708cb5 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadItem.java @@ -0,0 +1,82 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.download; + +import org.chromium.content.browser.DownloadInfo; + +/** + * A generic class representing a download item. The item can be either downloaded through the + * Android DownloadManager, or through Chrome's network stack + */ +public class DownloadItem { + static final long INVALID_DOWNLOAD_ID = -1L; + private boolean mUseAndroidDownloadManager; + private DownloadInfo mDownloadInfo; + private long mDownloadId = INVALID_DOWNLOAD_ID; + + public DownloadItem(boolean useAndroidDownloadManager, DownloadInfo info) { + mUseAndroidDownloadManager = useAndroidDownloadManager; + mDownloadInfo = info; + } + + /** + * Sets the system download ID retrieved from Android DownloadManager. + * + * @param downloadId ID from the Android DownloadManager. + */ + public void setSystemDownloadId(long downloadId) { + mDownloadId = downloadId; + } + + /** + * @return whether the download item has a valid system download ID. + */ + public boolean hasSystemDownloadId() { + return mDownloadId != INVALID_DOWNLOAD_ID; + } + + /** + * @return notification ID for this download. + */ + public int getNotificationId() { + if (mUseAndroidDownloadManager) { + return (int) mDownloadId; + } + return mDownloadInfo.getNotificationId(); + } + + /** + * @return System download ID from the Android DownloadManager. + */ + public long getSystemDownloadId() { + return mDownloadId; + } + + /** + * @return String ID that uniquely identifies the download. + */ + public String getId() { + if (mUseAndroidDownloadManager) { + return String.valueOf(mDownloadId); + } + return mDownloadInfo.getDownloadGuid(); + } + + /** + * @return Info about the download. + */ + public DownloadInfo getDownloadInfo() { + return mDownloadInfo; + } + + /** + * Sets the system download info. + * + * @param info Download information. + */ + public void setDownloadInfo(DownloadInfo info) { + mDownloadInfo = info; + } +} diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java index 676662e..90efe4e 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java @@ -55,6 +55,9 @@ import java.util.concurrent.atomic.AtomicBoolean; * Chrome implementation of the {@link DownloadController.DownloadNotificationService} interface. * This class is responsible for keeping track of which downloads are in progress. It generates * updates for progress of downloads and handles cleaning up of interrupted progress notifications. + * TODO(qinmin): move BroadcastReceiver inheritance into DownloadManagerDelegate, as it handles all + * Android DownloadManager interactions. And DownloadManagerService should not know download Id + * issued by Android DownloadManager. */ public class DownloadManagerService extends BroadcastReceiver implements DownloadController.DownloadNotificationService, @@ -68,7 +71,6 @@ public class DownloadManagerService extends BroadcastReceiver implements protected static final String PENDING_OMA_DOWNLOADS = "PendingOMADownloads"; private static final String UNKNOWN_MIME_TYPE = "application/unknown"; private static final long UPDATE_DELAY_MILLIS = 1000; - private static final long INVALID_DOWNLOAD_ID = -1L; private static final int UNKNOWN_DOWNLOAD_STATUS = -1; // Values for the histogram MobileDownloadResumptionCount. @@ -101,8 +103,8 @@ public class DownloadManagerService extends BroadcastReceiver implements private static DownloadManagerService sDownloadManagerService; private final SharedPreferences mSharedPrefs; - private final ConcurrentHashMap<Integer, DownloadProgress> mDownloadProgressMap = - new ConcurrentHashMap<Integer, DownloadProgress>(4, 0.75f, 2); + private final ConcurrentHashMap<String, DownloadProgress> mDownloadProgressMap = + new ConcurrentHashMap<String, DownloadProgress>(4, 0.75f, 2); private final DownloadNotifier mDownloadNotifier; // Delay between UI updates. @@ -113,12 +115,11 @@ public class DownloadManagerService extends BroadcastReceiver implements private final Handler mHandler; private final Context mContext; - private final LongSparseArray<DownloadInfo> mPendingDownloads = - new LongSparseArray<DownloadInfo>(); + private final LongSparseArray<DownloadItem> mSystemDownloadIdMap = + new LongSparseArray<DownloadItem>(); // Using vector for thread safety. - @VisibleForTesting protected final Vector<Integer> mAutoResumableDownloadIds = - new Vector<Integer>(); - private DownloadBroadcastReceiver mDownloadBroadcastReceiver; + @VisibleForTesting protected final Vector<String> mAutoResumableDownloadIds = + new Vector<String>(); private OMADownloadHandler mOMADownloadHandler; private DownloadSnackbarController mDownloadSnackbarController; private long mNativeDownloadManagerService; @@ -132,22 +133,21 @@ public class DownloadManagerService extends BroadcastReceiver implements private static class DownloadProgress { final long mStartTimeInMillis; boolean mCanDownloadWhileMetered; - volatile DownloadInfo mDownloadInfo; + volatile DownloadItem mDownloadItem; volatile int mDownloadStatus; - volatile boolean mUpdateNotification; DownloadProgress(long startTimeInMillis, boolean canDownloadWhileMetered, - DownloadInfo downloadInfo, int downloadStatus, boolean updateNotification) { + DownloadItem downloadItem, int downloadStatus) { mStartTimeInMillis = startTimeInMillis; mCanDownloadWhileMetered = canDownloadWhileMetered; - mDownloadInfo = downloadInfo; + mDownloadItem = downloadItem; mDownloadStatus = downloadStatus; - mUpdateNotification = updateNotification; } } /** * Class representing an OMA download entry to be stored in SharedPrefs. + * TODO(qinmin): Move all OMA related class and functions to a separate class. */ @VisibleForTesting protected static class OMAEntry { @@ -258,41 +258,45 @@ public class DownloadManagerService extends BroadcastReceiver implements if (downloadInfo.getContentLength() == 0) { status = DOWNLOAD_STATUS_FAILED; } - updateDownloadProgress(downloadInfo, status); + updateDownloadProgress(new DownloadItem(false, downloadInfo), status); scheduleUpdateIfNeeded(); } @Override public void onDownloadUpdated(final DownloadInfo downloadInfo) { - updateDownloadProgress(downloadInfo, DOWNLOAD_STATUS_IN_PROGRESS); + DownloadItem item = new DownloadItem(false, downloadInfo); + updateDownloadProgress(item, DOWNLOAD_STATUS_IN_PROGRESS); // If user manually paused a download, this download is no longer auto resumable. if (downloadInfo.isPaused()) { - removeAutoResumableDownload(downloadInfo.getDownloadId()); + removeAutoResumableDownload(item.getId()); } scheduleUpdateIfNeeded(); } @Override public void onDownloadCancelled(final DownloadInfo downloadInfo) { - updateDownloadProgress(downloadInfo, DOWNLOAD_STATUS_CANCELLED); - removeAutoResumableDownload(downloadInfo.getDownloadId()); + DownloadItem item = new DownloadItem(false, downloadInfo); + updateDownloadProgress(new DownloadItem(false, downloadInfo), DOWNLOAD_STATUS_CANCELLED); + removeAutoResumableDownload(item.getId()); scheduleUpdateIfNeeded(); } @Override public void onDownloadInterrupted(final DownloadInfo downloadInfo, boolean isAutoResumable) { int status = DOWNLOAD_STATUS_INTERRUPTED; + DownloadItem item = new DownloadItem(false, downloadInfo); if (!downloadInfo.isResumable()) { status = DOWNLOAD_STATUS_FAILED; } else if (isAutoResumable) { - addAutoResumableDownload(downloadInfo.getDownloadId()); + addAutoResumableDownload(item.getId()); } - updateDownloadProgress(downloadInfo, status); + updateDownloadProgress(item, status); scheduleUpdateIfNeeded(); } /** * Clear any pending OMA downloads by reading them from shared prefs. + * TODO(qinmin): move this to a separate class. */ public void clearPendingOMADownloads() { if (mSharedPrefs.contains(PENDING_OMA_DOWNLOADS)) { @@ -307,25 +311,27 @@ public class DownloadManagerService extends BroadcastReceiver implements /** * Async task to clear the pending OMA download from SharedPrefs and inform * the OMADownloadHandler about download status. + * TODO(qinmin): move this to a separate file. */ private class ClearPendingOMADownloadTask extends AsyncTask<Void, Void, Pair<Integer, Boolean>> { - private DownloadInfo mDownloadInfo; - private final long mDownloadId; + private final DownloadItem mDownloadItem; private final String mInstallNotifyURI; + private DownloadInfo mDownloadInfo; private int mFailureReason; - public ClearPendingOMADownloadTask(long downloadId, String installNotifyURI) { - mDownloadId = downloadId; + public ClearPendingOMADownloadTask(DownloadItem downloadItem, String installNotifyURI) { + mDownloadItem = downloadItem; mInstallNotifyURI = installNotifyURI; - mDownloadInfo = mPendingDownloads.get(downloadId); + mDownloadInfo = downloadItem.getDownloadInfo(); } @Override public Pair<Integer, Boolean> doInBackground(Void...voids) { DownloadManager manager = (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE); - Cursor c = manager.query(new DownloadManager.Query().setFilterById(mDownloadId)); + Cursor c = manager.query(new DownloadManager.Query().setFilterById( + mDownloadItem.getSystemDownloadId())); int statusIndex = c.getColumnIndex(DownloadManager.COLUMN_STATUS); int reasonIndex = c.getColumnIndex(DownloadManager.COLUMN_REASON); int filenameIndex = c.getColumnIndex(DownloadManager.COLUMN_LOCAL_FILENAME); @@ -339,7 +345,6 @@ public class DownloadManagerService extends BroadcastReceiver implements // Chrome has been killed, reconstruct a DownloadInfo. mDownloadInfo = new DownloadInfo.Builder() .setFileName(fileName) - .setDownloadId((int) mDownloadId) .setDescription(c.getString( c.getColumnIndex(DownloadManager.COLUMN_DESCRIPTION))) .setMimeType(c.getString( @@ -352,10 +357,11 @@ public class DownloadManagerService extends BroadcastReceiver implements mDownloadInfo = DownloadInfo.Builder.fromDownloadInfo(mDownloadInfo) .setFileName(fileName) .build(); - canResolve = canResolveDownloadItem(mContext, mDownloadId); + mDownloadItem.setDownloadInfo(mDownloadInfo); + canResolve = canResolveDownloadItem(mContext, mDownloadItem); } else if (status == DownloadManager.STATUS_FAILED) { mFailureReason = c.getInt(reasonIndex); - manager.remove(mDownloadId); + manager.remove(mDownloadItem.getSystemDownloadId()); } } c.close(); @@ -364,15 +370,17 @@ public class DownloadManagerService extends BroadcastReceiver implements @Override protected void onPostExecute(Pair<Integer, Boolean> result) { + long downloadId = mDownloadItem.getSystemDownloadId(); if (result.first == DownloadManager.STATUS_SUCCESSFUL) { - mOMADownloadHandler.onDownloadCompleted(mDownloadInfo, mInstallNotifyURI); - removeOMADownloadFromSharedPrefs(mDownloadId); + mOMADownloadHandler.onDownloadCompleted( + mDownloadInfo, downloadId, mInstallNotifyURI); + removeOMADownloadFromSharedPrefs(downloadId); mDownloadSnackbarController.onDownloadSucceeded( - mDownloadInfo, mDownloadId, result.second); + mDownloadInfo, downloadId, result.second); } else if (result.first == DownloadManager.STATUS_FAILED) { mOMADownloadHandler.onDownloadFailed( - mDownloadInfo, mFailureReason, mInstallNotifyURI); - removeOMADownloadFromSharedPrefs(mDownloadId); + mDownloadInfo, downloadId, mFailureReason, mInstallNotifyURI); + removeOMADownloadFromSharedPrefs(downloadId); String fileName = mDownloadInfo.getFileName(); onDownloadFailed(fileName, mFailureReason); } @@ -382,13 +390,16 @@ public class DownloadManagerService extends BroadcastReceiver implements /** * Clear pending OMA downloads for a particular download ID. * - * @param downloadId Download identifier. - * @param info Information about the download. + * @param downloadId Download identifier from Android DownloadManager. * @param installNotifyURI URI to notify after installation. */ private void clearPendingOMADownload(long downloadId, String installNotifyURI) { - ClearPendingOMADownloadTask task = new ClearPendingOMADownloadTask( - downloadId, installNotifyURI); + DownloadItem item = mSystemDownloadIdMap.get(downloadId); + if (item == null) { + item = new DownloadItem(true, null); + item.setSystemDownloadId(downloadId); + } + ClearPendingOMADownloadTask task = new ClearPendingOMADownloadTask(item, installNotifyURI); task.execute(); } @@ -474,44 +485,43 @@ public class DownloadManagerService extends BroadcastReceiver implements /** * Updates notifications for all current downloads. Should not be called from UI thread. * - * @return A map that maps all download info to their corresponding download IDs in the - * download manager and whether the download can be resolved. If a download fails, - * its download ID is INVALID_DOWNLOAD_ID and the launching intent is null. If a - * download is cancelled, return an empty map so that no action needs to be taken. + * @return A map that maps all completed download to whether the download can be resolved. + * If a download fails, its system download ID is DownloadItem.INVALID_DOWNLOAD_ID. If + * a download is cancelled, return an empty map so that no action needs to be taken. */ - private Map<DownloadInfo, Pair<Long, Boolean>> updateAllNotifications() { + private Map<DownloadItem, Boolean> updateAllNotifications() { assert !ThreadUtils.runningOnUiThread(); - Map<DownloadInfo, Pair<Long, Boolean>> completionMap = - new HashMap<DownloadInfo, Pair<Long, Boolean>>(); + Map<DownloadItem, Boolean> downloadItems = new HashMap<DownloadItem, Boolean>(); for (DownloadProgress progress : mDownloadProgressMap.values()) { if (progress != null) { - DownloadInfo info = progress.mDownloadInfo; + DownloadItem item = progress.mDownloadItem; + DownloadInfo info = item.getDownloadInfo(); switch (progress.mDownloadStatus) { case DOWNLOAD_STATUS_COMPLETE: - mDownloadProgressMap.remove(info.getDownloadId()); - long downloadId = addCompletedDownload(info); - if (downloadId == INVALID_DOWNLOAD_ID) { - completionMap.put(info, Pair.create(INVALID_DOWNLOAD_ID, false)); - mDownloadNotifier.notifyDownloadFailed(info); - } else { + mDownloadProgressMap.remove(item.getId()); + boolean success = addCompletedDownload(item); + if (success) { boolean canResolve = isOMADownloadDescription(info) - || canResolveDownloadItem(mContext, downloadId); - completionMap.put(info, Pair.create(downloadId, canResolve)); + || canResolveDownloadItem(mContext, item); + downloadItems.put(item, canResolve); mDownloadNotifier.notifyDownloadSuccessful( - info, getLaunchIntentFromDownloadId(mContext, downloadId)); + info, getLaunchIntentFromDownloadId( + mContext, item.getSystemDownloadId())); broadcastDownloadSuccessful(info); + } else { + downloadItems.put(item, false); + mDownloadNotifier.notifyDownloadFailed(info); } break; case DOWNLOAD_STATUS_FAILED: - mDownloadProgressMap.remove(info.getDownloadId()); + mDownloadProgressMap.remove(item.getId()); mDownloadNotifier.notifyDownloadFailed(info); - completionMap.put(info, Pair.create(INVALID_DOWNLOAD_ID, false)); + downloadItems.put(item, false); Log.w(TAG, "Download failed: " + info.getFilePath()); break; case DOWNLOAD_STATUS_IN_PROGRESS: - if (!progress.mUpdateNotification) break; - if (progress.mDownloadInfo.isPaused()) { - mDownloadProgressMap.remove(info.getDownloadId()); + if (info.isPaused()) { + mDownloadProgressMap.remove(item.getId()); mDownloadNotifier.notifyDownloadPaused(info, false); if (info.isResumable()) { recordDownloadResumption(UMA_DOWNLOAD_RESUMPTION_MANUAL_PAUSE); @@ -522,16 +532,16 @@ public class DownloadManagerService extends BroadcastReceiver implements } break; case DOWNLOAD_STATUS_CANCELLED: - mDownloadProgressMap.remove(info.getDownloadId()); - mDownloadNotifier.cancelNotification(info.getDownloadId()); + mDownloadProgressMap.remove(item.getId()); + mDownloadNotifier.cancelNotification(item.getNotificationId()); break; case DOWNLOAD_STATUS_INTERRUPTED: // If the download can be auto resumed, keep it in the progress map so we // can resume it once network becomes available. boolean isAutoResumable = - mAutoResumableDownloadIds.contains(info.getDownloadId()); + mAutoResumableDownloadIds.contains(item.getId()); if (!isAutoResumable) { - mDownloadProgressMap.remove(info.getDownloadId()); + mDownloadProgressMap.remove(item.getId()); } mDownloadNotifier.notifyDownloadPaused(info, isAutoResumable); break; @@ -541,17 +551,17 @@ public class DownloadManagerService extends BroadcastReceiver implements } } } - return completionMap; + return downloadItems; } /** - * Add a completed download into DownloadManager. + * Adds a completed download into Android DownloadManager. * - * @param downloadInfo Information of the downloaded file. - * @return download ID if the download is added to the DownloadManager, or - * INVALID_DOWNLOAD_ID otherwise. + * @param downloadItem Information of the downloaded. + * @return true if the download is added to the DownloadManager, or false otherwise. */ - protected long addCompletedDownload(DownloadInfo downloadInfo) { + protected boolean addCompletedDownload(DownloadItem downloadItem) { + DownloadInfo downloadInfo = downloadItem.getDownloadInfo(); String mimeType = downloadInfo.getMimeType(); if (TextUtils.isEmpty(mimeType)) mimeType = UNKNOWN_MIME_TYPE; String description = downloadInfo.getDescription(); @@ -559,10 +569,12 @@ public class DownloadManagerService extends BroadcastReceiver implements try { // Exceptions can be thrown when calling this, although it is not // documented on Android SDK page. - return mDownloadManagerDelegate.addCompletedDownload( + long downloadId = mDownloadManagerDelegate.addCompletedDownload( mContext, downloadInfo.getFileName(), description, mimeType, downloadInfo.getFilePath(), downloadInfo.getContentLength(), downloadInfo.getOriginalUrl(), downloadInfo.getReferer()); + downloadItem.setSystemDownloadId(downloadId); + return true; } catch (RuntimeException e) { Log.w(TAG, "Failed to add the download item to DownloadManager: ", e); if (downloadInfo.getFilePath() != null) { @@ -572,21 +584,22 @@ public class DownloadManagerService extends BroadcastReceiver implements } } } - return INVALID_DOWNLOAD_ID; + return false; } /** * Handle auto opennable files after download completes. + * TODO(qinmin): move this to DownloadManagerDelegate. * - * @param info Information of the downloaded file. - * @param downloadId Download identifier issued by the android DownloadManager. + * @param download A download item. */ - private void handleAutoOpenAfterDownload(DownloadInfo info, long downloadId) { - if (isOMADownloadDescription(info)) { - mOMADownloadHandler.handleOMADownload(info, downloadId); + private void handleAutoOpenAfterDownload(DownloadItem download) { + if (isOMADownloadDescription(download.getDownloadInfo())) { + mOMADownloadHandler.handleOMADownload( + download.getDownloadInfo(), download.getSystemDownloadId()); return; } - openDownloadedContent(downloadId); + openDownloadedContent(download.getSystemDownloadId()); } /** @@ -597,32 +610,32 @@ public class DownloadManagerService extends BroadcastReceiver implements Runnable updateTask = new Runnable() { @Override public void run() { - new AsyncTask<Void, Void, Map<DownloadInfo, Pair<Long, Boolean>>>() { + new AsyncTask<Void, Void, Map<DownloadItem, Boolean>>() { @Override - public Map<DownloadInfo, Pair<Long, Boolean>> doInBackground( - Void... params) { + public Map<DownloadItem, Boolean> doInBackground(Void... params) { return updateAllNotifications(); } @Override protected void onPostExecute( - Map<DownloadInfo, Pair<Long, Boolean>> result) { - for (Map.Entry<DownloadInfo, Pair<Long, Boolean>> entry : - result.entrySet()) { - DownloadInfo info = entry.getKey(); - long downloadId = entry.getValue().first; - if (downloadId == INVALID_DOWNLOAD_ID) { + Map<DownloadItem, Boolean> result) { + for (Map.Entry<DownloadItem, Boolean> entry : result.entrySet()) { + DownloadItem download = entry.getKey(); + if (!download.hasSystemDownloadId()) { // TODO(qinmin): get the failure message from native. - onDownloadFailed(info.getFileName(), + onDownloadFailed(download.getDownloadInfo().getFileName(), DownloadManager.ERROR_UNKNOWN); + return; + } + boolean canResolve = entry.getValue(); + if (canResolve && shouldOpenAfterDownload( + download.getDownloadInfo())) { + handleAutoOpenAfterDownload(download); } else { - boolean canResolve = entry.getValue().second; - if (canResolve && shouldOpenAfterDownload(info)) { - handleAutoOpenAfterDownload(info, downloadId); - } else { - mDownloadSnackbarController.onDownloadSucceeded( - info, downloadId, canResolve); - } + mDownloadSnackbarController.onDownloadSucceeded( + download.getDownloadInfo(), + download.getSystemDownloadId(), + canResolve); } } } @@ -637,25 +650,21 @@ public class DownloadManagerService extends BroadcastReceiver implements /** * Updates the progress of a download. * - * @param downloadInfo Information about the download. + * @param downloadItem Information about the download. * @param downloadStatus Status of the download. */ - private void updateDownloadProgress(DownloadInfo downloadInfo, int downloadStatus) { - assert downloadInfo.hasDownloadId(); - int downloadId = downloadInfo.getDownloadId(); - DownloadProgress progress = mDownloadProgressMap.get(downloadId); + private void updateDownloadProgress(DownloadItem downloadItem, int downloadStatus) { + String id = downloadItem.getId(); + DownloadProgress progress = mDownloadProgressMap.get(id); if (progress == null) { - progress = new DownloadProgress(System.currentTimeMillis(), isActiveNetworkMetered(), - downloadInfo, downloadStatus, true); - mDownloadProgressMap.putIfAbsent(downloadId, progress); - } else if (progress.mDownloadInfo.isPaused() && downloadInfo.isPaused()) { - // when download is paused, this function can get called over and over. So stop updating - // the notifications unless the status changes. - progress.mUpdateNotification = false; + if (!downloadItem.getDownloadInfo().isPaused()) { + progress = new DownloadProgress(System.currentTimeMillis(), + isActiveNetworkMetered(), downloadItem, downloadStatus); + mDownloadProgressMap.putIfAbsent(id, progress); + } } else { progress.mDownloadStatus = downloadStatus; - progress.mDownloadInfo = downloadInfo; - progress.mUpdateNotification = true; + progress.mDownloadItem = downloadItem; } } @@ -674,21 +683,21 @@ public class DownloadManagerService extends BroadcastReceiver implements String action = intent.getAction(); if (!DownloadManager.ACTION_DOWNLOAD_COMPLETE.equals(action)) return; long downloadId = intent.getLongExtra(DownloadManager.EXTRA_DOWNLOAD_ID, - INVALID_DOWNLOAD_ID); - if (downloadId == INVALID_DOWNLOAD_ID) return; + DownloadItem.INVALID_DOWNLOAD_ID); + if (downloadId == DownloadItem.INVALID_DOWNLOAD_ID) return; boolean isPendingOMADownload = mOMADownloadHandler.isPendingOMADownload(downloadId); boolean isInOMASharedPrefs = isDownloadIdInOMASharedPrefs(downloadId); if (isPendingOMADownload || isInOMASharedPrefs) { clearPendingOMADownload(downloadId, null); - mPendingDownloads.remove(downloadId); + mSystemDownloadIdMap.remove(downloadId); return; } - DownloadInfo info = mPendingDownloads.get(downloadId); - if (info != null) { - DownloadCompletionTask task = new DownloadCompletionTask(info, downloadId); + DownloadItem downloadItem = mSystemDownloadIdMap.get(downloadId); + if (downloadItem != null) { + DownloadCompletionTask task = new DownloadCompletionTask(downloadItem); task.execute(); - mPendingDownloads.remove(downloadId); - if (mPendingDownloads.size() == 0) { + mSystemDownloadIdMap.remove(downloadId); + if (mSystemDownloadIdMap.size() == 0) { mContext.unregisterReceiver(this); } } @@ -698,27 +707,26 @@ public class DownloadManagerService extends BroadcastReceiver implements * Async task to handle completed downloads. */ private class DownloadCompletionTask extends AsyncTask<Void, Void, Pair<Integer, Boolean>> { - private final long mDownloadId; - private final DownloadInfo mDownloadInfo; + private final DownloadItem mDownloadItem; private int mFailureReason; - public DownloadCompletionTask(DownloadInfo info, long downloadId) { - mDownloadInfo = info; - mDownloadId = downloadId; + public DownloadCompletionTask(DownloadItem downloadItem) { + mDownloadItem = downloadItem; } @Override public Pair<Integer, Boolean> doInBackground(Void...voids) { final DownloadManager manager = (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE); - Cursor c = manager.query(new DownloadManager.Query().setFilterById(mDownloadId)); + Cursor c = manager.query(new DownloadManager.Query().setFilterById( + mDownloadItem.getSystemDownloadId())); int status = UNKNOWN_DOWNLOAD_STATUS; boolean canResolve = false; if (c.moveToNext()) { status = c.getInt(c.getColumnIndex(DownloadManager.COLUMN_STATUS)); if (status == DownloadManager.STATUS_SUCCESSFUL) { - canResolve = isOMADownloadDescription(mDownloadInfo) - || canResolveDownloadItem(mContext, mDownloadId); + canResolve = isOMADownloadDescription(mDownloadItem.getDownloadInfo()) + || canResolveDownloadItem(mContext, mDownloadItem); } else if (status == DownloadManager.STATUS_FAILED) { mFailureReason = c.getInt(c.getColumnIndex(DownloadManager.COLUMN_REASON)); } @@ -731,15 +739,17 @@ public class DownloadManagerService extends BroadcastReceiver implements protected void onPostExecute(Pair<Integer, Boolean> result) { switch (result.first) { case DownloadManager.STATUS_SUCCESSFUL: - if (shouldOpenAfterDownload(mDownloadInfo) && result.second) { - handleAutoOpenAfterDownload(mDownloadInfo, mDownloadId); + if (shouldOpenAfterDownload(mDownloadItem.getDownloadInfo()) && result.second) { + handleAutoOpenAfterDownload(mDownloadItem); } else { mDownloadSnackbarController.onDownloadSucceeded( - mDownloadInfo, mDownloadId, result.second); + mDownloadItem.getDownloadInfo(), + mDownloadItem.getSystemDownloadId(), + result.second); } break; case DownloadManager.STATUS_FAILED: - onDownloadFailed(mDownloadInfo.getFileName(), mFailureReason); + onDownloadFailed(mDownloadItem.getDownloadInfo().getFileName(), mFailureReason); break; default: break; @@ -753,34 +763,36 @@ public class DownloadManagerService extends BroadcastReceiver implements * content will be saved to the public directory on external storage. Otherwise, the * download will be saved in the app directory and user will not get any notifications * after download completion. + * TODO(qinmin): move this to DownloadManagerDelegate. * * @param info Download information about the download. * @param notifyCompleted Whether to notify the user after Downloadmanager completes the * download. */ - public void enqueueDownloadManagerRequest(final DownloadInfo info, boolean notifyCompleted) { - EnqueueDownloadRequestTask task = new EnqueueDownloadRequestTask(info); + public void enqueueDownloadManagerRequest( + final DownloadItem item, boolean notifyCompleted) { + EnqueueDownloadRequestTask task = new EnqueueDownloadRequestTask(item); task.execute(notifyCompleted); } /** * Async task to enqueue a download request into DownloadManager. */ - private class EnqueueDownloadRequestTask extends - AsyncTask<Boolean, Void, Boolean> { + private class EnqueueDownloadRequestTask extends AsyncTask<Boolean, Void, Boolean> { private long mDownloadId; - private DownloadInfo mDownloadInfo; + private final DownloadItem mDownloadItem; private int mFailureReason; - public EnqueueDownloadRequestTask(DownloadInfo downloadInfo) { - mDownloadInfo = downloadInfo; + public EnqueueDownloadRequestTask(DownloadItem downloadItem) { + mDownloadItem = downloadItem; } @Override public Boolean doInBackground(Boolean... booleans) { boolean notifyCompleted = booleans[0]; - Uri uri = Uri.parse(mDownloadInfo.getUrl()); + Uri uri = Uri.parse(mDownloadItem.getDownloadInfo().getUrl()); DownloadManager.Request request; + DownloadInfo info = mDownloadItem.getDownloadInfo(); try { request = new DownloadManager.Request(uri); } catch (IllegalArgumentException e) { @@ -791,17 +803,17 @@ public class DownloadManagerService extends BroadcastReceiver implements return false; } - request.setMimeType(mDownloadInfo.getMimeType()); + request.setMimeType(info.getMimeType()); try { if (notifyCompleted) { // Set downloaded file destination to /sdcard/Download or, should it be // set to one of several Environment.DIRECTORY* dirs depending on mimetype? request.setDestinationInExternalPublicDir( - Environment.DIRECTORY_DOWNLOADS, mDownloadInfo.getFileName()); + Environment.DIRECTORY_DOWNLOADS, info.getFileName()); } else { File dir = new File(mContext.getExternalFilesDir(null), DOWNLOAD_DIRECTORY); if (dir.mkdir() || dir.isDirectory()) { - File file = new File(dir, mDownloadInfo.getFileName()); + File file = new File(dir, info.getFileName()); request.setDestinationUri(Uri.fromFile(file)); } else { Log.e(TAG, "Cannot create download directory"); @@ -825,14 +837,14 @@ public class DownloadManagerService extends BroadcastReceiver implements request.setNotificationVisibility( DownloadManager.Request.VISIBILITY_VISIBLE); } - String description = mDownloadInfo.getDescription(); + String description = info.getDescription(); if (TextUtils.isEmpty(description)) { - description = mDownloadInfo.getFileName(); + description = info.getFileName(); } request.setDescription(description); - request.addRequestHeader("Cookie", mDownloadInfo.getCookie()); - request.addRequestHeader("Referer", mDownloadInfo.getReferer()); - request.addRequestHeader("User-Agent", mDownloadInfo.getUserAgent()); + request.addRequestHeader("Cookie", info.getCookie()); + request.addRequestHeader("Referer", info.getReferer()); + request.addRequestHeader("User-Agent", info.getUserAgent()); DownloadManager manager = (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE); @@ -854,13 +866,14 @@ public class DownloadManagerService extends BroadcastReceiver implements @Override protected void onPostExecute(Boolean result) { - boolean isPendingOMADownload = - mOMADownloadHandler.isPendingOMADownload(mDownloadInfo.getDownloadId()); + boolean isPendingOMADownload = mOMADownloadHandler.isPendingOMADownload( + mDownloadItem.getSystemDownloadId()); if (!result) { - onDownloadFailed(mDownloadInfo.getFileName(), mFailureReason); + onDownloadFailed(mDownloadItem.getDownloadInfo().getFileName(), mFailureReason); if (isPendingOMADownload) { mOMADownloadHandler.onDownloadFailed( - mDownloadInfo, DownloadManager.ERROR_UNKNOWN, null); + mDownloadItem.getDownloadInfo(), mDownloadItem.getSystemDownloadId(), + DownloadManager.ERROR_UNKNOWN, null); } return; } @@ -868,8 +881,8 @@ public class DownloadManagerService extends BroadcastReceiver implements if (isPendingOMADownload) { // A new downloadId is generated, needs to update the OMADownloadHandler // about this. - mDownloadInfo = mOMADownloadHandler.updateDownloadInfo( - mDownloadInfo, mDownloadId); + mOMADownloadHandler.updateDownloadInfo( + mDownloadItem.getSystemDownloadId(), mDownloadId); // TODO(qinmin): use a file instead of shared prefs to save the // OMA information in case chrome is killed. This will allow us to // save more information like cookies and user agent. @@ -879,11 +892,12 @@ public class DownloadManagerService extends BroadcastReceiver implements addOMADownloadToSharedPrefs(entry.generateSharedPrefsString()); } } - if (mPendingDownloads.size() == 0) { + if (mSystemDownloadIdMap.size() == 0) { mContext.registerReceiver(DownloadManagerService.this, new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE)); } - mPendingDownloads.put(mDownloadId, mDownloadInfo); + mDownloadItem.setSystemDownloadId(mDownloadId); + mSystemDownloadIdMap.put(mDownloadId, mDownloadItem); } } @@ -979,18 +993,19 @@ public class DownloadManagerService extends BroadcastReceiver implements * Return whether a download item can be resolved to any activity. * * @param context Context of the app. - * @param downloadId ID of the download item in DownloadManager. + * @param download A download item. * @return true if the download item can be resolved, or false otherwise. */ - private static boolean canResolveDownloadItem(Context context, long downloadId) { + private static boolean canResolveDownloadItem(Context context, DownloadItem download) { assert !ThreadUtils.runningOnUiThread(); - Intent intent = getLaunchIntentFromDownloadId(context, downloadId); + Intent intent = getLaunchIntentFromDownloadId(context, download.getSystemDownloadId()); return (intent == null) ? false : ExternalNavigationDelegateImpl.resolveIntent( context, intent, true); } /** * Launch the intent for a given download item. + * TODO(qinmin): Move this to DownloadManagerDelegate. * * @param downloadId ID of the download item in DownloadManager. */ @@ -1079,25 +1094,27 @@ public class DownloadManagerService extends BroadcastReceiver implements /** * Called to cancel a download notification. - * @param downloadId Id of the download. + * @param notificationId Notification Id of the download. */ - void cancelNotification(int downloadId) { - mDownloadNotifier.cancelNotification(downloadId); + void cancelNotification(int notificationId) { + mDownloadNotifier.cancelNotification(notificationId); } /** * Called to resume a paused download. - * @param downloadId Id of the download. + * @param notificationId Notification Id of the download. + * @param downloadGuid GUID of the download. * @param fileName Name of the download file. * @param hasUserGesture Whether the resumption is triggered by user gesture. */ @VisibleForTesting - protected void resumeDownload(int downloadId, String fileName, boolean hasUserGesture) { + protected void resumeDownload( + int notificationId, String downloadGuid, String fileName, boolean hasUserGesture) { int uma = hasUserGesture ? UMA_DOWNLOAD_RESUMPTION_CLICKED : UMA_DOWNLOAD_RESUMPTION_AUTO_STARTED; recordDownloadResumption(uma); if (hasUserGesture) { - DownloadProgress progress = mDownloadProgressMap.get(downloadId); + DownloadProgress progress = mDownloadProgressMap.get(downloadGuid); // If user manually resumes a download, update the connection type that the download // can start. If the previous connection type is metered, manually resuming on an // unmetered network should not affect the original connection type. @@ -1105,29 +1122,37 @@ public class DownloadManagerService extends BroadcastReceiver implements progress.mCanDownloadWhileMetered = isActiveNetworkMetered(); } } - nativeResumeDownload(mNativeDownloadManagerService, downloadId, fileName); + nativeResumeDownload(mNativeDownloadManagerService, notificationId, downloadGuid, fileName); } /** * Called to cancel a download. - * @param downloadId Id of the download. + * @param downloadGuid GUID of the download. */ - void cancelDownload(int downloadId) { - nativeCancelDownload(mNativeDownloadManagerService, downloadId); + void cancelDownload(String downloadGuid) { + nativeCancelDownload(mNativeDownloadManagerService, downloadGuid); } /** * Called to pause a download. - * @param downloadId Id of the download. + * @param downloadGuid GUID of the download. */ - void pauseDownload(int downloadId) { - nativePauseDownload(mNativeDownloadManagerService, downloadId); + void pauseDownload(String downloadGuid) { + nativePauseDownload(mNativeDownloadManagerService, downloadGuid); + // Calling pause will stop listening to the download item. Update its progress now. + DownloadProgress progress = mDownloadProgressMap.get(downloadGuid); + if (progress != null) { + DownloadInfo info = DownloadInfo.Builder.fromDownloadInfo( + progress.mDownloadItem.getDownloadInfo()).setIsPaused(true).build(); + onDownloadUpdated(info); + } } @CalledByNative - void onResumptionFailed(int downloadId, String fileName) { + void onResumptionFailed(int notificationId, String fileName) { mDownloadNotifier.notifyDownloadFailed( - new DownloadInfo.Builder().setDownloadId(downloadId).setFileName(fileName).build()); + new DownloadInfo.Builder().setNotificationId(notificationId).setFileName( + fileName).build()); recordDownloadResumption(UMA_DOWNLOAD_RESUMPTION_FAILED); } @@ -1135,9 +1160,9 @@ public class DownloadManagerService extends BroadcastReceiver implements * Called to record the UMA histograms for all auto paused downloads when browser is killed. */ private void recordAutoPausedDownloads() { - List<DownloadNotificationService.PendingNotification> downloads = - DownloadNotificationService.parseDownloadNotificationsFromSharedPrefs(mSharedPrefs); - for (DownloadNotificationService.PendingNotification download : downloads) { + List<DownloadSharedPreferenceEntry> entries = + DownloadNotificationService.parseDownloadSharedPrefs(mSharedPrefs); + for (int i = 0; i < entries.size(); ++i) { recordDownloadResumption(UMA_DOWNLOAD_RESUMPTION_BROWSER_KILLED); } } @@ -1173,25 +1198,25 @@ public class DownloadManagerService extends BroadcastReceiver implements /** * Helper method to add an auto resumable download. - * @param downloadId Id of the download item. + * @param guid Id of the download item. */ - private void addAutoResumableDownload(int downloadId) { + private void addAutoResumableDownload(String guid) { if (mAutoResumableDownloadIds.isEmpty() && !mIsNetworkChangeNotifierDisabled) { mNetworkChangeNotifier = new NetworkChangeNotifierAutoDetect(this, mContext, new RegistrationPolicyAlwaysRegister()); } - if (!mAutoResumableDownloadIds.contains(downloadId)) { - mAutoResumableDownloadIds.add(downloadId); + if (!mAutoResumableDownloadIds.contains(guid)) { + mAutoResumableDownloadIds.add(guid); } } /** * Helper method to remove an auto resumable download. - * @param downloadId Id of the download item. + * @param guid Id of the download item. */ - private void removeAutoResumableDownload(int downloadId) { + private void removeAutoResumableDownload(String guid) { if (mAutoResumableDownloadIds.isEmpty()) return; - mAutoResumableDownloadIds.remove(Integer.valueOf(downloadId)); + mAutoResumableDownloadIds.remove(guid); stopListenToConnectionChangeIfNotNeeded(); } @@ -1200,10 +1225,10 @@ public class DownloadManagerService extends BroadcastReceiver implements if (mAutoResumableDownloadIds.isEmpty()) return; if (connectionType == ConnectionType.CONNECTION_NONE) return; boolean isMetered = isActiveNetworkMetered(); - Iterator<Integer> iterator = mAutoResumableDownloadIds.iterator(); + Iterator<String> iterator = mAutoResumableDownloadIds.iterator(); while (iterator.hasNext()) { - final int downloadId = iterator.next(); - final DownloadProgress progress = mDownloadProgressMap.get(downloadId); + final String id = iterator.next(); + final DownloadProgress progress = mDownloadProgressMap.get(id); // Introduce some delay in each resumption so we don't start all of them immediately. if (progress.mCanDownloadWhileMetered || !isMetered) { // Remove the pending resumable item so that the task won't be posted again on the @@ -1214,7 +1239,8 @@ public class DownloadManagerService extends BroadcastReceiver implements mHandler.postDelayed(new Runnable() { @Override public void run() { - resumeDownload(downloadId, progress.mDownloadInfo.getFileName(), false); + resumeDownload(progress.mDownloadItem.getNotificationId(), id, + progress.mDownloadItem.getDownloadInfo().getFileName(), false); } }, mUpdateDelayInMillis); } @@ -1249,8 +1275,9 @@ public class DownloadManagerService extends BroadcastReceiver implements public void updateActiveNetworkList(int[] activeNetIds) {} private native long nativeInit(); - private native void nativeResumeDownload( - long nativeDownloadManagerService, int downloadId, String fileName); - private native void nativeCancelDownload(long nativeDownloadManagerService, int downloadId); - private native void nativePauseDownload(long nativeDownloadManagerService, int downloadId); + private native void nativeResumeDownload(long nativeDownloadManagerService, int notificationId, + String downloadGuid, String fileName); + private native void nativeCancelDownload( + long nativeDownloadManagerService, String downloadGuid); + private native void nativePauseDownload(long nativeDownloadManagerService, String downloadGuid); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java index b0a435a..c4f78fd 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java @@ -39,7 +39,8 @@ import java.util.Set; * Chrome gets killed. */ public class DownloadNotificationService extends Service { - static final String EXTRA_DOWNLOAD_ID = "DownloadId"; + static final String EXTRA_DOWNLOAD_NOTIFICATION_ID = "DownloadNotificationId"; + static final String EXTRA_DOWNLOAD_GUID = "DownloadGuid"; static final String EXTRA_DOWNLOAD_FILE_NAME = "DownloadFileName"; static final String ACTION_DOWNLOAD_CANCEL = "org.chromium.chrome.browser.download.DOWNLOAD_CANCEL"; @@ -58,50 +59,6 @@ public class DownloadNotificationService extends Service { private Context mContext; /** - * Class representing a pending notification entry. - */ - @VisibleForTesting - static class PendingNotification { - public final int downloadId; - public final String fileName; - public final boolean isResumable; - - PendingNotification(int downloadId, String fileName, boolean isResumable) { - this.downloadId = downloadId; - this.fileName = fileName; - this.isResumable = isResumable; - } - - /** - * Parse the pending notification from a String object in SharedPrefs. - * - * @param notification String containing the notification ID, file name and whether it is - * resumable. - * @return a PendingNotification object. - */ - static PendingNotification parseFromString(String notification) { - String[] values = notification.split(",", 3); - if (values.length == 3) { - try { - int id = Integer.parseInt(values[0]); - boolean isResumable = "1".equals(values[1]); - return new PendingNotification(id, values[2], isResumable); - } catch (NumberFormatException nfe) { - Log.w(TAG, "Exception while parsing pending download:" + notification); - } - } - return new PendingNotification(-1, "", false); - } - - /** - * Generate a string for the PendingNotification instance to be inserted into SharedPrefs. - */ - String getNotificationString() { - return downloadId + "," + (isResumable ? "1" : "0") + "," + fileName; - } - } - - /** * Class for clients to access. */ public class LocalBinder extends Binder { @@ -157,7 +114,8 @@ public class DownloadNotificationService extends Service { /** * Add a in-progress download notification. - * @param downloadId ID of the download. + * @param notificationId Notification ID of the download. + * @param downloadGuid GUID of the download. * @param fileName File name of the download. * @param percentage Percentage completed. Value should be between 0 to 100 if * the percentage can be determined, or -1 if it is unknown. @@ -165,9 +123,8 @@ public class DownloadNotificationService extends Service { * @param startTime Time when download started. * @param isResumable whether the download can be resumed. */ - public void notifyDownloadProgress( - int downloadId, String fileName, int percentage, long timeRemainingInMillis, - long startTime, boolean isResumable) { + public void notifyDownloadProgress(int notificationId, String downloadGuid, String fileName, + int percentage, long timeRemainingInMillis, long startTime, boolean isResumable) { boolean indeterminate = percentage == INVALID_DOWNLOAD_PERCENTAGE; NotificationCompat.Builder builder = buildNotification( android.R.drawable.stat_sys_download, fileName, null); @@ -181,40 +138,43 @@ public class DownloadNotificationService extends Service { if (startTime > 0) builder.setWhen(startTime); builder.addAction(android.R.drawable.ic_menu_close_clear_cancel, mContext.getResources().getString(R.string.download_notification_cancel_button), - buildPendingIntent(ACTION_DOWNLOAD_CANCEL, downloadId, fileName)); + buildPendingIntent(ACTION_DOWNLOAD_CANCEL, notificationId, downloadGuid, fileName)); if (isResumable) { builder.addAction(android.R.drawable.ic_media_pause, mContext.getResources().getString(R.string.download_notification_pause_button), - buildPendingIntent(ACTION_DOWNLOAD_PAUSE, downloadId, fileName)); + buildPendingIntent(ACTION_DOWNLOAD_PAUSE, notificationId, downloadGuid, + fileName)); } - updateNotification(downloadId, builder.build()); - addPendingDownloadToSharedPrefs(new PendingNotification(downloadId, fileName, isResumable)); + updateNotification(notificationId, builder.build()); + addEntryToSharedPrefs(new DownloadSharedPreferenceEntry( + notificationId, isResumable, true, downloadGuid, fileName)); } /** * Cancel a download notification. - * @param downloadId ID of the download. + * @param notificationId Notification ID of the download. */ - public void cancelNotification(int downloadId) { - mNotificationManager.cancel(NOTIFICATION_NAMESPACE, downloadId); - removePendingDownloadFromSharedPrefs(downloadId); + public void cancelNotification(int notificationId) { + mNotificationManager.cancel(NOTIFICATION_NAMESPACE, notificationId); + removeEntryFromSharedPrefs(notificationId); } /** * Change a download notification to paused state. - * @param downloadId ID of the download. + * @param notificationId Notification ID of the download. + * @param downloadGuid GUID of the download. * @param fileName File name of the download. * @param isResumable whether download is resumable. * @param isAutoResumable whether download is can be resumed automatically. */ - public void notifyDownloadPaused( - int downloadId, String fileName, boolean isResumable, boolean isAutoResumable) { + public void notifyDownloadPaused(int notificationId, String downloadGuid, String fileName, + boolean isResumable, boolean isAutoResumable) { NotificationCompat.Builder builder = buildNotification( android.R.drawable.ic_media_pause, fileName, mContext.getResources().getString(R.string.download_notification_paused)); PendingIntent cancelIntent = - buildPendingIntent(ACTION_DOWNLOAD_CANCEL, downloadId, fileName); + buildPendingIntent(ACTION_DOWNLOAD_CANCEL, notificationId, downloadGuid, fileName); builder.setDeleteIntent(cancelIntent); builder.addAction(android.R.drawable.ic_menu_close_clear_cancel, mContext.getResources().getString(R.string.download_notification_cancel_button), @@ -222,22 +182,23 @@ public class DownloadNotificationService extends Service { if (isResumable) { builder.addAction(android.R.drawable.stat_sys_download_done, mContext.getResources().getString(R.string.download_notification_resume_button), - buildPendingIntent(ACTION_DOWNLOAD_RESUME, downloadId, fileName)); + buildPendingIntent(ACTION_DOWNLOAD_RESUME, notificationId, downloadGuid, + fileName)); } - updateNotification(downloadId, builder.build()); + updateNotification(notificationId, builder.build()); // If download is not auto resumable, there is no need to keep it in SharedPreferences. if (!isResumable || !isAutoResumable) { - removePendingDownloadFromSharedPrefs(downloadId); + removeEntryFromSharedPrefs(notificationId); } } /** * Add a download successful notification. - * @param downloadId ID of the download. + * @param notificationId Notification ID of the download. * @param fileName File name of the download. * @param intent Intent to launch when clicking the notification. */ - public void notifyDownloadSuccessful(int downloadId, String fileName, Intent intent) { + public void notifyDownloadSuccessful(int notificationId, String fileName, Intent intent) { NotificationCompat.Builder builder = buildNotification( android.R.drawable.stat_sys_download_done, fileName, @@ -246,23 +207,23 @@ public class DownloadNotificationService extends Service { builder.setContentIntent(PendingIntent.getActivity( mContext, 0, intent, PendingIntent.FLAG_UPDATE_CURRENT)); } - updateNotification(downloadId, builder.build()); - removePendingDownloadFromSharedPrefs(downloadId); + updateNotification(notificationId, builder.build()); + removeEntryFromSharedPrefs(notificationId); } /** * Add a download failed notification. - * @param downloadId ID of the download. + * @param notificationId Notification ID of the download. * @param fileName File name of the download. * @param intent Intent to launch when clicking the notification. */ - public void notifyDownloadFailed(int downloadId, String fileName) { + public void notifyDownloadFailed(int notificationId, String fileName) { NotificationCompat.Builder builder = buildNotification( android.R.drawable.stat_sys_download_done, fileName, mContext.getResources().getString(R.string.download_notification_failed)); - updateNotification(downloadId, builder.build()); - removePendingDownloadFromSharedPrefs(downloadId); + updateNotification(notificationId, builder.build()); + removeEntryFromSharedPrefs(notificationId); } /** @@ -270,26 +231,27 @@ public class DownloadNotificationService extends Service { */ @VisibleForTesting void pauseAllDownloads() { - List<PendingNotification> notifications = - parseDownloadNotificationsFromSharedPrefs(mSharedPrefs); - for (int i = 0; i < notifications.size(); ++i) { - PendingNotification notification = notifications.get(i); - if (notification.downloadId > 0) { - notifyDownloadPaused(notification.downloadId, notification.fileName, - notification.isResumable, true); + List<DownloadSharedPreferenceEntry> entries = parseDownloadSharedPrefs(mSharedPrefs); + for (int i = 0; i < entries.size(); ++i) { + DownloadSharedPreferenceEntry entry = entries.get(i); + if (entry.notificationId > 0) { + notifyDownloadPaused(entry.notificationId, entry.downloadGuid, entry.fileName, + entry.isResumable, true); } } } - private PendingIntent buildPendingIntent(String action, int downloadId, String fileName) { + private PendingIntent buildPendingIntent( + String action, int notificationId, String downloadGuid, String fileName) { ComponentName component = new ComponentName( mContext.getPackageName(), DownloadBroadcastReceiver.class.getName()); Intent intent = new Intent(action); intent.setComponent(component); - intent.putExtra(EXTRA_DOWNLOAD_ID, downloadId); + intent.putExtra(EXTRA_DOWNLOAD_NOTIFICATION_ID, notificationId); + intent.putExtra(EXTRA_DOWNLOAD_GUID, downloadGuid); intent.putExtra(EXTRA_DOWNLOAD_FILE_NAME, fileName); return PendingIntent.getBroadcast( - mContext, downloadId, intent, PendingIntent.FLAG_UPDATE_CURRENT); + mContext, notificationId, intent, PendingIntent.FLAG_UPDATE_CURRENT); } /** @@ -316,8 +278,10 @@ public class DownloadNotificationService extends Service { * @param intent Intent with the download operation. */ private void handleDownloadOperation(final Intent intent) { - final int downloadId = IntentUtils.safeGetIntExtra( - intent, DownloadNotificationService.EXTRA_DOWNLOAD_ID, -1); + final int notificationId = IntentUtils.safeGetIntExtra( + intent, DownloadNotificationService.EXTRA_DOWNLOAD_NOTIFICATION_ID, -1); + final String guid = IntentUtils.safeGetStringExtra( + intent, DownloadNotificationService.EXTRA_DOWNLOAD_GUID); final String fileName = IntentUtils.safeGetStringExtra( intent, DownloadNotificationService.EXTRA_DOWNLOAD_FILE_NAME); // If browser process already goes away, the download should have already paused. Do nothing @@ -336,16 +300,16 @@ public class DownloadNotificationService extends Service { // TODO(qinmin): Alternatively, we can delete the downloaded content on // SD card, and remove the download ID from the SharedPreferences so we // don't need to restart the browser process. http://crbug.com/579643. - service.cancelDownload(downloadId); - cancelNotification(downloadId); + service.cancelDownload(guid); + cancelNotification(notificationId); break; case ACTION_DOWNLOAD_PAUSE: - service.pauseDownload(downloadId); + service.pauseDownload(guid); break; case ACTION_DOWNLOAD_RESUME: - service.resumeDownload(downloadId, fileName, true); - notifyDownloadProgress( - downloadId, fileName, INVALID_DOWNLOAD_PERCENTAGE, 0, 0, true); + service.resumeDownload(notificationId, guid, fileName, true); + notifyDownloadProgress(notificationId, guid, fileName, + INVALID_DOWNLOAD_PERCENTAGE, 0, 0, true); break; default: Log.e(TAG, "Unrecognized intent action.", intent); @@ -384,52 +348,56 @@ public class DownloadNotificationService extends Service { && !ACTION_DOWNLOAD_PAUSE.equals(intent.getAction())) { return false; } - if (!intent.hasExtra(DownloadNotificationService.EXTRA_DOWNLOAD_ID) - || !intent.hasExtra(DownloadNotificationService.EXTRA_DOWNLOAD_FILE_NAME)) { + if (!intent.hasExtra(EXTRA_DOWNLOAD_NOTIFICATION_ID) + || !intent.hasExtra(EXTRA_DOWNLOAD_FILE_NAME) + || !intent.hasExtra(EXTRA_DOWNLOAD_GUID)) { return false; } - final int downloadId = IntentUtils.safeGetIntExtra( - intent, DownloadNotificationService.EXTRA_DOWNLOAD_ID, -1); - if (downloadId == -1) return false; - final String fileName = IntentUtils.safeGetStringExtra( - intent, DownloadNotificationService.EXTRA_DOWNLOAD_FILE_NAME); + final int notificationId = + IntentUtils.safeGetIntExtra(intent, EXTRA_DOWNLOAD_NOTIFICATION_ID, -1); + if (notificationId == -1) return false; + final String fileName = IntentUtils.safeGetStringExtra(intent, EXTRA_DOWNLOAD_FILE_NAME); if (fileName == null) return false; + final String guid = IntentUtils.safeGetStringExtra(intent, EXTRA_DOWNLOAD_GUID); + if (!DownloadSharedPreferenceEntry.isValidGUID(guid)) return false; return true; } /** - * Add a pending download to SharedPrefs, the string consists of the download ID, file name and - * whether it is resumable. If the download ID already exists in SharedPrefs, do nothing. - * @param pendingNotification Pending download entry. + * Add a DownloadSharedPreferenceEntry to SharedPrefs. If the notification ID already exists + * in SharedPrefs, do nothing. + * @param DownloadSharedPreferenceEntry A DownloadSharedPreferenceEntry to be added. */ - private void addPendingDownloadToSharedPrefs(PendingNotification pendingNotification) { - Set<String> pendingDownloads = DownloadManagerService.getStoredDownloadInfo( + private void addEntryToSharedPrefs(DownloadSharedPreferenceEntry pendingEntry) { + Set<String> entries = DownloadManagerService.getStoredDownloadInfo( mSharedPrefs, PENDING_DOWNLOAD_NOTIFICATIONS); - for (String download : pendingDownloads) { - PendingNotification notification = PendingNotification.parseFromString(download); - if (notification.downloadId == pendingNotification.downloadId) return; + for (String entryString : entries) { + DownloadSharedPreferenceEntry entry = + DownloadSharedPreferenceEntry.parseFromString(entryString); + if (entry.notificationId == pendingEntry.notificationId) return; } - pendingDownloads.add(pendingNotification.getNotificationString()); + entries.add(pendingEntry.getSharedPreferenceString()); DownloadManagerService.storeDownloadInfo( - mSharedPrefs, PENDING_DOWNLOAD_NOTIFICATIONS, pendingDownloads); + mSharedPrefs, PENDING_DOWNLOAD_NOTIFICATIONS, entries); } /** - * Removes a pending donwload from SharedPrefs. - * @param downloadId ID to be removed. + * Removes a DownloadSharedPreferenceEntry from SharedPrefs. + * @param notificationId Notification ID to be removed. */ - private void removePendingDownloadFromSharedPrefs(int downloadId) { - Set<String> pendingDownloads = DownloadManagerService.getStoredDownloadInfo( + private void removeEntryFromSharedPrefs(int notificationId) { + Set<String> entries = DownloadManagerService.getStoredDownloadInfo( mSharedPrefs, PENDING_DOWNLOAD_NOTIFICATIONS); - for (String download : pendingDownloads) { - PendingNotification notification = PendingNotification.parseFromString(download); - if (notification.downloadId == downloadId) { - pendingDownloads.remove(download); - if (pendingDownloads.isEmpty()) { + for (String entryString : entries) { + DownloadSharedPreferenceEntry entry = + DownloadSharedPreferenceEntry.parseFromString(entryString); + if (entry.notificationId == notificationId) { + entries.remove(entryString); + if (entries.isEmpty()) { mSharedPrefs.edit().remove(PENDING_DOWNLOAD_NOTIFICATIONS).apply(); } else { DownloadManagerService.storeDownloadInfo( - mSharedPrefs, PENDING_DOWNLOAD_NOTIFICATIONS, pendingDownloads); + mSharedPrefs, PENDING_DOWNLOAD_NOTIFICATIONS, entries); } break; } @@ -444,17 +412,17 @@ public class DownloadNotificationService extends Service { } /** - * Parse the download notifications from the shared preference and return a list of them. - * @return a list of parsed notifications. + * Parse the DownloadSharedPreferenceEntry from the shared preference and return a list of them. + * @return a list of parsed DownloadSharedPreferenceEntry. */ - static List<PendingNotification> parseDownloadNotificationsFromSharedPrefs( + static List<DownloadSharedPreferenceEntry> parseDownloadSharedPrefs( SharedPreferences prefs) { - List<PendingNotification> result = new ArrayList<PendingNotification>(); + List<DownloadSharedPreferenceEntry> result = new ArrayList<DownloadSharedPreferenceEntry>(); if (prefs.contains(PENDING_DOWNLOAD_NOTIFICATIONS)) { - Set<String> pendingDownloads = DownloadManagerService.getStoredDownloadInfo( + Set<String> entries = DownloadManagerService.getStoredDownloadInfo( prefs, PENDING_DOWNLOAD_NOTIFICATIONS); - for (String download : pendingDownloads) { - result.add(PendingNotification.parseFromString(download)); + for (String entryString : entries) { + result.add(DownloadSharedPreferenceEntry.parseFromString(entryString)); } } return result; diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java index d75619d..4c16ae4 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java @@ -43,9 +43,9 @@ public interface DownloadNotifier { /** * Cancel the notification for a download. - * @param downloadId the downloadId of the cancelled download. + * @param notificationId The notification ID of the cancelled download. */ - void cancelNotification(int downloadId); + void cancelNotification(int notificationId); /** * Called to clear all the pending download entries in SharedPreferences. diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java new file mode 100644 index 0000000..a231496 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java @@ -0,0 +1,96 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.download; + +import org.chromium.base.Log; +import org.chromium.base.VisibleForTesting; + +import java.util.UUID; + +/** + * Class representing the download information stored in SharedPreferences to construct a + * download notification. + */ +public class DownloadSharedPreferenceEntry { + private static final String TAG = "DownloadEntry"; + // Current version of the DownloadSharedPreferenceEntry. When changing the SharedPreference, + // we need to change the version number too. + @VisibleForTesting static final int VERSION = 1; + public final int notificationId; + public final boolean isResumable; + // This field is not yet used, but will soon be used. We add it here to avoid changing the + // format of the SharedPreference string again. + public final boolean isStartedOnMeteredNetwork; + public final String fileName; + public final String downloadGuid; + + DownloadSharedPreferenceEntry(int notificationId, boolean isResumable, + boolean isStartedOnMeteredNetwork, String guid, String fileName) { + this.notificationId = notificationId; + this.isResumable = isResumable; + this.isStartedOnMeteredNetwork = isStartedOnMeteredNetwork; + this.downloadGuid = guid; + this.fileName = fileName; + } + + /** + * Parse the pending notification from a String object in SharedPrefs. + * + * @param sharedPrefString String from SharedPreference, containing the notification ID, GUID, + * file name, whether it is resumable and whether download started on a metered network. + * @return a DownloadSharedPreferenceEntry object. + */ + static DownloadSharedPreferenceEntry parseFromString(String sharedPrefString) { + String[] values = sharedPrefString.split(",", 6); + if (values.length == 6) { + try { + int version = Integer.parseInt(values[0]); + // Ignore all SharedPreference entries that has an invalid version for now. + if (version != VERSION) { + return new DownloadSharedPreferenceEntry(-1, false, false, null, ""); + } + int id = Integer.parseInt(values[1]); + boolean isResumable = "1".equals(values[2]); + boolean isStartedOnMeteredNetwork = "1".equals(values[3]); + if (!isValidGUID(values[4])) { + return new DownloadSharedPreferenceEntry(-1, false, false, null, ""); + } + return new DownloadSharedPreferenceEntry( + id, isResumable, isStartedOnMeteredNetwork, values[4], values[5]); + } catch (NumberFormatException nfe) { + Log.w(TAG, "Exception while parsing pending download:" + sharedPrefString); + } + } + return new DownloadSharedPreferenceEntry(-1, false, false, null, ""); + } + + /** + * @return a string for the DownloadSharedPreferenceEntry instance to be inserted into + * SharedPrefs. + */ + String getSharedPreferenceString() { + return VERSION + "," + notificationId + "," + (isResumable ? "1" : "0") + "," + + (isStartedOnMeteredNetwork ? "1" : "0") + "," + downloadGuid + "," + fileName; + } + + /** + * Check if a string is a valid GUID. GUID is RFC 4122 compliant, it should have format + * xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx. + * TODO(qinmin): move this to base/. + * @return true if the string is a valid GUID, or false otherwise. + */ + static boolean isValidGUID(String guid) { + if (guid == null) return false; + try { + // Java UUID class doesn't check the length of the string. Need to convert it back to + // string so that we can validate the length of the original string. + UUID uuid = UUID.fromString(guid); + String uuidString = uuid.toString(); + return guid.equalsIgnoreCase(uuidString); + } catch (IllegalArgumentException e) { + return false; + } + } +} diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java index 354b656..d918315 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java @@ -35,7 +35,7 @@ public class DownloadSnackbarController implements SnackbarManager.SnackbarContr Pair<DownloadInfo, Long> download = (Pair<DownloadInfo, Long>) actionData; DownloadManagerService manager = DownloadManagerService.getDownloadManagerService(mContext); manager.openDownloadedContent(download.second); - manager.cancelNotification(download.first.getDownloadId()); + manager.cancelNotification(download.first.getNotificationId()); } @Override @@ -46,7 +46,7 @@ public class DownloadSnackbarController implements SnackbarManager.SnackbarContr * Called to display the download succeeded snackbar. * * @param downloadInfo Info of the download. - * @param downloadId Id of the download. + * @param downloadId Id of the download from Android DownloadManager. * @param canBeResolved Whether the download can be resolved to any activity. */ public void onDownloadSucceeded( diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java index 11002e7..0bbb1e0 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java @@ -244,13 +244,15 @@ public class OMADownloadHandler { // Send notification if required attributes are missing. if (omaInfo.getTypes().isEmpty() || getSize(omaInfo) <= 0 || omaInfo.isValueEmpty(OMA_OBJECT_URI)) { - sendNotification(omaInfo, mDownloadInfo, DOWNLOAD_STATUS_INVALID_DESCRIPTOR); + sendNotification(omaInfo, mDownloadInfo, DownloadItem.INVALID_DOWNLOAD_ID, + DOWNLOAD_STATUS_INVALID_DESCRIPTOR); return; } // Check version. Null version are treated as 1.0. String version = omaInfo.getValue(OMA_DD_VERSION); if (version != null && !version.startsWith("1.")) { - sendNotification(omaInfo, mDownloadInfo, DOWNLOAD_STATUS_INVALID_DDVERSION); + sendNotification(omaInfo, mDownloadInfo, DownloadItem.INVALID_DOWNLOAD_ID, + DOWNLOAD_STATUS_INVALID_DDVERSION); return; } // Check device capabilities. @@ -274,16 +276,17 @@ public class OMADownloadHandler { * Called when the content is successfully downloaded by the Android DownloadManager. * * @param downloadInfo The information about the download. + * @param downloadId Download Id from the Android DownloadManager. * @param notifyURI The previously saved installNotifyURI attribute. */ - public void onDownloadCompleted(DownloadInfo downloadInfo, String notifyURI) { - long downloadId = downloadInfo.getDownloadId(); + public void onDownloadCompleted(DownloadInfo downloadInfo, long downloadId, String notifyURI) { OMAInfo omaInfo = mPendingOMADownloads.get(downloadId); if (omaInfo == null) { omaInfo = new OMAInfo(); omaInfo.addAttributeValue(OMA_INSTALL_NOTIFY_URI, notifyURI); } - sendInstallNotificationAndNextStep(omaInfo, downloadInfo, DOWNLOAD_STATUS_SUCCESS); + sendInstallNotificationAndNextStep( + omaInfo, downloadInfo, downloadId, DOWNLOAD_STATUS_SUCCESS); mPendingOMADownloads.remove(downloadId); } @@ -291,10 +294,12 @@ public class OMADownloadHandler { * Called when android DownloadManager fails to download the content. * * @param downloadInfo The information about the download. + * @param downloadId Download Id from the Android DownloadManager. * @param reason The reason of failure. * @param notifyURI The previously saved installNotifyURI attribute. */ - public void onDownloadFailed(DownloadInfo downloadInfo, int reason, String notifyURI) { + public void onDownloadFailed( + DownloadInfo downloadInfo, long downloadId, int reason, String notifyURI) { String status = DOWNLOAD_STATUS_DEVICE_ABORTED; switch (reason) { case DownloadManager.ERROR_CANNOT_RESUME: @@ -311,13 +316,12 @@ public class OMADownloadHandler { default: break; } - long downloadId = downloadInfo.getDownloadId(); OMAInfo omaInfo = mPendingOMADownloads.get(downloadId); if (omaInfo == null) { // Just send the notification in this case. omaInfo = new OMAInfo(); omaInfo.addAttributeValue(OMA_INSTALL_NOTIFY_URI, notifyURI); - sendInstallNotificationAndNextStep(omaInfo, downloadInfo, status); + sendInstallNotificationAndNextStep(omaInfo, downloadInfo, downloadId, status); return; } showDownloadWarningDialog( @@ -332,11 +336,12 @@ public class OMADownloadHandler { * * @param omaInfo Information about the OMA content. * @param downloadInfo Information about the download. + * @param downloadId Id of the download in Android DownloadManager. * @param statusMessage The message to send to the notification server. */ private void sendInstallNotificationAndNextStep( - OMAInfo omaInfo, DownloadInfo downloadInfo, String statusMessage) { - if (!sendNotification(omaInfo, downloadInfo, statusMessage)) { + OMAInfo omaInfo, DownloadInfo downloadInfo, long downloadId, String statusMessage) { + if (!sendNotification(omaInfo, downloadInfo, downloadId, statusMessage)) { showNextUrlDialog(omaInfo); } } @@ -346,14 +351,15 @@ public class OMADownloadHandler { * * @param omaInfo Information about the OMA content. * @param downloadInfo Information about the download. + * @param downloadId Id of the download in Android DownloadManager. * @param statusMessage The message to send to the notification server. * @return true if the notification ise sent, or false otherwise. */ private boolean sendNotification( - OMAInfo omaInfo, DownloadInfo downloadInfo, String statusMessage) { + OMAInfo omaInfo, DownloadInfo downloadInfo, long downloadId, String statusMessage) { if (omaInfo == null) return false; if (omaInfo.isValueEmpty(OMA_INSTALL_NOTIFY_URI)) return false; - PostStatusTask task = new PostStatusTask(omaInfo, downloadInfo, statusMessage); + PostStatusTask task = new PostStatusTask(omaInfo, downloadInfo, downloadId, statusMessage); task.execute(); return true; } @@ -388,7 +394,9 @@ public class OMADownloadHandler { if (which == AlertDialog.BUTTON_POSITIVE) { downloadOMAContent(downloadId, downloadInfo, omaInfo); } else { - sendNotification(omaInfo, downloadInfo, DOWNLOAD_STATUS_USER_CANCELLED); + sendNotification(omaInfo, downloadInfo, + DownloadItem.INVALID_DOWNLOAD_ID, + DOWNLOAD_STATUS_USER_CANCELLED); } } }; @@ -419,7 +427,8 @@ public class OMADownloadHandler { @Override public void onClick(DialogInterface dialog, int which) { if (which == AlertDialog.BUTTON_POSITIVE) { - sendInstallNotificationAndNextStep(omaInfo, downloadInfo, statusMessage); + sendInstallNotificationAndNextStep(omaInfo, downloadInfo, + DownloadItem.INVALID_DOWNLOAD_ID, statusMessage); } } }; @@ -592,16 +601,16 @@ public class OMADownloadHandler { .setFileName(fileName) .setUrl(url) .setMimeType(mimeType) - .setDownloadId((int) downloadId) .setDescription(omaInfo.getValue(OMA_DESCRIPTION)) .setContentLength(getSize(omaInfo)) .build(); // If installNotifyURI is not empty, the downloaded content cannot // be used until the PostStatusTask gets a 200-series response. // Don't show complete notification until that happens. - DownloadManagerService.getDownloadManagerService(mContext) - .enqueueDownloadManagerRequest( - newInfo, omaInfo.isValueEmpty(OMA_INSTALL_NOTIFY_URI)); + DownloadItem item = new DownloadItem(true, newInfo); + item.setSystemDownloadId(downloadId); + DownloadManagerService.getDownloadManagerService(mContext).enqueueDownloadManagerRequest( + item, omaInfo.isValueEmpty(OMA_INSTALL_NOTIFY_URI)); mPendingOMADownloads.put(downloadId, omaInfo); } @@ -618,18 +627,13 @@ public class OMADownloadHandler { /** * Updates the download information with the new download Id. * - * @param downloadInfo Information about the download. + * @param oldDownloadId Old download Id from the DownloadManager. * @param newDownloadId New download Id from the DownloadManager. - * @return the new download information with the new Id. */ - public DownloadInfo updateDownloadInfo(DownloadInfo downloadInfo, long newDownloadId) { - long oldDownloadId = downloadInfo.getDownloadId(); + public void updateDownloadInfo(long oldDownloadId, long newDownloadId) { OMAInfo omaInfo = mPendingOMADownloads.get(oldDownloadId); mPendingOMADownloads.remove(oldDownloadId); mPendingOMADownloads.put(newDownloadId, omaInfo); - return DownloadInfo.Builder.fromDownloadInfo(downloadInfo) - .setDownloadId((int) newDownloadId) - .build(); } /** @@ -651,12 +655,14 @@ public class OMADownloadHandler { private final OMAInfo mOMAInfo; private final DownloadInfo mDownloadInfo; private final String mStatusMessage; + private final long mDownloadId; public PostStatusTask( - OMAInfo omaInfo, DownloadInfo downloadInfo, String statusMessage) { + OMAInfo omaInfo, DownloadInfo downloadInfo, long downloadId, String statusMessage) { mOMAInfo = omaInfo; mDownloadInfo = downloadInfo; mStatusMessage = statusMessage; + mDownloadId = downloadId; } @Override @@ -726,9 +732,9 @@ public class OMADownloadHandler { } } showNextUrlDialog(mOMAInfo); - } else if (mDownloadInfo.getDownloadId() > 0) { + } else if (mDownloadId != DownloadItem.INVALID_DOWNLOAD_ID) { // Remove the downloaded content. - manager.remove(mDownloadInfo.getDownloadId()); + manager.remove(mDownloadId); } } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java b/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java index 3f63aab..a744a76 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java @@ -36,7 +36,7 @@ public class SystemDownloadNotifier implements DownloadNotifier { private final Object mLock = new Object(); @Nullable private DownloadNotificationService mBoundService; private boolean mServiceStarted; - private Set<Integer> mActiveDownloadIds = new HashSet<Integer>(); + private Set<Integer> mActiveNotificationIds = new HashSet<Integer>(); private List<PendingNotificationInfo> mPendingNotifications = new ArrayList<PendingNotificationInfo>(); @@ -122,7 +122,7 @@ public class SystemDownloadNotifier implements DownloadNotifier { notifyDownloadFailed(info.downloadInfo); break; case DOWNLOAD_NOTIFICATION_TYPE_CANCEL: - cancelNotification(info.downloadInfo.getDownloadId()); + cancelNotification(info.downloadInfo.getNotificationId()); break; case DOWNLOAD_NOTIFICATION_TYPE_CLEAR: clearPendingDownloads(); @@ -150,7 +150,7 @@ public class SystemDownloadNotifier implements DownloadNotifier { */ private void stopServiceIfNeeded() { assert Thread.holdsLock(mLock); - if (mActiveDownloadIds.isEmpty() && mServiceStarted) { + if (mActiveNotificationIds.isEmpty() && mServiceStarted) { stopService(); mServiceStarted = false; } @@ -179,8 +179,8 @@ public class SystemDownloadNotifier implements DownloadNotifier { } @Override - public void cancelNotification(int downloadId) { - DownloadInfo info = new DownloadInfo.Builder().setDownloadId(downloadId).build(); + public void cancelNotification(int notificationId) { + DownloadInfo info = new DownloadInfo.Builder().setNotificationId(notificationId).build(); updateDownloadNotification(DOWNLOAD_NOTIFICATION_TYPE_CANCEL, info, null, -1, false); } @@ -227,9 +227,9 @@ public class SystemDownloadNotifier implements DownloadNotifier { synchronized (mLock) { startAndBindToServiceIfNeeded(); if (type == DOWNLOAD_NOTIFICATION_TYPE_PROGRESS) { - mActiveDownloadIds.add(info.getDownloadId()); + mActiveNotificationIds.add(info.getNotificationId()); } else if (type != DOWNLOAD_NOTIFICATION_TYPE_CLEAR) { - mActiveDownloadIds.remove(info.getDownloadId()); + mActiveNotificationIds.remove(info.getNotificationId()); } if (mBoundService == null) { // We need to wait for the service to connect before we can handle @@ -240,27 +240,30 @@ public class SystemDownloadNotifier implements DownloadNotifier { } else { switch (type) { case DOWNLOAD_NOTIFICATION_TYPE_PROGRESS: - mBoundService.notifyDownloadProgress(info.getDownloadId(), - info.getFileName(), info.getPercentCompleted(), - info.getTimeRemainingInMillis(), startTime, info.isResumable()); + + mBoundService.notifyDownloadProgress(info.getNotificationId(), + info.getDownloadGuid(), info.getFileName(), + info.getPercentCompleted(), info.getTimeRemainingInMillis(), + startTime, info.isResumable()); break; case DOWNLOAD_NOTIFICATION_TYPE_PAUSE: assert info.isResumable(); - mBoundService.notifyDownloadPaused(info.getDownloadId(), info.getFileName(), + mBoundService.notifyDownloadPaused(info.getNotificationId(), + info.getDownloadGuid(), info.getFileName(), info.isResumable(), isAutoResumable); break; case DOWNLOAD_NOTIFICATION_TYPE_SUCCESS: mBoundService.notifyDownloadSuccessful( - info.getDownloadId(), info.getFileName(), intent); + info.getNotificationId(), info.getFileName(), intent); stopServiceIfNeeded(); break; case DOWNLOAD_NOTIFICATION_TYPE_FAILURE: mBoundService.notifyDownloadFailed( - info.getDownloadId(), info.getFileName()); + info.getNotificationId(), info.getFileName()); stopServiceIfNeeded(); break; case DOWNLOAD_NOTIFICATION_TYPE_CANCEL: - mBoundService.cancelNotification(info.getDownloadId()); + mBoundService.cancelNotification(info.getNotificationId()); stopServiceIfNeeded(); break; case DOWNLOAD_NOTIFICATION_TYPE_CLEAR: diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index d683fb8..6a6ab30 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni @@ -281,10 +281,12 @@ chrome_java_sources = [ "java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeTabInfo.java", "java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java", "java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java", + "java/src/org/chromium/chrome/browser/download/DownloadItem.java", "java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java", "java/src/org/chromium/chrome/browser/download/DownloadManagerService.java", "java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java", "java/src/org/chromium/chrome/browser/download/DownloadNotifier.java", + "java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java", "java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java", "java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java", "java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java", diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java index c32a7af..4c3927b 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java @@ -33,6 +33,7 @@ import java.util.HashSet; import java.util.Queue; import java.util.Random; import java.util.Set; +import java.util.UUID; import java.util.concurrent.ConcurrentLinkedQueue; /** @@ -131,8 +132,8 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { } @Override - public void cancelNotification(int downloadId) { - assertCorrectExpectedCall(MethodID.CANCEL_DOWNLOAD_ID, downloadId); + public void cancelNotification(int notificationId) { + assertCorrectExpectedCall(MethodID.CANCEL_DOWNLOAD_ID, notificationId); } @Override @@ -225,7 +226,6 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { static class MockOMADownloadHandler extends OMADownloadHandler { protected boolean mSuccess; protected String mNofityURI; - protected DownloadInfo mDownloadInfo; protected long mDownloadId; MockOMADownloadHandler(Context context) { @@ -237,7 +237,8 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { } @Override - public void onDownloadCompleted(DownloadInfo downloadInfo, String notifyURI) { + public void onDownloadCompleted( + DownloadInfo downloadInfo, long downloadId, String notifyURI) { mSuccess = true; mNofityURI = notifyURI; } @@ -248,13 +249,8 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { } @Override - public DownloadInfo updateDownloadInfo(DownloadInfo downloadInfo, long newDownloadId) { - mDownloadInfo = downloadInfo; + public void updateDownloadInfo(long oldDownloadId, long newDownloadId) { mDownloadId = newDownloadId; - mDownloadInfo = DownloadInfo.Builder.fromDownloadInfo(downloadInfo) - .setDownloadId((int) newDownloadId) - .build(); - return mDownloadInfo; } @Override @@ -273,8 +269,9 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { } @Override - protected long addCompletedDownload(DownloadInfo downloadInfo) { - return 1L; + protected boolean addCompletedDownload(DownloadItem downloadItem) { + downloadItem.setSystemDownloadId(1L); + return true; } @Override @@ -286,7 +283,8 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { protected void init() {} @Override - protected void resumeDownload(int downloadId, String fileName, boolean hasUserGesture) { + protected void resumeDownload(int notificationId, String downloadGuid, String fileName, + boolean hasUserGesture) { mResumed = true; } } @@ -298,9 +296,9 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { } private DownloadInfo getDownloadInfo() { - return new Builder().setContentLength(100) - .setDownloadId(mRandom.nextInt(1000)) - .setHasDownloadId(true) + return new Builder() + .setContentLength(100) + .setDownloadGuid(UUID.randomUUID().toString()) .build(); } @@ -534,7 +532,6 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { try { DownloadInfo info = new DownloadInfo.Builder() - .setDownloadId(0) .setMimeType(OMADownloadHandler.OMA_DRM_MESSAGE_MIME) .setFileName("test.gzip") .setUrl(testServer.getURL("/chrome/test/data/android/download/test.gzip")) @@ -546,14 +543,15 @@ public class DownloadManagerServiceTest extends InstrumentationTestCase { final MockOMADownloadHandler handler = new MockOMADownloadHandler(context); dService.setOMADownloadHandler(handler); handler.setDownloadId(0); - dService.enqueueDownloadManagerRequest(info, true); + DownloadItem item = new DownloadItem(true, info); + item.setSystemDownloadId(0); + dService.enqueueDownloadManagerRequest(item, true); CriteriaHelper.pollUiThread(new Criteria() { @Override public boolean isSatisfied() { return handler.mDownloadId != 0; } }); - handler.mDownloadId = handler.mDownloadInfo.getDownloadId(); Set<String> downloads = dService.getStoredDownloadInfo( PreferenceManager.getDefaultSharedPreferences(context), DownloadManagerService.PENDING_OMA_DOWNLOADS); diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java index 26bf7cc..d0b47c4 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java @@ -18,6 +18,7 @@ import org.chromium.base.test.util.Feature; import java.util.HashSet; import java.util.Set; +import java.util.UUID; /** * Tests of {@link DownloadNotificationService}. @@ -73,10 +74,10 @@ public class DownloadNotificationServiceTest extends Context mockContext = new AdvancedMockContext(getSystemContext()); getService().setContext(mockContext); Set<String> notifications = new HashSet<String>(); - notifications.add(new DownloadNotificationService.PendingNotification(1, "test1", true) - .getNotificationString()); - notifications.add(new DownloadNotificationService.PendingNotification(2, "test2", true) - .getNotificationString()); + notifications.add(new DownloadSharedPreferenceEntry(1, true, true, + UUID.randomUUID().toString(), "test1").getSharedPreferenceString()); + notifications.add(new DownloadSharedPreferenceEntry(2, true, true, + UUID.randomUUID().toString(), "test2").getSharedPreferenceString()); SharedPreferences sharedPrefs = PreferenceManager.getDefaultSharedPreferences(mockContext); SharedPreferences.Editor editor = sharedPrefs.edit(); @@ -103,7 +104,7 @@ public class DownloadNotificationServiceTest extends getService().setContext(mockContext); startNotificationService(); DownloadNotificationService service = bindNotificationService(); - service.notifyDownloadProgress(1, "test", -1, 1L, 1L, true); + service.notifyDownloadProgress(1, UUID.randomUUID().toString(), "test", -1, 1L, 1L, true); assertEquals(1, getService().getNotificationIds().size()); assertTrue(getService().getNotificationIds().contains(1)); @@ -123,18 +124,21 @@ public class DownloadNotificationServiceTest extends @SmallTest @Feature({"Download"}) public void testParseDownloadNotifications() { - String notification = "1,0,test.pdf"; - DownloadNotificationService.PendingNotification pendingNotification = - DownloadNotificationService.PendingNotification.parseFromString(notification); - assertEquals(1, pendingNotification.downloadId); - assertEquals("test.pdf", pendingNotification.fileName); - assertFalse(pendingNotification.isResumable); - - notification = "2,1,test,2.pdf"; - pendingNotification = - DownloadNotificationService.PendingNotification.parseFromString(notification); - assertEquals(2, pendingNotification.downloadId); - assertEquals("test,2.pdf", pendingNotification.fileName); - assertTrue(pendingNotification.isResumable); + String uuid = UUID.randomUUID().toString(); + String notification = + DownloadSharedPreferenceEntry.VERSION + ",1,0,1," + uuid + ",test.pdf"; + DownloadSharedPreferenceEntry entry = + DownloadSharedPreferenceEntry.parseFromString(notification); + assertEquals(1, entry.notificationId); + assertEquals("test.pdf", entry.fileName); + assertFalse(entry.isResumable); + assertEquals(uuid, entry.downloadGuid); + + notification = DownloadSharedPreferenceEntry.VERSION + ",2,1,1," + uuid + ",test,2.pdf"; + entry = DownloadSharedPreferenceEntry.parseFromString(notification); + assertEquals(2, entry.notificationId); + assertEquals("test,2.pdf", entry.fileName); + assertTrue(entry.isResumable); + assertEquals(uuid, entry.downloadGuid); } } diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTestBase.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTestBase.java index 7cb1982..e6b3341 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTestBase.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTestBase.java @@ -225,10 +225,11 @@ public abstract class DownloadTestBase extends ChromeActivityTestCaseBase<Chrome @Override public void enqueueDownloadManagerRequest( - final DownloadInfo info, boolean notifyCompleted) { + final DownloadItem item, boolean notifyCompleted) { // Intentionally do not call super, since DownloadManager does not work in test // environment. - mEnqueueHttpGetDownloadCallbackHelper.notifyCalled(info, notifyCompleted); + mEnqueueHttpGetDownloadCallbackHelper.notifyCalled( + item.getDownloadInfo(), notifyCompleted); } } diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java index bc0f05a..af5f904 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java @@ -42,8 +42,8 @@ public class MockDownloadNotificationService extends DownloadNotificationService } @Override - public void cancelNotification(int downloadId) { - mNotificationIds.remove(Integer.valueOf(downloadId)); + public void cancelNotification(int notificationId) { + mNotificationIds.remove(Integer.valueOf(notificationId)); } @Override diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java index 766b879..c441ca7 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java @@ -13,6 +13,8 @@ import org.chromium.base.test.util.AdvancedMockContext; import org.chromium.base.test.util.Feature; import org.chromium.content.browser.DownloadInfo; +import java.util.UUID; + /** * Tests of {@link SystemDownloadNotifier}. */ @@ -67,7 +69,8 @@ public class SystemDownloadNotifierTest extends InstrumentationTestCase { @SmallTest @Feature({"Download"}) public void testNotificationNotHandledUntilServiceConnection() { - DownloadInfo info = new DownloadInfo.Builder().setDownloadId(1).build(); + DownloadInfo info = new DownloadInfo.Builder() + .setDownloadGuid(UUID.randomUUID().toString()).setNotificationId(1).build(); mDownloadNotifier.notifyDownloadProgress(info, 1L); assertTrue(mDownloadNotifier.mStarted); @@ -83,10 +86,12 @@ public class SystemDownloadNotifierTest extends InstrumentationTestCase { @Feature({"Download"}) public void testServiceStoppedWhenAllDownloadsFinish() { onServiceConnected(); - DownloadInfo info = new DownloadInfo.Builder().setDownloadId(1).build(); + DownloadInfo info = new DownloadInfo.Builder() + .setDownloadGuid(UUID.randomUUID().toString()).setNotificationId(1).build(); mDownloadNotifier.notifyDownloadProgress(info, 1L); assertTrue(mDownloadNotifier.mStarted); - DownloadInfo info2 = new DownloadInfo.Builder().setDownloadId(2).build(); + DownloadInfo info2 = new DownloadInfo.Builder() + .setDownloadGuid(UUID.randomUUID().toString()).setNotificationId(2).build(); mDownloadNotifier.notifyDownloadProgress(info2, 1L); mDownloadNotifier.notifyDownloadFailed(info); diff --git a/chrome/browser/android/download/chrome_download_delegate.cc b/chrome/browser/android/download/chrome_download_delegate.cc index 5fa5db1..8ce3fac 100644 --- a/chrome/browser/android/download/chrome_download_delegate.cc +++ b/chrome/browser/android/download/chrome_download_delegate.cc @@ -48,14 +48,17 @@ static jboolean IsDownloadDangerous(JNIEnv* env, } // Called when a dangerous download is validated. -static void DangerousDownloadValidated(JNIEnv* env, - const JavaParamRef<jclass>& clazz, - const JavaParamRef<jobject>& tab, - jint download_id, - jboolean accept) { +static void DangerousDownloadValidated( + JNIEnv* env, + const JavaParamRef<jclass>& clazz, + const JavaParamRef<jobject>& tab, + const JavaParamRef<jstring>& jdownload_guid, + jboolean accept) { + std::string download_guid = + base::android::ConvertJavaStringToUTF8(env, jdownload_guid); TabAndroid* tab_android = TabAndroid::GetNativeTab(env, tab); DownloadControllerAndroid::Get()->DangerousDownloadValidated( - tab_android->web_contents(), download_id, accept); + tab_android->web_contents(), download_guid, accept); } // static diff --git a/chrome/browser/android/download/download_manager_service.cc b/chrome/browser/android/download/download_manager_service.cc index 0e0ed61..935f174 100644 --- a/chrome/browser/android/download/download_manager_service.cc +++ b/chrome/browser/android/download/download_manager_service.cc @@ -57,26 +57,34 @@ DownloadManagerService::~DownloadManagerService() { manager_->RemoveObserver(this); } -void DownloadManagerService::ResumeDownload(JNIEnv* env, - jobject obj, - uint32_t download_id, - jstring fileName) { - ResumeDownloadInternal(download_id, ConvertJavaStringToUTF8(env, fileName), - true); +void DownloadManagerService::ResumeDownload( + JNIEnv* env, + jobject obj, + uint32_t download_id, + const JavaParamRef<jstring>& jdownload_guid, + jstring fileName) { + std::string download_guid = ConvertJavaStringToUTF8(env, jdownload_guid); + ResumeDownloadInternal(download_id, download_guid, + ConvertJavaStringToUTF8(env, fileName), true); } -void DownloadManagerService::CancelDownload(JNIEnv* env, - jobject obj, - uint32_t download_id) { - CancelDownloadInternal(download_id, true); +void DownloadManagerService::CancelDownload( + JNIEnv* env, + jobject obj, + const JavaParamRef<jstring>& jdownload_guid) { + std::string download_guid = ConvertJavaStringToUTF8(env, jdownload_guid); + CancelDownloadInternal(download_guid, true); } -void DownloadManagerService::PauseDownload(JNIEnv* env, - jobject obj, - uint32_t download_id) { - content::DownloadItem* item = manager_->GetDownload(download_id); +void DownloadManagerService::PauseDownload( + JNIEnv* env, + jobject obj, + const JavaParamRef<jstring>& jdownload_guid) { + std::string download_guid = ConvertJavaStringToUTF8(env, jdownload_guid); + content::DownloadItem* item = manager_->GetDownloadByGuid(download_guid); if (item) item->Pause(); + item->RemoveObserver(content::DownloadControllerAndroid::Get()); } void DownloadManagerService::ManagerGoingDown( @@ -96,14 +104,16 @@ void DownloadManagerService::ResumeDownloadItem(content::DownloadItem* item, resume_callback_for_testing_.Run(true); } -void DownloadManagerService::ResumeDownloadInternal(uint32_t download_id, - const std::string& fileName, - bool retry) { +void DownloadManagerService::ResumeDownloadInternal( + uint32_t download_id, + const std::string& download_guid, + const std::string& fileName, + bool retry) { if (!manager_) { OnResumptionFailed(download_id, fileName); return; } - content::DownloadItem* item = manager_->GetDownload(download_id); + content::DownloadItem* item = manager_->GetDownloadByGuid(download_guid); if (item) { ResumeDownloadItem(item, fileName); return; @@ -120,25 +130,27 @@ void DownloadManagerService::ResumeDownloadInternal(uint32_t download_id, // created item might not be loaded from download history. So user might wait // indefinitely to see the failed notification. See http://crbug.com/577893. base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&DownloadManagerService::ResumeDownloadInternal, - base::Unretained(this), download_id, fileName, false), + FROM_HERE, base::Bind(&DownloadManagerService::ResumeDownloadInternal, + base::Unretained(this), download_id, download_guid, + fileName, false), base::TimeDelta::FromMilliseconds(kRetryIntervalInMilliseconds)); } -void DownloadManagerService::CancelDownloadInternal(uint32_t download_id, - bool retry) { +void DownloadManagerService::CancelDownloadInternal( + const std::string& download_guid, + bool retry) { if (!manager_) return; - content::DownloadItem* item = manager_->GetDownload(download_id); + content::DownloadItem* item = manager_->GetDownloadByGuid(download_guid); if (item) { item->Cancel(true); + item->RemoveObserver(content::DownloadControllerAndroid::Get()); return; } if (retry) { base::MessageLoop::current()->PostDelayedTask( FROM_HERE, base::Bind(&DownloadManagerService::CancelDownloadInternal, - base::Unretained(this), download_id, false), + base::Unretained(this), download_guid, false), base::TimeDelta::FromMilliseconds(kRetryIntervalInMilliseconds)); } } diff --git a/chrome/browser/android/download/download_manager_service.h b/chrome/browser/android/download/download_manager_service.h index f1a5f9e..aa65a32 100644 --- a/chrome/browser/android/download/download_manager_service.h +++ b/chrome/browser/android/download/download_manager_service.h @@ -13,6 +13,8 @@ #include "base/macros.h" #include "content/public/browser/download_manager.h" +using base::android::JavaParamRef; + namespace content { class DownloadItem; } @@ -29,20 +31,26 @@ class DownloadManagerService : public content::DownloadManager::Observer { content::DownloadManager* manager); ~DownloadManagerService() override; - // Called to resume downloading the item that has ID equal to |download_id|. - // If the DownloadItem is not yet created, retry after a while. + // Called to resume downloading the item that has GUID equal to + // |jdownload_guid|. If the DownloadItem is not yet created, retry after + // a while. void ResumeDownload(JNIEnv* env, jobject obj, uint32_t download_id, + const JavaParamRef<jstring>& jdownload_guid, jstring fileName); - // Called to cancel a download item that has ID equal to |download_id|. + // Called to cancel a download item that has GUID equal to |jdownload_guid|. // If the DownloadItem is not yet created, retry after a while. - void CancelDownload(JNIEnv* env, jobject obj, uint32_t download_id); + void CancelDownload(JNIEnv* env, + jobject obj, + const JavaParamRef<jstring>& jdownload_guid); - // Called to pause a download item that has ID equal to |download_id|. + // Called to pause a download item that has GUID equal to |jdownload_guid|. // If the DownloadItem is not yet created, do nothing as it is already paused. - void PauseDownload(JNIEnv* env, jobject obj, uint32_t download_id); + void PauseDownload(JNIEnv* env, + jobject obj, + const JavaParamRef<jstring>& jdownload_guid); // content::DownloadManager::Observer methods. void ManagerGoingDown(content::DownloadManager* manager) override; @@ -58,12 +66,13 @@ class DownloadManagerService : public content::DownloadManager::Observer { // Helper function to start the download resumption. If |retry| is true, // chrome will retry the resumption if the download item is not loaded. void ResumeDownloadInternal(uint32_t download_id, + const std::string& download_guid, const std::string& fileName, bool retry); // Helper function to cancel a download. If |retry| is true, // chrome will retry the cancellation if the download item is not loaded. - void CancelDownloadInternal(uint32_t download_id, bool retry); + void CancelDownloadInternal(const std::string& download_guid, bool retry); // Called to notify the java side that download resumption failed. void OnResumptionFailed(uint32_t download_id, const std::string& fileName); diff --git a/chrome/browser/android/download/download_manager_service_unittest.cc b/chrome/browser/android/download/download_manager_service_unittest.cc index b987c8b..c5d60d7a 100644 --- a/chrome/browser/android/download/download_manager_service_unittest.cc +++ b/chrome/browser/android/download/download_manager_service_unittest.cc @@ -36,9 +36,10 @@ class DownloadManagerServiceTest : public testing::Test { &manager_)), finished_(false), success_(false) { - ON_CALL(manager_, GetDownload(_)) + ON_CALL(manager_, GetDownloadByGuid(_)) .WillByDefault( - ::testing::Invoke(this, &DownloadManagerServiceTest::GetDownload)); + ::testing::Invoke(this, + &DownloadManagerServiceTest::GetDownloadByGuid)); } void OnResumptionDone(bool success) { @@ -46,12 +47,14 @@ class DownloadManagerServiceTest : public testing::Test { success_ = success; } - void StartDownload(int download_id) { + void StartDownload(const std::string& download_guid) { JNIEnv* env = base::android::AttachCurrentThread(); service_->set_resume_callback_for_testing(base::Bind( &DownloadManagerServiceTest::OnResumptionDone, base::Unretained(this))); service_->ResumeDownload( - env, nullptr, download_id, + env, nullptr, 0, JavaParamRef<jstring>( + env, base::android::ConvertUTF8ToJavaString( + env, download_guid).obj()), base::android::ConvertUTF8ToJavaString(env, "test").obj()); while (!finished_) message_loop_.RunUntilIdle(); @@ -64,7 +67,9 @@ class DownloadManagerServiceTest : public testing::Test { } protected: - content::DownloadItem* GetDownload(uint32_t) { return download_.get(); } + content::DownloadItem* GetDownloadByGuid(const std::string&) { + return download_.get(); + } base::MessageLoop message_loop_; scoped_ptr<content::MockDownloadItem> download_; @@ -78,7 +83,7 @@ class DownloadManagerServiceTest : public testing::Test { // Test that resumption will fail if no download item is found before times out. TEST_F(DownloadManagerServiceTest, ResumptionTimeOut) { - StartDownload(1); + StartDownload("0000"); EXPECT_FALSE(success_); } @@ -86,13 +91,13 @@ TEST_F(DownloadManagerServiceTest, ResumptionTimeOut) { // resumed. TEST_F(DownloadManagerServiceTest, ResumptionWithResumableItem) { CreateDownloadItem(true); - StartDownload(1); + StartDownload("0000"); EXPECT_TRUE(success_); } // Test that resumption fails if the target download item is not resumable. TEST_F(DownloadManagerServiceTest, ResumptionWithNonResumableItem) { CreateDownloadItem(false); - StartDownload(1); + StartDownload("0000"); EXPECT_FALSE(success_); } diff --git a/chrome/browser/android/download/mock_download_controller_android.cc b/chrome/browser/android/download/mock_download_controller_android.cc index f072e9e..7551330 100644 --- a/chrome/browser/android/download/mock_download_controller_android.cc +++ b/chrome/browser/android/download/mock_download_controller_android.cc @@ -34,8 +34,9 @@ void MockDownloadControllerAndroid::StartContextMenuDownload( } void MockDownloadControllerAndroid::DangerousDownloadValidated( - content::WebContents* web_contents, int download_id, bool accept) { -} + content::WebContents* web_contents, + const std::string& download_guid, + bool accept) {} void MockDownloadControllerAndroid::AcquireFileAccessPermission( content::WebContents* web_contents, diff --git a/chrome/browser/android/download/mock_download_controller_android.h b/chrome/browser/android/download/mock_download_controller_android.h index 50e621e..d74e7dfe 100644 --- a/chrome/browser/android/download/mock_download_controller_android.h +++ b/chrome/browser/android/download/mock_download_controller_android.h @@ -33,9 +33,9 @@ class MockDownloadControllerAndroid const content::ContextMenuParams& params, content::WebContents* web_contents, bool is_link, const std::string& extra_headers) override; - void DangerousDownloadValidated( - content::WebContents* web_contents, int download_id, - bool accept) override; + void DangerousDownloadValidated(content::WebContents* web_contents, + const std::string& download_guid, + bool accept) override; void AcquireFileAccessPermission( content::WebContents* web_contents, const AcquireFileAccessPermissionCallback& callback) override; diff --git a/content/browser/android/download_controller_android_impl.cc b/content/browser/android/download_controller_android_impl.cc index a09532f..abbd4c3 100644 --- a/content/browser/android/download_controller_android_impl.cc +++ b/content/browser/android/download_controller_android_impl.cc @@ -453,6 +453,8 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { OnDangerousDownload(item); JNIEnv* env = base::android::AttachCurrentThread(); + ScopedJavaLocalRef<jstring> jguid = + ConvertUTF8ToJavaString(env, item->GetGuid()); ScopedJavaLocalRef<jstring> jurl = ConvertUTF8ToJavaString(env, item->GetURL().spec()); ScopedJavaLocalRef<jstring> jmime_type = @@ -473,8 +475,9 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { Java_DownloadController_onDownloadUpdated( env, GetJavaObject()->Controller(env).obj(), jurl.obj(), jmime_type.obj(), jfilename.obj(), jpath.obj(), - item->GetReceivedBytes(), item->GetId(), item->PercentComplete(), - time_delta.InMilliseconds(), item->HasUserGesture(), item->IsPaused(), + item->GetReceivedBytes(), item->GetId(), jguid.obj(), + item->PercentComplete(), time_delta.InMilliseconds(), + item->HasUserGesture(), item->IsPaused(), // Get all requirements that allows a download to be resumable. !item->GetBrowserContext()->IsOffTheRecord()); break; @@ -488,12 +491,13 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { Java_DownloadController_onDownloadCompleted( env, GetJavaObject()->Controller(env).obj(), jurl.obj(), jmime_type.obj(), jfilename.obj(), jpath.obj(), - item->GetReceivedBytes(), item->GetId(), + item->GetReceivedBytes(), item->GetId(), jguid.obj(), joriginal_url.obj(), jreferrer_url.obj(), item->HasUserGesture()); break; case DownloadItem::CANCELLED: Java_DownloadController_onDownloadCancelled( - env, GetJavaObject()->Controller(env).obj(), item->GetId()); + env, GetJavaObject()->Controller(env).obj(), item->GetId(), + jguid.obj()); break; case DownloadItem::INTERRUPTED: // When device loses/changes network, we get a NETWORK_TIMEOUT, @@ -502,8 +506,9 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { Java_DownloadController_onDownloadInterrupted( env, GetJavaObject()->Controller(env).obj(), jurl.obj(), jmime_type.obj(), jfilename.obj(), jpath.obj(), - item->GetReceivedBytes(), item->GetId(), item->CanResume(), - IsInterruptedDownloadAutoResumable(item)); + item->GetReceivedBytes(), item->GetId(), jguid.obj(), + item->CanResume(), IsInterruptedDownloadAutoResumable(item)); + item->RemoveObserver(this); break; case DownloadItem::MAX_DOWNLOAD_STATE: NOTREACHED(); @@ -514,12 +519,14 @@ void DownloadControllerAndroidImpl::OnDangerousDownload(DownloadItem* item) { JNIEnv* env = base::android::AttachCurrentThread(); ScopedJavaLocalRef<jstring> jfilename = ConvertUTF8ToJavaString( env, item->GetTargetFilePath().BaseName().value()); + ScopedJavaLocalRef<jstring> jguid = + ConvertUTF8ToJavaString(env, item->GetGuid()); ScopedJavaLocalRef<jobject> view_core = GetContentViewCoreFromWebContents( item->GetWebContents()); if (!view_core.is_null()) { Java_DownloadController_onDangerousDownload( env, GetJavaObject()->Controller(env).obj(), view_core.obj(), - jfilename.obj(), item->GetId()); + jfilename.obj(), jguid.obj()); } } @@ -559,12 +566,14 @@ void DownloadControllerAndroidImpl::StartContextMenuDownload( } void DownloadControllerAndroidImpl::DangerousDownloadValidated( - WebContents* web_contents, int download_id, bool accept) { + WebContents* web_contents, + const std::string& download_guid, + bool accept) { if (!web_contents) return; DownloadManagerImpl* dlm = static_cast<DownloadManagerImpl*>( BrowserContext::GetDownloadManager(web_contents->GetBrowserContext())); - DownloadItem* item = dlm->GetDownload(download_id); + DownloadItem* item = dlm->GetDownloadByGuid(download_guid); if (!item) return; if (accept) diff --git a/content/browser/android/download_controller_android_impl.h b/content/browser/android/download_controller_android_impl.h index 9045d29..78d6544 100644 --- a/content/browser/android/download_controller_android_impl.h +++ b/content/browser/android/download_controller_android_impl.h @@ -102,7 +102,7 @@ class DownloadControllerAndroidImpl : public DownloadControllerAndroid { bool is_link, const std::string& extra_headers) override; void DangerousDownloadValidated(WebContents* web_contents, - int download_id, + const std::string& download_guid, bool accept) override; // DownloadItem::Observer interface. diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java index 4caff13..5153062 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java @@ -26,9 +26,9 @@ public interface ContentViewDownloadDelegate { * Notify the host application that a download has an extension indicating * a dangerous file type. * @param filename File name of the downloaded file. - * @param downloadId The download id. + * @param downloadGuid The download GUID. */ - void onDangerousDownload(String filename, int downloadId); + void onDangerousDownload(String filename, String downloadGuid); /** * Called when file access has been requested to complete a download. diff --git a/content/public/android/java/src/org/chromium/content/browser/DownloadController.java b/content/public/android/java/src/org/chromium/content/browser/DownloadController.java index 63a5eb7..5e2c29f 100644 --- a/content/public/android/java/src/org/chromium/content/browser/DownloadController.java +++ b/content/public/android/java/src/org/chromium/content/browser/DownloadController.java @@ -118,11 +118,13 @@ public class DownloadController { /** * Notifies the download delegate that a download completed and passes along info about the * download. This can be either a POST download or a GET download with authentication. + * TODO(qinmin): Generate the notificationId in the java side, the native downloadId is going + * to be deprecated. */ @CalledByNative private void onDownloadCompleted(String url, String mimeType, String filename, String path, - long contentLength, int downloadId, String originalUrl, String refererUrl, - boolean hasUserGesture) { + long contentLength, int notificationId, String downloadGuid, String originalUrl, + String refererUrl, boolean hasUserGesture) { if (sDownloadNotificationService == null) return; DownloadInfo downloadInfo = new DownloadInfo.Builder() .setUrl(url) @@ -131,8 +133,8 @@ public class DownloadController { .setFilePath(path) .setContentLength(contentLength) .setDescription(filename) - .setDownloadId(downloadId) - .setHasDownloadId(true) + .setNotificationId(notificationId) + .setDownloadGuid(downloadGuid) .setOriginalUrl(originalUrl) .setReferer(refererUrl) .setHasUserGesture(hasUserGesture) @@ -146,7 +148,8 @@ public class DownloadController { */ @CalledByNative private void onDownloadInterrupted(String url, String mimeType, String filename, String path, - long contentLength, int downloadId, boolean isResumable, boolean isAutoResumable) { + long contentLength, int notificationId, String downloadGuid, boolean isResumable, + boolean isAutoResumable) { if (sDownloadNotificationService == null) return; DownloadInfo downloadInfo = new DownloadInfo.Builder() .setUrl(url) @@ -155,8 +158,8 @@ public class DownloadController { .setFilePath(path) .setContentLength(contentLength) .setDescription(filename) - .setDownloadId(downloadId) - .setHasDownloadId(true) + .setNotificationId(notificationId) + .setDownloadGuid(downloadGuid) .setIsResumable(isResumable) .build(); sDownloadNotificationService.onDownloadInterrupted(downloadInfo, isAutoResumable); @@ -164,14 +167,17 @@ public class DownloadController { /** * Called when a download was cancelled. - * @param downloadId Id of the download item. + * @param notificationId Notification Id of the download item. + * @param downloadGuid GUID of the download item. + * TODO(qinmin): Generate the notificationId in the java side, the native downloadId is going + * to be deprecated. */ @CalledByNative - private void onDownloadCancelled(int downloadId) { + private void onDownloadCancelled(int notificationId, String downloadGuid) { if (sDownloadNotificationService == null) return; DownloadInfo downloadInfo = new DownloadInfo.Builder() - .setDownloadId(downloadId) - .setHasDownloadId(true) + .setNotificationId(notificationId) + .setDownloadGuid(downloadGuid) .build(); sDownloadNotificationService.onDownloadCancelled(downloadInfo); } @@ -179,10 +185,12 @@ public class DownloadController { /** * Notifies the download delegate about progress of a download. Downloads that use Chrome * network stack use custom notification to display the progress of downloads. + * TODO(qinmin): Generate the notificationId in the java side, the native downloadId is going + * to be deprecated. */ @CalledByNative - private void onDownloadUpdated(String url, String mimeType, String filename, - String path, long contentLength, int downloadId, int percentCompleted, + private void onDownloadUpdated(String url, String mimeType, String filename, String path, + long contentLength, int notificationId, String downloadGuid, int percentCompleted, long timeRemainingInMs, boolean hasUserGesture, boolean isPaused, boolean isResumable) { if (sDownloadNotificationService == null) return; DownloadInfo downloadInfo = new DownloadInfo.Builder() @@ -192,8 +200,8 @@ public class DownloadController { .setFilePath(path) .setContentLength(contentLength) .setDescription(filename) - .setDownloadId(downloadId) - .setHasDownloadId(true) + .setNotificationId(notificationId) + .setDownloadGuid(downloadGuid) .setPercentCompleted(percentCompleted) .setTimeRemainingInMillis(timeRemainingInMs) .setHasUserGesture(hasUserGesture) @@ -207,11 +215,10 @@ public class DownloadController { * Notifies the download delegate that a dangerous download started. */ @CalledByNative - private void onDangerousDownload(ContentViewCore view, String filename, - int downloadId) { + private void onDangerousDownload(ContentViewCore view, String filename, String downloadGuid) { ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromView(view); if (downloadDelegate != null) { - downloadDelegate.onDangerousDownload(filename, downloadId); + downloadDelegate.onDangerousDownload(filename, downloadGuid); } } diff --git a/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java b/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java index 39da754..f6297be 100644 --- a/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java +++ b/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java @@ -18,8 +18,8 @@ public final class DownloadInfo { private final String mReferer; private final String mOriginalUrl; private final long mContentLength; - private final boolean mHasDownloadId; - private final int mDownloadId; + private final int mNotificationId; + private final String mDownloadGuid; private final boolean mHasUserGesture; private final String mContentDisposition; private final boolean mIsGETRequest; @@ -39,8 +39,8 @@ public final class DownloadInfo { mReferer = builder.mReferer; mOriginalUrl = builder.mOriginalUrl; mContentLength = builder.mContentLength; - mHasDownloadId = builder.mHasDownloadId; - mDownloadId = builder.mDownloadId; + mNotificationId = builder.mNotificationId; + mDownloadGuid = builder.mDownloadGuid; mHasUserGesture = builder.mHasUserGesture; mIsGETRequest = builder.mIsGETRequest; mContentDisposition = builder.mContentDisposition; @@ -94,12 +94,12 @@ public final class DownloadInfo { return mIsGETRequest; } - public boolean hasDownloadId() { - return mHasDownloadId; + public int getNotificationId() { + return mNotificationId; } - public int getDownloadId() { - return mDownloadId; + public String getDownloadGuid() { + return mDownloadGuid; } public boolean hasUserGesture() { @@ -144,8 +144,8 @@ public final class DownloadInfo { private String mOriginalUrl; private long mContentLength; private boolean mIsGETRequest; - private boolean mHasDownloadId; - private int mDownloadId; + private int mNotificationId; + private String mDownloadGuid; private boolean mHasUserGesture; private String mContentDisposition; private int mPercentCompleted = -1; @@ -208,13 +208,13 @@ public final class DownloadInfo { return this; } - public Builder setHasDownloadId(boolean hasDownloadId) { - mHasDownloadId = hasDownloadId; + public Builder setNotificationId(int notificationId) { + mNotificationId = notificationId; return this; } - public Builder setDownloadId(int downloadId) { - mDownloadId = downloadId; + public Builder setDownloadGuid(String downloadGuid) { + mDownloadGuid = downloadGuid; return this; } @@ -270,8 +270,8 @@ public final class DownloadInfo { .setReferer(downloadInfo.getReferer()) .setOriginalUrl(downloadInfo.getOriginalUrl()) .setContentLength(downloadInfo.getContentLength()) - .setHasDownloadId(downloadInfo.hasDownloadId()) - .setDownloadId(downloadInfo.getDownloadId()) + .setNotificationId(downloadInfo.getNotificationId()) + .setDownloadGuid(downloadInfo.getDownloadGuid()) .setHasUserGesture(downloadInfo.hasUserGesture()) .setContentDisposition(downloadInfo.getContentDisposition()) .setIsGETRequest(downloadInfo.isGETRequest()) diff --git a/content/public/browser/android/download_controller_android.h b/content/public/browser/android/download_controller_android.h index e048afd..2ef664e 100644 --- a/content/public/browser/android/download_controller_android.h +++ b/content/public/browser/android/download_controller_android.h @@ -41,8 +41,9 @@ class CONTENT_EXPORT DownloadControllerAndroid : public DownloadItem::Observer { bool is_link, const std::string& extra_headers) = 0; // Called when a dangerous download item is verified or rejected. - virtual void DangerousDownloadValidated( - WebContents* web_contents, int download_id, bool accept) = 0; + virtual void DangerousDownloadValidated(WebContents* web_contents, + const std::string& download_guid, + bool accept) = 0; // Callback when user permission prompt finishes. Args: whether file access // permission is acquired. |