Warn users about potentially unwanted downloads.

Warn users about potentially unwanted downloads.

Exposes POTENTIALLY_UNWANTED ClientDownloadResponse verdict from the
SafeBrowsing service to ChromeDownloadManagerDelegate. Downloads for
which this verdict is seen will be marked with the danger type
DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED.

A dangerous download prompt will be displayed on the download shelf
indicating that the download may make changes to the browser or
system.

Changes are for Windows only since the POTENTIALLY_UNWANTED signal is
only implemented for Windows.

BUG=262983
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213289

[asanka, 2013-07-22 20:06:02]

A screenshot of the warning is at
https://code.google.com/p/chromium/issues/detail?id=262983#c3.

jam:
  chrome/browser/automation/
  content/public/browser/download_danger_type.h

benjhayden:
  content/browser/download/*
  chrome/browser/download/*
  chrome/browser/extensions/api/downloads/
  chrome/browser/history/

jhawkins:
  chrome/browser/resources/downloads/
  chrome/browser/ui/webui/

mattm:
  chrome/browser/safe_browsing/
  net/

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

jar:
  tools/metrics/histograms/histograms.xml

[benjhayden, 2013-07-22 20:16:18]

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/c...
File chrome/browser/download/chrome_download_manager_delegate.cc (right):

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/c...
chrome/browser/download/chrome_download_manager_delegate.cc:544: case
DownloadProtectionService::POTENTIALLY_UNWANTED:
Maybe collapse these cases with item->OnContentCheckCompleted(result)?

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/d...
File chrome/browser/download/download_item_model.cc (right):

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/d...
chrome/browser/download/download_item_model.cc:335:
ui::ElideFilename(download_->GetFileNameToReportUser(),
pre-compute elided_filename to reduce code duplication?

https://codereview.chromium.org/19863005/diff/14001/chrome/common/extensions/...
File chrome/common/extensions/api/downloads.idl (right):

https://codereview.chromium.org/19863005/diff/14001/chrome/common/extensions/...
chrome/common/extensions/api/downloads.idl:79: //     <dt>unwanted</dt>
"unwanted" is very vague.
How about "modifiesSettings"?

[sky, 2013-07-22 21:19:49]

LGTM

[jam, 2013-07-22 23:35:01]

lgtm

[mattm, 2013-07-22 23:44:43]

safe_browsing and net lgtm

[James Hawkins, 2013-07-23 00:52:02]

lgtm

[mattm, 2013-07-23 01:03:36]

oh, I might suggest changing the commit message though.  "Implement support for
potentially unwanted downloads." sounds like we will now support downloading
unwanted things

[jar (doing other things), 2013-07-23 03:06:29]

histograms.xml LGTM

[asanka, 2013-07-23 15:10:36]

Thanks everyone!

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/c...
File chrome/browser/download/chrome_download_manager_delegate.cc (right):

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/c...
chrome/browser/download/chrome_download_manager_delegate.cc:544: case
DownloadProtectionService::POTENTIALLY_UNWANTED:
On 2013/07/22 20:16:19, benjhayden_chromium wrote:
> Maybe collapse these cases with item->OnContentCheckCompleted(result)?

Done.

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/d...
File chrome/browser/download/download_item_model.cc (right):

https://codereview.chromium.org/19863005/diff/14001/chrome/browser/download/d...
chrome/browser/download/download_item_model.cc:335:
ui::ElideFilename(download_->GetFileNameToReportUser(),
On 2013/07/22 20:16:19, benjhayden_chromium wrote:
> pre-compute elided_filename to reduce code duplication?

Done.

https://codereview.chromium.org/19863005/diff/14001/chrome/common/extensions/...
File chrome/common/extensions/api/downloads.idl (right):

https://codereview.chromium.org/19863005/diff/14001/chrome/common/extensions/...
chrome/common/extensions/api/downloads.idl:79: //     <dt>unwanted</dt>
On 2013/07/22 20:16:19, benjhayden_chromium wrote:
> "unwanted" is very vague.
> How about "modifiesSettings"?

This is intentionally vague. Chrome gets this signal from the SafeBrowsing
service. The service currently generates this signal based on whether a binary
changes browser or system settings. This is likely to change in the future.

In general, I'm not sure whether the danger type constants should be part of the
(imminently stabilizing) extensions API. The danger types have changed in the
past and will probably change in number and meaning in the future. I don't think
each such change should require a change to a stable extensions API.

I can change this to potentiallyUnwanted so that it's clear that this is a fuzzy
signal. Or we could fold this into 'file'. I'll upload a new patchset once we
figure out what to do here.

[asanka, 2013-07-23 15:42:18]

Updated the enum description for downloads.idl based on OOB discussion with
benjhayden.

[asanka, 2013-07-23 15:46:53]

On 2013/07/23 01:03:36, mattm wrote:
> oh, I might suggest changing the commit message though.  "Implement support
for
> potentially unwanted downloads." sounds like we will now support downloading
> unwanted things

Haha. Yes. Updated the commit message.

[benjhayden, 2013-07-23 15:49:00]

lgtm

[commit-bot: I haz the power, 2013-07-23 20:07:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/19863005/56002

[commit-bot: I haz the power, 2013-07-23 21:03:23]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-23 21:24:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/19863005/56002

[commit-bot: I haz the power, 2013-07-24 00:16:43]

Change committed as 213289