Disable downloads from "file:" or "data:" URLs.

Disable downloads from "file:" or "data:" URLs.

BUG=119129
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128744

[Randy Smith (Not in Mondays), 2012-03-20 19:12:51]

Andy, could you give this a look?

[ahendrickson, 2012-03-20 19:56:35]

https://chromiumcodereview.appspot.com/9762002/diff/1/chrome/browser/download...
File chrome/browser/download/download_browsertest.cc (right):

https://chromiumcodereview.appspot.com/9762002/diff/1/chrome/browser/download...
chrome/browser/download/download_browsertest.cc:2490: for (size_t i = 0; i <
arraysize(urls); i++) {
Nit: Use ARRAYSIZE_UNSAFE()?

https://chromiumcodereview.appspot.com/9762002/diff/1/content/browser/downloa...
File content/browser/download/download_resource_handler.cc (right):

https://chromiumcodereview.appspot.com/9762002/diff/1/content/browser/downloa...
content/browser/download/download_resource_handler.cc:103:
CallStartedCB(download_id_, net::ERR_DISALLOWED_URL_SCHEME);
Should we be doing a test at the time when we determine that a link should be
downloaded?

[ahendrickson, 2012-03-21 19:47:45]

I withdraw the question about when CallStartedCB is called.

LGTM.

[Randy Smith (Not in Mondays), 2012-03-21 19:48:36]

http://codereview.chromium.org/9762002/diff/1/chrome/browser/download/downloa...
File chrome/browser/download/download_browsertest.cc (right):

http://codereview.chromium.org/9762002/diff/1/chrome/browser/download/downloa...
chrome/browser/download/download_browsertest.cc:2490: for (size_t i = 0; i <
arraysize(urls); i++) {
On 2012/03/20 19:56:35, ahendrickson wrote:
> Nit: Use ARRAYSIZE_UNSAFE()?

Andy and I talked offline, and agreed that by the comments in ARRAYSIZE_UNSAFE,
I should use arraysize() if I can.  And the code compiles and runs :-}.

http://codereview.chromium.org/9762002/diff/1/content/browser/download/downlo...
File content/browser/download/download_resource_handler.cc (right):

http://codereview.chromium.org/9762002/diff/1/content/browser/download/downlo...
content/browser/download/download_resource_handler.cc:103:
CallStartedCB(download_id_, net::ERR_DISALLOWED_URL_SCHEME);
On 2012/03/20 19:56:35, ahendrickson wrote:
> Should we be doing a test at the time when we determine that a link should be
> downloaded?

Andy and I talked offline, and agreed that doing it in a single place that any
download (either current or future) needed to go through was preferable to
having multiple test locations, even if that would avoid creating the
DownloadResourceHandler.  I may look into putting this in OnWillStart(), if that
can be made to work.

[Randy Smith (Not in Mondays), 2012-03-21 20:55:24]

Ben, could you take a look at test.js?  Sadrul, could you take a look at the
other chrome/test stuff?

[Randy Smith (Not in Mondays), 2012-03-21 20:56:19]

Sorry, repeating with reviewers actually added to the reviewers line :-}:

Ben, could you take a look at test.js?  Sadrul, could you take a look at the
other chrome/test stuff?

[sadrul, 2012-03-21 21:06:13]

LGTM

[benjhayden, 2012-03-22 13:30:40]

LGTM

https://chromiumcodereview.appspot.com/9762002/diff/11001/chrome/browser/down...
File chrome/browser/download/download_browsertest.cc (right):

https://chromiumcodereview.appspot.com/9762002/diff/11001/chrome/browser/down...
chrome/browser/download/download_browsertest.cc:2507: EXPECT_EQ(0u,
downloads.size());
Don't you also need to flush the UI message loop?
Does this expect fail without your change?

[commit-bot: I haz the power, 2012-03-24 13:44:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdsmith@chromium.org/9762002/21002

[commit-bot: I haz the power, 2012-03-24 16:11:02]

Change committed as 128744

[Aaron Boodman, 2012-03-28 22:30:06]

Can someone please cc me to crbug.com/119129?

[eligrey, 2016-01-29 00:47:33]

Can someone cc me to crbug.com/119129 as well?