Add support for [PutForwards] IDL extended attribute

Add support for [PutForwards] IDL extended attribute

Add support for [PutForwards] IDL extended attribute in the Perl
bindings generator and use it for Document.location:
http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#document

This CL also gets rid of custom bindings code for Document.location
and uses [PutForwards] instead.

R=haraken, abarth
BUG=307045
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159674

[Inactive, 2013-10-14 21:14:30]

For comparison, the generated code looks like:

static void locationAttributeGetter(v8::Local<v8::String> name, const
v8::PropertyCallbackInfo<v8::Value>& info)
{
    Document* imp = V8Document::toNative(info.Holder());
    v8SetReturnValueFast(info, imp->location(), imp);
}

static void locationAttributeSetter(v8::Local<v8::String> name,
v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<void>& info)
{
    Document* proxyImp = V8Document::toNative(info.Holder());
    Location* imp = proxyImp->location();
    if (!imp)
        return;
    V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<>, v, value);
    imp->setHref(activeDOMWindow(), firstDOMWindow(), v);
}

[Inactive, 2013-10-14 21:18:09]

https://codereview.chromium.org/27207002/diff/5001/Source/core/dom/Document.idl
File Source/core/dom/Document.idl (left):

https://codereview.chromium.org/27207002/diff/5001/Source/core/dom/Document.i...
Source/core/dom/Document.idl:133: [Custom, Replaceable, PerWorldBindings,
ActivityLogging=AccessForIsolatedWorlds] readonly attribute Location location;
I removed [Replaceable, PerWorldBindings] because they had no impact on the
generated code (since it was custom code).

- [Replaceable] is not in the specification so I don't think we want this
anyway.
- However, let me know if you would like me to keep [PerWorldBindings]. We can
actually use [PerWorldBindings] now that the code is generated.

[haraken, 2013-10-15 00:15:36]

LGTM

https://codereview.chromium.org/27207002/diff/5001/Source/bindings/scripts/co...
File Source/bindings/scripts/code_generator_v8.pm (right):

https://codereview.chromium.org/27207002/diff/5001/Source/bindings/scripts/co...
Source/bindings/scripts/code_generator_v8.pm:1866: my $attrType =
$attribute->type;

This is not necessary. $attrType is already defined above.

https://codereview.chromium.org/27207002/diff/5001/Source/bindings/tests/idls...
File Source/bindings/tests/idls/TestNode.idl (right):

https://codereview.chromium.org/27207002/diff/5001/Source/bindings/tests/idls...
Source/bindings/tests/idls/TestNode.idl:25: attribute DOMString href;

Shall we add a comment that these attributes are for [PutForwards] written in
TestObj.idl ?

https://codereview.chromium.org/27207002/diff/5001/Source/core/dom/Document.idl
File Source/core/dom/Document.idl (left):

https://codereview.chromium.org/27207002/diff/5001/Source/core/dom/Document.i...
Source/core/dom/Document.idl:133: [Custom, Replaceable, PerWorldBindings,
ActivityLogging=AccessForIsolatedWorlds] readonly attribute Location location;
On 2013/10/14 21:18:10, Chris Dumez wrote:
> I removed [Replaceable, PerWorldBindings] because they had no impact on the
> generated code (since it was custom code).
> 
> - [Replaceable] is not in the specification so I don't think we want this
> anyway.

Removing [Replaceable] sounds reasonable.

> - However, let me know if you would like me to keep [PerWorldBindings]. We can
> actually use [PerWorldBindings] now that the code is generated.

Just to confirm: Does [ActivityLogging] imply [PerWorldBindings]?
[ActivityLogging] requires per-world binding code. If [ActivityLogging] implies
[PerWorldBindings], I'm OK with removing [PerWorldBindings].

[Nils Barth (inactive), 2013-10-15 02:13:29]

Thanks for doing this Chris!
Could you update the documentation, indicating that this is supported?
(Preferably before landing, so it's not forgotten...)
Thanks!
https://sites.google.com/a/chromium.org/dev/blink/webidl/blink-idl-extended-a...

[Inactive, 2013-10-15 02:46:08]

On 2013/10/15 02:13:29, Nils Barth wrote:
> Thanks for doing this Chris!
> Could you update the documentation, indicating that this is supported?
> (Preferably before landing, so it's not forgotten...)
> Thanks!
>
https://sites.google.com/a/chromium.org/dev/blink/webidl/blink-idl-extended-a...

Of course, I always try to keep it up to date.

[Nils Barth (inactive), 2013-10-15 07:17:55]

On 2013/10/15 02:46:08, Chris Dumez wrote:
> On 2013/10/15 02:13:29, Nils Barth wrote:
> > Could you update the documentation, indicating that this is supported?
> Of course, I always try to keep it up to date.

Thanks for the good citizenship!

[commit-bot: I haz the power, 2013-10-15 13:39:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/27207002/16001

[Inactive, 2013-10-15 13:46:33]

Updated the doc:
https://sites.google.com/a/chromium.org/dev/blink/webidl/blink-idl-extended-a...

[commit-bot: I haz the power, 2013-10-15 16:03:32]

Change committed as 159674

[arv (Not doing code reviews), 2013-10-15 16:23:06]

Any reason you didn't update Window location in this CL?