Visual tweaks for download items.

* Error and warning icon use was reversed from Material guidelines,
  other Chrome use, etc.  Swap these to fix.
* "Warning" icon + "normal" color doesn't make sense; use the warning
  color there.
* Apply the same styling to the filename in |file_name_label_| as in the
  other labels.  This only has a visible effect when new-style warnings
  are enabled.
* Schedule a paint when |complete_animation_| ends, too, not just when
  it progresses; this ensures the download item doesn't get "stuck until
  mouseover" when canceling, for example.

Bug: none
Change-Id: Icffc49980c23c3b42133c5c54af195924b33084c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333658
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795386}

chrome/browser/resources/downloads/item.js

@@ -327,24 +327,24 @@ Polymer({
         if ((loadTimeData.getBoolean('requestsApVerdicts') &&
              dangerType === DangerType.UNCOMMON_CONTENT) ||
             dangerType === DangerType.SENSITIVE_CONTENT_WARNING) {
-          return 'cr:error';
+          return 'cr:warning';
         }
 
-        const WARNING_TYPES = [
+        const ERROR_TYPES = [
           DangerType.SENSITIVE_CONTENT_BLOCK,
           DangerType.BLOCKED_TOO_LARGE,
           DangerType.BLOCKED_PASSWORD_PROTECTED,
         ];
-        if (WARNING_TYPES.includes(dangerType)) {
-          return 'cr:warning';
+        if (ERROR_TYPES.includes(dangerType)) {
+          return 'cr:error';
         }
 
         if (this.data.state === States.ASYNC_SCANNING) {
-          return 'cr:error';
+          return 'cr:info';
         }
       }
       if (this.isDangerous_) {
-        return 'cr:warning';
+        return 'cr:error';
       }
       if (!this.useFileIcon_) {
         return 'cr:insert-drive-file';

chrome/browser/ui/views/download/download_item_view.cc

@@ -96,6 +96,7 @@
 #include "ui/views/metadata/metadata_header_macros.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
 #include "ui/views/style/typography.h"
+#include "ui/views/vector_icons.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 #include "url/gurl.h"
@@ -266,6 +267,7 @@ DownloadItemView::DownloadItemView(DownloadUIModel::DownloadUIModelPtr model,
   const base::string16 filename = ElidedFilename(*file_name_label_);
   file_name_label_->SetText(filename);
   file_name_label_->set_can_process_events_within_subtree(false);
+  StyleFilename(*file_name_label_, 0, filename.length());
 
   status_label_ = AddChildView(std::make_unique<views::Label>(
       base::string16(), CONTEXT_DOWNLOAD_SHELF_STATUS));
@@ -470,20 +472,23 @@ void DownloadItemView::OnDownloadUpdated() {
 }
 
 void DownloadItemView::OnDownloadOpened() {
+  SetEnabled(false);
   file_name_label_->SetDefaultTextStyle(views::style::STYLE_DISABLED);
   const base::string16 filename = ElidedFilename(*file_name_label_);
-  file_name_label_->SetText(
-      l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_OPENING, filename));
+  size_t filename_offset;
+  file_name_label_->SetText(l10n_util::GetStringFUTF16(
+      IDS_DOWNLOAD_STATUS_OPENING, filename, &filename_offset));
+  StyleFilename(*file_name_label_, filename_offset, filename.length());
 
-  SetEnabled(false);
   const auto reenable = [](base::WeakPtr<DownloadItemView> view) {
     if (!view)
       return;
+    view->SetEnabled(true);
     auto* label = view->file_name_label_;
     label->SetDefaultTextStyle(views::style::STYLE_PRIMARY);
     const base::string16 filename = view->ElidedFilename(*label);
     label->SetText(filename);
-    view->SetEnabled(true);
+    StyleFilename(*label, 0, filename.length());
   };
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
@@ -498,11 +503,13 @@ void DownloadItemView::OnDownloadDestroyed() {
 }
 
 void DownloadItemView::AnimationProgressed(const gfx::Animation* animation) {
-  // We don't care if what animation (body button/drop button/complete),
-  // is calling back, as they all have to go through the same paint call.
   SchedulePaint();
 }
 
+void DownloadItemView::AnimationEnded(const gfx::Animation* animation) {
+  AnimationProgressed(animation);
+}
+
 void DownloadItemView::MaybeSubmitDownloadToFeedbackService(
     DownloadCommands::Command download_command) {
   if (!model_->ShouldAllowDownloadFeedback() ||
@@ -974,18 +981,18 @@ ui::ImageModel DownloadItemView::GetIcon() const {
   // TODO(pkasting): Use a child view (ImageView subclass?) to display the icon
   // instead of recomputing this and drawing manually.
 
-  // TODO(pkasting): Some names/icons here are backwards.
   // TODO(drubery): Replace these sizes with layout provider constants when the
   // new UX is fully launched.
-  const int help_or_error_icon_size = UseNewWarnings() ? 20 : 27;
+  const int non_error_icon_size = UseNewWarnings() ? 20 : 27;
   const auto kWarning = ui::ImageModel::FromVectorIcon(
-      vector_icons::kErrorIcon, ui::NativeTheme::kColorId_AlertSeverityMedium,
-      help_or_error_icon_size);
+      vector_icons::kWarningIcon, ui::NativeTheme::kColorId_AlertSeverityMedium,
+      non_error_icon_size);
   const auto kError = ui::ImageModel::FromVectorIcon(
-      vector_icons::kWarningIcon, ui::NativeTheme::kColorId_AlertSeverityHigh,
+      vector_icons::kErrorIcon, ui::NativeTheme::kColorId_AlertSeverityHigh,
       UseNewWarnings() ? 20 : 24);
 
-  switch (model_->GetDangerType()) {
+  const auto danger_type = model_->GetDangerType();
+  switch (danger_type) {
     case download::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT:
       return safe_browsing::AdvancedProtectionStatusManagerFactory::
                      GetForProfile(model_->profile())
@@ -1001,15 +1008,15 @@ ui::ImageModel DownloadItemView::GetIcon() const {
     case download::DOWNLOAD_DANGER_TYPE_BLOCKED_PASSWORD_PROTECTED:
     case download::DOWNLOAD_DANGER_TYPE_SENSITIVE_CONTENT_BLOCK:
       return kError;
-    case download::DOWNLOAD_DANGER_TYPE_ASYNC_SCANNING:
     case download::DOWNLOAD_DANGER_TYPE_SENSITIVE_CONTENT_WARNING:
-      return ui::ImageModel::FromVectorIcon(
-          vector_icons::kErrorIcon, ui::NativeTheme::kColorId_DefaultIconColor,
-          help_or_error_icon_size);
+      return kWarning;
+    case download::DOWNLOAD_DANGER_TYPE_ASYNC_SCANNING:
     case download::DOWNLOAD_DANGER_TYPE_PROMPT_FOR_SCANNING:
       return ui::ImageModel::FromVectorIcon(
-          vector_icons::kHelpIcon, ui::NativeTheme::kColorId_DefaultIconColor,
-          help_or_error_icon_size);
+          (danger_type == download::DOWNLOAD_DANGER_TYPE_ASYNC_SCANNING)
+              ? views::kInfoIcon
+              : vector_icons::kHelpIcon,
+          ui::NativeTheme::kColorId_DefaultIconColor, non_error_icon_size);
     case download::DOWNLOAD_DANGER_TYPE_BLOCKED_UNSUPPORTED_FILETYPE:
     case download::DOWNLOAD_DANGER_TYPE_DEEP_SCANNED_SAFE:
     case download::DOWNLOAD_DANGER_TYPE_DEEP_SCANNED_OPENED_DANGEROUS:

chrome/browser/ui/views/download/download_item_view.h

@@ -98,6 +98,7 @@ class DownloadItemView : public views::View,
 
   // views::AnimationDelegateViews:
   void AnimationProgressed(const gfx::Animation* animation) override;
+  void AnimationEnded(const gfx::Animation* animation) override;
 
   // Returns the DownloadUIModel object belonging to this item.
   DownloadUIModel* model() { return model_.get(); }

chrome/test/data/webui/downloads/item_tests.js

@@ -66,7 +66,7 @@ suite('item tests', function() {
                hideDate: false,
                dangerType: DangerType.SENSITIVE_CONTENT_BLOCK,
              }));
-    assertEquals(item.computeIcon_(), 'cr:warning');
+    assertEquals(item.computeIcon_(), 'cr:error');
     assertFalse(item.useFileIcon_);
 
     item.set('data', createDownload({
@@ -74,7 +74,7 @@ suite('item tests', function() {
                hideDate: false,
                dangerType: DangerType.BLOCKED_TOO_LARGE,
              }));
-    assertEquals(item.computeIcon_(), 'cr:warning');
+    assertEquals(item.computeIcon_(), 'cr:error');
     assertFalse(item.useFileIcon_);
 
     item.set('data', createDownload({
@@ -82,7 +82,7 @@ suite('item tests', function() {
                hideDate: false,
                dangerType: DangerType.BLOCKED_PASSWORD_PROTECTED,
              }));
-    assertEquals(item.computeIcon_(), 'cr:warning');
+    assertEquals(item.computeIcon_(), 'cr:error');
     assertFalse(item.useFileIcon_);
   });