bindings: Introduces type-check for the internal pointers.

Source/bindings/templates/interface.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file has been auto-generated by {{code_generator}}. DO NOT MODIFY!
 
 #ifndef {{v8_class}}_h
 #define {{v8_class}}_h
 
 {% filter conditional(conditional_string) %}
@@ skipping 14 matching lines @@
 };
 
 {% endif %}
 class {{v8_class}} {
 public:
     static bool hasInstance(v8::Handle<v8::Value>, v8::Isolate*);
     static v8::Handle<v8::Object> findInstanceInPrototypeChain(v8::Handle<v8::Value>, v8::Isolate*);
     static v8::Handle<v8::FunctionTemplate> domTemplate(v8::Isolate*);
     static {{cpp_class}}* toNative(v8::Handle<v8::Object> object)
     {
-        return fromInternalPointer(object->GetAlignedPointerFromInternalField(v8DOMWrapperObjectIndex));
+        return fromInternalPointer(blink::toInternalPointer(object));

[haraken, 2014/08/08 09:37:23]

Do we need blink:: ? The same comment for other parts where you added blink::.

[Yuki, 2014/08/08 13:01:25]

Yes, we need blink:: to solve ambiguity between blink::V8T::toInternalPointer(T*
impl) and blink::toInternalPointer(v8::Handle<v8::Object>).

     }
     static {{cpp_class}}* toNativeWithTypeCheck(v8::Isolate*, v8::Handle<v8::Value>);
     static const WrapperTypeInfo wrapperTypeInfo;
-    static void derefObject(void*);
+    static void derefObject(ScriptWrappableBase* internalPointer);

[haraken, 2014/08/08 09:37:23]

Omit |internalPointer|. Blink normally doesn't add variable names in method
declarations. The same comment for other parts in this CL.

[Yuki, 2014/08/08 13:01:25]

I read the style guide at
http://dev.chromium.org/blink/coding-style
especially at [Names] section's 9, which says "Leave meaningless variable names
out of function declarations. A good rule of thumb is if the parameter type name
contains the parameter name (without trailing numbers or pluralization), then
the parameter name isn't needed. Usually, there should be a parameter name for
bools, strings, and numerical types."

I thought that "ScriptWrappableBase" and "internalPointer" differ and it's
helpful to put the name explicitly so that people understand that we need to
pass "internal pointer" here.

[haraken, 2014/08/08 13:14:30]

I think the issue is we're using a word "internalPointer" although it's actually
a ScriptWrapperBase's pointer. It should be named "wrappableBase" or something
like that. Then we can remove the variable name.

You can do this in a separate CL.

     {% if has_visit_dom_wrapper %}
-    static void visitDOMWrapper(void*, const v8::Persistent<v8::Object>&, v8::Isolate*);
+    static void visitDOMWrapper(ScriptWrappableBase* internalPointer, const v8::Persistent<v8::Object>&, v8::Isolate*);
     {% endif %}
     {% if is_active_dom_object %}
     static ActiveDOMObject* toActiveDOMObject(v8::Handle<v8::Object>);
     {% endif %}
     {% if is_event_target %}
     static EventTarget* toEventTarget(v8::Handle<v8::Object>);
     {% endif %}
     {% if interface_name == 'Window' %}
     static v8::Handle<v8::ObjectTemplate> getShadowObjectTemplate(v8::Isolate*);
     {% endif %}
@@ skipping 81 matching lines @@
 #if ENABLE(OILPAN)
     static const int persistentHandleIndex = v8DefaultWrapperInternalFieldCount + {{custom_internal_field_counter}};
     static const int internalFieldCount = v8DefaultWrapperInternalFieldCount + {{custom_internal_field_counter}} + 1;
 #else
     static const int internalFieldCount = v8DefaultWrapperInternalFieldCount + {{custom_internal_field_counter}};
 #endif
     {% else %}
     static const int internalFieldCount = v8DefaultWrapperInternalFieldCount + {{custom_internal_field_counter}};
     {% endif %}
     {# End custom internal fields #}
-    static inline void* toInternalPointer({{cpp_class}}* impl)
+    static inline ScriptWrappableBase* toInternalPointer({{cpp_class}}* impl)
     {
         {% if parent_interface %}
         return V8{{parent_interface}}::toInternalPointer(impl);
         {% else %}
-        return impl;
+        return reinterpret_cast<ScriptWrappableBase*>(static_cast<void*>(impl));

[haraken, 2014/08/08 09:37:23]

Do we need static_cast<void*> ? The same comment for other parts where you added
static_cast<void*>.

[Yuki, 2014/08/08 13:01:25]

The other parts really need this trick because there are cases that T doesn't
inherit from ScriptWrappableBase.

This part is a special exception that I would like to keep the existing behavior
as much as possible in this CL.  I'm planning to change the behavior at
http://crrev.com/420233002 .

We have been using pointers to most-base DOM class as the internal pointers, not
poitners to ScriptWrappable or ScriptWrappableBase.  For example, pointer to
EventTarget as the internal pointer of Node.  This cast returns a pointer to
EventTarget as the internal pointer of Node as same as before.

         {% endif %}
     }
 
-    static inline {{cpp_class}}* fromInternalPointer(void* object)
+    static inline {{cpp_class}}* fromInternalPointer(ScriptWrappableBase* internalPointer)
     {
         {% if parent_interface %}
-        return static_cast<{{cpp_class}}*>(V8{{parent_interface}}::fromInternalPointer(object));
+        return static_cast<{{cpp_class}}*>(V8{{parent_interface}}::fromInternalPointer(internalPointer));
         {% else %}
-        return static_cast<{{cpp_class}}*>(object);
+        return reinterpret_cast<{{cpp_class}}*>(static_cast<void*>(internalPointer));
         {% endif %}
     }
     {% if interface_name == 'Window' %}
     static bool namedSecurityCheckCustom(v8::Local<v8::Object> host, v8::Local<v8::Value> key, v8::AccessType, v8::Local<v8::Value> data);
     static bool indexedSecurityCheckCustom(v8::Local<v8::Object> host, uint32_t index, v8::AccessType, v8::Local<v8::Value> data);
     {% endif %}
     static void installPerContextEnabledProperties(v8::Handle<v8::Object>, {{cpp_class}}*, v8::Isolate*){% if has_per_context_enabled_attributes %};
     {% else %} { }
     {% endif %}
     static void installPerContextEnabledMethods(v8::Handle<v8::Object>, v8::Isolate*){% if per_context_enabled_methods %};
@@ skipping 119 matching lines @@
     v8SetReturnValueFast(callbackInfo, impl.get(), wrappable);
 }
 
 {% if has_event_constructor %}
 bool initialize{{cpp_class}}({{cpp_class}}Init&, const Dictionary&, ExceptionState&, const v8::FunctionCallbackInfo<v8::Value>& info, const String& = "");
 
 {% endif %}
 }
 {% endfilter %}
 #endif // {{v8_class}}_h