Polish for downloads home v2

- Update caption messages for in-progress downloads.
- Fix assert failure around being unable to resume.
- Fix notification persisting after cancel request.

BUG=864223

Change-Id: Ie60aea0288bbd076599d71b89fa60e1ce54ffd29
Reviewed-on: https://chromium-review.googlesource.com/c/1258243
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596438}

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

@@ -234,10 +234,23 @@ public final class DownloadInfo {
 
     /**
      * Helper method to build a {@link DownloadInfo} from an {@link OfflineItem}.
-     * @param item The {@link OfflineItem} to mimic.
-     * @return     A {@link DownloadInfo} containing the relevant fields from {@code item}.
+     * @param item    The {@link OfflineItem} to mimic.
+     * @param visuals The {@link OfflineItemVisuals} to mimic.
+     * @return        A {@link DownloadInfo} containing the relevant fields from {@code item}.
      */
     public static DownloadInfo fromOfflineItem(OfflineItem item, OfflineItemVisuals visuals) {
+        return builderFromOfflineItem(item, visuals).build();
+    }
+
+    /**
+     * Helper method to build a {@link DownloadInfo.Builder} from an {@link OfflineItem}.
+     * @param item    The {@link OfflineItem} to mimic.
+     * @param visuals The {@link OfflineItemVisuals} to mimic.
+     * @return        A {@link DownloadInfo.Builder} containing the relevant fields from
+     *                {@code item}.
+     */
+    public static DownloadInfo.Builder builderFromOfflineItem(
+            OfflineItem item, OfflineItemVisuals visuals) {
         int state;
         switch (item.state) {
             case OfflineItemState.COMPLETE:
@@ -285,8 +298,7 @@ public final class DownloadInfo {
                 .setIcon(visuals == null ? null : visuals.icon)
                 .setPendingState(item.pendingState)
                 .setFailState(item.failState)
-                .setShouldPromoteOrigin(item.promoteOrigin)
-                .build();
+                .setShouldPromoteOrigin(item.promoteOrigin);
     }
 
     /**
@@ -311,9 +323,12 @@ public final class DownloadInfo {
         offlineItem.isOffTheRecord = downloadInfo.isOffTheRecord();
         offlineItem.mimeType = downloadInfo.getMimeType();
         offlineItem.progress = downloadInfo.getProgress();
+        offlineItem.timeRemainingMs = downloadInfo.getTimeRemainingInMillis();
         offlineItem.isDangerous = downloadInfo.getIsDangerous();
+        offlineItem.pendingState = downloadInfo.getPendingState();
         offlineItem.failState = downloadInfo.getFailState();
         offlineItem.promoteOrigin = downloadInfo.getShouldPromoteOrigin();
+
         switch (downloadInfo.state()) {
             case DownloadState.IN_PROGRESS:
                 offlineItem.state = downloadInfo.isPaused() ? OfflineItemState.PAUSED

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

@@ -1033,6 +1033,8 @@ public class DownloadManagerService
                             .build();
             onDownloadCancelled(info);
             removeDownloadProgress(id.id);
+        } else {
+            mDownloadNotifier.notifyDownloadCanceled(id);
         }
         recordDownloadFinishedUMA(DownloadStatus.CANCELLED, id.id, 0);
     }

chrome/android/java/src/org/chromium/chrome/browser/download/home/glue/DownloadGlue.java

@@ -126,8 +126,14 @@ public class DownloadGlue implements DownloadObserver {
 
     /** @see OfflineContentProvider#resumeDownload(ContentId) */
     public void resumeDownload(OfflineItem item, boolean hasUserGesture) {
-        DownloadItem downloadItem = new DownloadItem(
-                false /* useAndroidDownloadManager */, DownloadInfo.fromOfflineItem(item, null));
+        DownloadInfo.Builder builder = DownloadInfo.builderFromOfflineItem(item, null);
+
+        // This is a temporary hack to work around the assumption that the DownloadItem passed to
+        // DownloadManagerService#resumeDownload() will not be paused.
+        builder.setIsPaused(false);
+
+        DownloadItem downloadItem =
+                new DownloadItem(false /* useAndroidDownloadManager */, builder.build());
         DownloadManagerService.getDownloadManagerService().resumeDownload(
                 item.id, downloadItem, hasUserGesture);
     }

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/UiUtils.java

@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.util.MathUtils;
 import org.chromium.components.offline_items_collection.OfflineItem;
 import org.chromium.components.offline_items_collection.OfflineItem.Progress;
 import org.chromium.components.offline_items_collection.OfflineItemFilter;
+import org.chromium.components.offline_items_collection.OfflineItemProgressUnit;
 import org.chromium.components.offline_items_collection.OfflineItemState;
 import org.chromium.components.url_formatter.UrlFormatter;
 
@@ -113,6 +114,93 @@ public final class UiUtils {
                 R.string.download_manager_list_item_description, displaySize, displayUrl);
     }
 
+    /**
+     * Generates a detailed caption for downloads that are in-progress.
+     * @param item The {@link OfflineItem} to generate a caption for.
+     * @return     The {@link CharSequence} representing the caption.
+     */
+    public static CharSequence generateInProgressLongCaption(OfflineItem item) {
+        Context context = ContextUtils.getApplicationContext();
+        assert item.state != OfflineItemState.COMPLETE;
+
+        OfflineItem.Progress progress = item.progress;
+
+        // Make sure we have a valid OfflineItem.Progress to parse even if it's just for the failed
+        // message.
+        if (progress == null) {
+            if (item.totalSizeBytes > 0) {
+                progress = new OfflineItem.Progress(
+                        0, item.totalSizeBytes, OfflineItemProgressUnit.BYTES);
+            } else {
+                progress = new OfflineItem.Progress(0, 100L, OfflineItemProgressUnit.PERCENTAGE);
+            }
+        }
+
+        CharSequence progressString = DownloadUtils.getProgressTextForNotification(progress);
+        CharSequence statusString = null;
+
+        switch (item.state) {
+            case OfflineItemState.PENDING:
+                // TODO(crbug.com/891421): Add detailed pending state string from
+                // DownloadUtils.getPendingStatusString().
+                statusString = context.getString(R.string.download_manager_pending);
+                break;
+            case OfflineItemState.IN_PROGRESS:
+                if (item.timeRemainingMs > 0) {
+                    statusString = DownloadUtils.formatRemainingTime(context, item.timeRemainingMs);
+                }
+                break;
+            case OfflineItemState.FAILED: // Intentional fallthrough.
+            case OfflineItemState.CANCELLED: // Intentional fallthrough.
+            case OfflineItemState.INTERRUPTED:
+                // TODO(crbug.com/891421): Add detailed failure state string from
+                // DownloadUtils.getFailStatusString().
+                statusString = context.getString(R.string.download_manager_failed);
+                break;
+            case OfflineItemState.PAUSED:
+                statusString = context.getString(R.string.download_manager_paused);
+                break;
+            case OfflineItemState.COMPLETE: // Intentional fallthrough.
+            default:
+                assert false;
+        }
+
+        if (statusString == null) return progressString;
+
+        return context.getString(
+                R.string.download_manager_in_progress_description, progressString, statusString);
+    }
+
+    /**
+     * Generates a short caption for downloads that are in-progress.
+     * @param item The {@link OfflineItem} to generate a short caption for.
+     * @return     The {@link CharSequence} representing the caption.
+     */
+    public static CharSequence generateInProgressShortCaption(OfflineItem item) {
+        Context context = ContextUtils.getApplicationContext();
+
+        switch (item.state) {
+            case OfflineItemState.PENDING:
+                return context.getString(R.string.download_manager_pending);
+            case OfflineItemState.IN_PROGRESS:
+                if (item.timeRemainingMs > 0) {
+                    return DownloadUtils.formatRemainingTime(context, item.timeRemainingMs);
+                } else {
+                    return DownloadUtils.getProgressTextForNotification(item.progress);
+                }
+            case OfflineItemState.FAILED: // Intentional fallthrough.
+            case OfflineItemState.CANCELLED: // Intentional fallthrough.
+            case OfflineItemState.INTERRUPTED:
+                return context.getString(R.string.download_manager_failed);
+            case OfflineItemState.PAUSED:
+                return context.getString(R.string.download_manager_paused);
+            case OfflineItemState.COMPLETE: // Intentional fallthrough.
+            default:
+                assert false;
+                return "";
+        }
+    }
+
     /** @return Whether or not {@code item} can show a thumbnail in the UI. */
     public static boolean canHaveThumbnails(OfflineItem item) {
         switch (item.filter) {

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/holder/InProgressImageViewHolder.java

@@ -11,7 +11,6 @@ import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.TextView;
 
-import org.chromium.chrome.browser.download.DownloadUtils;
 import org.chromium.chrome.browser.download.home.list.ListItem;
 import org.chromium.chrome.browser.download.home.list.ListProperties;
 import org.chromium.chrome.browser.download.home.list.UiUtils;
@@ -61,8 +60,8 @@ public class InProgressImageViewHolder extends ListItemViewHolder {
         OfflineItem offlineItem = ((ListItem.OfflineItemListItem) item).item;
 
         mPlaceholder.setContentDescription(offlineItem.title);
-        // TODO(shaktisahu): Create status string for the new specs.
-        mCaption.setText(DownloadUtils.getProgressTextForNotification(offlineItem.progress));
+        mCaption.setText(UiUtils.generateInProgressShortCaption(offlineItem));
+
         mCancelButton.setOnClickListener(
                 v -> properties.get(ListProperties.CALLBACK_CANCEL).onResult(offlineItem));
 

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/holder/InProgressVideoViewHolder.java

@@ -10,7 +10,6 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.widget.TextView;
 
-import org.chromium.chrome.browser.download.DownloadUtils;
 import org.chromium.chrome.browser.download.home.list.ListItem;
 import org.chromium.chrome.browser.download.home.list.ListProperties;
 import org.chromium.chrome.browser.download.home.list.UiUtils;
@@ -55,10 +54,10 @@ public class InProgressVideoViewHolder extends ListItemViewHolder {
         OfflineItem offlineItem = ((ListItem.OfflineItemListItem) item).item;
 
         mTitle.setText(offlineItem.title);
+        mCaption.setText(UiUtils.generateInProgressLongCaption(offlineItem));
+
         mCancelButton.setOnClickListener(
                 v -> properties.get(ListProperties.CALLBACK_CANCEL).onResult(offlineItem));
-        // TODO(shaktisahu): Create status string for the new specs.
-        mCaption.setText(DownloadUtils.getProgressTextForNotification(offlineItem.progress));
 
         UiUtils.setProgressForOfflineItem(mActionButton, offlineItem);
         mActionButton.setOnClickListener(view -> {

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/holder/InProgressViewHolder.java

@@ -10,7 +10,6 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.widget.TextView;
 
-import org.chromium.chrome.browser.download.DownloadUtils;
 import org.chromium.chrome.browser.download.home.list.ListItem;
 import org.chromium.chrome.browser.download.home.list.ListProperties;
 import org.chromium.chrome.browser.download.home.list.UiUtils;
@@ -55,10 +54,10 @@ public class InProgressViewHolder extends ListItemViewHolder {
         OfflineItem offlineItem = ((ListItem.OfflineItemListItem) item).item;
 
         mTitle.setText(offlineItem.title);
+        mCaption.setText(UiUtils.generateInProgressLongCaption(offlineItem));
+
         mCancelButton.setOnClickListener(
                 v -> properties.get(ListProperties.CALLBACK_CANCEL).onResult(offlineItem));
-        // TODO(shaktisahu): Create status string for the new specs.
-        mCaption.setText(DownloadUtils.getProgressTextForNotification(offlineItem.progress));
 
         UiUtils.setProgressForOfflineItem(mActionButton, offlineItem);
         mActionButton.setOnClickListener(view -> {

chrome/android/java/src/org/chromium/chrome/browser/infobar/DownloadProgressInfoBar.java

@@ -88,7 +88,7 @@ public class DownloadProgressInfoBar extends InfoBar {
      * @return The tab associated with this infobar.
      */
     public Tab getTab() {
-        return nativeGetTab(getNativeInfoBarPtr());
+        return getNativeInfoBarPtr() == 0 ? null : nativeGetTab(getNativeInfoBarPtr());
     }
 
     /**

chrome/android/java/strings/android_chrome_strings.grd

@@ -2499,13 +2499,13 @@ To obtain new licenses, connect to the internet and play your downloaded content
         <ph name="FILE_SIZE">%1$s<ex>1.56 MB</ex></ph> - Updated <ph name="TIME_SINCE_UPDATE">%2$s<ex>4 minutes ago</ex></ph>
       </message>
       <message name="IDS_DOWNLOAD_MANAGER_LIST_ITEM_DESCRIPTION" desc="Text containing the download list item description.">
-        <ph name="FILE_SIZE">%1$s<ex>1.56 MB</ex></ph> - <ph name="DESCRIPTION">%2$s<ex>www.example.com</ex></ph>
+        <ph name="FILE_SIZE">%1$s<ex>1.56 MB</ex></ph> <ph name="SEPARATOR">•</ph> <ph name="DESCRIPTION">%2$s<ex>www.example.com</ex></ph>
       </message>
       <message name="IDS_DOWNLOAD_MANAGER_FILES_TAB" desc="Tab text for showing the files a user explicitly downloaded.">
         My Files
       </message>
       <message name="IDS_DOWNLOAD_MANAGER_PREFETCH_CAPTION" desc="Text containing the prefetched article description.">
-        <ph name="DESCRIPTION">%1$s<ex>www.example.com</ex></ph> - <ph name="FILE_SIZE">%2$s<ex>1.56 MB</ex></ph>
+        <ph name="DESCRIPTION">%1$s<ex>www.example.com</ex></ph> <ph name="SEPARATOR">•</ph> <ph name="FILE_SIZE">%2$s<ex>1.56 MB</ex></ph>
       </message>
       <message name="IDS_DOWNLOAD_MANAGER_PREFETCH_TAB_EMPTY" desc="Tab text indicating that there is no prefetched content.">
         No content here
@@ -2516,6 +2516,18 @@ To obtain new licenses, connect to the internet and play your downloaded content
       <message name="IDS_DOWNLOAD_MANAGER_N_HOURS" desc="How many hours ago an item was downloaded (if downloaded on the same day). [ICU Syntax]">
         {HOURS, plural, =1 {# hr} other {# hrs}}
       </message>
+      <message name="IDS_DOWNLOAD_MANAGER_PAUSED" desc="Generic text shown as part of an aggregated message when a download is paused.">
+        paused
+      </message>
+      <message name="IDS_DOWNLOAD_MANAGER_PENDING" desc="Generic text shown as part of an aggregated message when a download is waiting to start.">
+        pending
+      </message>
+      <message name="IDS_DOWNLOAD_MANAGER_FAILED" desc="Generic text shown as part of an aggregated message when a download has failed.">
+        failed
+      </message>
+      <message name="IDS_DOWNLOAD_MANAGER_IN_PROGRESS_DESCRIPTION" desc="Text containing two components of an in-progress string description.">
+        <ph name="FILE_SIZE_OF_TOTAL">%1$s<ex>1.56 MB / 10MB</ex></ph> <ph name="SEPARATOR">•</ph> <ph name="DESCRIPTION">%2$s<ex>failed</ex></ph>
+      </message>
 
       <!-- Browsing History UI -->
       <message name="IDS_HISTORY_MANAGER_EMPTY" desc="Indicates that there are no browsing history items.">