Record download target connection security state

Record download target connection security state

As part of HTTP-bad effort, we want to learn about and then indicate the
insecure state of HTTP downloads. This CL add UMA metric to record the
connection security state of download target, indicating whether the final
resolved download url and the redirects before it are secure or not.

BUG=645521
Committed: https://crrev.com/82b8e5071d4088fc18daf80f6f49047ef5a6ff5e
Cr-Commit-Position: refs/heads/master@{#422148}

[Jialiu Lin, 2016-09-29 00:15:57]

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

[commit-bot: I haz the power, 2016-09-29 00:16:18]

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-09-29 02:02:35]

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

[commit-bot: I haz the power, 2016-09-29 02:02:36]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2016-09-29 17:26:14]

Description was changed from

==========
record download target connection security state

BUG=645521
==========

to

==========
Record download target connection security state

As part of HTTP-bad effort, we want to learn about and then indicate the
insecure state of HTTP downloads. This CL add UMA metric to record the
connection security state of download target, indicating whether the final
resolved download url and the redirects before it are secure or not.

BUG=645521
==========

[Jialiu Lin, 2016-09-29 17:27:15]

jialiul@chromium.org changed reviewers:
+ asanka@chromium.org, holte@chromium.org

[Jialiu Lin, 2016-09-29 17:27:16]

asanka@, holte@, could you take a look? Thanks!

[Steven Holte, 2016-09-29 20:10:33]

lgtm

[asanka, 2016-09-29 21:13:40]

lgtm % nits and a question

https://codereview.chromium.org/2378333002/diff/40001/content/browser/downloa...
File content/browser/download/download_manager_impl.cc (right):

https://codereview.chromium.org/2378333002/diff/40001/content/browser/downloa...
content/browser/download/download_manager_impl.cc:131: if
(download_url.SchemeIsHTTPOrHTTPS()) {
Do we care cross-scheme redirects? E.g. things like HTTP -> FTP?

https://codereview.chromium.org/2378333002/diff/40001/content/browser/downloa...
content/browser/download/download_manager_impl.cc:136: if
(url_chain[i].SchemeIs(url::kHttpScheme)) {
!url.SchemeIsCryptographic() ?

[Jialiu Lin, 2016-09-29 21:29:29]

Thanks, asanka@!

https://codereview.chromium.org/2378333002/diff/40001/content/browser/downloa...
File content/browser/download/download_manager_impl.cc (right):

https://codereview.chromium.org/2378333002/diff/40001/content/browser/downloa...
content/browser/download/download_manager_impl.cc:131: if
(download_url.SchemeIsHTTPOrHTTPS()) {
On 2016/09/29 at 21:13:40, asanka wrote:
> Do we care cross-scheme redirects? E.g. things like HTTP -> FTP?

If the final download url is not http or https, then we record it as
DOWNLOAD_NONE_HTTPX no matter how redirects looks like.

https://codereview.chromium.org/2378333002/diff/40001/content/browser/downloa...
content/browser/download/download_manager_impl.cc:136: if
(url_chain[i].SchemeIs(url::kHttpScheme)) {
On 2016/09/29 at 21:13:40, asanka wrote:
> !url.SchemeIsCryptographic() ?

Ah, yes, that's better. Fixed. Thanks!.

[Jialiu Lin, 2016-09-30 16:12:43]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2016-09-30 16:12:44]

The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org,
holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2378333002/#ps60001
(title: "address asanka's comments")

[commit-bot: I haz the power, 2016-09-30 16:13:02]

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-09-30 17:43:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-30 17:45:42]

Description was changed from

==========
Record download target connection security state

As part of HTTP-bad effort, we want to learn about and then indicate the
insecure state of HTTP downloads. This CL add UMA metric to record the
connection security state of download target, indicating whether the final
resolved download url and the redirects before it are secure or not.

BUG=645521
==========

to

==========
Record download target connection security state

As part of HTTP-bad effort, we want to learn about and then indicate the
insecure state of HTTP downloads. This CL add UMA metric to record the
connection security state of download target, indicating whether the final
resolved download url and the redirects before it are secure or not.

BUG=645521

Committed: https://crrev.com/82b8e5071d4088fc18daf80f6f49047ef5a6ff5e
Cr-Commit-Position: refs/heads/master@{#422148}
==========

[commit-bot: I haz the power, 2016-09-30 17:45:42]

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