Teach SSLHostStateDelegate about subresources with cert errors

Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171, 636986
Committed: https://crrev.com/854d78b541e9a8132f50985fc434d4d43c7417b1
Cr-Commit-Position: refs/heads/master@{#411543}

[estark, 2016-08-09 18:22:20]

Description was changed from

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

This is based on top of https://codereview.chromium.org/2224023003/.

BUG=634171
==========

to

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171
==========

[estark, 2016-08-11 16:02:59]

Description was changed from

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171
==========

to

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171,636986
==========

[estark, 2016-08-11 16:06:04]

estark@chromium.org changed reviewers:
+ jam@chromium.org, jww@chromium.org

[estark, 2016-08-11 16:06:04]

jam, this is #3/4.

Also sending to jww@ for SSLHostStateDelegate expertise.

[jww, 2016-08-11 18:50:16]

A few nits and clarifications. Thanks!

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
File chrome/browser/ssl/chrome_ssl_host_state_delegate.h (right):

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
chrome/browser/ssl/chrome_ssl_host_state_delegate.h:124:
std::set<BrokenHostEntry> ran_content_with_cert_errors_hosts_;
nit: Can you add a #include<set>? This should have been there already, but
apparently it's missing.

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
File chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc (right):

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc:235:
state->HostRanInsecureContent(
This looks identical to the set of tests right above it (starting at line 226).
Did you mean for this to be the reverse, where you mark a host as having run
mixed content, and verify that it's not marking it as having run cert error
content?

Even if I'm missing something about how this is different, you should still add
the test case I just mentioned anyway.

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
File content/browser/ssl/ssl_policy_backend.cc (right):

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:23: host, id,
SSLHostStateDelegate::MIXED_CONTENT);
Okay, this has me very confused. I think maybe that you're planning a later
patch or something, but can you explain to me why this only has the possibility
of marking mixed content, and not cert errors?

If this is just to simulate current behavior, then that's fine, but please add a
comment somewhere to clarify it, and add a TODO saying that you're going to fix
it.

If this code is only reachable by mixed content (although I don't think that's
the case), that's also fine, but then I'd like to see the methods renamed to
reflect that, since otherwise it gives the impression that this magically
handles all cases.

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:32: return
ssl_host_state_delegate_->DidHostRunInsecureContent(
This is especially confusing. I would expect this to check
DidHostRunInsecureContent for *both* MIXED_CONTENT and CERT_ERRORS_CONTENT.

[jam, 2016-08-11 19:53:13]

lgtm

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
File content/browser/ssl/ssl_policy_backend.cc (right):

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:23: host, id,
SSLHostStateDelegate::MIXED_CONTENT);
On 2016/08/11 18:50:15, jww wrote:
> Okay, this has me very confused. I think maybe that you're planning a later
> patch or something, but can you explain to me why this only has the
possibility
> of marking mixed content, and not cert errors?
> 
> If this is just to simulate current behavior, then that's fine, but please add
a
> comment somewhere to clarify it, and add a TODO saying that you're going to
fix
> it.
> 
> If this code is only reachable by mixed content (although I don't think that's
> the case), that's also fine, but then I'd like to see the methods renamed to
> reflect that, since otherwise it gives the impression that this magically
> handles all cases.

jww: since i got the next patch to review in this series,
https://codereview.chromium.org/2226363002/diff/1/content/browser/ssl/ssl_pol...
does what you suspect of adding new methods which when it lands will make this
easy to read i think

https://codereview.chromium.org/2225213004/diff/1/content/public/browser/ssl_...
File content/public/browser/ssl_host_state_delegate.h (right):

https://codereview.chromium.org/2225213004/diff/1/content/public/browser/ssl_...
content/public/browser/ssl_host_state_delegate.h:62: int pid,
nit: (in a followup if you agree), I just noticed that this code says "pid"
which is a bit misleading. the id here is usually called child_id in interfaces,
as opposed to a pid, because it's not the OS' process ID. it's chrome's unique
ID per process (i.e.
https://cs.chromium.org/chromium/src/content/public/browser/render_process_ho...)

[estark, 2016-08-11 20:58:38]

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
File chrome/browser/ssl/chrome_ssl_host_state_delegate.h (right):

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
chrome/browser/ssl/chrome_ssl_host_state_delegate.h:124:
std::set<BrokenHostEntry> ran_content_with_cert_errors_hosts_;
On 2016/08/11 18:50:15, jww wrote:
> nit: Can you add a #include<set>? This should have been there already, but
> apparently it's missing.

Done.

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
File chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc (right):

https://codereview.chromium.org/2225213004/diff/1/chrome/browser/ssl/chrome_s...
chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc:235:
state->HostRanInsecureContent(
On 2016/08/11 18:50:15, jww wrote:
> This looks identical to the set of tests right above it (starting at line
226).
> Did you mean for this to be the reverse, where you mark a host as having run
> mixed content, and verify that it's not marking it as having run cert error
> content?
> 
> Even if I'm missing something about how this is different, you should still
add
> the test case I just mentioned anyway.

As best as I can interpret past-Emily's intentions, I think the test case on
line 226 was supposed to be marking a MIXED_CONTENT site as CERT_ERRORS_CONTENT,
and this test case was supposed to be marking a non-MIXED_CONTENT site as
CERT_ERRORS_CONTENT.

I added some comments to each block to explain what it's doing, and added some
expectations to the test cases above to test the case you suggested (marking a
MIXED_CONTENT site should not marked it as CERT_ERRORS_CONTENT).

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
File content/browser/ssl/ssl_policy_backend.cc (right):

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:23: host, id,
SSLHostStateDelegate::MIXED_CONTENT);
On 2016/08/11 19:53:12, jam wrote:
> On 2016/08/11 18:50:15, jww wrote:
> > Okay, this has me very confused. I think maybe that you're planning a later
> > patch or something, but can you explain to me why this only has the
> possibility
> > of marking mixed content, and not cert errors?
> > 
> > If this is just to simulate current behavior, then that's fine, but please
add
> a
> > comment somewhere to clarify it, and add a TODO saying that you're going to
> fix
> > it.
> > 
> > If this code is only reachable by mixed content (although I don't think
that's
> > the case), that's also fine, but then I'd like to see the methods renamed to
> > reflect that, since otherwise it gives the impression that this magically
> > handles all cases.
> 
> jww: since i got the next patch to review in this series,
>
https://codereview.chromium.org/2226363002/diff/1/content/browser/ssl/ssl_pol...
> does what you suspect of adding new methods which when it lands will make this
> easy to read i think

Urgh, yeah, I tried to split up a giant CL into smallers CLs to make it easier
to review and might have gone too far to the other side of the line. :) Sorry
for the confusion, Joel. To explain a little more, this CL just gives
SSLHostStateDelegate the ability to track subresources with cert errors, but
doesn't actually tell SSLHostStateDelegate when a site has subresources with
cert errors.

The follow-up CL that jam linked to hooks everything up, so that when the
renderer tells the browser that it loaded a subresource with cert errors,
WebContents tells SSLManager who tells SSLPolicy who tells SSLPolicyBackend who
tells SSLHostStateDelegate that there's CONTENT_WITH_CERT_ERRORS.

Does that make any more sense? I'll add a TODO in SSLHostStateDelegate to
explain this as well. (The TODO will be done in the follow-up CL, but I'll add
it in case any people from the future are looking at this commit in isolation
and become similarly confused.)

(Also, next on my list of content/browser/ssl clean-up is to see if we can
collapse SSLManager, SSLPolicy, and SSLPolicyBackend, since AFAICT there's no
real reason for them to be split up and they all just kind of shuttle method
calls from one to the next.)

Finally, you raise a good point that these SSLManager/SSLPolicy/SSLPolicyBackend
methods should be renamed. Once this CL and the follow-up CL land,
HostRanInsecureContent() will handle only mixed content, not content with cert
errors, so it should be renamed to HostRanMixedContent(), and similarly for
DidHostRunInsecureContent(). I will file a bug for that but would like to do it
in a follow-up CL, if it's okay with you, since there's already so many moving
parts here.

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:32: return
ssl_host_state_delegate_->DidHostRunInsecureContent(
On 2016/08/11 18:50:15, jww wrote:
> This is especially confusing. I would expect this to check
> DidHostRunInsecureContent for *both* MIXED_CONTENT and CERT_ERRORS_CONTENT.

Same explanation as above, I think. In the follow-up CL, there's a separate
SSLPolicyBackend::DidHostRunContentWithCertErrors() method that passes
SSLHostStateDelegate::CONTENT_WITH_CERT_ERRORS.

https://codereview.chromium.org/2225213004/diff/1/content/public/browser/ssl_...
File content/public/browser/ssl_host_state_delegate.h (right):

https://codereview.chromium.org/2225213004/diff/1/content/public/browser/ssl_...
content/public/browser/ssl_host_state_delegate.h:62: int pid,
On 2016/08/11 19:53:13, jam wrote:
> nit: (in a followup if you agree), I just noticed that this code says "pid"
> which is a bit misleading. the id here is usually called child_id in
interfaces,
> as opposed to a pid, because it's not the OS' process ID. it's chrome's unique
> ID per process (i.e.
>
https://cs.chromium.org/chromium/src/content/public/browser/render_process_ho...)

That sounds reasonable, I will file a bug to come back and clean this up as soon
as I stop getting 500s from monorail :)

[jww, 2016-08-11 21:13:24]

lgtm

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
File content/browser/ssl/ssl_policy_backend.cc (right):

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:23: host, id,
SSLHostStateDelegate::MIXED_CONTENT);
On 2016/08/11 20:58:38, estark wrote:
> On 2016/08/11 19:53:12, jam wrote:
> > On 2016/08/11 18:50:15, jww wrote:
> > > Okay, this has me very confused. I think maybe that you're planning a
later
> > > patch or something, but can you explain to me why this only has the
> > possibility
> > > of marking mixed content, and not cert errors?
> > > 
> > > If this is just to simulate current behavior, then that's fine, but please
> add
> > a
> > > comment somewhere to clarify it, and add a TODO saying that you're going
to
> > fix
> > > it.
> > > 
> > > If this code is only reachable by mixed content (although I don't think
> that's
> > > the case), that's also fine, but then I'd like to see the methods renamed
to
> > > reflect that, since otherwise it gives the impression that this magically
> > > handles all cases.
> > 
> > jww: since i got the next patch to review in this series,
> >
>
https://codereview.chromium.org/2226363002/diff/1/content/browser/ssl/ssl_pol...
> > does what you suspect of adding new methods which when it lands will make
this
> > easy to read i think
> 
> Urgh, yeah, I tried to split up a giant CL into smallers CLs to make it easier
> to review and might have gone too far to the other side of the line. :) Sorry
> for the confusion, Joel. To explain a little more, this CL just gives
> SSLHostStateDelegate the ability to track subresources with cert errors, but
> doesn't actually tell SSLHostStateDelegate when a site has subresources with
> cert errors.
> 
> The follow-up CL that jam linked to hooks everything up, so that when the
> renderer tells the browser that it loaded a subresource with cert errors,
> WebContents tells SSLManager who tells SSLPolicy who tells SSLPolicyBackend
who
> tells SSLHostStateDelegate that there's CONTENT_WITH_CERT_ERRORS.
> 
> Does that make any more sense? I'll add a TODO in SSLHostStateDelegate to
> explain this as well. (The TODO will be done in the follow-up CL, but I'll add
> it in case any people from the future are looking at this commit in isolation
> and become similarly confused.)
> 
> (Also, next on my list of content/browser/ssl clean-up is to see if we can
> collapse SSLManager, SSLPolicy, and SSLPolicyBackend, since AFAICT there's no
> real reason for them to be split up and they all just kind of shuttle method
> calls from one to the next.)
> 
> Finally, you raise a good point that these
SSLManager/SSLPolicy/SSLPolicyBackend
> methods should be renamed. Once this CL and the follow-up CL land,
> HostRanInsecureContent() will handle only mixed content, not content with cert
> errors, so it should be renamed to HostRanMixedContent(), and similarly for
> DidHostRunInsecureContent(). I will file a bug for that but would like to do
it
> in a follow-up CL, if it's okay with you, since there's already so many moving
> parts here.

Great, thanks for the clarifications!

[jam, 2016-08-11 21:14:04]

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
File content/browser/ssl/ssl_policy_backend.cc (right):

https://codereview.chromium.org/2225213004/diff/1/content/browser/ssl/ssl_pol...
content/browser/ssl/ssl_policy_backend.cc:23: host, id,
SSLHostStateDelegate::MIXED_CONTENT);
On 2016/08/11 20:58:38, estark wrote:
> On 2016/08/11 19:53:12, jam wrote:
> > On 2016/08/11 18:50:15, jww wrote:
> > > Okay, this has me very confused. I think maybe that you're planning a
later
> > > patch or something, but can you explain to me why this only has the
> > possibility
> > > of marking mixed content, and not cert errors?
> > > 
> > > If this is just to simulate current behavior, then that's fine, but please
> add
> > a
> > > comment somewhere to clarify it, and add a TODO saying that you're going
to
> > fix
> > > it.
> > > 
> > > If this code is only reachable by mixed content (although I don't think
> that's
> > > the case), that's also fine, but then I'd like to see the methods renamed
to
> > > reflect that, since otherwise it gives the impression that this magically
> > > handles all cases.
> > 
> > jww: since i got the next patch to review in this series,
> >
>
https://codereview.chromium.org/2226363002/diff/1/content/browser/ssl/ssl_pol...
> > does what you suspect of adding new methods which when it lands will make
this
> > easy to read i think
> 
> Urgh, yeah, I tried to split up a giant CL into smallers CLs to make it easier
> to review and might have gone too far to the other side of the line. :) Sorry
> for the confusion, Joel. To explain a little more, this CL just gives
> SSLHostStateDelegate the ability to track subresources with cert errors, but
> doesn't actually tell SSLHostStateDelegate when a site has subresources with
> cert errors.
> 
> The follow-up CL that jam linked to hooks everything up, so that when the
> renderer tells the browser that it loaded a subresource with cert errors,
> WebContents tells SSLManager who tells SSLPolicy who tells SSLPolicyBackend
who
> tells SSLHostStateDelegate that there's CONTENT_WITH_CERT_ERRORS.
> 
> Does that make any more sense? I'll add a TODO in SSLHostStateDelegate to
> explain this as well. (The TODO will be done in the follow-up CL, but I'll add
> it in case any people from the future are looking at this commit in isolation
> and become similarly confused.)
> 
> (Also, next on my list of content/browser/ssl clean-up is to see if we can
> collapse SSLManager, SSLPolicy, and SSLPolicyBackend, since AFAICT there's no
> real reason for them to be split up and they all just kind of shuttle method
> calls from one to the next.)

That would be really great for readability. I've been confused about the
differences between them.

> 
> Finally, you raise a good point that these
SSLManager/SSLPolicy/SSLPolicyBackend
> methods should be renamed. Once this CL and the follow-up CL land,
> HostRanInsecureContent() will handle only mixed content, not content with cert
> errors, so it should be renamed to HostRanMixedContent(), and similarly for
> DidHostRunInsecureContent(). I will file a bug for that but would like to do
it
> in a follow-up CL, if it's okay with you, since there's already so many moving
> parts here.

[estark, 2016-08-12 02:48:05]

The CQ bit was checked by estark@chromium.org

[estark, 2016-08-12 02:48:05]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
jww@chromium.org
Link to the patchset: https://codereview.chromium.org/2225213004/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-12 02:48:19]

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-12 03:31:11]

Description was changed from

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171,636986
==========

to

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171,636986
==========

[commit-bot: I haz the power, 2016-08-12 03:31:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-12 03:33:01]

Description was changed from

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171,636986
==========

to

==========
Teach SSLHostStateDelegate about subresources with cert errors

This is the third in a series of CLs to create dedicated plumbing for
subresources with certificate errors, instead of treating them like
mixed content from the browser's perspective.

This CL teaches SSLHostStateDelegate (and its implementation
ChromeSSLHostStateDelegate) about subresources with certificate errors,
as distinct from mixed content. ChromeSSLHostStateDelegate keeps two
maps of broken hosts, one for mixed content and one for subresources
with cert errors. HostRanInsecureContent() and
DidHostRunInsecureContent() take an argument to indicate whether the
subresource in question was mixed or had cert errors. (An alternative
would be to have a separate set of methods for cert errors; I could be
convinced either way.)

#1: https://codereview.chromium.org/2224193003/
  Rename SecurityStateModel::MIXED_CONTENT_STATUS enum values
#2: https://codereview.chromium.org/2224023003/
  Teach SecurityStateModel about subresources with cert errors
#3: https://codereview.chromium.org/2225213004/
  (this CL)
#4: https://codereview.chromium.org/2226363002/
  Track subresources with cert errors separately from mixed content

BUG=634171,636986

Committed: https://crrev.com/854d78b541e9a8132f50985fc434d4d43c7417b1
Cr-Commit-Position: refs/heads/master@{#411543}
==========

[commit-bot: I haz the power, 2016-08-12 03:33:02]

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