bindings: Makes runtime-enabled attributes on prototype chains compilable.

bindings: Makes runtime-enabled attributes on prototype chains compilable.

Makes runtime-enabled "accessors" compilable.

BUG=43394, 424446
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190287

[Yuki, 2015-02-13 14:59:41]

yukishiino@chromium.org changed reviewers:
+ haraken@chromium.org

[Yuki, 2015-02-13 14:59:42]

Could you review this CL?

[haraken, 2015-02-13 15:08:24]

https://codereview.chromium.org/922233002/diff/1/Source/bindings/core/v8/V8DO...
File Source/bindings/core/v8/V8DOMConfiguration.h (right):

https://codereview.chromium.org/922233002/diff/1/Source/bindings/core/v8/V8DO...
Source/bindings/core/v8/V8DOMConfiguration.h:181: static inline void
installAccessor(v8::Handle<ObjectOrTemplate> instanceTemplate,
v8::Handle<ObjectOrTemplate> prototype, v8::Handle<v8::Signature> signature,
const AccessorConfiguration& accessor, v8::Isolate* isolate)

We might not want to duplicate the code between installAccessor and
installAccessors. Can we update installAccessors to call installAccessor and
thus share the code?

[Yuki, 2015-02-16 10:51:51]

https://codereview.chromium.org/922233002/diff/1/Source/bindings/core/v8/V8DO...
File Source/bindings/core/v8/V8DOMConfiguration.h (right):

https://codereview.chromium.org/922233002/diff/1/Source/bindings/core/v8/V8DO...
Source/bindings/core/v8/V8DOMConfiguration.h:181: static inline void
installAccessor(v8::Handle<ObjectOrTemplate> instanceTemplate,
v8::Handle<ObjectOrTemplate> prototype, v8::Handle<v8::Signature> signature,
const AccessorConfiguration& accessor, v8::Isolate* isolate)
On 2015/02/13 15:08:24, haraken wrote:
> 
> We might not want to duplicate the code between installAccessor and
> installAccessors. Can we update installAccessors to call installAccessor and
> thus share the code?

Refactored the code a bit.
Since it seemed that getting DOMWrapperWorld is not so cheap, I made a change to
keep it during for-loops.

Also installConstant seems misleading compared to installMethod, etc.  Thus I
renamed it to installConstantWithGetter.

[haraken, 2015-02-16 13:23:21]

LGTM, looks much nicer!

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/V8DOMConfiguration.cpp (left):

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/V8DOMConfiguration.cpp:73: // Constant installation

Can we keep this comment?

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/V8DOMConfiguration.cpp (right):

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/V8DOMConfiguration.cpp:109: void
V8DOMConfiguration::installAttributeInternal<v8::Object>(v8::Isolate*,
v8::Handle<v8::Object> instanceTemplate, v8::Handle<v8::Object> prototype, const
AttributeConfiguration&, const DOMWrapperWorld&);

Not related to your CL, I'm wondering why we need both Object version and
ObjectTemplate version.

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/V8DOMConfiguration.h (right):

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/V8DOMConfiguration.h:71: static void
installAttributes(v8::Isolate* isolate, v8::Handle<v8::ObjectTemplate>
instanceTemplate, v8::Handle<v8::ObjectTemplate> prototype, const
AttributeConfiguration* attributes, size_t attributeCount)

Can we uninline all of these methods? I don't think these methods are
performance-sensitive.

[Yuki, 2015-02-16 15:00:27]

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

[Yuki, 2015-02-16 15:02:05]

Could you take another look?

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/V8DOMConfiguration.cpp (left):

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/V8DOMConfiguration.cpp:73: // Constant installation
On 2015/02/16 13:23:20, haraken wrote:
> 
> Can we keep this comment?

Done.

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/V8DOMConfiguration.cpp (right):

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/V8DOMConfiguration.cpp:109: void
V8DOMConfiguration::installAttributeInternal<v8::Object>(v8::Isolate*,
v8::Handle<v8::Object> instanceTemplate, v8::Handle<v8::Object> prototype, const
AttributeConfiguration&, const DOMWrapperWorld&);
On 2015/02/16 13:23:20, haraken wrote:
> 
> Not related to your CL, I'm wondering why we need both Object version and
> ObjectTemplate version.

ObjectTemplate version is used for the most cases.

Object version is used for conditionally-enabled properties.

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/V8DOMConfiguration.h (right):

https://codereview.chromium.org/922233002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/V8DOMConfiguration.h:71: static void
installAttributes(v8::Isolate* isolate, v8::Handle<v8::ObjectTemplate>
instanceTemplate, v8::Handle<v8::ObjectTemplate> prototype, const
AttributeConfiguration* attributes, size_t attributeCount)
On 2015/02/16 13:23:20, haraken wrote:
> 
> Can we uninline all of these methods? I don't think these methods are
> performance-sensitive.

Done.

[haraken, 2015-02-16 23:27:14]

Still LGTM

[Yuki, 2015-02-17 02:31:44]

The CQ bit was checked by yukishiino@chromium.org

[commit-bot: I haz the power, 2015-02-17 02:32:07]

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

[commit-bot: I haz the power, 2015-02-17 02:35:54]

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