Mixed Content: introduce WebURLRequest::RequestContext checks.

Mixed Content: introduce WebURLRequest::RequestContext checks.

We're currently performing mixed content checks based on Resource::Type,
which isn't granular enough to support the behavior specified in the
mixed content specification (hence the introduction of the
MixedContentBlockingTreatment enum).

This patch introduces a new static mixed content checking method based
on WebURLRequest::RequestContext, which will allow us to remove most
of the complexity of MixedContentChecking in future patches.

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

[Mike West, 2014-09-05 07:02:05]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org

[Mike West, 2014-09-05 07:02:06]

Jochen, WDYT about this patch? It introduces a new static method in
MixedContentChecker, and adds ASSERTs which verify that the new method's
behavior matches the old method's behavior.

I'm reluctant to switch over to the new method in this patch, as I want to
change the error messages at the same time, since we now have a good idea of
exactly what content is being loaded. Doing so would mean a bunch of extra
complexity in this patch.

An alternative would be to switch over in this patch, and ensure that the
generated error messages were exactly the same as they currently are. I kinda
don't want to do that and then immediately throw it away.

WDYT?

[Mike West, 2014-09-05 07:22:59]

On 2014/09/05 07:02:06, Mike West wrote:
> Jochen, WDYT about this patch? It introduces a new static method in
> MixedContentChecker, and adds ASSERTs which verify that the new method's
> behavior matches the old method's behavior.
> 
> I'm reluctant to switch over to the new method in this patch, as I want to
> change the error messages at the same time, since we now have a good idea of
> exactly what content is being loaded. Doing so would mean a bunch of extra
> complexity in this patch.
> 
> An alternative would be to switch over in this patch, and ensure that the
> generated error messages were exactly the same as they currently are. I kinda
> don't want to do that and then immediately throw it away.
> 
> WDYT?

Chromium-side bots failing because the previous patch hasn't rolled in yet. :(

[Mike West, 2014-09-05 07:52:08]

On 2014/09/05 07:22:59, Mike West wrote:
> On 2014/09/05 07:02:06, Mike West wrote:
> > Jochen, WDYT about this patch? It introduces a new static method in
> > MixedContentChecker, and adds ASSERTs which verify that the new method's
> > behavior matches the old method's behavior.
> > 
> > I'm reluctant to switch over to the new method in this patch, as I want to
> > change the error messages at the same time, since we now have a good idea of
> > exactly what content is being loaded. Doing so would mean a bunch of extra
> > complexity in this patch.
> > 
> > An alternative would be to switch over in this patch, and ensure that the
> > generated error messages were exactly the same as they currently are. I
kinda
> > don't want to do that and then immediately throw it away.
> > 
> > WDYT?
> 
> Chromium-side bots failing because the previous patch hasn't rolled in yet. :(

(Though note that they all passed on patchset #1, which includes the
not-yet-rolled patch)

[jochen (gone - plz use gerrit), 2014-09-05 08:26:15]

https://codereview.chromium.org/537983002/diff/20001/Source/core/loader/Mixed...
File Source/core/loader/MixedContentChecker.cpp (right):

https://codereview.chromium.org/537983002/diff/20001/Source/core/loader/Mixed...
Source/core/loader/MixedContentChecker.cpp:65: MixedContentChecker::ContextType
contextTypeFromContext(WebURLRequest::RequestContext context)
static?

https://codereview.chromium.org/537983002/diff/20001/Source/core/loader/Mixed...
File Source/core/loader/MixedContentChecker.h (right):

https://codereview.chromium.org/537983002/diff/20001/Source/core/loader/Mixed...
Source/core/loader/MixedContentChecker.h:48: Blockable,
why is this public?

what about ContextTypeBlockable, etc..

https://codereview.chromium.org/537983002/diff/20001/Source/core/loader/Mixed...
Source/core/loader/MixedContentChecker.h:54: MixedContentChecker(LocalFrame*);
explicit?

[Mike West, 2014-09-05 10:37:44]

Thanks. Mind taking another look?

[jochen (gone - plz use gerrit), 2014-09-09 08:11:57]

lgtm

https://codereview.chromium.org/537983002/diff/60001/Source/core/loader/Mixed...
File Source/core/loader/MixedContentChecker.cpp (right):

https://codereview.chromium.org/537983002/diff/60001/Source/core/loader/Mixed...
Source/core/loader/MixedContentChecker.cpp:46: } // namespace
not needed?

[Mike West, 2014-09-10 08:35:32]

Thanks! I need to reland that DocumentLoader patch that got reverted on Monday
before I can land this patch. I guess I'll wait to do that until the crasher has
actually been tracked down and killed.

[Mike West, 2014-09-11 07:32:48]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-09-11 07:33:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/537983002/80001

[commit-bot: I haz the power, 2014-09-11 08:43:15]

Committed patchset #5 (id:80001) as 181804