make window interceptors DoNoCheckSecurity

make window interceptors DoNoCheckSecurity

This marks them as "all can read". The actually security check is then done in
the custom bindings code for windows.

R=haraken@chromium.org, jl@opera.com

BUG=404300
Patch by Dan Carney <dcarney@chromium.org>;

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

[haraken, 2015-02-12 09:45:58]

LGTM

Jens might want to take another look though.

https://codereview.chromium.org/921683002/diff/1/Source/bindings/core/v8/cust...
File Source/bindings/core/v8/custom/V8WindowCustom.cpp (left):

https://codereview.chromium.org/921683002/diff/1/Source/bindings/core/v8/cust...
Source/bindings/core/v8/custom/V8WindowCustom.cpp:389: if (key->IsString()) {

I'm assuming that the removed checks are now conducted in the V8 side, right?

https://codereview.chromium.org/921683002/diff/1/Source/bindings/core/v8/cust...
File Source/bindings/core/v8/custom/V8WindowCustom.cpp (right):

https://codereview.chromium.org/921683002/diff/1/Source/bindings/core/v8/cust...
Source/bindings/core/v8/custom/V8WindowCustom.cpp:373: static bool
SecurityCheck(v8::Local<v8::Object> host)

SecurityCheck => securityCheck

[jochen (gone - plz use gerrit), 2015-02-12 09:52:04]

New patchsets have been uploaded after l-g-t-m from haraken@chromium.org

[jochen (gone - plz use gerrit), 2015-02-12 09:52:17]

https://codereview.chromium.org/921683002/diff/1/Source/bindings/core/v8/cust...
File Source/bindings/core/v8/custom/V8WindowCustom.cpp (left):

https://codereview.chromium.org/921683002/diff/1/Source/bindings/core/v8/cust...
Source/bindings/core/v8/custom/V8WindowCustom.cpp:389: if (key->IsString()) {
On 2015/02/12 at 09:45:58, haraken wrote:
> I'm assuming that the removed checks are now conducted in the V8 side, right?

no, it's moved into the getter

[Jens Widell, 2015-02-12 09:57:13]

The previous patch added [AllCanread] to the getters defined in
TestInterface2.idl. Any reason you didn't carry that change over to this patch?

[jochen (gone - plz use gerrit), 2015-02-12 10:01:16]

added them back. ptal

[Jens Widell, 2015-02-12 10:10:05]

On 2015/02/12 10:01:16, jochen (slow) wrote:
> added them back. ptal

Thanks. LGTM.

[jochen (gone - plz use gerrit), 2015-02-12 10:10:43]

lgtm

[jochen (gone - plz use gerrit), 2015-02-12 10:10:52]

meh, wrong button

[jochen (gone - plz use gerrit), 2015-02-12 10:10:55]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2015-02-12 10:12:04]

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

[dcarney, 2015-02-12 12:00:49]

On 2015/02/12 10:12:04, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/921683002/40001

testinterface2 now has an expected bunch of originsafemethod crap being
generated, which is what i was worried about.  you'll have to somehow prevent
that from happening...

[jochen (gone - plz use gerrit), 2015-02-12 12:02:19]

The CQ bit was unchecked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2015-02-12 12:02:39]

On 2015/02/12 at 12:00:49, dcarney wrote:
> On 2015/02/12 10:12:04, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >  https://chromium-cq-status.appspot.com/patch-status/921683002/40001
> 
> testinterface2 now has an expected bunch of originsafemethod crap being
generated, which is what i was worried about.  you'll have to somehow prevent
that from happening...

meh

[Jens Widell, 2015-02-12 12:03:58]

On 2015/02/12 12:00:49, dcarney wrote:
> On 2015/02/12 10:12:04, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >  https://chromium-cq-status.appspot.com/patch-status/921683002/40001
> 
> testinterface2 now has an expected bunch of originsafemethod crap being
> generated, which is what i was worried about.  you'll have to somehow prevent
> that from happening...

But that's because it uses "getter X item(Y y);" which defines both a property
getter and an operation. Window.idl only has "getter X (Y y);" which defines
just the property getter.

If any real interface has the former and we want to apply the attribute to only
one aspect of it, we could always split it into two, I guess:

  X item(Y y);
  getter X (Y y);

[jochen (gone - plz use gerrit), 2015-02-12 12:19:49]

Dan, what's your take on this? Land as is?

[dcarney, 2015-02-12 13:03:04]

On 2015/02/12 12:19:49, jochen (slow) wrote:
> Dan, what's your take on this? Land as is?

land it, but maybe mark a different test interface as DoNotCheckSecurity so we
get the code generation coverage?

[jochen (gone - plz use gerrit), 2015-02-12 13:36:51]

New patchsets have been uploaded after l-g-t-m from jl@opera.com

[jochen (gone - plz use gerrit), 2015-02-12 13:37:17]

like this?

[dcarney, 2015-02-12 14:01:33]

dcarney@chromium.org changed reviewers:
+ dcarney@chromium.org

[dcarney, 2015-02-12 14:01:34]

lgtm

[jochen (gone - plz use gerrit), 2015-02-12 14:02:21]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2015-02-12 14:02:53]

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

[commit-bot: I haz the power, 2015-02-12 14:56:38]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190054