Commit 81fb77eb authored by qinmin's avatar qinmin Committed by Commit bot

Fix an issue that notification is not posted if download is interrupted on start

If a download is interrupted on start, notification is not posted.
However, the download will show up in download home.
This CL fixes the issue by allowing interrupted downloads to post new notifications.

BUG=714798

Review-Url: https://codereview.chromium.org/2848723004
Cr-Commit-Position: refs/heads/master@{#468355}
parent c418f3fa
...@@ -770,7 +770,9 @@ public class DownloadManagerService extends BroadcastReceiver implements ...@@ -770,7 +770,9 @@ public class DownloadManagerService extends BroadcastReceiver implements
DownloadUmaStatsEntry entry = getUmaStatsEntry(downloadItem.getId()); DownloadUmaStatsEntry entry = getUmaStatsEntry(downloadItem.getId());
if (entry == null) { if (entry == null) {
addUmaStatsEntry(new DownloadUmaStatsEntry( addUmaStatsEntry(new DownloadUmaStatsEntry(
downloadItem.getId(), startTime, 0, false, false, bytesReceived, 0)); downloadItem.getId(), startTime,
downloadStatus == DOWNLOAD_STATUS_INTERRUPTED ? 1 : 0, false, false,
bytesReceived, 0));
} else if (updateBytesReceived(entry, bytesReceived)) { } else if (updateBytesReceived(entry, bytesReceived)) {
storeUmaEntries(); storeUmaEntries();
} }
......
...@@ -576,7 +576,8 @@ public class DownloadNotificationService extends Service { ...@@ -576,7 +576,8 @@ public class DownloadNotificationService extends Service {
// Move all regular downloads to pending. Don't propagate the pause because // Move all regular downloads to pending. Don't propagate the pause because
// if native is still working and it triggers an update, then the service will be // if native is still working and it triggers an update, then the service will be
// restarted. // restarted.
notifyDownloadPaused(entry.id, !entry.isOffTheRecord, true, entry.isTransient, null); notifyDownloadPaused(entry.id, entry.fileName, !entry.isOffTheRecord, true,
entry.isOffTheRecord, entry.isTransient, null);
} }
} }
...@@ -897,26 +898,28 @@ public class DownloadNotificationService extends Service { ...@@ -897,26 +898,28 @@ public class DownloadNotificationService extends Service {
/** /**
* Change a download notification to paused state. * Change a download notification to paused state.
* @param id The {@link ContentId} of the download. * @param id The {@link ContentId} of the download.
* @param fileName File name of the download.
* @param isResumable Whether download can be resumed. * @param isResumable Whether download can be resumed.
* @param isAutoResumable whether download is can be resumed automatically. * @param isAutoResumable Whether download is can be resumed automatically.
* @param isOffTheRecord Whether the download is off the record.
* @param isTransient Whether or not clicking on the download should launch downloads home. * @param isTransient Whether or not clicking on the download should launch downloads home.
* @param icon A {@link Bitmap} to be used as the large icon for display. * @param icon A {@link Bitmap} to be used as the large icon for display.
*/ */
public void notifyDownloadPaused(ContentId id, boolean isResumable, boolean isAutoResumable, public void notifyDownloadPaused(ContentId id, String fileName, boolean isResumable,
boolean isTransient, Bitmap icon) { boolean isAutoResumable, boolean isOffTheRecord, boolean isTransient, Bitmap icon) {
DownloadSharedPreferenceEntry entry = DownloadSharedPreferenceEntry entry =
mDownloadSharedPreferenceHelper.getDownloadSharedPreferenceEntry(id); mDownloadSharedPreferenceHelper.getDownloadSharedPreferenceEntry(id);
if (entry == null) return;
if (!isResumable) { if (!isResumable) {
notifyDownloadFailed(id, entry.fileName, icon); notifyDownloadFailed(id, fileName, icon);
return; return;
} }
// If download is already paused, do nothing. // If download is already paused, do nothing.
if (!entry.isAutoResumable) return; if (entry != null && !entry.isAutoResumable) return;
boolean canDownloadWhileMetered = entry == null ? false : entry.canDownloadWhileMetered;
// If download is interrupted due to network disconnection, show download pending state. // If download is interrupted due to network disconnection, show download pending state.
if (isAutoResumable) { if (isAutoResumable) {
notifyDownloadPending(id, entry.fileName, entry.isOffTheRecord, notifyDownloadPending(id, fileName, isOffTheRecord, canDownloadWhileMetered,
entry.canDownloadWhileMetered, isTransient, icon); isTransient, icon);
stopTrackingInProgressDownload(id, true); stopTrackingInProgressDownload(id, true);
return; return;
} }
...@@ -924,38 +927,37 @@ public class DownloadNotificationService extends Service { ...@@ -924,38 +927,37 @@ public class DownloadNotificationService extends Service {
String contentText = mContext.getResources().getString( String contentText = mContext.getResources().getString(
R.string.download_notification_paused); R.string.download_notification_paused);
ChromeNotificationBuilder builder = ChromeNotificationBuilder builder =
buildNotification(R.drawable.ic_download_pause, entry.fileName, contentText); buildNotification(R.drawable.ic_download_pause, fileName, contentText);
int notificationId = entry == null ? getNotificationId(id) : entry.notificationId;
if (!isTransient) { if (!isTransient) {
// Clicking on an in-progress download sends the user to see all their downloads. // Clicking on an in-progress download sends the user to see all their downloads.
Intent downloadHomeIntent = buildActionIntent( Intent downloadHomeIntent = buildActionIntent(
mContext, DownloadManager.ACTION_NOTIFICATION_CLICKED, null, false); mContext, DownloadManager.ACTION_NOTIFICATION_CLICKED, null, false);
builder.setContentIntent(PendingIntent.getBroadcast(mContext, entry.notificationId, builder.setContentIntent(PendingIntent.getBroadcast(mContext, notificationId,
downloadHomeIntent, PendingIntent.FLAG_UPDATE_CURRENT)); downloadHomeIntent, PendingIntent.FLAG_UPDATE_CURRENT));
} }
builder.setAutoCancel(false); builder.setAutoCancel(false);
if (icon != null) builder.setLargeIcon(icon); if (icon != null) builder.setLargeIcon(icon);
Intent resumeIntent = Intent resumeIntent =
buildActionIntent(mContext, ACTION_DOWNLOAD_RESUME, id, entry.isOffTheRecord); buildActionIntent(mContext, ACTION_DOWNLOAD_RESUME, id, isOffTheRecord);
builder.addAction(R.drawable.ic_file_download_white_24dp, builder.addAction(R.drawable.ic_file_download_white_24dp,
mContext.getResources().getString(R.string.download_notification_resume_button), mContext.getResources().getString(R.string.download_notification_resume_button),
buildPendingIntent(resumeIntent, entry.notificationId)); buildPendingIntent(resumeIntent, notificationId));
Intent cancelIntent = Intent cancelIntent =
buildActionIntent(mContext, ACTION_DOWNLOAD_CANCEL, id, entry.isOffTheRecord); buildActionIntent(mContext, ACTION_DOWNLOAD_CANCEL, id, isOffTheRecord);
builder.addAction(R.drawable.btn_close_white, builder.addAction(R.drawable.btn_close_white,
mContext.getResources().getString(R.string.download_notification_cancel_button), mContext.getResources().getString(R.string.download_notification_cancel_button),
buildPendingIntent(cancelIntent, entry.notificationId)); buildPendingIntent(cancelIntent, notificationId));
PendingIntent deleteIntent = isTransient PendingIntent deleteIntent = isTransient
? buildPendingIntent(cancelIntent, entry.notificationId) ? buildPendingIntent(cancelIntent, notificationId)
: buildSummaryIconIntent(entry.notificationId); : buildSummaryIconIntent(notificationId);
builder.setDeleteIntent(deleteIntent); builder.setDeleteIntent(deleteIntent);
updateNotification(entry.notificationId, builder.build(), id, updateNotification(notificationId, builder.build(), id,
new DownloadSharedPreferenceEntry(id, entry.notificationId, entry.isOffTheRecord, new DownloadSharedPreferenceEntry(id, notificationId, isOffTheRecord,
entry.canDownloadWhileMetered, entry.fileName, isAutoResumable, canDownloadWhileMetered, fileName, isAutoResumable, isTransient));
isTransient));
stopTrackingInProgressDownload(id, true); stopTrackingInProgressDownload(id, true);
} }
...@@ -1170,7 +1172,8 @@ public class DownloadNotificationService extends Service { ...@@ -1170,7 +1172,8 @@ public class DownloadNotificationService extends Service {
// TODO(dtrainor): Should we spin up native to make sure we have the icon? Or maybe // TODO(dtrainor): Should we spin up native to make sure we have the icon? Or maybe
// build a Java cache for easy access. // build a Java cache for easy access.
notifyDownloadPaused( notifyDownloadPaused(
entry.id, !entry.isOffTheRecord, false, entry.isTransient, null); entry.id, entry.fileName, !entry.isOffTheRecord, false,
entry.isOffTheRecord, entry.isTransient, null);
hideSummaryNotificationIfNecessary(-1); hideSummaryNotificationIfNecessary(-1);
return; return;
} }
...@@ -1219,7 +1222,8 @@ public class DownloadNotificationService extends Service { ...@@ -1219,7 +1222,8 @@ public class DownloadNotificationService extends Service {
} else if (ACTION_DOWNLOAD_PAUSE.equals(intent.getAction())) { } else if (ACTION_DOWNLOAD_PAUSE.equals(intent.getAction())) {
// TODO(dtrainor): Consider hitting the delegate and rely on that to update the // TODO(dtrainor): Consider hitting the delegate and rely on that to update the
// state. // state.
notifyDownloadPaused(entry.id, true, false, entry.isTransient, null); notifyDownloadPaused(entry.id, entry.fileName, true, false,
entry.isOffTheRecord, entry.isTransient, null);
downloadServiceDelegate.pauseDownload(entry.id, entry.isOffTheRecord); downloadServiceDelegate.pauseDownload(entry.id, entry.isOffTheRecord);
} else if (ACTION_DOWNLOAD_RESUME.equals(intent.getAction())) { } else if (ACTION_DOWNLOAD_RESUME.equals(intent.getAction())) {
// TODO(dtrainor): Consider hitting the delegate and rely on that to update the // TODO(dtrainor): Consider hitting the delegate and rely on that to update the
......
...@@ -275,11 +275,13 @@ public class SystemDownloadNotifier implements DownloadNotifier, Observer { ...@@ -275,11 +275,13 @@ public class SystemDownloadNotifier implements DownloadNotifier, Observer {
break; break;
case DOWNLOAD_NOTIFICATION_TYPE_PAUSE: case DOWNLOAD_NOTIFICATION_TYPE_PAUSE:
mBoundService.notifyDownloadPaused( mBoundService.notifyDownloadPaused(
info.getContentId(), true, false, info.getIsTransient(), info.getIcon()); info.getContentId(), info.getFileName(), true, false, info.isOffTheRecord(),
info.getIsTransient(), info.getIcon());
break; break;
case DOWNLOAD_NOTIFICATION_TYPE_INTERRUPT: case DOWNLOAD_NOTIFICATION_TYPE_INTERRUPT:
mBoundService.notifyDownloadPaused(info.getContentId(), info.isResumable(), mBoundService.notifyDownloadPaused(info.getContentId(), info.getFileName(),
notificationInfo.isAutoResumable, info.getIsTransient(), info.getIcon()); info.isResumable(), notificationInfo.isAutoResumable, info.isOffTheRecord(),
info.getIsTransient(), info.getIcon());
break; break;
case DOWNLOAD_NOTIFICATION_TYPE_SUCCESS: case DOWNLOAD_NOTIFICATION_TYPE_SUCCESS:
final int notificationId = mBoundService.notifyDownloadSuccessful( final int notificationId = mBoundService.notifyDownloadSuccessful(
......
...@@ -49,12 +49,6 @@ class AndroidUIControllerDelegate : public DownloadUIController::Delegate { ...@@ -49,12 +49,6 @@ class AndroidUIControllerDelegate : public DownloadUIController::Delegate {
void AndroidUIControllerDelegate::OnNewDownloadReady( void AndroidUIControllerDelegate::OnNewDownloadReady(
content::DownloadItem* item) { content::DownloadItem* item) {
// The Android DownloadController is only interested in IN_PROGRESS downloads.
// Ones which are INTERRUPTED etc. can't be handed over to the Android
// DownloadManager.
if (item->GetState() != content::DownloadItem::IN_PROGRESS)
return;
DownloadControllerBase::Get()->OnDownloadStarted(item); DownloadControllerBase::Get()->OnDownloadStarted(item);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment