Issue metadata
Sign in to add a comment
|
Chrome crash when clicking download successful notification |
|||||||||||||||||||||||||||
Issue descriptionNot sure if this goes into M55, but this happens on trunk Steps to reproduce: (1) download something (2) when download finishes, kill Chrome by swiping it away from recents (3) click download notification Expected result: Chrome opens the download if possible, or launches an intent to open it Actual result: See an chrome crashed warning dialog The problem happens because when DownloadBroadcastReceiver handles the notification click event, chrome goes to the native side to check if the Mimetype can be handled. However, if Chrome is killed, the native DownloadManagerService is not being initialized, and thus causing a crash
,
Oct 21 2016
,
Oct 21 2016
I can repro on latest Chrome Beta 55.0.2883.18 so yeah, this is a beta blocker. This needs to be fixed and merged back to branch 2883 by next Tuesday at 5 PM, let's hustle on a fix please.
,
Oct 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff25b0171fdd6663cc16000071c8d65c3cde1cb0 commit ff25b0171fdd6663cc16000071c8d65c3cde1cb0 Author: qinmin <qinmin@chromium.org> Date: Tue Oct 25 01:08:52 2016 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} [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageNotificationBridge.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java [modify] https://crrev.com/ff25b0171fdd6663cc16000071c8d65c3cde1cb0/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java
,
Oct 25 2016
,
Oct 25 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Oct 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc commit e92cfc834916b71c7e7dd8613d5fd2ba7da706bc Author: Min Qin <qinmin@chromium.org> Date: Tue Oct 25 17:05:47 2016 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 TBR=dfalcantara@chromium.org BUG= 658012 Review-Url: https://codereview.chromium.org/2446643004 Cr-Commit-Position: refs/heads/master@{#427204} (cherry picked from commit ff25b0171fdd6663cc16000071c8d65c3cde1cb0) Review URL: https://codereview.chromium.org/2444383002 . Cr-Commit-Position: refs/branch-heads/2883@{#283} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageNotificationBridge.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java
,
Oct 25 2016
Should be fixed, marking as such, reopen if further work is required.
,
Oct 25 2016
,
Oct 26 2016
This issue is fixed in current M55 build 55.0.2883.28
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc commit e92cfc834916b71c7e7dd8613d5fd2ba7da706bc Author: Min Qin <qinmin@chromium.org> Date: Tue Oct 25 17:05:47 2016 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 TBR=dfalcantara@chromium.org BUG= 658012 Review-Url: https://codereview.chromium.org/2446643004 Cr-Commit-Position: refs/heads/master@{#427204} (cherry picked from commit ff25b0171fdd6663cc16000071c8d65c3cde1cb0) Review URL: https://codereview.chromium.org/2444383002 . Cr-Commit-Position: refs/branch-heads/2883@{#283} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageNotificationBridge.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationServiceTest.java [modify] https://crrev.com/e92cfc834916b71c7e7dd8613d5fd2ba7da706bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java
,
Oct 28 2016
,
Nov 4 2016
[Automated comment] removing mislabelled merge-merged-2840
,
Nov 4 2016
[Automated comment] removing mislabelled merge-merged-2840
,
Jan 24 2017
It's reproducible on M 55.0.2883.91. Could you check this issue again?
,
Jan 24 2017
#10 mentioned that this is fixed, so I think #15 is something new. Do you have the logcat or stacktrace for #15?
,
Jan 30 2017
This is reproduced on M 55.0.2883.91 in Android 7.0, but not in Android 6.0. I think this is related to the following. https://developer.android.com/about/versions/nougat/android-7.0-changes.html#sharing-files I attach the logcat. Could you confirm it?
,
Jan 30 2017
So looks like this is checking file URI sharing between apps. Dan, any idea how file URI sharing works in N? 01-30 14:54:25.571 E/AndroidRuntime( 9522): java.lang.RuntimeException: Unable to start receiver org.chromium.chrome.browser.download.DownloadBroadcastReceiver: android.os.FileUriExposedException: file:///storage/emulated/0/Download/white-house-says-deliberately-omitted-194423484%20(1).html exposed beyond app through ClipData.Item.getUri() 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.ActivityThread.handleReceiver(ActivityThread.java:3018) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.ActivityThread.-wrap18(ActivityThread.java) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1544) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.os.Handler.dispatchMessage(Handler.java:102) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.os.Looper.loop(Looper.java:154) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.ActivityThread.main(ActivityThread.java:6077) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at java.lang.reflect.Method.invoke(Native Method) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755) 01-30 14:54:25.571 E/AndroidRuntime( 9522): Caused by: android.os.FileUriExposedException: file:///storage/emulated/0/Download/white-house-says-deliberately-omitted-194423484%20(1).html exposed beyond app through ClipData.Item.getUri() 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.os.StrictMode.onFileUriExposed(StrictMode.java:1799) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.net.Uri.checkFileUriExposed(Uri.java:2346) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.content.ClipData.prepareToLeaveProcess(ClipData.java:832) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.content.Intent.prepareToLeaveProcess(Intent.java:8909) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.content.Intent.prepareToLeaveProcess(Intent.java:8894) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.PendingIntent.getActivity(PendingIntent.java:340) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.PendingIntent.getActivity(PendingIntent.java:302) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at org.chromium.chrome.browser.download.DownloadUtils.getMediaViewerIntentForDownloadItem(DownloadUtils.java:378) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at org.chromium.chrome.browser.download.DownloadManagerService.getLaunchIntentFromDownloadId(DownloadManagerService.java:1000) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at org.chromium.chrome.browser.download.DownloadBroadcastReceiver.onReceive(DownloadBroadcastReceiver.java:1066) 01-30 14:54:25.571 E/AndroidRuntime( 9522): at android.app.ActivityThread.handleReceiver(ActivityThread.java:3011) 01-30 14:54:25.571 E/AndroidRuntime( 9522): ... 8 more
,
Jan 30 2017
I thought that was your thing? Theresa, do you have any idea why these URIs are problematic? Do you think we could just use the content URI in this case?
,
Jan 30 2017
Here's the Android documentation on that exception: https://developer.android.com/reference/android/os/FileUriExposedException.html The exception that is thrown when an application exposes a file:// Uri to another app. This exposure is discouraged since the receiving app may not have access to the shared path. For example, the receiving app may not have requested the READ_EXTERNAL_STORAGE runtime permission, or the platform may be sharing the Uri across user profile boundaries. Instead, apps should use content:// Uris so the platform can extend temporary permission for the receiving app to access the resource. This is only thrown for applications targeting N or higher. Applications targeting earlier SDK versions are allowed to share file:// Uri, but it's strongly discouraged. Based on that, I would expect this exception to be thrown when tapping on the "Open with..." in the CCT used to display downloaded items (although I haven't been able to reproduce it there). It seems odd that it would be thrown when creating a media viewer intent, however, because we should be staying within the same app. We can probably use the content:// URIs, but we'd need to test to make sure everything still works as expected. Issue 683881 discusses this a bit.
,
Jan 30 2017
yasutaka.kaneda.nf@kyocera.jp or seiyon.park@gmail.com - can you please confirm that the repro steps are the same as in the original bug report?
,
Jan 31 2017
The repro steps are the same as in the original bug report. I attach the video.
,
Jan 31 2017
,
Jan 31 2017
yasutaka.kaneda.nf@kyocera.jp, thank you for the video and confirmation of the repro steps. I was able to reproduce this on 55.0.2883.91 and 57.0.2987.9 on my Pixel running N. Min/Dan - should we re-open this bug as an M57 blocker (it's a little late for 56 unfortunately) or should we file a new one? I think the issue may be that the Chrome process is no longer running when the download notification is tapped, causing the exception. If Chrome is still running when the notification is tapped, this issue does not occur.
,
Jan 31 2017
,
Jan 31 2017
I'm confused about why the process being gone would make a difference, but reopened :/
,
Jan 31 2017
I'm seeing the exception thrown on an O device but it's not crashing with trunk/M58.
,
Jan 31 2017
Min: even if I address the crash, the DownloadBroadcastReceiver still runs tasks on the UI thread when it probably shouldn't, causing StrictMode violations to occur.
,
Feb 1 2017
Yeah, this doesn't seem to be crashing for me on 58 with N. Hit it with M56 on the same device, though, so I wonder if something landed to mitigate it.
,
Feb 2 2017
I can still reproduce on the most recent Canary, 58.0.2998.3, but not on a local ToT build.
,
Feb 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ef71dc0ec1f189d366e294f644ceeacbc7dab48a commit ef71dc0ec1f189d366e294f644ceeacbc7dab48a Author: dfalcantara <dfalcantara@chromium.org> Date: Thu Feb 02 19:40:35 2017 [Downloads] Use content:// URIs in more places * Use content:// URIs in more places instead of file:// URIs. This avoids problems with Android N+ complaining about passing around file:// URIs. * Allow Intents for Media Viewer Custom Tabs to have an additional URI that can override the one passed along via the Intent's data. The file:// URI is necessary to deal with Custom Tabs displaying "content:" and a database ID instead of the filename and its location. BUG= 658012 Review-Url: https://codereview.chromium.org/2671573002 Cr-Commit-Position: refs/heads/master@{#447821} [modify] https://crrev.com/ef71dc0ec1f189d366e294f644ceeacbc7dab48a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java [modify] https://crrev.com/ef71dc0ec1f189d366e294f644ceeacbc7dab48a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java [modify] https://crrev.com/ef71dc0ec1f189d366e294f644ceeacbc7dab48a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
,
Feb 2 2017
,
Feb 2 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9e65e42e3965f2b86054c35c381588f6a339403 commit d9e65e42e3965f2b86054c35c381588f6a339403 Author: dfalcantara@chromium.org <dfalcantara@chromium.org> Date: Thu Feb 02 23:04:45 2017 [Downloads] Use content:// URIs in more places * Use content:// URIs in more places instead of file:// URIs. This avoids problems with Android N+ complaining about passing around file:// URIs. * Allow Intents for Media Viewer Custom Tabs to have an additional URI that can override the one passed along via the Intent's data. The file:// URI is necessary to deal with Custom Tabs displaying "content:" and a database ID instead of the filename and its location. BUG= 658012 TBR=qinmin,twellington,yusufo Original-Review-Url: https://codereview.chromium.org/2671573002 Original-Cr-Commit-Position: refs/heads/master@{#447821} Review-Url: https://codereview.chromium.org/2669243007 . Cr-Commit-Position: refs/branch-heads/2987@{#285} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/d9e65e42e3965f2b86054c35c381588f6a339403/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java [modify] https://crrev.com/d9e65e42e3965f2b86054c35c381588f6a339403/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java [modify] https://crrev.com/d9e65e42e3965f2b86054c35c381588f6a339403/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
,
Feb 2 2017
Assigning back to Min to look into the StrictMode violations.
,
Feb 14 2017
Not see this crash on recent builds
,
Feb 14 2017
,
Apr 26 2017
|
||||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||||
Comment 1 by krav...@chromium.org
, Oct 21 2016