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}

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<String> MIME_TYPES_TO_OPEN = new ArrayList<String>(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);

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.

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

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

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();

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);
         }