bindings: Retires manual dispatching in createV8{HTML,SVG}Wrapper, etc.

Source/build/scripts/templates/ElementWrapperFactory.cpp.tmpl

 {% from "macros.tmpl" import license %}

[haraken, 2014/09/04 02:17:46]

I hope you completely remove ElementWrapperFactory.{h,cpp} and
make_element_factory.py in a follow-up CL :)

[Yuki, 2014/09/04 04:47:29]

Yes, will do.

 {{ license() }}
 
 #include "config.h"
 #include "V8{{namespace}}ElementWrapperFactory.h"
 
 #include "{{namespace}}Names.h"
 #include "bindings/core/v8/CustomElementWrapper.h"
 {% for tag in tags|sort if tag.has_js_interface %}
 #include "bindings/core/v8/V8{{tag.interface}}.h"
 {% endfor %}
 {% for tag in tags|sort if tag.has_js_interface %}
 #include "core/{{namespace|lower}}/{{tag.js_interface}}.h"
 {% endfor %}
 #include "core/{{namespace|lower}}/{{fallback_js_interface}}.h"
 #include "core/dom/Document.h"
 #include "core/frame/Settings.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "wtf/StdLibExtras.h"
 
 namespace blink {
 
 using namespace {{namespace}}Names;
 
-typedef v8::Handle<v8::Object> (*Create{{namespace}}ElementWrapperFunction)({{namespace}}Element*, v8::Handle<v8::Object> creationContext, v8::Isolate*);
-
-static v8::Handle<v8::Object> create{{namespace}}ElementWrapper({{namespace}}Element*, v8::Handle<v8::Object>, v8::Isolate*)
-{
-    ASSERT_NOT_REACHED();
-    return v8::Handle<v8::Object>();
-}
-{% for js_interface, list in tags|sort|selectattr('has_js_interface')|groupby('js_interface') %}
-{% filter enable_conditional(list[0].Conditional) %}
-static v8::Handle<v8::Object> create{{js_interface}}Wrapper({{namespace}}Element* element, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
-{
-    {% if list[0].runtimeEnabled %}
-    if (!RuntimeEnabledFeatures::{{list[0].runtimeEnabled}}Enabled())

[haraken, 2014/09/04 02:17:46]

It is wrong to just remove this code. We need to fallback to an
HTMLUnknownElement::wrap() if the element is not runtime-enabled.

[Yuki, 2014/09/04 04:47:29]

Hmm.  If the element is not runtime-enabled, then the DOM object is never
created, I think.  I'm seeing
https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/blin...
and
https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/blin...

If the flag is not enabled, then a HTMLUnknownElement is instantiated instead of
T and its wrap method uses V8HTMLUnknownElement as expected instead of V8T.

In anycase, if the DOM object is created as an instance of class T, it's wrong
to wrap it as V8HTMLUnknownElement, it must be wrapped as V8T.

[haraken, 2014/09/04 04:54:52]

What happens if:

<div id="foo">
<audio></audio>
</div>
<script>
div = document.getElementById("foo");
div.firstChild;  // I think this should return a wrapper of HTMLUnknownElement
if the runtime-enabled flag is not on for webaudio.
</script>

[Yuki, 2014/09/04 14:10:14]

As talked offline, if the runtime flag is not enabled, then a HTMLUnknownElement
is instantiated and wrapped as V8HTMLUnknownElement as same as before.  Should
be no problem.

-        return createV8{{namespace}}FallbackWrapper(to{{fallback_js_interface}}(element), creationContext, isolate);
-    {% endif %}
-    return wrap(static_cast<{{js_interface}}*>(element), creationContext, isolate);
-}
-{% endfilter %}
-{% endfor %}
-
-v8::Handle<v8::Object> createV8{{namespace}}Wrapper({{namespace}}Element* element, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
-{
-    typedef HashMap<StringImpl*, Create{{namespace}}ElementWrapperFunction> FunctionMap;
-    DEFINE_STATIC_LOCAL(FunctionMap, map, ());
-    if (map.isEmpty()) {
-    {% for tag in tags|sort %}
-    {% filter enable_conditional(tag.Conditional) %}
-        map.set({{tag|symbol}}Tag.localName().impl(), create{{tag.js_interface}}Wrapper);
-    {% endfilter %}
-    {% endfor %}
-    }
-
-    Create{{namespace}}ElementWrapperFunction createWrapperFunction = map.get(element->localName().impl());
-    if (createWrapperFunction == create{{namespace}}ElementWrapper)
-        createWrapperFunction = createV8{{namespace}}DirectWrapper;

[haraken, 2014/09/04 02:17:46]

I think it's correct to just remove createV8HTMLDirectWrapper in this CL, but
the reason is never trivial. If I'm understanding things correctly, it's correct
because:

- createV8HTMLDirectWrapper was used to redirect X::wrap to
V8HTMLElement::createWrapper for X that doesn't have a corresponding IDL file
(e.g., <noscript>).

- V8HTMLElement::createWrapper is doing exactly the same thing as
ScriptWrappable::wrap.

- Thus it's correct to remove createV8HTMLDirectWrapper and make X::wrap use
ScriptWrappable::wrap.

[Yuki, 2014/09/04 04:47:29]

That's right.  In addition, it must be fine to use V8T for an instance of T
rather than V8HTMLElement (although T is a subclass of HTMLElement).

[haraken, 2014/09/04 04:54:52]

There is no V8T for the classes that had been using createV8HTMLDirectWrapper.
For example, HTMLNoScriptElement doesn't have a corresponding IDL file and thus
V8HTMLNoScriptElement is not created.

Either way I think it's safe to just remove the DirectWrapper code.

[Yuki, 2014/09/04 14:10:14]

Yes, there are such cases.  In those cases, they are handled by the superclass
which has a corresponding V8T class.  For the case of HTMLNoScriptElement, the
superclass HTMLElement takes care of wrapping and V8HTMLElement is used.

-    if (element->isCustomElement())
-        return CustomElementWrapper<{{namespace}}Element, V8{{namespace}}Element>::wrap(element, creationContext, isolate, createWrapperFunction);
-
-    if (createWrapperFunction)
-        return createWrapperFunction(element, creationContext, isolate);
-    {% if fallback_js_interface == namespace + 'Element' %}
-    return V8{{fallback_js_interface}}::createWrapper(element, creationContext, isolate);
-    {% else %}
-    return wrap(to{{fallback_js_interface}}(element), creationContext, isolate);
-    {% endif %}
-}
-
 const WrapperTypeInfo* findWrapperTypeFor{{namespace}}TagName(const AtomicString& name)
 {
     typedef HashMap<StringImpl*, const WrapperTypeInfo*> NameTypeMap;
     DEFINE_STATIC_LOCAL(NameTypeMap, map, ());
     if (map.isEmpty()) {
         // FIXME: This seems wrong. We should list every interface here, not
         // just the ones that have specialized JavaScript interfaces.
         {% for tag in tags|sort if tag.has_js_interface %}
         {% filter enable_conditional(tag.Conditional) %}
         map.set({{tag|symbol}}Tag.localName().impl(), &V8{{tag.js_interface}}::wrapperTypeInfo);
         {% endfilter %}
         {% endfor %}
     }
 
     if (const WrapperTypeInfo* result = map.get(name.impl()))
         return result;
 
     return &V8{{fallback_js_interface}}::wrapperTypeInfo;
 }
 
-}
+} // namespace blink