Prototype an 'allow-unsandboxed-auxiliary' sandbox flag.

Prototype an 'allow-unsandboxed-auxiliary' sandbox flag.

This is a new flag for `<iframe sandbox="...">` which will allow a
sandboxed document to spawn new windows without forcing the sandboxing
flags upon them. This will allow, for example, a third-party advertisement
to be safely sandboxed without forcing the same restrictions upon a
landing page.

Intent to Implement: https://groups.google.com/a/chromium.org/d/msg/blink-dev/ZOhUntjhc94/EEXYKs9k1bgJ

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

[Mike West, 2015-05-12 11:02:03]

mkwst@chromium.org changed reviewers:
+ philipj@opera.com

[Mike West, 2015-05-12 11:02:03]

Hey Philip, WDYT (both of the patch, and the Intent)? :)

-mike

[philipj_slow, 2015-05-13 10:36:19]

lgtm

https://codereview.chromium.org/1139933002/diff/1/Source/core/dom/SandboxFlag...
File Source/core/dom/SandboxFlags.cpp (right):

https://codereview.chromium.org/1139933002/diff/1/Source/core/dom/SandboxFlag...
Source/core/dom/SandboxFlags.cpp:70: if
(RuntimeEnabledFeatures::unsandboxedAuxiliaryEnabled())
Should this check not be moved into the containing if so that when it's disabled
the token will still be treated as an error?

https://codereview.chromium.org/1139933002/diff/1/Source/core/page/CreateWind...
File Source/core/page/CreateWindow.cpp (right):

https://codereview.chromium.org/1139933002/diff/1/Source/core/page/CreateWind...
Source/core/page/CreateWindow.cpp:121: if
(openerFrame.document()->isSandboxed(SandboxUnsandboxedAuxiliary))
The name is quite cryptic in this context. If you sandbox something unsandboxed,
it's no longer unsandboxed... Since the meaning is inverted from the syntax,
something like SandboxAuxiliaryBrowsingContexts would be clearer, but that would
make it harder to find this code if you just know the syntax.

No suggestion for how to make this better, just hoping you have an idea :)

[Mike West, 2015-05-13 11:22:01]

https://codereview.chromium.org/1139933002/diff/1/Source/core/dom/SandboxFlag...
File Source/core/dom/SandboxFlags.cpp (right):

https://codereview.chromium.org/1139933002/diff/1/Source/core/dom/SandboxFlag...
Source/core/dom/SandboxFlags.cpp:70: if
(RuntimeEnabledFeatures::unsandboxedAuxiliaryEnabled())
On 2015/05/13 at 10:36:19, philipj_UTC2 wrote:
> Should this check not be moved into the containing if so that when it's
disabled the token will still be treated as an error?

It should!

https://codereview.chromium.org/1139933002/diff/1/Source/core/page/CreateWind...
File Source/core/page/CreateWindow.cpp (right):

https://codereview.chromium.org/1139933002/diff/1/Source/core/page/CreateWind...
Source/core/page/CreateWindow.cpp:121: if
(openerFrame.document()->isSandboxed(SandboxUnsandboxedAuxiliary))
On 2015/05/13 at 10:36:19, philipj_UTC2 wrote:
> The name is quite cryptic in this context. If you sandbox something
unsandboxed, it's no longer unsandboxed... Since the meaning is inverted from
the syntax, something like SandboxAuxiliaryBrowsingContexts would be clearer,
but that would make it harder to find this code if you just know the syntax.

That's a fair point. Maybe something like
"SandboxPropagatesToAuxiliaryBrowsingContexts"? That seems more consistent with
the way flags are defined in HTML (I assume this would end up as something like
"Sandbox Propagates to Auxiliary Browsing Contexts flag" in
https://html.spec.whatwg.org/multipage/browsers.html#sandboxing). *shrug*

[Mike West, 2015-05-13 11:22:04]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2015-05-13 11:22:05]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1139933002/#ps20001
(title: "Feedback.")

[commit-bot: I haz the power, 2015-05-13 11:22:15]

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

[philipj_slow, 2015-05-13 12:29:04]

https://codereview.chromium.org/1139933002/diff/1/Source/core/page/CreateWind...
File Source/core/page/CreateWindow.cpp (right):

https://codereview.chromium.org/1139933002/diff/1/Source/core/page/CreateWind...
Source/core/page/CreateWindow.cpp:121: if
(openerFrame.document()->isSandboxed(SandboxUnsandboxedAuxiliary))
On 2015/05/13 11:22:01, Mike West wrote:
> On 2015/05/13 at 10:36:19, philipj_UTC2 wrote:
> > The name is quite cryptic in this context. If you sandbox something
> unsandboxed, it's no longer unsandboxed... Since the meaning is inverted from
> the syntax, something like SandboxAuxiliaryBrowsingContexts would be clearer,
> but that would make it harder to find this code if you just know the syntax.
> 
> That's a fair point. Maybe something like
> "SandboxPropagatesToAuxiliaryBrowsingContexts"? That seems more consistent
with
> the way flags are defined in HTML (I assume this would end up as something
like
> "Sandbox Propagates to Auxiliary Browsing Contexts flag" in
> https://html.spec.whatwg.org/multipage/browsers.html#sandboxing). *shrug*

That name is much more clear for now, thanks!

[commit-bot: I haz the power, 2015-05-13 12:41:08]

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