[Bindings] Replace V8HiddenValue in generated code with V8PrivateProperty

third_party/WebKit/Source/bindings/templates/attributes.cpp.tmpl

 {% from 'utilities.cpp.tmpl' import declare_enum_validation_variable, v8_value_to_local_cpp_value %}
 
 {##############################################################################}
 {% macro attribute_getter(attribute, world_suffix) %}
 static void {{attribute.name}}AttributeGetter{{world_suffix}}(
 {%- if attribute.is_data_type_property %}
 const v8::PropertyCallbackInfo<v8::Value>& info
 {%- else %}
 const v8::FunctionCallbackInfo<v8::Value>& info
 {%- endif %}) {
@@ skipping 42 matching lines @@
   // safe to downcast here.
   LocalDOMWindow* impl = toLocalDOMWindow({{v8_class}}::toImpl(holder));
   {% endif %}{# attribute.is_check_security_for_receiver #}
   {% else %}
   {{cpp_class}}* impl = {{v8_class}}::toImpl(holder);
   {% endif %}{# local_dom_window_only #}
   {% endif %}{# not attribute.is_static #}
 
   {% if attribute.cached_attribute_validation_method %}
   // [CachedAttribute]
-  v8::Local<v8::String> propertyName = v8AtomicString(info.GetIsolate(), "{{attribute.name}}");
+  V8PrivateProperty::Symbol propertySymbol =
+      V8PrivateProperty::createSymbol(info.GetIsolate(), "{{cpp_class}}#{{attribute.name.capitalize()}}");
   if (!impl->{{attribute.cached_attribute_validation_method}}()) {
-    v8::Local<v8::Value> v8Value = V8HiddenValue::getHiddenValue(ScriptState::forFunctionObject(info), holder, propertyName);
-    if (!v8Value.IsEmpty() && !v8Value->IsUndefined()) {
+    v8::Local<v8::Value> v8Value = propertySymbol.getOrUndefined(holder);
+    if (!v8Value->IsUndefined()) {

[Yuki, 2017/03/31 13:38:29]

With this change, we can no longer cache the value of |undefined|.  I don't know
if there is a case, but if the attribute value is |undefined|, we'd like to
cache |undefined|.

[peria, 2017/04/03 04:58:43]

IIUC, the old code also did not cache "undefined".

       v8SetReturnValue(info, v8Value);
       return;
     }
   }
   {% endif %}
 
   {% if attribute.is_check_security_for_receiver and not attribute.is_data_type_property %}
   // Perform a security check for the receiver object.
   {{define_exception_state}}
   {% if local_dom_window_only %}
@@ skipping 52 matching lines @@
   {% if attribute.reflect_only %}
   {{release_only_check(attribute.reflect_only, attribute.reflect_missing,
                        attribute.reflect_invalid, attribute.reflect_empty,
                        attribute.cpp_value)
       | indent(2)}}
   {% endif %}
 
   {% if attribute.cached_attribute_validation_method %}
   // [CachedAttribute]
   v8::Local<v8::Value> v8Value({{attribute.cpp_value_to_v8_value}});
-  V8HiddenValue::setHiddenValue(ScriptState::forFunctionObject(info), holder, propertyName, v8Value);
+  propertySymbol.set(holder, v8Value);
   {% endif %}
 
   {% if attribute.is_explicit_nullable %}
   if (isNull) {
     v8SetReturnValueNull(info);
     return;
   }
   {% endif %}
 
   {% if attribute.is_keep_alive_for_gc %}
   // Keep the wrapper object for the return value alive as long as |this|
   // object is alive in order to save creation time of the wrapper object.
   if ({{attribute.cpp_value}} && DOMDataStore::setReturnValue{{world_suffix}}(info.GetReturnValue(), {{attribute.cpp_value}}))
     return;
   v8::Local<v8::Value> v8Value(ToV8({{attribute.cpp_value}}, holder, info.GetIsolate()));
-  V8HiddenValue::setHiddenValue(ScriptState::current(info.GetIsolate()), holder, v8AtomicString(info.GetIsolate(), "KeepAlive#{{interface_name}}#{{attribute.name}}"), v8Value);
+  V8PrivateProperty::createSymbol(
+      info.GetIsolate(), "KeepAlive#{{interface_name}}#{{attribute.name}}")
+      .set(holder, v8Value);
   {% endif %}
 
   {% if world_suffix %}
   {{attribute.v8_set_return_value_for_main_world}};
   {% else %}
   {{attribute.v8_set_return_value}};
   {% endif %}
 
   {% if attribute.is_save_same_object %}
   // [SaveSameObject]
@@ skipping 108 matching lines @@
 {% macro attribute_setter(attribute, world_suffix) %}
 static void {{attribute.name}}AttributeSetter{{world_suffix}}(
 {%- if attribute.has_cross_origin_setter %}
 v8::Local<v8::Value> v8Value, const V8CrossOriginSetterInfo& info
 {%- elif attribute.is_data_type_property %}
 v8::Local<v8::Value> v8Value, const v8::PropertyCallbackInfo<void>& info
 {%- else %}
 v8::Local<v8::Value> v8Value, const v8::FunctionCallbackInfo<v8::Value>& info
 {%- endif %}) {
   {% filter format_remove_duplicates(['ExceptionState exceptionState']) %}
+  v8::Isolate* isolate = info.GetIsolate();
+  ALLOW_UNUSED_LOCAL(isolate);
+
   {% set define_exception_state -%}
-  ExceptionState exceptionState(info.GetIsolate(), ExceptionState::SetterContext, "{{interface_name}}", "{{attribute.name}}");
+  ExceptionState exceptionState(isolate, ExceptionState::SetterContext, "{{interface_name}}", "{{attribute.name}}");
   {%- endset %}
 
   {% if attribute.is_lenient_this %}
   // [LenientThis]
   // Make sure that info.Holder() really points to an instance if [LenientThis].
-  if (!{{v8_class}}::hasInstance(info.Holder(), info.GetIsolate()))
+  if (!{{v8_class}}::hasInstance(info.Holder(), isolate))
     return; // Return silently because of [LenientThis].
   {% endif %}
 
   {% if not attribute.is_static and not attribute.is_replaceable %}
   v8::Local<v8::Object> holder = info.Holder();
   {% if attribute.is_put_forwards %}
   {{cpp_class}}* proxyImpl = {{v8_class}}::toImpl(holder);
   {{attribute.cpp_type}} impl = WTF::getPtr(proxyImpl->{{attribute.name}}());
   if (!impl)
     return;
@@ skipping 11 matching lines @@
   {% else %}
   {{cpp_class}}* impl = {{v8_class}}::toImpl(holder);
   {% endif %}{# local_dom_window_only #}
   {% endif %}
   {% endif %}
 
   {% if attribute.is_check_security_for_receiver and not attribute.is_data_type_property %}
   // Perform a security check for the receiver object.
   {{define_exception_state}}
   {% if local_dom_window_only %}
-  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), uncheckedImpl, exceptionState)) {
+  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(isolate), uncheckedImpl, exceptionState)) {
   {% else %}
-  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), impl, exceptionState)) {
+  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(isolate), impl, exceptionState)) {
   {% endif %}{# local_dom_window_only #}
     v8SetReturnValue(info, v8Value);
     return;
   }
   {% if local_dom_window_only %}
   LocalDOMWindow* impl = toLocalDOMWindow(uncheckedImpl);
   {% endif %}{# local_dom_window_only #}
   {% endif %}
 
   {% if attribute.is_check_security_for_return_value %}
 #error Attribute setter with the security check for the return value is not supported.  Since the return value is the given value to be set, it\'s meaningless to perform the security check for the return value.
   {% endif %}
 
   {% if attribute.is_custom_element_callbacks or
         (attribute.is_reflect and not (attribute.idl_type == 'DOMString' and is_node)) %}
   // Skip on compact node DOMString getters.
   V0CustomElementProcessingStack::CallbackDeliveryScope deliveryScope;
   {% endif %}
 
   {% if attribute.has_setter_exception_state %}
   {{define_exception_state}}
   {% endif %}
 
   // Prepare the value to be set.
   {% if attribute.idl_type == 'EventHandler' %}
   {% if not is_node %}
-  moveEventListenerToNewWrapper(info.GetIsolate(), holder, {{attribute.event_handler_getter_expression}}, v8Value, {{v8_class}}::eventListenerCacheIndex);
+  moveEventListenerToNewWrapper(isolate, holder, {{attribute.event_handler_getter_expression}}, v8Value, {{v8_class}}::eventListenerCacheIndex);
   {% endif %}
   {% else %}{# not EventHandler #}
   {{v8_value_to_local_cpp_value(attribute) | indent(2)}}
   {% endif %}
 
   {% if attribute.has_type_checking_interface %}
   // Type check per: http://heycam.github.io/webidl/#es-interface
   if (!cppValue{% if attribute.is_nullable %} && !isUndefinedOrNull(v8Value){% endif %}) {
     exceptionState.throwTypeError("The provided value is not of type '{{attribute.idl_type}}'.");
     return;
   }
   {% endif %}
 
   {% if attribute.enum_values %}
   // Type check per: http://heycam.github.io/webidl/#dfn-attribute-setter
   // Returns undefined without setting the value if the value is invalid.
   DummyExceptionStateForTesting dummyExceptionState;
   {{declare_enum_validation_variable(attribute.enum_values) | indent(2)}}
   if (!isValidEnum(cppValue, validValues, WTF_ARRAY_LENGTH(validValues), "{{attribute.enum_type}}", dummyExceptionState)) {
-    currentExecutionContext(info.GetIsolate())->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, dummyExceptionState.message()));
+    currentExecutionContext(isolate)->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, dummyExceptionState.message()));
     return;
   }
   {% endif %}
 
   {% if attribute.is_call_with_execution_context or attribute.is_setter_call_with_execution_context %}
-  ExecutionContext* executionContext = currentExecutionContext(info.GetIsolate());
+  ExecutionContext* executionContext = currentExecutionContext(isolate);
   {% endif %}
 
   {% if attribute.is_call_with_script_state %}
   {% if attribute.is_static %}
   ScriptState* scriptState = ScriptState::forFunctionObject(info);
   {% else %}
   ScriptState* scriptState = ScriptState::forReceiverObject(info);
   {% endif %}
   {% endif %}
 
   {% if attribute.is_replaceable %}
-  v8::Local<v8::String> propertyName = v8AtomicString(info.GetIsolate(), "{{attribute.name}}");
+  v8::Local<v8::String> propertyName = v8AtomicString(isolate, "{{attribute.name}}");
   {% endif %}
   {{attribute.cpp_setter}};
 
   {% if attribute.cached_attribute_validation_method %}
   // [CachedAttribute]
-  // Invalidate the cached value.
-  V8HiddenValue::deleteHiddenValue(ScriptState::forFunctionObject(info), holder, v8AtomicString(info.GetIsolate(), "{{attribute.name}}"));
+  // Invalidate the cached value. Actually deleting the attribute would make
+  // force the object into dictionary mode which is unnecessarily slow.
+  // Instead, we replace the cached value with "undefined".

[Yuki, 2017/03/31 13:38:29]

Is this really happening so often?  I'd expect that it's rare and negligible.

[peria, 2017/04/03 04:58:43]

I agree. It happens rarely.

+  V8PrivateProperty::createSymbol(
+      isolate, "{{cpp_class}}#{{attribute.name.capitalize()}}")
+      .set(holder, v8::Undefined(isolate));
   {% endif %}
 }
 {% endfilter %}{# format_remove_duplicates #}
 {% endmacro %}
 
 
 {##############################################################################}
 {% macro attribute_setter_callback(attribute, world_suffix) %}
 void {{v8_class_or_partial}}::{{attribute.name}}AttributeSetterCallback{{world_suffix}}(
 {%- if attribute.is_data_type_property %}
@@ skipping 98 matching lines @@
 static const V8DOMConfiguration::AccessorConfiguration accessorConfiguration[] = {
   {{attribute_configuration(attribute)}}
 };
 for (const auto& accessorConfig : accessorConfiguration)
   V8DOMConfiguration::installAccessor(isolate, world, v8::Local<v8::Object>(), prototypeObject, interfaceObject, signature, accessorConfig);
 {% endfilter %}{# runtime_enabled #}
 {% endfilter %}{# secure_context #}
 {% endfilter %}{# exposed #}
 {% endfor %}
 {% endmacro %}