From a2b28b8c60fd0e5cbf5b01e3681a39f014247026 Mon Sep 17 00:00:00 2001 From: qinmin Date: Thu, 30 Jul 2015 10:47:34 -0700 Subject: [PATCH] Refactor the code to handle auto open after download completes If a pdf is downloaded by android DownloadManager, it could be opened after download completes. However, for downloads that are handled by chrome, we don't do that even if there are user gestures. In this change: 1. For download handled by Chrome, Move the download completion handling out of DownloadNotifier. The DownloadNotifier should only talk to notification manager, and should not talk to DownloadManager. 2. For all downloads(chrome or DownloadManager), use the same logic to determine if they should be auto opened BUG=478304 Review URL: https://codereview.chromium.org/1267593002 Cr-Commit-Position: refs/heads/master@{#341155} --- .../download/DownloadManagerService.java | 74 ++++++++++++++----- .../browser/download/DownloadNotifier.java | 4 +- .../download/SystemDownloadNotifier.java | 31 +------- .../download/DownloadManagerServiceTest.java | 3 +- .../download_controller_android_impl.cc | 7 +- .../content/browser/DownloadController.java | 7 +- 6 files changed, 69 insertions(+), 57 deletions(-) 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 67a6b0f02f52c..e4fc223e491b0 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 @@ -18,10 +18,10 @@ import android.os.Handler; import android.preference.PreferenceManager; import android.text.TextUtils; -import android.util.Log; import android.util.LongSparseArray; import android.widget.Toast; +import org.chromium.base.Log; import org.chromium.base.ThreadUtils; import org.chromium.base.VisibleForTesting; import org.chromium.base.annotations.SuppressFBWarnings; @@ -46,10 +46,11 @@ */ public class DownloadManagerService extends BroadcastReceiver implements DownloadController.DownloadNotificationService { - private static final String TAG = "DownloadNotificationService"; + private static final String TAG = "cr.DownloadService"; private static final String DOWNLOAD_NOTIFICATION_IDS = "DownloadNotificationIds"; private static final String DOWNLOAD_DIRECTORY = "Download"; 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; // Set will be more expensive to initialize, so use an ArrayList here. private static final List MIME_TYPES_TO_OPEN = new ArrayList(Arrays.asList( @@ -435,7 +436,8 @@ private boolean updateAllNotifications() { case COMPLETE: removeProgressNotificationForDownload(progress.mDownloadInfo .getDownloadId()); - ret = mDownloadNotifier.notifyDownloadSuccessful(progress.mDownloadInfo); + ret = ret && addCompletedDownload(progress.mDownloadInfo); + mDownloadNotifier.notifyDownloadSuccessful(progress.mDownloadInfo); broadcastDownloadSuccessful(progress.mDownloadInfo); break; case FAILED: @@ -453,6 +455,56 @@ private boolean updateAllNotifications() { return ret; } + /** + * Add a completed download into DownloadManager. + * + * @param downloadInfo Information of the downloaded file. + * @return true if the download is added to the DownloadManager, or false otherwise. + */ + private boolean addCompletedDownload(DownloadInfo downloadInfo) { + String mimeType = downloadInfo.getMimeType(); + if (TextUtils.isEmpty(mimeType)) mimeType = UNKNOWN_MIME_TYPE; + String description = downloadInfo.getDescription(); + if (TextUtils.isEmpty(description)) description = downloadInfo.getFileName(); + DownloadManager manager = + (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE); + try { + long downloadId = manager.addCompletedDownload( + downloadInfo.getFileName(), description, true, mimeType, + downloadInfo.getFilePath(), downloadInfo.getContentLength(), true); + if (shouldOpenAfterDownload(downloadInfo)) { + handleAutoOpenAfterDownload(downloadInfo, downloadId); + } + } catch (IllegalArgumentException e) { + // TODO(qinmin): Properly handle the case that we fail to add a completed + // download item to DownloadManager + Log.w(TAG, "Failed to add the download item to DownloadManager: " + e); + return false; + } + return true; + } + + /** + * Handle auto opennable files after download completes. + * + * @param info Information of the downloaded file. + * @param downloadId Download identifier issued by the android DownloadManager. + */ + private void handleAutoOpenAfterDownload(DownloadInfo info, long downloadId) { + if (OMADownloadHandler.OMA_DOWNLOAD_DESCRIPTOR_MIME.equalsIgnoreCase(info.getMimeType())) { + mOMADownloadHandler.handleOMADownload(info, downloadId); + return; + } + DownloadManager manager = + (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE); + Uri uri = manager.getUriForDownloadedFile(downloadId); + Intent launchIntent = new Intent(Intent.ACTION_VIEW); + launchIntent.setDataAndType(uri, manager.getMimeTypeForDownloadedFile(downloadId)); + launchIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK + | Intent.FLAG_GRANT_READ_URI_PERMISSION); + openIntent(mContext, launchIntent, true); + } + /** * Schedule an update if there is no update scheduled. */ @@ -529,10 +581,6 @@ protected void setOMADownloadHandler(OMADownloadHandler omaDownloadHandler) { mOMADownloadHandler = omaDownloadHandler; } - protected OMADownloadHandler getOMADownloadHandler() { - return mOMADownloadHandler; - } - @Override public void onReceive(Context context, Intent intent) { String action = intent.getAction(); @@ -556,17 +604,7 @@ public void onReceive(Context context, Intent intent) { switch (status) { case DownloadManager.STATUS_SUCCESSFUL: mPendingAutoOpenDownloads.remove(downloadId); - if (OMADownloadHandler.OMA_DOWNLOAD_DESCRIPTOR_MIME.equalsIgnoreCase( - info.getMimeType())) { - mOMADownloadHandler.handleOMADownload(info, downloadId); - break; - } - Uri uri = manager.getUriForDownloadedFile(downloadId); - Intent launchIntent = new Intent(Intent.ACTION_VIEW); - launchIntent.setDataAndType( - uri, manager.getMimeTypeForDownloadedFile(downloadId)); - launchIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); - openIntent(mContext, launchIntent, true); + handleAutoOpenAfterDownload(info, downloadId); break; case DownloadManager.STATUS_FAILED: mPendingAutoOpenDownloads.remove(downloadId); 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 1083b7047d5a2..053a730675bdb 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 @@ -13,10 +13,8 @@ public interface DownloadNotifier { /** * Add a download successful notification. * @param downloadInfo info about the successful download. - * @return true if the download is successfully added to the android DownloadManager, - * or false otherwise. */ - boolean notifyDownloadSuccessful(DownloadInfo downloadInfo); + void notifyDownloadSuccessful(DownloadInfo downloadInfo); /** * Add a download failed notification. 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 098da90705b68..2430d2dc0dd6b 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 @@ -4,13 +4,10 @@ package org.chromium.chrome.browser.download; -import android.app.DownloadManager; import android.app.Notification; import android.app.NotificationManager; import android.content.Context; import android.support.v4.app.NotificationCompat; -import android.text.TextUtils; -import android.util.Log; import org.chromium.chrome.R; @@ -25,12 +22,9 @@ * service to create download notifications. */ public class SystemDownloadNotifier implements DownloadNotifier { - private static final String UNKNOWN_MIME_TYPE = "application/unknown"; - private static final String LOGTAG = "SystemDownloadNotifier"; private static final String NOTIFICATION_NAMESPACE = "SystemDownloadNotifier"; private final NotificationManager mNotificationManager; - private final DownloadManager mDownloadManager; private final Context mApplicationContext; @@ -42,8 +36,6 @@ public SystemDownloadNotifier(Context context) { mApplicationContext = context.getApplicationContext(); mNotificationManager = (NotificationManager) mApplicationContext .getSystemService(Context.NOTIFICATION_SERVICE); - mDownloadManager = (DownloadManager) mApplicationContext - .getSystemService(Context.DOWNLOAD_SERVICE); } /** @@ -61,27 +53,8 @@ public void cancelNotification(int downloadId) { } @Override - public boolean notifyDownloadSuccessful(DownloadInfo downloadInfo) { - String mimeType = downloadInfo.getMimeType(); - if (TextUtils.isEmpty(mimeType)) mimeType = UNKNOWN_MIME_TYPE; - - try { - String description = downloadInfo.getDescription(); - if (TextUtils.isEmpty(description)) description = downloadInfo.getFileName(); - long downloadId = mDownloadManager.addCompletedDownload( - downloadInfo.getFileName(), description, true, mimeType, - downloadInfo.getFilePath(), downloadInfo.getContentLength(), true); - if (OMADownloadHandler.OMA_DOWNLOAD_DESCRIPTOR_MIME.equalsIgnoreCase(mimeType)) { - DownloadManagerService.getDownloadManagerService(mApplicationContext) - .getOMADownloadHandler().handleOMADownload(downloadInfo, downloadId); - } - } catch (IllegalArgumentException e) { - // TODO(qinmin): Properly handle the case that we fail to add a completed - // download item to DownloadManager - Log.w(LOGTAG, "Failed to add the download item to DownloadManager: " + e); - return false; - } - return true; + public void notifyDownloadSuccessful(DownloadInfo downloadInfo) { + // TODO(qinmin): figure out what needs to be done here. } @Override 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 44653fb4c54e2..bb7391d174af2 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 @@ -101,9 +101,8 @@ void assertCorrectExpectedCall(MethodID methodId, Object param) { } @Override - public boolean notifyDownloadSuccessful(DownloadInfo downloadInfo) { + public void notifyDownloadSuccessful(DownloadInfo downloadInfo) { assertCorrectExpectedCall(MethodID.DOWNLOAD_SUCCESSFUL, downloadInfo); - return true; } @Override diff --git a/content/browser/android/download_controller_android_impl.cc b/content/browser/android/download_controller_android_impl.cc index e571780efcc5c..5d21117faeb17 100644 --- a/content/browser/android/download_controller_android_impl.cc +++ b/content/browser/android/download_controller_android_impl.cc @@ -457,7 +457,8 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { env, GetJavaObject()->Controller(env).obj(), base::android::GetApplicationContext(), jurl.obj(), jmime_type.obj(), jfilename.obj(), jpath.obj(), item->GetReceivedBytes(), true, - item->GetId(), item->PercentComplete(), time_delta.InMilliseconds()); + item->GetId(), item->PercentComplete(), time_delta.InMilliseconds(), + item->HasUserGesture()); break; } case DownloadItem::COMPLETE: @@ -470,7 +471,7 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { env, GetJavaObject()->Controller(env).obj(), base::android::GetApplicationContext(), jurl.obj(), jmime_type.obj(), jfilename.obj(), jpath.obj(), item->GetReceivedBytes(), true, - item->GetId()); + item->GetId(), item->HasUserGesture()); break; case DownloadItem::CANCELLED: // TODO(shashishekhar): An interrupted download can be resumed. Android @@ -482,7 +483,7 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) { env, GetJavaObject()->Controller(env).obj(), base::android::GetApplicationContext(), jurl.obj(), jmime_type.obj(), jfilename.obj(), jpath.obj(), item->GetReceivedBytes(), false, - item->GetId()); + item->GetId(), item->HasUserGesture()); break; case DownloadItem::MAX_DOWNLOAD_STATE: NOTREACHED(); 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 5634f6f97c148..e16933fc8f011 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 @@ -108,7 +108,8 @@ public void onDownloadStarted(ContentViewCore view, String filename, String mime */ @CalledByNative public void onDownloadCompleted(Context context, String url, String mimeType, - String filename, String path, long contentLength, boolean successful, int downloadId) { + String filename, String path, long contentLength, boolean successful, int downloadId, + boolean hasUserGesture) { if (sDownloadNotificationService != null) { DownloadInfo downloadInfo = new DownloadInfo.Builder() .setUrl(url) @@ -120,6 +121,7 @@ public void onDownloadCompleted(Context context, String url, String mimeType, .setDescription(filename) .setDownloadId(downloadId) .setHasDownloadId(true) + .setHasUserGesture(hasUserGesture) .build(); sDownloadNotificationService.onDownloadCompleted(downloadInfo); } @@ -132,7 +134,7 @@ public void onDownloadCompleted(Context context, String url, String mimeType, @CalledByNative public void onDownloadUpdated(Context context, String url, String mimeType, String filename, String path, long contentLength, boolean successful, int downloadId, - int percentCompleted, long timeRemainingInMs) { + int percentCompleted, long timeRemainingInMs, boolean hasUserGesture) { if (sDownloadNotificationService != null) { DownloadInfo downloadInfo = new DownloadInfo.Builder() .setUrl(url) @@ -146,6 +148,7 @@ public void onDownloadUpdated(Context context, String url, String mimeType, .setHasDownloadId(true) .setPercentCompleted(percentCompleted) .setTimeRemainingInMillis(timeRemainingInMs) + .setHasUserGesture(hasUserGesture) .build(); sDownloadNotificationService.onDownloadUpdated(downloadInfo); }