Change Dismiss button to Discard and cancel download

Change Dismiss button to Discard and cancel download

For malicious downloads, change the Dismiss button in DownloadItemView
to Discard button and cancel download when user clicks on it.

BUG=594596
Committed: https://crrev.com/388b9dc52751cac09f531aef419c02d642e9bb9c
Cr-Commit-Position: refs/heads/master@{#407324}

[Jialiu Lin, 2016-07-19 17:59:24]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 18:00:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 18:57:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-19 18:57:04]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2016-07-19 21:39:08]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 21:40:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-07-19 22:11:16]

jialiul@chromium.org changed reviewers:
+ holte@chromium.org, msw@chromium.org, rsesek@chromium.org

[Jialiu Lin, 2016-07-19 22:11:17]

As discussed in https://crbug.com/594596, we decided to change the "Dismiss"
button of malicious download warning into a "Discard" button, and cancel the
download when user clicks on it.
This change will not impact the download feedback service, but will deprecate
the clickjacking.dismiss_download UMA metric. 

+msw@chromium.org  for chrome/browser/ui/views/download/download_item_view* 
(WIN and Linux)
+rsesek@chromium.org for chrome/app/nibs/DownloadItem.xib (Mac)
+holte@chromium.org for minor change in histograms.xml


Thanks!

[commit-bot: I haz the power, 2016-07-20 00:10:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 00:10:06]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Robert Sesek, 2016-07-20 18:37:11]

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
File chrome/app/nibs/DownloadItem.xib (right):

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
chrome/app/nibs/DownloadItem.xib:101: <action selector="discardDownload:"
target="-2" id="114"/>
This is a method name but it hasn't been renamed in code. Did you test this?

[Jialiu Lin, 2016-07-20 19:05:34]

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
File chrome/app/nibs/DownloadItem.xib (right):

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
chrome/app/nibs/DownloadItem.xib:101: <action selector="discardDownload:"
target="-2" id="114"/>
On 2016/07/20 at 18:37:11, Robert Sesek wrote:
> This is a method name but it hasn't been renamed in code. Did you test this?

Yes, tested on Mac build. dismissMaliciousDownload is an existing function:
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/download/downloa...

[Robert Sesek, 2016-07-20 19:57:58]

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
File chrome/app/nibs/DownloadItem.xib (right):

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
chrome/app/nibs/DownloadItem.xib:101: <action selector="discardDownload:"
target="-2" id="114"/>
On 2016/07/20 19:05:33, Jialiu Lin wrote:
> On 2016/07/20 at 18:37:11, Robert Sesek wrote:
> > This is a method name but it hasn't been renamed in code. Did you test this?
> 
> Yes, tested on Mac build. dismissMaliciousDownload is an existing function:
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/download/downloa...

Right, I was expecting dismissMaliciousDownload: to either be renamed in the
.h/mm file or removed. It looks like the XIB was the only caller.

[Jialiu Lin, 2016-07-20 20:05:59]

dismissMaliciousDownload function removed from download_item_controller.h/mm.
Thanks for catching this.

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
File chrome/app/nibs/DownloadItem.xib (right):

https://codereview.chromium.org/2164723002/diff/20001/chrome/app/nibs/Downloa...
chrome/app/nibs/DownloadItem.xib:101: <action selector="discardDownload:"
target="-2" id="114"/>
On 2016/07/20 at 19:57:58, Robert Sesek wrote:
> On 2016/07/20 19:05:33, Jialiu Lin wrote:
> > On 2016/07/20 at 18:37:11, Robert Sesek wrote:
> > > This is a method name but it hasn't been renamed in code. Did you test
this?
> > 
> > Yes, tested on Mac build. dismissMaliciousDownload is an existing function:
> >
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/download/downloa...
> 
> Right, I was expecting dismissMaliciousDownload: to either be renamed in the
.h/mm file or removed. It looks like the XIB was the only caller.
Oops, mistake in my previous reply. I actually meant "discardDownload" is an
existing function. 
And you're right, "dissmissMaliciousDownload" function is no longer needed.
Removed it from download_item_controller.h/mm

[Jialiu Lin, 2016-07-20 20:06:44]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-20 20:07:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Robert Sesek, 2016-07-20 20:14:10]

lgtm

[msw, 2016-07-20 20:24:32]

https://codereview.chromium.org/2164723002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/download/download_item_view_md.cc (left):

https://codereview.chromium.org/2164723002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/download/download_item_view_md.cc:979:
model_.IsMalicious() ? IDS_DISMISS_DOWNLOAD : IDS_DISCARD_DOWNLOAD;
Remove IDS_DISMISS_DOWNLOAD from chrome/app/generated_resources.grd

[Jialiu Lin, 2016-07-20 20:30:02]

Thanks, msw@!

https://codereview.chromium.org/2164723002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/download/download_item_view_md.cc (left):

https://codereview.chromium.org/2164723002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/download/download_item_view_md.cc:979:
model_.IsMalicious() ? IDS_DISMISS_DOWNLOAD : IDS_DISCARD_DOWNLOAD;
On 2016/07/20 at 20:24:32, msw wrote:
> Remove IDS_DISMISS_DOWNLOAD from chrome/app/generated_resources.grd

Done.

[msw, 2016-07-20 20:37:54]

lgtm

[Steven Holte, 2016-07-20 23:50:15]

histograms lgtm

[Jialiu Lin, 2016-07-21 02:08:52]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2016-07-21 02:08:52]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/2164723002/#ps60001
(title: "remove IDS_DISMISS_DOWNLOAD from generated_resources.grd")

[Jialiu Lin, 2016-07-21 02:09:09]

The CQ bit was unchecked by jialiul@chromium.org

[commit-bot: I haz the power, 2016-07-21 02:09:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2016-07-21 02:09:21]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 02:09:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 03:25:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-21 03:25:36]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2016-07-22 23:50:54]

The CQ bit was checked by jialiul@chromium.org

[commit-bot: I haz the power, 2016-07-22 23:51:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-23 00:59:19]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-23 01:02:29]

Description was changed from

==========
Change Dismiss button to Discard and cancel download

For malicious downloads, change the Dismiss button in DownloadItemView
to Discard button and cancel download when user clicks on it.

BUG=594596
==========

to

==========
Change Dismiss button to Discard and cancel download

For malicious downloads, change the Dismiss button in DownloadItemView
to Discard button and cancel download when user clicks on it.

BUG=594596

Committed: https://crrev.com/388b9dc52751cac09f531aef419c02d642e9bb9c
Cr-Commit-Position: refs/heads/master@{#407324}
==========

[commit-bot: I haz the power, 2016-07-23 01:02:31]

Patchset 4 (id:??) landed as
https://crrev.com/388b9dc52751cac09f531aef419c02d642e9bb9c
Cr-Commit-Position: refs/heads/master@{#407324}