Remove [OverrideBuiltins] from HTMLCollection

Source/bindings/scripts/code_generator_v8.pm

 # Copyright (C) 2005, 2006 Nikolas Zimmermann <zimmermann@kde.org>
 # Copyright (C) 2006 Anders Carlsson <andersca@mac.com>
 # Copyright (C) 2006 Samuel Weinig <sam.weinig@gmail.com>
 # Copyright (C) 2006 Alexey Proskuryakov <ap@webkit.org>
 # Copyright (C) 2006 Apple Computer, Inc.
 # Copyright (C) 2007, 2008, 2009, 2012 Google Inc.
 # Copyright (C) 2009 Cameron McCormack <cam@mcc.id.au>
 # Copyright (C) Research In Motion Limited 2010. All rights reserved.
 # Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
 # Copyright (C) 2012 Ericsson AB. All rights reserved.
@@ skipping 4063 matching lines @@
     my $isNull = GenerateIsNullExpression($returnType, "element");
     my $nativeValue = "element";
     $nativeValue .= ".release()" if (IsRefPtrType($returnType));
     my $returnJSValueCode = NativeToJSValue($namedGetterFunction->type, $namedGetterFunction->extendedAttributes, $nativeValue, "    ", "", "info.GetIsolate()", "info", "collection", "", "return");
     my $raisesExceptions = $namedGetterFunction->extendedAttributes->{"RaisesException"};
     my $methodCallCode = GenerateMethodCall($returnType, "element", "collection->${methodName}", "propertyName", $raisesExceptions);
 
     my $code = "static void namedPropertyGetter(v8::Local<v8::String> name, const v8::PropertyCallbackInfo<v8::Value>& info)\n";
     $code .= "{\n";
     if (!$interface->extendedAttributes->{"OverrideBuiltins"}) {
+        $code .= "    if (info.Holder()->HasRealNamedProperty(name))\n";

[arv (Not doing code reviews), 2014/02/08 18:13:58]

I looked into this too.

I find this confusing. Maybe it is just bad V8 API names?

I would think we need to do something like:

1. Check own property for real property
2. Check prototype chain for real property
3. Check for own callback property
4. Check prototype chain for callback property

With this change, based on the (possibly bad) V8 names. It looks like we are
only doing 1 & 2.

Don't we need to look for callback properties to make sure that an ID/name is
not shadowing the length, item and namedItem?

[Inactive, 2014/02/08 18:57:04]

I looked at the V8 implementation and 
HasRealNamedProperty() works for callback properties as well. Therefore, I
believe the other check is redundant and can be removed.

[Inactive, 2014/02/08 21:34:04]

Here is the implementation of those 2 methods if you want to compare:
http://pastebin.com/AvmgTwuV

+        $code .= "        return;\n";
         $code .= "    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())\n";

[arv (Not doing code reviews), 2014/02/08 18:13:58]

So confusing. Why is this Get and not Has?

[Inactive, 2014/02/08 18:57:04]

This API does not have an Has equivalent for some reason, only a Get.

         $code .= "        return;\n";
-        $code .= "    if (info.Holder()->HasRealNamedCallbackProperty(name))\n";
-        $code .= "        return;\n";
-        $code .= "    if (info.Holder()->HasRealNamedProperty(name))\n";
-        $code .= "        return;\n";
         $code .= "\n";
     }
     $code .= "    ${implClassName}* collection = ${v8ClassName}::toNative(info.Holder());\n";
     $code .= "    AtomicString propertyName = toCoreAtomicString(name);\n";
     if ($raisesExceptions) {
         $code .= "    ExceptionState exceptionState(info.Holder(), info.GetIsolate());\n";
     }
     $code .= $methodCallCode . "\n";
     if ($raisesExceptions) {
         $code .= "    if (exceptionState.throwIfNeeded())\n";
@@ skipping 16 matching lines @@
     my $interfaceName = $interface->name;
 
     my $type = $namedSetterFunction->parameters->[1]->type;
     my $raisesExceptions = $namedSetterFunction->extendedAttributes->{"RaisesException"};
     my $treatNullAs = $namedSetterFunction->parameters->[1]->extendedAttributes->{"TreatNullAs"};
     my $treatUndefinedAs = $namedSetterFunction->parameters->[1]->extendedAttributes->{"TreatUndefinedAs"};
 
     my $code = "static void namedPropertySetter(v8::Local<v8::String> name, v8::Local<v8::Value> jsValue, const v8::PropertyCallbackInfo<v8::Value>& info)\n";
     $code .= "{\n";
     if (!$interface->extendedAttributes->{"OverrideBuiltins"}) {
+        $code .= "    if (info.Holder()->HasRealNamedProperty(name))\n";
+        $code .= "        return;\n";
         $code .= "    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())\n";
         $code .= "        return;\n";
-        $code .= "    if (info.Holder()->HasRealNamedCallbackProperty(name))\n";
-        $code .= "        return;\n";
-        $code .= "    if (info.Holder()->HasRealNamedProperty(name))\n";
-        $code .= "        return;\n";
         $code .= "\n";
     }
 
     my $asSetterValue = 0;
     $code .= "    ${implClassName}* collection = ${v8ClassName}::toNative(info.Holder());\n";
     $code .= JSValueToNativeStatement($namedSetterFunction->parameters->[0]->type, $namedSetterFunction->extendedAttributes, $asSetterValue, "name", "propertyName", "    ", "info.GetIsolate()");
     $code .= JSValueToNativeStatement($namedSetterFunction->parameters->[1]->type, $namedSetterFunction->extendedAttributes, $asSetterValue, "jsValue", "propertyValue", "    ", "info.GetIsolate()");
 
     my $extraArguments = "";
     if ($raisesExceptions || IsIntegerType($type)) {
@@ skipping 2341 matching lines @@
         if ($parameter->type eq "SerializedScriptValue") {
             return 1;
         } elsif (IsIntegerType($parameter->type)) {
             return 1;
         }
     }
     return 0;
 }
 
 1;