Remove the WARNING security level.

Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - blink::WebURLResponse::SecurityStyleWarning
  - blink::ResourceResponse::SecurityStyleWarning
  - Generated for the browser process: content::devtools::security::kSecurityStateWarning

BUG=645698

[lgarron, 2016-09-10 00:40:12]

Description was changed from

==========
Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - Generated for the browser process:
content::devtools::security::kSecurityStateWarning

BUG=
==========

to

==========
Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - Generated for the browser process:
content::devtools::security::kSecurityStateWarning

BUG=645698
==========

[lgarron, 2016-09-10 00:46:08]

Patchset #1 (id:1) has been deleted

[lgarron, 2016-09-10 00:46:21]

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

[commit-bot: I haz the power, 2016-09-10 00:46:48]

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-10 00:59:00]

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

[commit-bot: I haz the power, 2016-09-10 00:59:01]

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

[lgarron, 2016-09-10 06:58:14]

Description was changed from

==========
Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - Generated for the browser process:
content::devtools::security::kSecurityStateWarning

BUG=645698
==========

to

==========
Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - blink::WebURLResponse::SecurityStyleWarning
  - blink::ResourceResponse::SecurityStyleWarning
  - Generated for the browser process:
content::devtools::security::kSecurityStateWarning

BUG=645698
==========

[lgarron, 2016-09-10 07:07:01]

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

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

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-10 08:43:09]

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

[commit-bot: I haz the power, 2016-09-10 08:43:10]

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...)

[lgarron, 2016-09-12 19:29:39]

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

[commit-bot: I haz the power, 2016-09-12 19:30:15]

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-12 19:43:39]

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

[commit-bot: I haz the power, 2016-09-12 19:43:40]

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

[lgarron, 2016-09-12 20:31:56]

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

[lgarron, 2016-09-12 20:32:02]

Patchset #3 (id:60001) has been deleted

[commit-bot: I haz the power, 2016-09-12 20:32:19]

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-12 22:35:24]

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

[commit-bot: I haz the power, 2016-09-12 22:35:25]

Dry run: This issue passed the CQ dry run.

[lgarron, 2016-09-12 22:36:56]

rsleevi@: You're not an OWNER for most of this, but since estark@ is out I think
you know the most. Does this look alright to you, or am I missing any risks or
implications?

[Ryan Sleevi, 2016-09-12 23:23:07]

I'm really swamped this week, so I don't anticipate reviewing this before
Emily's return.

[lgarron, 2016-09-14 00:03:00]

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

[lgarron, 2016-09-14 00:03:01]

estark@, could you review this #yolo-sized commit?

[estark, 2016-09-14 01:21:54]

I have a question (inline) about whether we really should map
SECURITY_POLICY_WARNING to SECURITY_STYLE_UNAUTHENTICATED. Unfortunately I don't
know what to do instead. :( I wonder if we should keep SECURITY_STYLE_WARNING
and handle it properly in devtools instead of deleting it? (Right now it looks
to me like we just ignore it in devtools, which is definitely sad.)

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java
(right):

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java:200:
public void testShortWarningHTTPSUrl() {
This rename looks like it changed the wrong thing, maybe? Should it be
testVeryShortInsecureHTTPSUrl()?

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappVisibilityTest.java
(left):

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappVisibilityTest.java:49:
assertTrue(shouldShowTopControls(WEBAPP_URL, "http://sub.originalwebsite.com",
I'm wondering if having a test for sub.originalwebsite.com was significant here,
to test that the top controls are shown for nonsecure subdomains in addition to
unrelated nonsecure URLs. i.e. maybe you should keep this test case but change
it to SECURITY_ERROR instead of SECURITY_WARNING.

https://codereview.chromium.org/2329153002/diff/80001/chrome/browser/ssl/chro...
File chrome/browser/ssl/chrome_security_state_model_client.cc (right):

https://codereview.chromium.org/2329153002/diff/80001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:72: return
content::SECURITY_STYLE_UNAUTHENTICATED;
Hrmm, this means that we'll show "This page is not secure" in devtools for this
state. That seems confusing.

On the other hand, right now it looks like we don't show anything at all for
this state. :(

(https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...)

[estark, 2016-09-14 12:12:44]

On 2016/09/14 01:21:54, estark wrote:
> I have a question (inline) about whether we really should map
> SECURITY_POLICY_WARNING to SECURITY_STYLE_UNAUTHENTICATED. Unfortunately I
don't
> know what to do instead. :( I wonder if we should keep SECURITY_STYLE_WARNING
> and handle it properly in devtools instead of deleting it? (Right now it looks
> to me like we just ignore it in devtools, which is definitely sad.)

Thinking about this some more, because jetlag. Another possibility might be to
map SECURITY_POLICY_WARNING to SECURITY_STYLE_AUTHENTICATED for devtools, and
add an info or warning explanation to explain that an admin-provided cert is in
use. To me that seems to best capture what we want to communicate: the page is
secure, as long as you trust your admin-provided cert.

> 
>
https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
> File
>
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java
> (right):
> 
>
https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
>
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java:200:
> public void testShortWarningHTTPSUrl() {
> This rename looks like it changed the wrong thing, maybe? Should it be
> testVeryShortInsecureHTTPSUrl()?
> 
>
https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
> File
>
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappVisibilityTest.java
> (left):
> 
>
https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
>
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappVisibilityTest.java:49:
> assertTrue(shouldShowTopControls(WEBAPP_URL, "http://sub.originalwebsite.com",
> I'm wondering if having a test for http://sub.originalwebsite.com was
significant here,
> to test that the top controls are shown for nonsecure subdomains in addition
to
> unrelated nonsecure URLs. i.e. maybe you should keep this test case but change
> it to SECURITY_ERROR instead of SECURITY_WARNING.
> 
>
https://codereview.chromium.org/2329153002/diff/80001/chrome/browser/ssl/chro...
> File chrome/browser/ssl/chrome_security_state_model_client.cc (right):
> 
>
https://codereview.chromium.org/2329153002/diff/80001/chrome/browser/ssl/chro...
> chrome/browser/ssl/chrome_security_state_model_client.cc:72: return
> content::SECURITY_STYLE_UNAUTHENTICATED;
> Hrmm, this means that we'll show "This page is not secure" in devtools for
this
> state. That seems confusing.
> 
> On the other hand, right now it looks like we don't show anything at all for
> this state. :(
> 
>
(https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...)

[lgarron, 2016-09-19 22:15:51]

I like the idea of cleaning up what we should for Chrome OS policy certs first.
Will try that after rpop@ is back (tomorrow) and can help me get enterprise test
accounts.

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java
(right):

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java:200:
public void testShortWarningHTTPSUrl() {
On 2016/09/14 at 01:21:54, estark wrote:
> This rename looks like it changed the wrong thing, maybe? Should it be
testVeryShortInsecureHTTPSUrl()?

My best guess is that the existing code used "very short" to mean 3 spans, while
"short" is 4 spans.

Broken HTTPS is 4 spans (as opposed to WARNING, which had 3 spans because the
scheme was not struck throught.)

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappVisibilityTest.java
(left):

https://codereview.chromium.org/2329153002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappVisibilityTest.java:49:
assertTrue(shouldShowTopControls(WEBAPP_URL, "http://sub.originalwebsite.com",
On 2016/09/14 at 01:21:54, estark wrote:
> I'm wondering if having a test for sub.originalwebsite.com was significant
here, to test that the top controls are shown for nonsecure subdomains in
addition to unrelated nonsecure URLs. i.e. maybe you should keep this test case
but change it to SECURITY_ERROR instead of SECURITY_WARNING.

Good idea; done.

[lgarron, 2016-09-19 22:16:26]

Ooh, just found
https://www.chromium.org/developers/how-tos/enterprise/running-the-cloud-poli...

[vinodsonkusare77_gmail.com, 2016-09-20 08:17:14]

https://youtu.be/yD6KioQhnAc

On Tue, Sep 20, 2016 at 3:46 AM, <lgarron@chromium.org> wrote:

> Ooh, just found
> https://www.chromium.org/developers/how-tos/enterprise/
> running-the-cloud-policy-test-server
>
> https://codereview.chromium.org/2329153002/
>
> --
> You received this message because you are subscribed to the Google Groups
> "DevTools Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to devtools-reviews+unsubscribe@chromium.org.
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[vinodsonkusare77_gmail.com, 2016-09-20 08:17:14]

https://youtu.be/yD6KioQhnAc

On Tue, Sep 20, 2016 at 3:46 AM, <lgarron@chromium.org> wrote:

> Ooh, just found
> https://www.chromium.org/developers/how-tos/enterprise/
> running-the-cloud-policy-test-server
>
> https://codereview.chromium.org/2329153002/
>
> --
> You received this message because you are subscribed to the Google Groups
> "DevTools Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to devtools-reviews+unsubscribe@chromium.org.
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[estark, 2016-09-20 17:26:08]

Description was changed from

==========
Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - blink::WebURLResponse::SecurityStyleWarning
  - blink::ResourceResponse::SecurityStyleWarning
  - Generated for the browser process:
content::devtools::security::kSecurityStateWarning

BUG=645698
==========

to

==========
Remove the WARNING security level.

- content::SECURITY_STYLE_WARNING
- SecurityStateModel::SECURITY_WARNING
  - Generated for Android: ConnectionSecurityLevel.SECURITY_WARNING
- Security[Agent].SecurityState.warning (DevTools)
  - blink::WebURLResponse::SecurityStyleWarning
  - blink::ResourceResponse::SecurityStyleWarning
  - Generated for the browser process:
content::devtools::security::kSecurityStateWarning

BUG=645698
==========

[estark, 2016-09-20 17:26:08]

estark@chromium.org changed reviewers:
- vinodsonkusare77@gmail.com

[estark, 2016-09-22 17:44:06]

Lucas, did you want me to look at this CL again, or are we waiting on enterprise
test accounts?

[lgarron, 2016-09-22 18:29:33]

On 2016/09/22 at 17:44:06, estark wrote:
> Lucas, did you want me to look at this CL again, or are we waiting on
enterprise test accounts?

Enterprise test accounts. I think we have instructions now, but I need to make
them. :-)