DevTools security panel: explain subresources with cert errors separately

DevTools security panel: explain subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
Committed: https://crrev.com/f61f3dfeb0af8a4b4d08b855689d73291dc31f8b
Cr-Commit-Position: refs/heads/master@{#415383}

[estark, 2016-08-26 00:55:39]

Description was changed from

==========
DevTools security panel: highlight subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

to

==========
DevTools security panel: highlight subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

[estark, 2016-08-26 01:39:42]

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

[estark, 2016-08-26 01:40:02]

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

[estark, 2016-08-26 01:40:02]

pfeldman, PTAL?

[commit-bot: I haz the power, 2016-08-26 01:40:14]

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

[estark, 2016-08-26 01:41:56]

Description was changed from

==========
DevTools security panel: highlight subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

to

==========
DevTools security panel: explain subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

[commit-bot: I haz the power, 2016-08-26 01:56:08]

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

[commit-bot: I haz the power, 2016-08-26 01:56:11]

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

[estark, 2016-08-26 02:11:18]

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

[commit-bot: I haz the power, 2016-08-26 02:11:49]

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-08-26 06:21:28]

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

[commit-bot: I haz the power, 2016-08-26 06:21:29]

Dry run: This issue passed the CQ dry run.

[pfeldman, 2016-08-26 17:20:29]

devtools and content lgtm, but you need someone to look at chrome.

[estark, 2016-08-26 17:23:23]

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

[estark, 2016-08-26 17:23:24]

Thanks pfeldman.

felt, could you please look at the changes in //chrome for another pair of eyes?

[felt, 2016-08-30 13:54:10]

Gahhhh sorry this got mislabeled in my filters, will review this morning.

[felt, 2016-08-30 16:28:52]

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

https://codereview.chromium.org/2286553002/diff/60001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:227: if
(!is_cert_status_error || is_cert_status_minor_error) {
should that be an && instead of an ||?

[estark, 2016-08-30 16:36:57]

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

https://codereview.chromium.org/2286553002/diff/60001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:227: if
(!is_cert_status_error || is_cert_status_minor_error) {
On 2016/08/30 16:28:52, felt wrote:
> should that be an && instead of an ||?

Nope, I think || is right. |is_cert_status_error| is true if there is any error
(major or minor). So this condition is supposed to be true if there were no cert
errors at all, or if there was only a minor cert error.

[felt, 2016-08-30 16:41:41]

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

https://codereview.chromium.org/2286553002/diff/60001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:227: if
(!is_cert_status_error || is_cert_status_minor_error) {
On 2016/08/30 16:36:57, estark wrote:
> On 2016/08/30 16:28:52, felt wrote:
> > should that be an && instead of an ||?
> 
> Nope, I think || is right. |is_cert_status_error| is true if there is any
error
> (major or minor). So this condition is supposed to be true if there were no
cert
> errors at all, or if there was only a minor cert error.

That's what I'm confused about -- you want this to be populated if there were no
cert errors at all? The comment says that this is for recording cert errors,
which doesn't make sense when there are no cert errors. Could you update the
comment?

[estark, 2016-08-30 17:27:58]

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

https://codereview.chromium.org/2286553002/diff/60001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:227: if
(!is_cert_status_error || is_cert_status_minor_error) {
On 2016/08/30 16:41:41, felt wrote:
> On 2016/08/30 16:36:57, estark wrote:
> > On 2016/08/30 16:28:52, felt wrote:
> > > should that be an && instead of an ||?
> > 
> > Nope, I think || is right. |is_cert_status_error| is true if there is any
> error
> > (major or minor). So this condition is supposed to be true if there were no
> cert
> > errors at all, or if there was only a minor cert error.
> 
> That's what I'm confused about -- you want this to be populated if there were
no
> cert errors at all? The comment says that this is for recording cert errors,
> which doesn't make sense when there are no cert errors. Could you update the
> comment?

Ahhh, ok, I think I understand the confusion now.
ran/displayed_content_with_cert_errors record the presence of subresources with
cert errors, but we only populate those fields if the main resource was loaded
without cert errors. I updated the comment to try to explain this better, lmk if
it makes sense or not.

[estark, 2016-08-30 17:34:26]

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

[commit-bot: I haz the power, 2016-08-30 17:34:54]

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

[felt, 2016-08-30 18:35:51]

cool thx, it seems clearer now. lgtm

[commit-bot: I haz the power, 2016-08-30 18:56:13]

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

[commit-bot: I haz the power, 2016-08-30 18:56:14]

Dry run: This issue passed the CQ dry run.

[estark, 2016-08-30 18:58:01]

The CQ bit was checked by estark@chromium.org

[estark, 2016-08-30 18:58:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2286553002/#ps80001
(title: "try to make comment more clear")

[commit-bot: I haz the power, 2016-08-30 18:58:34]

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-08-30 19:03:45]

Description was changed from

==========
DevTools security panel: explain subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

to

==========
DevTools security panel: explain subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

[commit-bot: I haz the power, 2016-08-30 19:03:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-30 19:06:25]

Description was changed from

==========
DevTools security panel: explain subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172
==========

to

==========
DevTools security panel: explain subresources with cert errors separately

The browser process used to record subresources that were loaded with
certificate errors as mixed content. This meant that DevTools wasn't
able to distinguish mixed content (subresources loaded over HTTP) from
subresources loaded with certificate errors. On a page that loaded
subresources with certificate errors, DevTools would show the mixed
content explanation, which was confusing/inaccurate because it described
"HTTP resources".

The commits in https://crbug.com/634171 created separate plumbing in the
browser process for subresources with certificate errors. This CL uses
that plumbing to distinguish these two cases for DevTools, and adds
separate bullets to the security panel when there are subresources with
cert errors on the page.

I have also renamed various instances of "insecure content" to "mixed
content" to be more precise: trying to use "insecure content" to mean
any type of insecure content, and "mixed content" to mean specifically
content loaded over HTTP on an HTTPS page. (There are still some more
places throughout Chrome that need to be cleaned up with this
terminology.)

(For subresources with cert errors, there is no link to the Network
panel with a filter like there is for mixed content; we can perhaps add
that later.)

BUG=587172

Committed: https://crrev.com/f61f3dfeb0af8a4b4d08b855689d73291dc31f8b
Cr-Commit-Position: refs/heads/master@{#415383}
==========

[commit-bot: I haz the power, 2016-08-30 19:06:26]

Patchset 5 (id:??) landed as
https://crrev.com/f61f3dfeb0af8a4b4d08b855689d73291dc31f8b
Cr-Commit-Position: refs/heads/master@{#415383}