[bindings] Include V8HiddenValue only if attribute has CachedAttribute or marked keep alive for GC.

[bindings] Include V8HiddenValue only if attribute has CachedAttribute or marked keep alive for GC.

Include the header, V8HiddenValue.h, only if an IDL attribute has CachedAttribute or its has been marked as keep alive
for GC.

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

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

[vivekg, 2015-04-01 06:11:59]

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

[vivekg, 2015-04-01 06:12:00]

This popped up while revisiting History.idl for removal of custom bindings. 

Jens added the recent support for |Custom=CallEpilogue| and the history methods
pushState and replaceState just clears the |state| hidden value.

This can also be auto-generated with an extended attribute,
ClearCachedAttributes=(...) like

interface History {
    ...
    [ClearCachedAttributes=(state, ...), RaisesException] void pushState(...);
    [ClearCachedAttributes=(state, ...), RaisesException] void
replaceState(...);
}

WDYT?

[vivekg, 2015-04-01 07:03:34]

WIP patch added here: https://codereview.chromium.org/1046423002 for your
feedback. Thanks

[bashi, 2015-04-01 09:02:28]

This CL LGTM.

On 2015/04/01 06:12:00, vivekg_ wrote:
> This popped up while revisiting History.idl for removal of custom bindings. 
> 
> Jens added the recent support for |Custom=CallEpilogue| and the history
methods
> pushState and replaceState just clears the |state| hidden value.
> 
> This can also be auto-generated with an extended attribute,
> ClearCachedAttributes=(...) like
> 
> interface History {
>     ...
>     [ClearCachedAttributes=(state, ...), RaisesException] void pushState(...);
>     [ClearCachedAttributes=(state, ...), RaisesException] void
> replaceState(...);
> }
> 
> WDYT?

Do you have other instances in which we can use ClearCachedAttributes? Removing
custom bindings is great, but adding blink specific extended attributes is also
something we want to avoid as much as possible.

[vivekg, 2015-04-01 09:06:29]

On 2015/04/01 at 09:02:28, bashi wrote:
> This CL LGTM.
> 

Thanks.

> On 2015/04/01 06:12:00, vivekg_ wrote:
> > This popped up while revisiting History.idl for removal of custom bindings. 
> > 
> > Jens added the recent support for |Custom=CallEpilogue| and the history
methods
> > pushState and replaceState just clears the |state| hidden value.
> > 
> > This can also be auto-generated with an extended attribute,
> > ClearCachedAttributes=(...) like
> > 
> > interface History {
> >     ...
> >     [ClearCachedAttributes=(state, ...), RaisesException] void
pushState(...);
> >     [ClearCachedAttributes=(state, ...), RaisesException] void
> > replaceState(...);
> > }
> > 
> > WDYT?
> 
> Do you have other instances in which we can use ClearCachedAttributes?
Removing custom bindings is great, but adding blink specific extended attributes
is also something we want to avoid as much as possible.

I agree, the reason I ended up suggesting this is to counter the CachedAttribute
i.e. providing counter mechanism to remove something added by CachedAttribute
within the interface scope.
As of now, I think this is the only place where its used.

[vivekg, 2015-04-01 09:11:10]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2015-04-01 09:11:44]

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

[haraken, 2015-04-01 09:13:40]

LGTM

[commit-bot: I haz the power, 2015-04-01 11:26:18]

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