Rename SecurityStyleExplanations and WebSecurityStyle fields

Rename SecurityStyleExplanations and WebSecurityStyle fields

This CL renames various SecurityStyleExplanations fields and WebSecurityStyle
values to be more accurate and consistent with the rest of the codebase.
- unauthenticated => neutral
- authenticated => secure (a page can be fully authenticated but not marked with
  the state that was formerly called authenticated, e.g. because of Safe
  Browsing checks)
- authentication broken => insecure

BUG=703882
Review-Url: https://codereview.chromium.org/2770703002
Cr-Commit-Position: refs/heads/master@{#459559}
Committed: https://chromium.googlesource.com/chromium/src/+/90ff901e2d8127ff990319818f93de838fcd8990

[estark, 2017-03-22 17:30:01]

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

[commit-bot: I haz the power, 2017-03-22 17:30:32]

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

[estark, 2017-03-22 17:32:27]

estark@chromium.org changed reviewers:
+ elawrence@chromium.org

[estark, 2017-03-22 17:32:28]

elawrence, PTAL and let me know if these new names/comments make sense to you?
Thanks!

https://codereview.chromium.org/2770703002/diff/1/content/child/web_url_loade...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/2770703002/diff/1/content/child/web_url_loade...
content/child/web_url_loader_impl.cc:267: // security info (such as a
certificate) attached. Use WebSecurityStyleUnknown
(unrelated comment cleanup while I was here)

https://codereview.chromium.org/2770703002/diff/1/content/public/browser/secu...
File content/public/browser/security_style_explanations.h (right):

https://codereview.chromium.org/2770703002/diff/1/content/public/browser/secu...
content/public/browser/security_style_explanations.h:72: // Explanations
corresponding to each security level.
(removed the instruction about how to display explanations because I think that
should be really be up to the UI layer, not this code's concern)

[commit-bot: I haz the power, 2017-03-22 17:40:38]

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

[commit-bot: I haz the power, 2017-03-22 17:40:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[estark, 2017-03-22 17:49:27]

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

[commit-bot: I haz the power, 2017-03-22 17:50: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, 2017-03-22 19:32:05]

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

[commit-bot: I haz the power, 2017-03-22 19:32:06]

Dry run: This issue passed the CQ dry run.

[elawrence, 2017-03-22 19:43:07]

LGTM, I think this is a lot clearer, thank you!

https://codereview.chromium.org/2770703002/diff/20001/chrome/browser/ssl/secu...
File chrome/browser/ssl/security_state_tab_helper_browser_tests.cc (right):

https://codereview.chromium.org/2770703002/diff/20001/chrome/browser/ssl/secu...
chrome/browser/ssl/security_state_tab_helper_browser_tests.cc:1834:
DidChangeVisibleSecurityStateObserverNonsecureConnection) {
The name of this test seems a bit misleading, especially now that we've changed
WSSAuthenticated to WSSSecure. Maybe rename to DCVSSOObsoleteTLSProtocol or
something?

https://codereview.chromium.org/2770703002/diff/20001/content/public/browser/...
File content/public/browser/security_style_explanations.h (right):

https://codereview.chromium.org/2770703002/diff/20001/content/public/browser/...
content/public/browser/security_style_explanations.h:27: // severity levels.
This comment remains a bit confusing, partly because we start by talking about a
"page" and end up talking about a "site." Fixing that word alone is probably
sufficient, but maybe we could provide a little more context? 
 
SecurityStyleExplanations provide context for why the specific SecurityStyle was
chosen for the page. 

Each page has a single SecurityStyle, which is chosen based on factors like
whether the page was delivered over HTTPS with a valid certificate, is free of
mixed-content, does not use a deprecated protocol, and is not flagged as
dangerous. 

Each factor that impacts the SecurityStyle has an accompanying
SecurityStyleExplanation that contains a human-readable explanation of the
factor. A single page may contain multiple explanations, each of which may have
a different severity level ("secure", "warning", "insecure" and "info").


---
I think part of the reason that this is so wordy is that it's written to be
generic (despite the primary use within the Developer Tools). But we probably
can just say "secure protocol" instead of "cryptographically secure transport",
etc.

[estark, 2017-03-22 20:40:55]

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

[estark, 2017-03-22 20:41:28]

estark@chromium.org changed reviewers:
+ nparker@chromium.org, pfeldman@chromium.org

[estark, 2017-03-22 20:41:29]

Thanks Eric!

+pfeldman for content/ and third_party/WebKit
+nparker for chrome/browser/safe_browsing

https://codereview.chromium.org/2770703002/diff/20001/chrome/browser/ssl/secu...
File chrome/browser/ssl/security_state_tab_helper_browser_tests.cc (right):

https://codereview.chromium.org/2770703002/diff/20001/chrome/browser/ssl/secu...
chrome/browser/ssl/security_state_tab_helper_browser_tests.cc:1834:
DidChangeVisibleSecurityStateObserverNonsecureConnection) {
On 2017/03/22 19:43:07, elawrence wrote:
> The name of this test seems a bit misleading, especially now that we've
changed
> WSSAuthenticated to WSSSecure. Maybe rename to DCVSSOObsoleteTLSProtocol or
> something?

Done.

https://codereview.chromium.org/2770703002/diff/20001/content/public/browser/...
File content/public/browser/security_style_explanations.h (right):

https://codereview.chromium.org/2770703002/diff/20001/content/public/browser/...
content/public/browser/security_style_explanations.h:27: // severity levels.
On 2017/03/22 19:43:07, elawrence wrote:
> This comment remains a bit confusing, partly because we start by talking about
a
> "page" and end up talking about a "site." Fixing that word alone is probably
> sufficient, but maybe we could provide a little more context? 
>  
> SecurityStyleExplanations provide context for why the specific SecurityStyle
was
> chosen for the page. 
> 
> Each page has a single SecurityStyle, which is chosen based on factors like
> whether the page was delivered over HTTPS with a valid certificate, is free of
> mixed-content, does not use a deprecated protocol, and is not flagged as
> dangerous. 
> 
> Each factor that impacts the SecurityStyle has an accompanying
> SecurityStyleExplanation that contains a human-readable explanation of the
> factor. A single page may contain multiple explanations, each of which may
have
> a different severity level ("secure", "warning", "insecure" and "info").
> 
> 
> ---
> I think part of the reason that this is so wordy is that it's written to be
> generic (despite the primary use within the Developer Tools). But we probably
> can just say "secure protocol" instead of "cryptographically secure
transport",
> etc.

Done, thanks for the suggested wording.

[commit-bot: I haz the power, 2017-03-22 20:41:53]

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

[Nathan Parker, 2017-03-22 20:47:46]

safe_browsing/* LGTM

[commit-bot: I haz the power, 2017-03-22 22:28:39]

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

[commit-bot: I haz the power, 2017-03-22 22:28:40]

Dry run: This issue passed the CQ dry run.

[estark, 2017-03-24 17:51:34]

pfeldman: friendly ping

[pfeldman, 2017-03-24 17:56:35]

lgtm

[estark, 2017-03-24 17:58:27]

The CQ bit was checked by estark@chromium.org

[estark, 2017-03-24 17:58:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
elawrence@chromium.org
Link to the patchset: https://codereview.chromium.org/2770703002/#ps40001
(title: "elawrence comments")

[commit-bot: I haz the power, 2017-03-24 17:58:57]

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, 2017-03-24 21:34:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490378307549890,
"parent_rev": "7ee130f7b0f2780cba2af28fd7c9f85efd72b867", "commit_rev":
"90ff901e2d8127ff990319818f93de838fcd8990"}

[commit-bot: I haz the power, 2017-03-24 21:35:15]

Description was changed from

==========
Rename SecurityStyleExplanations and WebSecurityStyle fields

This CL renames various SecurityStyleExplanations fields and WebSecurityStyle
values to be more accurate and consistent with the rest of the codebase.
- unauthenticated => neutral
- authenticated => secure (a page can be fully authenticated but not marked with
  the state that was formerly called authenticated, e.g. because of Safe
  Browsing checks)
- authentication broken => insecure

BUG=703882
==========

to

==========
Rename SecurityStyleExplanations and WebSecurityStyle fields

This CL renames various SecurityStyleExplanations fields and WebSecurityStyle
values to be more accurate and consistent with the rest of the codebase.
- unauthenticated => neutral
- authenticated => secure (a page can be fully authenticated but not marked with
  the state that was formerly called authenticated, e.g. because of Safe
  Browsing checks)
- authentication broken => insecure

BUG=703882

Review-Url: https://codereview.chromium.org/2770703002
Cr-Commit-Position: refs/heads/master@{#459559}
Committed:
https://chromium.googlesource.com/chromium/src/+/90ff901e2d8127ff990319818f93...
==========

[commit-bot: I haz the power, 2017-03-24 21:35:18]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/90ff901e2d8127ff990319818f93...