[Bindings] Use SetNativeDataProperty to install features on instances

third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 77 matching lines @@
 }
 
 void InstallAttributeInternal(
     v8::Isolate* isolate,
     v8::Local<v8::Object> instance,
     v8::Local<v8::Object> prototype,
     const V8DOMConfiguration::AttributeConfiguration& attribute,
     const DOMWrapperWorld& world) {
   if (!WorldConfigurationApplies(attribute, world))
     return;
+
   v8::Local<v8::Name> name = V8AtomicString(isolate, attribute.name);
+  v8::AccessorNameGetterCallback getter = attribute.getter;
+  v8::AccessorNameSetterCallback setter = attribute.setter;
+  v8::Local<v8::Value> data =
+      v8::External::New(isolate, const_cast<WrapperTypeInfo*>(attribute.data));
 
-  // This method is only being used for installing interfaces which are
-  // enabled through origin trials. Assert here that it is being called with
-  // an attribute configuration for a constructor.
-  // TODO(iclelland): Relax this constraint and allow arbitrary data-type
-  // properties to be added here.
-  DCHECK_EQ(&V8ConstructorAttributeGetter, attribute.getter);

[haraken, 2017/04/13 08:06:09]

Can we keep this assert?

This assert is important since non-data properties should not use
InstallAttributeInternal (which should actually be renamed to
InstallConstructorDataProperty).

Not related to this CL, but we should really consider renaming methods in this
file. They are confusing...

[peria, 2017/04/13 09:51:21]

done
Agreed. I'll rename these functions (and also local variables) in another CL.

-
-  V8PerContextData* per_context_data =
-      V8PerContextData::From(isolate->GetCurrentContext());
-  v8::Local<v8::Function> data =
-      per_context_data->ConstructorForType(attribute.data);
-
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
   DCHECK(attribute.property_location_configuration);
   if (attribute.property_location_configuration &
-      V8DOMConfiguration::kOnInstance)
-    instance
-        ->DefineOwnProperty(
-            isolate->GetCurrentContext(), name, data,
-            static_cast<v8::PropertyAttribute>(attribute.attribute))
-        .ToChecked();
+      V8DOMConfiguration::kOnInstance) {
+    DCHECK(instance

[bashi, 2017/04/13 08:08:36]

Why do we need DCHECK()? ToChecked() should crash on failures.

[peria, 2017/04/13 09:51:21]

Hmm, I thought returning false can be independent from internal exceptions, but
as far as I read V8 code, SetAccessor seems to return iff it has a pending
exception.
So removed.

+               ->SetAccessor(
+                   context, name, getter, setter, data, v8::DEFAULT,
+                   static_cast<v8::PropertyAttribute>(attribute.attribute))
+               .ToChecked());
+  }
   if (attribute.property_location_configuration &
-      V8DOMConfiguration::kOnPrototype)
-    prototype
-        ->DefineOwnProperty(
-            isolate->GetCurrentContext(), name, data,
-            static_cast<v8::PropertyAttribute>(attribute.attribute))
-        .ToChecked();
+      V8DOMConfiguration::kOnPrototype) {
+    DCHECK(prototype
+               ->SetAccessor(
+                   context, name, getter, setter, data, v8::DEFAULT,
+                   static_cast<v8::PropertyAttribute>(attribute.attribute))
+               .ToChecked());
+  }
   if (attribute.property_location_configuration &
       V8DOMConfiguration::kOnInterface)
     NOTREACHED();
 }
 
 void InstallLazyDataAttributeInternal(
     v8::Isolate* isolate,
     v8::Local<v8::ObjectTemplate> instance_template,
     v8::Local<v8::ObjectTemplate> prototype_template,
     const V8DOMConfiguration::AttributeConfiguration& attribute,
@@ skipping 565 matching lines @@
     v8::Isolate* isolate,
     v8::Local<v8::ObjectTemplate> object_template,
     const char* class_string) {
   object_template->Set(
       v8::Symbol::GetToStringTag(isolate),
       V8AtomicString(isolate, class_string),
       static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum));
 }
 
 }  // namespace blink