Add a setting to strictly block "blockable" mixed content.

Add a setting to strictly block "blockable" mixed content.

If the new 'StrictlyBlockBlockableMixedContent' setting is 'true', the
MixedContentChecker will suppress messages to the embedder about
blockable mixed content, opting instead to block the content entirely
without giving the embedder the opportunity to override the decision.

This setting is 'false' by default, but I sure would like to make it
'true' by default instead.

blink-dev thread: https://groups.google.com/a/chromium.org/d/msg/blink-dev/1yWQaXmkXpA/BDV1-dC9su4J

BUG=513163
R=yoav@yoav.ws

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199372

[Mike West, 2015-07-23 09:47:19]

Hey Yoav, since you're watching this on Twitter, how about reviewing a patch? :)

[Yoav Weiss, 2015-07-23 09:53:24]

Sure :)

Is there a way to test that the setting is actually doing its thing?

https://codereview.chromium.org/1248313002/diff/1/Source/web/WebSettingsImpl.cpp
File Source/web/WebSettingsImpl.cpp (right):

https://codereview.chromium.org/1248313002/diff/1/Source/web/WebSettingsImpl....
Source/web/WebSettingsImpl.cpp:627: 
Nit: extra new line

[Mike West, 2015-07-23 10:30:05]

On 2015/07/23 at 09:53:24, yoav wrote:
> Sure :)
> 
> Is there a way to test that the setting is actually doing its thing?

It doesn't involve any web-visible behavioral change in Blink. If the setting is
false, we block blockable mixed content and send a signal to the embedder. If
the setting is true, we block blockable mixed content, but don't send a signal
to the embedder.

I could probably add an embedder-side test, however. I'll look into that.

> 
>
https://codereview.chromium.org/1248313002/diff/1/Source/web/WebSettingsImpl.cpp
> File Source/web/WebSettingsImpl.cpp (right):
> 
>
https://codereview.chromium.org/1248313002/diff/1/Source/web/WebSettingsImpl....
> Source/web/WebSettingsImpl.cpp:627: 
> Nit: extra new line

Done.

[Yoav Weiss, 2015-07-23 10:50:19]

On 2015/07/23 10:30:05, Mike West wrote:
> On 2015/07/23 at 09:53:24, yoav wrote:
> > Sure :)
> > 
> > Is there a way to test that the setting is actually doing its thing?
> 
> It doesn't involve any web-visible behavioral change in Blink. If the setting
is
> false, we block blockable mixed content and send a signal to the embedder. If
> the setting is true, we block blockable mixed content, but don't send a signal
> to the embedder.
> 
> I could probably add an embedder-side test, however. I'll look into that.
> 

I'd be happy with adding that in a followup CL (since in any case, it's embedded
side). LGTM

[philipj_slow, 2015-07-23 11:44:27]

Is there any reason to have the setting at all, as opposed to just blocking and
giving the embedder no say in the matter?

[Mike West, 2015-07-23 11:53:00]

On 2015/07/23 at 11:44:27, philipj wrote:
> Is there any reason to have the setting at all, as opposed to just blocking
and giving the embedder no say in the matter?

1. Depends on what Opera wants to do. :)

2. I have a suspicion that we might need to make this a bit more flexible; one
of the initial pieces of feedback on Twitter was that HTTPS Everywhere often
produces mixed content errors, which some folks like opting into. Perhaps it
would be reasonable for those kinds of extensions to expose an extension API
which would enable the shield for particular pages?

If it turns out that we don't need the setting, I'm happy to rip it out and just
block things. I'm good at that. :)

[philipj_slow, 2015-07-23 12:06:25]

On 2015/07/23 11:53:00, Mike West wrote:
> On 2015/07/23 at 11:44:27, philipj wrote:
> > Is there any reason to have the setting at all, as opposed to just blocking
> and giving the embedder no say in the matter?
> 
> 1. Depends on what Opera wants to do. :)

This isn't my call, but I think it's very likely we'll remove the extra UI too
when Chrome does.

> 2. I have a suspicion that we might need to make this a bit more flexible; one
> of the initial pieces of feedback on Twitter was that HTTPS Everywhere often
> produces mixed content errors, which some folks like opting into. Perhaps it
> would be reasonable for those kinds of extensions to expose an extension API
> which would enable the shield for particular pages?
> 
> If it turns out that we don't need the setting, I'm happy to rip it out and
just
> block things. I'm good at that. :)

Do you mean that some people will want to keep the shield as a way of noticing
these issues without checking the console?

Is there a Chromium CL which hooks up the setting to something? What I'm really
asking is where it will be set to true, to check if any Opera product has to do
it manually or not. Enabling the setting by default, as I know you'd like, would
be nice.

[Mike West, 2015-07-23 12:13:21]

On 2015/07/23 at 12:06:25, philipj wrote:
> On 2015/07/23 11:53:00, Mike West wrote:
> > On 2015/07/23 at 11:44:27, philipj wrote:
> > > Is there any reason to have the setting at all, as opposed to just
blocking
> > and giving the embedder no say in the matter?
> > 
> > 1. Depends on what Opera wants to do. :)
> 
> This isn't my call, but I think it's very likely we'll remove the extra UI too
when Chrome does.

Ok, I'm marking Opera down as "SUPER STRONGLY SUPPORTIVE AND THEY'RE GOING TO DO
THIS THEMSELVES REGARDLESS OF WHETHER OR NOT I LAND THIS PATCH". That seems
reasonable, right? :)
 
> Do you mean that some people will want to keep the shield as a way of noticing
these issues without checking the console?

I mean that HTTPS Everywhere forcibly migrates some pages to HTTPS before
they're ready, and that often causes insecure requests. See
https://twitter.com/passy/status/624150669558194176 for that suggestion.

> Is there a Chromium CL which hooks up the setting to something? What I'm
really asking is where it will be set to true, to check if any Opera product has
to do it manually or not. Enabling the setting by default, as I know you'd like,
would be nice.

Not yet. Assuming no one strenuously objects on the list, the setting would be
toggled in `content::RenderView::ApplyWebPreferences` based on a content-level
CLI switch, which means Opera would get it for free.

[Mike West, 2015-07-23 12:13:54]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-07-23 12:14:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1248313002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1248313002/20001

[commit-bot: I haz the power, 2015-07-23 12:17:17]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199372