IDL compiler: [CheckSecurity=Frame] interfaces (initial)

IDL compiler: [CheckSecurity=Frame] interfaces (initial)

This adds support for [CheckSecurity=Frame] interface,
but not for [CheckSecurity] + [DoNotCheckSecurity], as in:
[CheckSecurity=Frame] interface { [DoNotCheckSecurity] void foo(); };
(i.e., DomainSafeFunctionGetter/Setter).

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

[Nils Barth (inactive), 2013-11-19 06:56:05]

First [CheckSecurity] CL;
breaking up b/c otherwise a bit big.

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/code_...
File Source/bindings/scripts/code_generator_v8.pm (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/code_...
Source/bindings/scripts/code_generator_v8.pm:4117: if
($interface->extendedAttributes->{"CheckSecurity"}) {
This just makes sure the includes happen at the "generate interface" level,
for consistency with Python.
Doesn't actually change output, since always included eventually.
(Not removing other includes in Perl to keep the Perl internally consistent.)

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':
Moved from templates to here b/c need to do an overall check,
otherwise generates an empty array.

[haraken, 2013-11-19 07:05:06]

LGTM

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':

standard_methods => has_standard_method ?

BTW, would you summarize what method is a non-standard method for me? I'm
curious why the distinction between standard/non-standard is needed.

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:82: {% block security_check_functions %}

security_check_functions => check_security_functions

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:84: bool
indexedSecurityCheck(v8::Local<v8::Object> host, uint32_t index, v8::AccessType
type, v8::Local<v8::Value>)

indexedSecurityCheck => checkSecurityForIndexedGetter ?

host => wrapper

index, type => should be omitted

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:90: bool
namedSecurityCheck(v8::Local<v8::Object> host, v8::Local<v8::Value> key,
v8::AccessType type, v8::Local<v8::Value>)

namedSecurityCheck => securityCheckForNamedGetter ?

host => wrapper

key, type => should be omitted

https://codereview.chromium.org/61523003/diff/1/Tools/Scripts/webkitpy/bindin...
File Tools/Scripts/webkitpy/bindings/main.py (left):

https://codereview.chromium.org/61523003/diff/1/Tools/Scripts/webkitpy/bindin...
Tools/Scripts/webkitpy/bindings/main.py:45: 'TestCheckSecurityInterface.idl',

Don't you need to 'git rm' the IDL file as well?

[Nils Barth (inactive), 2013-11-19 08:10:09]

Revised:
* restructured code some,
* question about function names

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':
On 2013/11/19 07:05:06, haraken wrote:
> standard_methods => has_standard_method ?

Rearranged to compute a value for each method,
so we can use an overall flag and a filter in the template.

> BTW, would you summarize what method is a non-standard method for me? I'm
> curious why the distinction between standard/non-standard is needed.

I've renamed this to |do_generate_method_configuration|,
as that's what it does.
Other methods (custom signature etc.) have their configuration
set somewhere else, rather than in the main
V8DOMConfiguration::MethodConfiguration
array.

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:82: {% block security_check_functions %}
On 2013/11/19 07:05:06, haraken wrote:
> 
> security_check_functions => check_security_functions

(Pending choice of name for functions themselves.)

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:84: bool
indexedSecurityCheck(v8::Local<v8::Object> host, uint32_t index, v8::AccessType
type, v8::Local<v8::Value>)
On 2013/11/19 07:05:06, haraken wrote:
> indexedSecurityCheck => checkSecurityForIndexedGetter ?
> 
> host => wrapper
> 
> index, type => should be omitted

Will fix in followup (with Perl).

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:90: bool
namedSecurityCheck(v8::Local<v8::Object> host, v8::Local<v8::Value> key,
v8::AccessType type, v8::Local<v8::Value>)
On 2013/11/19 07:05:06, haraken wrote:
> namedSecurityCheck => securityCheckForNamedGetter ?

It's also for setters, right?
How about |namedAccessorSecurityCheck|?

> host => wrapper
> key, type => should be omitted

Will do in followup (with Perl).

https://codereview.chromium.org/61523003/diff/1/Tools/Scripts/webkitpy/bindin...
File Tools/Scripts/webkitpy/bindings/main.py (left):

https://codereview.chromium.org/61523003/diff/1/Tools/Scripts/webkitpy/bindin...
Tools/Scripts/webkitpy/bindings/main.py:45: 'TestCheckSecurityInterface.idl',
On 2013/11/19 07:05:06, haraken wrote:
> Don't you need to 'git rm' the IDL file as well?

This is the SKIP list;
I'm turning the test on in Python by removing it from the blacklist.

https://codereview.chromium.org/61523003/diff/1/Tools/Scripts/webkitpy/bindin...
Tools/Scripts/webkitpy/bindings/main.py:45: 'TestCheckSecurityInterface.idl',
On 2013/11/19 07:05:06, haraken wrote:
> Don't you need to 'git rm' the IDL file as well?

This is the SKIP list (blacklist);
I'm turning the test *on* by removing this line.

[commit-bot: I haz the power, 2013-11-19 08:10:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/61523003/70001

[haraken, 2013-11-19 08:52:47]

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':
On 2013/11/19 08:10:09, Nils Barth wrote:
> On 2013/11/19 07:05:06, haraken wrote:
> > standard_methods => has_standard_method ?
> 
> Rearranged to compute a value for each method,
> so we can use an overall flag and a filter in the template.
> 
> > BTW, would you summarize what method is a non-standard method for me? I'm
> > curious why the distinction between standard/non-standard is needed.
> 
> I've renamed this to |do_generate_method_configuration|,
> as that's what it does.
> Other methods (custom signature etc.) have their configuration
> set somewhere else, rather than in the main
> V8DOMConfiguration::MethodConfiguration
> array.

It's OK in this CL, but the core issue is we have configuration code in multiple
places.

> (custom signature etc.)

What is 'etc'? I want to understand what makes it harder to unify these
configurations.

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/templates/int...
Source/bindings/templates/interface.cpp:90: bool
namedSecurityCheck(v8::Local<v8::Object> host, v8::Local<v8::Value> key,
v8::AccessType type, v8::Local<v8::Value>)
On 2013/11/19 08:10:09, Nils Barth wrote:
> On 2013/11/19 07:05:06, haraken wrote:
> > namedSecurityCheck => securityCheckForNamedGetter ?
> 
> It's also for setters, right?
> How about |namedAccessorSecurityCheck|?

Probably "checkSecurityForNamedProperty" ? We use "namedPropertyGetter" and
"namedPropertySetter" in bindings.

[haraken, 2013-11-19 08:58:05]

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':
> It's OK in this CL, but the core issue is we have configuration code in
multiple
> places.
> 
> > (custom signature etc.)
> 
> What is 'etc'? I want to understand what makes it harder to unify these
> configurations.

Supporting indexed/named properties will also add more complexity to the
configuration code, so we might want to clean up the configuration code at this
point.

[commit-bot: I haz the power, 2013-11-19 09:48:59]

Change committed as 162286

[Nils Barth (inactive), 2013-11-20 06:36:18]

(Followup notes/replies.)

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':
On 2013/11/19 08:52:47, haraken wrote:
> > (custom signature etc.)
> 
> What is 'etc'? I want to understand what makes it harder to unify these
> configurations.

The full check is in v8_methods.py:
'do_not_check_signature': not(this_custom_signature or is_static or
    v8_utilities.has_extended_attribute(method,
        ['DoNotCheckSignature', 'NotEnumerable', 'ReadOnly',
         'RuntimeEnabled', 'Unforgeable'])),

There are various reasons for installation different places;
some of these are:
If [RuntimeEnabled] we need to check the runtime enabled function before
installing the attributes,
if [PerContextEnabled] we need to check the per context enabled function.

https://codereview.chromium.org/61523003/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_interface.py:106: 'standard_methods':
On 2013/11/19 08:58:05, haraken wrote:
> Supporting indexed/named properties will also add more complexity to the
> configuration code, so we might want to clean up the configuration code at
this
> point.

Got it; let's do it then!