Fixing a crash when clicking download notification while Chrome is killed

When clicking on download notification, Chrome checks if the download file
can be opened by itself.
This calls into a native function.
However, if user kills Chrome, and then clicks the notification, this
generates a Broadcast to Chrome.
When BroadcastReceiver runs, native library is not loaded.
As a result, this generates a crash.
This CL solves the problem by performing the mime type checking when
download completes.
So that we don't need to check it when BroadcastReceiver runs

BUG=658012

Review-Url: https://codereview.chromium.org/2446643004
Cr-Commit-Position: refs/heads/master@{#427204}

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java

@@ -61,8 +61,10 @@ public class DownloadBroadcastReceiver extends BroadcastReceiver {
         } else {
             String downloadFilename = IntentUtils.safeGetStringExtra(
                     intent, DownloadNotificationService.EXTRA_DOWNLOAD_FILE_PATH);
+            boolean isSupportedMimeType =  IntentUtils.safeGetBooleanExtra(
+                    intent, DownloadNotificationService.EXTRA_IS_SUPPORTED_MIME_TYPE, false);
             Intent launchIntent = DownloadManagerService.getLaunchIntentFromDownloadId(
-                    context, downloadFilename, id);
+                    context, downloadFilename, id, isSupportedMimeType);
             if (!DownloadUtils.fireOpenIntentForDownload(context, launchIntent)) {
                 DownloadManagerService.openDownloadsPage(context);
             }

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java

@@ -38,8 +38,6 @@ public class DownloadManagerDelegate {
                 (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE);
         NotificationManagerCompat notificationManager = NotificationManagerCompat.from(mContext);
         boolean useSystemNotification = !notificationManager.areNotificationsEnabled();
-        String newMimeType =
-                ChromeDownloadDelegate.remapGenericMimeType(mimeType, originalUrl, fileName);
         if (Build.VERSION.SDK_INT > Build.VERSION_CODES.M) {
             Class<?> c = manager.getClass();
             try {
@@ -48,7 +46,7 @@ public class DownloadManagerDelegate {
                 Method method = c.getMethod("addCompletedDownload", args);
                 Uri originalUri = Uri.parse(originalUrl);
                 Uri refererUri = referer == null ? Uri.EMPTY : Uri.parse(referer);
-                return (Long) method.invoke(manager, fileName, description, true, newMimeType, path,
+                return (Long) method.invoke(manager, fileName, description, true, mimeType, path,
                         length, useSystemNotification, originalUri, refererUri);
             } catch (SecurityException e) {
                 Log.e(TAG, "Cannot access the needed method.");
@@ -60,7 +58,7 @@ public class DownloadManagerDelegate {
                 Log.e(TAG, "Error accessing the needed method.");
             }
         }
-        return manager.addCompletedDownload(fileName, description, true, newMimeType, path, length,
+        return manager.addCompletedDownload(fileName, description, true, mimeType, path, length,
                 useSystemNotification);
     }
 
@@ -149,7 +147,7 @@ public class DownloadManagerDelegate {
                         canResolve = DownloadManagerService.isOMADownloadDescription(
                                 mDownloadItem.getDownloadInfo())
                                 || DownloadManagerService.canResolveDownloadItem(
-                                        mContext, mDownloadItem);
+                                        mContext, mDownloadItem, false);
                     }
                 } else if (status == DownloadManager.STATUS_FAILED) {
                     downloadStatus = DownloadManagerService.DOWNLOAD_STATUS_FAILED;

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

@@ -141,6 +141,7 @@ public class DownloadManagerService extends BroadcastReceiver implements
         int mDownloadStatus;
         boolean mIsAutoResumable;
         boolean mIsUpdated;
+        boolean mIsSupportedMimeType;
 
         DownloadProgress(long startTimeInMillis, boolean canDownloadWhileMetered,
                 DownloadItem downloadItem, int downloadStatus) {
@@ -159,6 +160,7 @@ public class DownloadManagerService extends BroadcastReceiver implements
             mDownloadStatus = progress.mDownloadStatus;
             mIsAutoResumable = progress.mIsAutoResumable;
             mIsUpdated = progress.mIsUpdated;
+            mIsSupportedMimeType = progress.mIsSupportedMimeType;
         }
     }
 
@@ -288,11 +290,18 @@ public class DownloadManagerService extends BroadcastReceiver implements
     @Override
     public void onDownloadCompleted(final DownloadInfo downloadInfo) {
         int status = DOWNLOAD_STATUS_COMPLETE;
+        String mimeType = downloadInfo.getMimeType();
         if (downloadInfo.getContentLength() == 0) {
             status = DOWNLOAD_STATUS_FAILED;
+        } else {
+            String origMimeType = mimeType;
+            if (TextUtils.isEmpty(origMimeType)) origMimeType = UNKNOWN_MIME_TYPE;
+            mimeType = ChromeDownloadDelegate.remapGenericMimeType(
+                    origMimeType, downloadInfo.getOriginalUrl(), downloadInfo.getFileName());
         }
-
-        DownloadItem downloadItem = new DownloadItem(false, downloadInfo);
+        DownloadInfo newInfo =
+                DownloadInfo.Builder.fromDownloadInfo(downloadInfo).setMimeType(mimeType).build();
+        DownloadItem downloadItem = new DownloadItem(false, newInfo);
         updateDownloadProgress(downloadItem, status);
         scheduleUpdateIfNeeded();
     }
@@ -400,7 +409,7 @@ public class DownloadManagerService extends BroadcastReceiver implements
                             .setFileName(title)
                             .build();
                     mDownloadItem.setDownloadInfo(mDownloadInfo);
-                    canResolve = canResolveDownloadItem(mContext, mDownloadItem);
+                    canResolve = canResolveDownloadItem(mContext, mDownloadItem, false);
                 } else if (status == DownloadManager.STATUS_FAILED) {
                     mFailureReason = c.getInt(reasonIndex);
                     manager.remove(mDownloadItem.getSystemDownloadId());
@@ -542,10 +551,11 @@ public class DownloadManagerService extends BroadcastReceiver implements
                     boolean success = addCompletedDownload(item);
                     if (success) {
                         boolean canResolve = isOMADownloadDescription(info)
-                                || canResolveDownloadItem(mContext, item);
+                                || canResolveDownloadItem(
+                                        mContext, item, progress.mIsSupportedMimeType);
                         long systemDownloadId = item.getSystemDownloadId();
                         mDownloadNotifier.notifyDownloadSuccessful(
-                                info, systemDownloadId, canResolve);
+                                info, systemDownloadId, canResolve, progress.mIsSupportedMimeType);
                         broadcastDownloadSuccessful(info);
                     } else {
                         downloadItems.add(item);
@@ -588,17 +598,15 @@ public class DownloadManagerService extends BroadcastReceiver implements
      */
     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();
         if (TextUtils.isEmpty(description)) description = downloadInfo.getFileName();
         try {
             // Exceptions can be thrown when calling this, although it is not
             // documented on Android SDK page.
             long downloadId = mDownloadManagerDelegate.addCompletedDownload(
-                    downloadInfo.getFileName(), description, mimeType, downloadInfo.getFilePath(),
-                    downloadInfo.getContentLength(), downloadInfo.getOriginalUrl(),
-                    downloadInfo.getReferer());
+                    downloadInfo.getFileName(), description, downloadInfo.getMimeType(),
+                    downloadInfo.getFilePath(), downloadInfo.getContentLength(),
+                    downloadInfo.getOriginalUrl(), downloadInfo.getReferer());
             downloadItem.setSystemDownloadId(downloadId);
             return true;
         } catch (RuntimeException e) {
@@ -689,6 +697,8 @@ public class DownloadManagerService extends BroadcastReceiver implements
      * @param downloadStatus Status of the download.
      */
     private void updateDownloadProgress(DownloadItem downloadItem, int downloadStatus) {
+        boolean isSupportedMimeType = (downloadStatus == DOWNLOAD_STATUS_COMPLETE)
+                ? isSupportedMimeType(downloadItem.getDownloadInfo().getMimeType()) : false;
         String id = downloadItem.getId();
         DownloadProgress progress = mDownloadProgressMap.get(id);
         if (progress == null) {
@@ -697,6 +707,7 @@ public class DownloadManagerService extends BroadcastReceiver implements
                 progress = new DownloadProgress(
                         startTime, isActiveNetworkMetered(mContext), downloadItem, downloadStatus);
                 progress.mIsUpdated = true;
+                progress.mIsSupportedMimeType = isSupportedMimeType;
                 mDownloadProgressMap.put(id, progress);
                 if (getUmaStatsEntry(downloadItem.getId()) == null) {
                     addUmaStatsEntry(new DownloadUmaStatsEntry(
@@ -710,6 +721,7 @@ public class DownloadManagerService extends BroadcastReceiver implements
         progress.mDownloadItem = downloadItem;
         progress.mIsUpdated = true;
         progress.mIsAutoResumable = mAutoResumableDownloadIds.contains(id);
+        progress.mIsSupportedMimeType = isSupportedMimeType;
         DownloadUmaStatsEntry entry;
         switch (downloadStatus) {
             case DOWNLOAD_STATUS_COMPLETE:
@@ -951,10 +963,12 @@ public class DownloadManagerService extends BroadcastReceiver implements
      * @param context    Context of the app.
      * @param filePath   Path to the file.
      * @param downloadId ID of the download item in DownloadManager.
+     * @param isSupportedMimeType Whether the MIME type is supported by browser.
      * @return the intent to launch for the given download item.
      */
     static Intent getLaunchIntentFromDownloadId(
-            Context context, @Nullable String filePath, long downloadId) {
+            Context context, @Nullable String filePath, long downloadId,
+            boolean isSupportedMimeType) {
         assert !ThreadUtils.runningOnUiThread();
         DownloadManager manager =
                 (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE);
@@ -962,7 +976,7 @@ public class DownloadManagerService extends BroadcastReceiver implements
         if (contentUri == null) return null;
 
         String mimeType = manager.getMimeTypeForDownloadedFile(downloadId);
-        if (isSupportedMimeType(mimeType)) {
+        if (isSupportedMimeType) {
             // Redirect the user to an internal media viewer.  The file path is necessary to show
             // the real file path to the user instead of a content:// download ID.
             Uri fileUri = contentUri;
@@ -978,13 +992,15 @@ public class DownloadManagerService extends BroadcastReceiver implements
      *
      * @param context Context of the app.
      * @param download A download item.
+     * @param isSupportedMimeType Whether the MIME type is supported by browser.
      * @return true if the download item can be resolved, or false otherwise.
      */
-    static boolean canResolveDownloadItem(Context context, DownloadItem download) {
+    static boolean canResolveDownloadItem(Context context, DownloadItem download,
+            boolean isSupportedMimeType) {
         assert !ThreadUtils.runningOnUiThread();
         Intent intent = getLaunchIntentFromDownloadId(
                 context, download.getDownloadInfo().getFilePath(),
-                download.getSystemDownloadId());
+                download.getSystemDownloadId(), isSupportedMimeType);
         return (intent == null) ? false : ExternalNavigationDelegateImpl.resolveIntent(
                 context, intent, true);
     }
@@ -997,11 +1013,12 @@ public class DownloadManagerService extends BroadcastReceiver implements
      * @param downloadId   ID of the download item in DownloadManager.
      */
     protected void openDownloadedContent(final DownloadInfo downloadInfo, final long downloadId) {
+        final boolean isSupportedMimeType = isSupportedMimeType(downloadInfo.getMimeType());
         new AsyncTask<Void, Void, Intent>() {
             @Override
             public Intent doInBackground(Void... params) {
                 return getLaunchIntentFromDownloadId(
-                        mContext, downloadInfo.getFilePath(), downloadId);
+                        mContext, downloadInfo.getFilePath(), downloadId, isSupportedMimeType);
             }
 
             @Override

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java

@@ -60,6 +60,7 @@ public class DownloadNotificationService extends Service {
     static final String EXTRA_NOTIFICATION_DISMISSED = "NotificationDismissed";
     static final String EXTRA_DOWNLOAD_IS_OFF_THE_RECORD = "DownloadIsOffTheRecord";
     static final String EXTRA_DOWNLOAD_IS_OFFLINE_PAGE = "DownloadIsOfflinePage";
+    static final String EXTRA_IS_SUPPORTED_MIME_TYPE = "IsSupportedMimeType";
     static final String ACTION_DOWNLOAD_CANCEL =
             "org.chromium.chrome.browser.download.DOWNLOAD_CANCEL";
     static final String ACTION_DOWNLOAD_PAUSE =
@@ -339,12 +340,14 @@ public class DownloadNotificationService extends Service {
      * @param filePath Full path to the download.
      * @param fileName Filename of the download.
      * @param systemDownloadId Download ID assigned by system DownloadManager.
+     * @param isOfflinePage Whether the download is for offline page.
+     * @param isSupportedMimeType Whether the MIME type can be viewed inside browser.
      * @return ID of the successful download notification. Used for removing the notification when
      *         user click on the snackbar.
      */
     public int notifyDownloadSuccessful(
             String downloadGuid, String filePath, String fileName, long systemDownloadId,
-            boolean isOfflinePage) {
+            boolean isOfflinePage, boolean isSupportedMimeType) {
         int notificationId = getNotificationId(downloadGuid);
         NotificationCompat.Builder builder = buildNotification(
                 R.drawable.offline_pin, fileName,
@@ -360,6 +363,7 @@ public class DownloadNotificationService extends Service {
             long[] idArray = {systemDownloadId};
             intent.putExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS, idArray);
             intent.putExtra(EXTRA_DOWNLOAD_FILE_PATH, filePath);
+            intent.putExtra(EXTRA_IS_SUPPORTED_MIME_TYPE, isSupportedMimeType);
         }
         intent.setComponent(component);
         builder.setContentIntent(PendingIntent.getBroadcast(

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java

@@ -13,9 +13,10 @@ public interface DownloadNotifier {
      * @param downloadInfo info about the successful download.
      * @param systemDownloadId The system download ID assigned to the download.
      * @param canResolve Whether the download can be resolved to any activity.
+     * @param isSupportedMimeType Whether the MIME type can be viewed inside browser.
      */
     void notifyDownloadSuccessful(DownloadInfo downloadInfo, long systemDownloadId,
-            boolean canResolve);
+            boolean canResolve, boolean isSupportedMimeType);
 
     /**
      * Add a download failed notification.

chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java

@@ -55,6 +55,7 @@ public class SystemDownloadNotifier implements DownloadNotifier {
         public boolean canDownloadWhileMetered;
         public boolean canResolve;
         public long systemDownloadId;
+        public boolean isSupportedMimeType;
 
         public PendingNotificationInfo(int type, DownloadInfo downloadInfo) {
             this.type = type;
@@ -178,11 +179,12 @@ public class SystemDownloadNotifier implements DownloadNotifier {
 
     @Override
     public void notifyDownloadSuccessful(DownloadInfo downloadInfo, long systemDownloadId,
-            boolean canResolve) {
+            boolean canResolve, boolean isSupportedMimeType) {
         PendingNotificationInfo info =
                 new PendingNotificationInfo(DOWNLOAD_NOTIFICATION_TYPE_SUCCESS, downloadInfo);
         info.canResolve = canResolve;
         info.systemDownloadId = systemDownloadId;
+        info.isSupportedMimeType = isSupportedMimeType;
         updateDownloadNotification(info);
     }
 
@@ -281,7 +283,8 @@ public class SystemDownloadNotifier implements DownloadNotifier {
                     case DOWNLOAD_NOTIFICATION_TYPE_SUCCESS:
                         final int notificationId = mBoundService.notifyDownloadSuccessful(
                                 info.getDownloadGuid(), info.getFilePath(), info.getFileName(),
-                                notificationInfo.systemDownloadId, info.isOfflinePage());
+                                notificationInfo.systemDownloadId, info.isOfflinePage(),
+                                notificationInfo.isSupportedMimeType);
                         onSuccessNotificationShown(notificationInfo, notificationId);
                         stopServiceIfNeeded();
                         break;

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageNotificationBridge.java

@@ -39,7 +39,7 @@ public class OfflinePageNotificationBridge {
                                             .setIsOffTheRecord(false)
                                             .build();
 
-        notifier.notifyDownloadSuccessful(downloadInfo, -1, false);
+        notifier.notifyDownloadSuccessful(downloadInfo, -1, false, true);
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java

@@ -122,48 +122,53 @@ public class DownloadManagerServiceTest extends NativeLibraryTestBase {
             return new Pair<MethodID, Object>(methodId, param);
         }
 
-        void assertCorrectExpectedCall(MethodID methodId, Object param) {
+        void assertCorrectExpectedCall(MethodID methodId, Object param, boolean matchParams) {
             Log.w("MockDownloadNotifier", "Called: " + methodId);
             assertFalse("Unexpected call:, no call expected, but got: " + methodId,
                     mExpectedCalls.isEmpty());
             Pair<MethodID, Object> actual = getMethodSignature(methodId, param);
             Pair<MethodID, Object> expected = mExpectedCalls.poll();
             assertEquals("Unexpected call", expected.first, actual.first);
-            assertTrue("Incorrect arguments", MatchHelper.macthes(expected.second, actual.second));
+            if (matchParams) {
+                assertTrue(
+                        "Incorrect arguments", MatchHelper.macthes(expected.second, actual.second));
+            }
         }
 
         @Override
         public void notifyDownloadSuccessful(DownloadInfo downloadInfo,
-                long systemDownloadId, boolean canResolve) {
-            assertCorrectExpectedCall(MethodID.DOWNLOAD_SUCCESSFUL, downloadInfo);
-            super.notifyDownloadSuccessful(downloadInfo, systemDownloadId, canResolve);
+                long systemDownloadId, boolean canResolve, boolean isSupportedMimeType) {
+            assertCorrectExpectedCall(MethodID.DOWNLOAD_SUCCESSFUL, downloadInfo, false);
+            assertEquals("application/unknown", downloadInfo.getMimeType());
+            super.notifyDownloadSuccessful(downloadInfo, systemDownloadId, canResolve,
+                    isSupportedMimeType);
         }
 
         @Override
         public void notifyDownloadFailed(DownloadInfo downloadInfo) {
-            assertCorrectExpectedCall(MethodID.DOWNLOAD_FAILED, downloadInfo);
+            assertCorrectExpectedCall(MethodID.DOWNLOAD_FAILED, downloadInfo, true);
 
         }
 
         @Override
         public void notifyDownloadProgress(
                 DownloadInfo downloadInfo, long startTime, boolean canDownloadWhileMetered) {
-            assertCorrectExpectedCall(MethodID.DOWNLOAD_PROGRESS, downloadInfo);
+            assertCorrectExpectedCall(MethodID.DOWNLOAD_PROGRESS, downloadInfo, true);
         }
 
         @Override
         public void notifyDownloadPaused(DownloadInfo downloadInfo) {
-            assertCorrectExpectedCall(MethodID.DOWNLOAD_PAUSED, downloadInfo);
+            assertCorrectExpectedCall(MethodID.DOWNLOAD_PAUSED, downloadInfo, true);
         }
 
         @Override
         public void notifyDownloadInterrupted(DownloadInfo downloadInfo, boolean isAutoResumable) {
-            assertCorrectExpectedCall(MethodID.DOWNLOAD_INTERRUPTED, downloadInfo);
+            assertCorrectExpectedCall(MethodID.DOWNLOAD_INTERRUPTED, downloadInfo, true);
         }
 
         @Override
         public void notifyDownloadCanceled(String downloadGuid) {
-            assertCorrectExpectedCall(MethodID.CANCEL_DOWNLOAD_ID, downloadGuid);
+            assertCorrectExpectedCall(MethodID.CANCEL_DOWNLOAD_ID, downloadGuid, true);
         }
 
         @Override

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java

@@ -246,7 +246,7 @@ public class DownloadNotificationServiceTest extends
                 sharedPrefs, DownloadNotificationService.PENDING_DOWNLOAD_NOTIFICATIONS);
         assertEquals(3, entries.size());
 
-        service.notifyDownloadSuccessful(guid1, "/path/to/success", "success", 100L, false);
+        service.notifyDownloadSuccessful(guid1, "/path/to/success", "success", 100L, false, false);
         entries = DownloadManagerService.getStoredDownloadInfo(
                 sharedPrefs, DownloadNotificationService.PENDING_DOWNLOAD_NOTIFICATIONS);
         assertEquals(2, entries.size());
@@ -272,7 +272,7 @@ public class DownloadNotificationServiceTest extends
         startNotificationService();
         DownloadNotificationService service = bindNotificationService();
         String guid = UUID.randomUUID().toString();
-        service.notifyDownloadSuccessful(guid, "/path/to/test", "test", 100L, false);
+        service.notifyDownloadSuccessful(guid, "/path/to/test", "test", 100L, false, false);
         assertEquals(1, getService().getNotificationIds().size());
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java

@@ -102,7 +102,7 @@ public class SystemDownloadNotifierTest extends InstrumentationTestCase {
 
         mDownloadNotifier.notifyDownloadFailed(info);
         assertTrue(mDownloadNotifier.mStarted);
-        mDownloadNotifier.notifyDownloadSuccessful(info2, 100L, true);
+        mDownloadNotifier.notifyDownloadSuccessful(info2, 100L, true, false);
         assertFalse(mDownloadNotifier.mStarted);
     }
 }