Update chrome://downloads UX to display deep scanning

chrome://downloads was previously displaying the in progress UX for
downloads that were in the middle of scanning. This could be confusing
given that we may remain in this state for several minutes. Instead, we
should indicate to the user that the file is being scanned.

This CL also removes the ".dangerous #description" selectors that could
cause warning text to change color. These currently aren't doing
anything (the text is black).

Screenshot: https://screenshot.googleplex.com/sTApKg55f4n.png
Bug: 1048891
Change-Id: I3fb736818814dd0343871c08ca230435899bf5aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063043Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744040}

chrome/app/generated_resources.grd

@@ -1744,6 +1744,10 @@ are declared in tools/grit/grit_rule.gni.
          desc="Message shown to the user on chrome://downloads page to explain that this download is blocked because it is mixed-content.">
         This file can't be downloaded securely.
       </message>
+      <message name="IDS_BLOCK_REASON_DEEP_SCANNING"
+        desc="Message shown on chrome://downloads when a download is being scanned">
+        This file is being scanned.
+      </message>
       <message name="IDS_DEEP_SCANNED_SAFE_DESCRIPTION"
         desc="Message shown to the user on chrome://downloads page to explain this this download was deep scanned, and found to be safe.">
         Scan complete, no issues identified.

chrome/app/generated_resources_grd/IDS_BLOCK_REASON_DEEP_SCANNING.png.sha1

+7e5cb61f27a35050f60a77f0dcf622a0b9c62551
\ No newline at end of file

chrome/browser/resources/downloads/constants.js

@@ -35,4 +35,5 @@ export const States = {
   DANGEROUS: 'DANGEROUS',
   INTERRUPTED: 'INTERRUPTED',
   MIXED_CONTENT: 'MIXED_CONTENT',
+  ASYNC_SCANNING: 'ASYNC_SCANNING',
 };

chrome/browser/resources/downloads/item.html

@@ -135,27 +135,34 @@
     opacity: .5;
   }
 
-  #file-icon-wrapper iron-icon[icon='cr:insert-drive-file'] {
+  #file-icon-wrapper iron-icon[icon-color='paper-grey'] {
     color: var(--paper-grey-400);
   }
 
-  #file-icon-wrapper iron-icon[icon='cr:warning'],
-  .dangerous #description {
+  #file-icon-wrapper iron-icon[icon-color='red'] {
     color: var(--google-red-700);
   }
 
-  #file-icon-wrapper iron-icon[icon='cr:error'],
-  .dangerous #description {
+  #file-icon-wrapper iron-icon[icon-color='yellow'] {
     color: var(--google-yellow-500);
   }
 
   @media (prefers-color-scheme: dark) {
-    #file-icon-wrapper iron-icon[icon='cr:warning'],
-    .dangerous #description {
+    #file-icon-wrapper iron-icon[icon-color='red'] {
       color: var(--google-red-refresh-300);
     }
   }
 
+  #file-icon-wrapper iron-icon[icon-color='grey'] {
+    color: var(--google-grey-700);
+  }
+
+  @media (prefers-color-scheme: dark) {
+    #file-icon-wrapper iron-icon[icon-color='grey'] {
+      color: var(--google-grey-500);
+    }
+  }
+
   #name,
   #file-link,
   #url {
@@ -269,7 +276,8 @@
   }
 
   #pauseOrResume,
-  #dangerous .action-button {
+  #dangerous .action-button,
+  #openNow {
     margin-inline-end: 8px;
   }
 </style>
@@ -286,10 +294,13 @@
     attribute and remove aria-hidden="true" from #file-icon-wrapper
   -->
   <div id="file-icon-wrapper" class="icon-wrapper" aria-hidden="true">
-    <img class="icon" id="file-icon" alt="" hidden="[[!useFileIcon_]]">
+    <img class="icon" id="file-icon" alt="" hidden="[[!useFileIcon_]]"
+    icon-color$="[[computeIconColor_(isDangerous_, data.dangerType,
+    useFileIcon_)]]">
     <iron-icon class="icon" icon$="[[computeIcon_(
         isDangerous_, data.dangerType, useFileIcon_)]]"
-        hidden="[[useFileIcon_]]"></iron-icon>
+        hidden="[[useFileIcon_]]" icon-color$="[[computeIconColor_(isDangerous_,
+        data.dangerType, useFileIcon_)]]"></iron-icon>
   </div>
 
   <div id="details">
@@ -351,6 +362,14 @@
           </cr-button>
         </div>
       </template>
+      <template is="dom-if" if="[[showOpenNow_]]">
+        <div role="gridcell">
+          <cr-button on-click="onOpenNowTap_" id="openNow" class="action-button"
+                     focus-row-control focus-type="open">
+            $i18n{controlOpenNow}
+          </cr-button>
+        </div>
+      </template>
       <template is="dom-if" if="[[showCancel_]]">
         <div role="gridcell">
           <cr-button on-click="onCancelTap_" focus-row-control

chrome/browser/resources/downloads/item.js

@@ -116,6 +116,13 @@ Polymer({
         value: false,
       },
 
+      /** @private */
+      showOpenNow_: {
+        computed: 'computeShowOpenNow_(data.state)',
+        type: Boolean,
+        value: false,
+      },
+
       useFileIcon_: Boolean,
     },
 
@@ -289,6 +296,9 @@ Polymer({
           }
           break;
 
+        case States.ASYNC_SCANNING:
+          return loadTimeData.getString('asyncScanningDownloadDesc');
+
         case States.IN_PROGRESS:
         case States.PAUSED:  // Fallthrough.
           return data.progressStatusText;
@@ -314,7 +324,6 @@ Polymer({
     computeIcon_() {
       if (this.data) {
         const dangerType = this.data.dangerType;
-
         if ((loadTimeData.getBoolean('requestsApVerdicts') &&
              dangerType === DangerType.UNCOMMON_CONTENT) ||
             dangerType === DangerType.SENSITIVE_CONTENT_WARNING) {
@@ -329,6 +338,10 @@ Polymer({
         if (WARNING_TYPES.includes(dangerType)) {
           return 'cr:warning';
         }
+
+        if (this.data.state === States.ASYNC_SCANNING) {
+          return 'cr:error';
+        }
       }
       if (this.isDangerous_) {
         return 'cr:warning';
@@ -339,6 +352,41 @@ Polymer({
       return '';
     },
 
+    /**
+     * @return {string}
+     * @private
+     */
+    computeIconColor_() {
+      if (this.data) {
+        const dangerType = this.data.dangerType;
+        if ((loadTimeData.getBoolean('requestsApVerdicts') &&
+             dangerType === DangerType.UNCOMMON_CONTENT) ||
+            dangerType === DangerType.SENSITIVE_CONTENT_WARNING) {
+          return 'yellow';
+        }
+
+        const WARNING_TYPES = [
+          DangerType.SENSITIVE_CONTENT_BLOCK,
+          DangerType.BLOCKED_TOO_LARGE,
+          DangerType.BLOCKED_PASSWORD_PROTECTED,
+        ];
+        if (WARNING_TYPES.includes(dangerType)) {
+          return 'red';
+        }
+
+        if (this.data.state === States.ASYNC_SCANNING) {
+          return 'grey';
+        }
+      }
+      if (this.isDangerous_) {
+        return 'red';
+      }
+      if (!this.useFileIcon_) {
+        return 'paper-grey';
+      }
+      return '';
+    },
+
     /**
      * @return {boolean}
      * @private
@@ -432,7 +480,8 @@ Polymer({
      */
     computeShowCancel_() {
       return this.data.state === States.IN_PROGRESS ||
-          this.data.state === States.PAUSED;
+          this.data.state === States.PAUSED ||
+          this.data.state === States.ASYNC_SCANNING;
     },
 
     /**
@@ -440,7 +489,16 @@ Polymer({
      * @private
      */
     computeShowProgress_() {
-      return this.showCancel_ && this.data.percent >= -1;
+      return this.showCancel_ && this.data.percent >= -1 &&
+          this.data.state !== States.ASYNC_SCANNING;
+    },
+
+    /**
+     * @return {boolean}
+     * @private
+     */
+    computeShowOpenNow_() {
+      return this.data.state === States.ASYNC_SCANNING;
     },
 
     /**
@@ -499,13 +557,16 @@ Polymer({
         this.useFileIcon_ = false;
       } else if (OVERRIDDEN_ICON_TYPES.includes(this.data.dangerType)) {
         this.useFileIcon_ = false;
+      } else if (this.data.state === States.ASYNC_SCANNING) {
+        this.useFileIcon_ = false;
       } else {
         this.$.url.href = assert(this.data.url);
         const path = this.data.filePath;
         IconLoader.getInstance()
             .loadIcon(this.$['file-icon'], path)
             .then(success => {
-              if (path === this.data.filePath) {
+              if (path === this.data.filePath && this.data.state !==
+                  States.ASYNC_SCANNING) {
                 this.useFileIcon_ = success;
               }
             });
@@ -523,6 +584,11 @@ Polymer({
       this.mojoHandler_.discardDangerous(this.data.id);
     },
 
+    /** @private */
+    onOpenNowTap_() {
+      this.mojoHandler_.openDuringScanningRequiringGesture(this.data.id);
+    },
+
     /**
      * @private
      * @param {Event} e

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

@@ -348,12 +348,6 @@ void DownloadItemView::OnDownloadUpdated() {
              model_->GetState() != DownloadItem::CANCELLED) {
     if (!IsShowingDeepScanning())
       TransitionToDeepScanningDialog();
-
-    if (should_open_while_scanning_ &&
-        model_->GetState() == DownloadItem::COMPLETE) {
-      should_open_while_scanning_ = false;
-      model_->OpenDownload();
-    }
   } else {
     TransitionToNormalMode();
   }
@@ -1626,7 +1620,7 @@ base::string16 DownloadItemView::ElidedFilename() {
 
 void DownloadItemView::OpenDownloadDuringAsyncScanning() {
   model_->CompleteSafeBrowsingScan();
-  should_open_while_scanning_ = true;
+  model_->SetOpenWhenComplete(true);
 }
 
 void DownloadItemView::StyleFilenameInLabel(views::StyledLabel* label) {

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

@@ -439,10 +439,6 @@ class DownloadItemView : public views::View,
   // Deep scanning open now button.
   views::MdTextButton* open_now_button_ = nullptr;
 
-  // Whether the user has selected "open now" on a download pending asynchronous
-  // scanning.
-  bool should_open_while_scanning_ = false;
-
   // Deep scanning modal dialog confirming choice to "open now".
   TabModalConfirmDialog* open_now_modal_dialog_;
 

chrome/browser/ui/webui/downloads/downloads.mojom

@@ -47,6 +47,11 @@ interface PageHandler {
   Cancel(string id);
   ClearAll();
   OpenDownloadsFolderRequiringGesture();
+
+  // Opens this download with the given |id| while it is being scanned by Safe
+  // Browsing. This completes the scan early. This requires a user gesture on
+  // the WebUI.
+  OpenDuringScanningRequiringGesture(string id);
 };
 
 interface Page {

chrome/browser/ui/webui/downloads/downloads_dom_handler.cc

@@ -71,6 +71,7 @@ enum DownloadsDOMEvent {
   DOWNLOADS_DOM_EVENT_OPEN_FOLDER = 10,
   DOWNLOADS_DOM_EVENT_RESUME = 11,
   DOWNLOADS_DOM_EVENT_RETRY_DOWNLOAD = 12,
+  DOWNLOADS_DOM_EVENT_OPEN_DURING_SCANNING = 13,
   DOWNLOADS_DOM_EVENT_MAX
 };
 
@@ -355,6 +356,23 @@ void DownloadsDOMHandler::OpenDownloadsFolderRequiringGesture() {
   }
 }
 
+void DownloadsDOMHandler::OpenDuringScanningRequiringGesture(
+    const std::string& id) {
+  if (!GetWebUIWebContents()->HasRecentInteractiveInputEvent()) {
+    LOG(ERROR) << "OpenDownloadsFolderRequiringGesture received without recent "
+                  "user interaction";
+    return;
+  }
+
+  CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_OPEN_DURING_SCANNING);
+  download::DownloadItem* download = GetDownloadByStringId(id);
+  if (download) {
+    DownloadItemModel model(download);
+    model.SetOpenWhenComplete(true);
+    model.CompleteSafeBrowsingScan();
+  }
+}
+
 // DownloadsDOMHandler, private: --------------------------------------------
 
 content::DownloadManager* DownloadsDOMHandler::GetMainNotifierManager() const {

chrome/browser/ui/webui/downloads/downloads_dom_handler.h

@@ -61,6 +61,7 @@ class DownloadsDOMHandler : public content::WebContentsObserver,
   void Cancel(const std::string& id) override;
   void ClearAll() override;
   void OpenDownloadsFolderRequiringGesture() override;
+  void OpenDuringScanningRequiringGesture(const std::string& id) override;
 
  protected:
   // These methods are for mocking so that most of this class does not actually

chrome/browser/ui/webui/downloads/downloads_list_tracker.cc

@@ -275,6 +275,9 @@ downloads::mojom::DataPtr DownloadsListTracker::CreateDownloadData(
         state = "DANGEROUS";
       } else if (download_item->IsMixedContent()) {
         state = "MIXED_CONTENT";
+      } else if (download_item->GetDangerType() ==
+                 download::DOWNLOAD_DANGER_TYPE_ASYNC_SCANNING) {
+        state = "ASYNC_SCANNING";
       } else if (download_item->IsPaused()) {
         state = "PAUSED";
       } else {

chrome/browser/ui/webui/downloads/downloads_ui.cc

@@ -123,6 +123,7 @@ content::WebUIDataSource* CreateDownloadsUIHTMLSource(Profile* profile) {
       {"controlRemoveFromListAriaLabel", IDS_DOWNLOAD_LINK_REMOVE_ARIA_LABEL},
       {"controlRetry", IDS_DOWNLOAD_LINK_RETRY},
       {"controlledByUrl", IDS_DOWNLOAD_BY_EXTENSION_URL},
+      {"controlOpenNow", IDS_OPEN_DOWNLOAD_NOW},
       {"toastClearedAll", IDS_DOWNLOAD_TOAST_CLEARED_ALL},
       {"toastRemovedFromList", IDS_DOWNLOAD_TOAST_REMOVED_FROM_LIST},
       {"undo", IDS_DOWNLOAD_UNDO},
@@ -140,6 +141,8 @@ content::WebUIDataSource* CreateDownloadsUIHTMLSource(Profile* profile) {
                              IDS_BLOCK_REASON_UNWANTED_DOWNLOAD);
   source->AddLocalizedString("mixedContentDownloadDesc",
                              IDS_BLOCK_REASON_MIXED_CONTENT);
+  source->AddLocalizedString("asyncScanningDownloadDesc",
+                             IDS_BLOCK_REASON_DEEP_SCANNING);
   if (browser_defaults::kDownloadPageHasShowInFolder)
     source->AddLocalizedString("controlShowInFolder", IDS_DOWNLOAD_LINK_SHOW);
 

tools/metrics/histograms/enums.xml

@@ -16197,6 +16197,7 @@ to ensure that the crash string is shown properly on the user-facing crash UI.
   <int value="10" label="OpenFolder"/>
   <int value="11" label="Resume"/>
   <int value="12" label="RetryDownload"/>
+  <int value="13" label="OpenDuringScanning"/>
 </enum>
 
 <enum name="DownloadedFileAction">