Make customElements.define faster

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

Index: third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
index 8382d22ad1110f6abc6e82d37fd3b254a61f30c6..6db5be5af3fb715755e1b898990485fbfcd50ac8 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
@@ -24,36 +24,22 @@
 
 namespace blink {
 
-// Retrieves the custom elements constructor -> name map, creating it
-// if necessary.
-static v8::Local<v8::Map> EnsureCustomElementRegistryMap(
-    ScriptState* script_state,
-    CustomElementRegistry* registry) {
-  CHECK(script_state->World().IsMainWorld());
-  v8::Isolate* isolate = script_state->GetIsolate();
-
-  V8PrivateProperty::Symbol symbol =
-      V8PrivateProperty::GetCustomElementRegistryMap(isolate);
-  v8::Local<v8::Object> wrapper = ToV8(registry, script_state).As<v8::Object>();
-  v8::Local<v8::Value> map = symbol.GetOrUndefined(wrapper);
-  if (map->IsUndefined()) {
-    map = v8::Map::New(isolate);
-    symbol.Set(wrapper, map);
-  }
-  return map.As<v8::Map>();
-}
-
 ScriptCustomElementDefinition* ScriptCustomElementDefinition::ForConstructor(
     ScriptState* script_state,
     CustomElementRegistry* registry,
     const v8::Local<v8::Value>& constructor) {
-  v8::Local<v8::Map> map =
-      EnsureCustomElementRegistryMap(script_state, registry);
-  v8::Local<v8::Value> name_value =
-      map->Get(script_state->GetContext(), constructor).ToLocalChecked();
-  if (!name_value->IsString())
+  if (!constructor->IsObject())
+    return nullptr;
+  auto private_id =
+      script_state->PerContextData()->GetPrivateCustomElementDefinitionId();
+  v8::Local<v8::Value> id_value;
+  if (!V8Call(constructor.As<v8::Object>()->GetPrivate(
+                  script_state->GetContext(), private_id),
+              id_value))
     return nullptr;
-  AtomicString name = ToCoreAtomicString(name_value.As<v8::String>());
+  if (!id_value->IsUint32())
+    return nullptr;
+  uint32_t id = id_value.As<v8::Uint32>()->Value();
 
   // This downcast is safe because only
   // ScriptCustomElementDefinitions have a name associated with a V8
@@ -79,9 +65,15 @@ ScriptCustomElementDefinition* ScriptCustomElementDefinition::ForConstructor(
   // currently only one implementation of CustomElementDefinition in
   // product code and that is ScriptCustomElementDefinition. But
   // that may change in the future.
-  CustomElementDefinition* definition = registry->DefinitionForName(name);
+  CustomElementDefinition* definition = registry->DefinitionForId(id);
   CHECK(definition);
-  return static_cast<ScriptCustomElementDefinition*>(definition);
+  ScriptCustomElementDefinition* scriptDefinition =
+      static_cast<ScriptCustomElementDefinition*>(definition);
+  // v8::Object::GetPrivate notes that private properties may be
+  // inherited in future, so check that the definition's constructor
+  // is exactly the passed object.
+  return scriptDefinition->constructor_.Get() == constructor ? scriptDefinition
+                                                             : nullptr;
 }
 
 using SymbolGetter = V8PrivateProperty::Symbol (*)(v8::Isolate*);
@@ -105,42 +97,24 @@ ScriptCustomElementDefinition* ScriptCustomElementDefinition::Create(
     ScriptState* script_state,
     CustomElementRegistry* registry,
     const CustomElementDescriptor& descriptor,
+    CustomElementDefinition::Id id,
     const v8::Local<v8::Object>& constructor,
     const v8::Local<v8::Function>& connected_callback,
     const v8::Local<v8::Function>& disconnected_callback,
     const v8::Local<v8::Function>& adopted_callback,
     const v8::Local<v8::Function>& attribute_changed_callback,
     const HashSet<AtomicString>& observed_attributes) {
-  ScriptCustomElementDefinition* definition = new ScriptCustomElementDefinition(
+  auto private_id =
+      script_state->PerContextData()->GetPrivateCustomElementDefinitionId();
+  CHECK(constructor
+            ->SetPrivate(
+                script_state->GetContext(), private_id,
+                v8::Integer::NewFromUnsigned(script_state->GetIsolate(), id))
+            .ToChecked());
+  return new ScriptCustomElementDefinition(
       script_state, descriptor, constructor, connected_callback,
       disconnected_callback, adopted_callback, attribute_changed_callback,
       observed_attributes);
-
-  // Add a constructor -> name mapping to the registry.
-  v8::Local<v8::Value> name_value =
-      V8String(script_state->GetIsolate(), descriptor.GetName());
-  v8::Local<v8::Map> map =
-      EnsureCustomElementRegistryMap(script_state, registry);
-  map->Set(script_state->GetContext(), constructor, name_value)
-      .ToLocalChecked();
-  definition->constructor_.SetPhantom();
-
-  // We add the callbacks here to keep them alive. We use the name as
-  // the key because it is unique per-registry.
-  v8::Local<v8::Object> object = v8::Object::New(script_state->GetIsolate());
-  KeepAlive(object, V8PrivateProperty::GetCustomElementConnectedCallback,
-            connected_callback, definition->connected_callback_, script_state);
-  KeepAlive(object, V8PrivateProperty::GetCustomElementDisconnectedCallback,
-            disconnected_callback, definition->disconnected_callback_,
-            script_state);
-  KeepAlive(object, V8PrivateProperty::GetCustomElementAdoptedCallback,
-            adopted_callback, definition->adopted_callback_, script_state);
-  KeepAlive(object, V8PrivateProperty::GetCustomElementAttributeChangedCallback,
-            attribute_changed_callback, definition->attribute_changed_callback_,
-            script_state);
-  map->Set(script_state->GetContext(), name_value, object).ToLocalChecked();
-
-  return definition;
 }
 
 ScriptCustomElementDefinition::ScriptCustomElementDefinition(
@@ -154,7 +128,21 @@ ScriptCustomElementDefinition::ScriptCustomElementDefinition(
     const HashSet<AtomicString>& observed_attributes)
     : CustomElementDefinition(descriptor, observed_attributes),
       script_state_(script_state),
-      constructor_(script_state->GetIsolate(), constructor) {}
+      constructor_(script_state->GetIsolate(), this, constructor),
+      connected_callback_(this),
+      disconnected_callback_(this),
+      adopted_callback_(this),
+      attribute_changed_callback_(this) {
+  v8::Isolate* isolate = script_state->GetIsolate();
+  if (!connected_callback.IsEmpty())
+    connected_callback_.Set(isolate, connected_callback);
+  if (!disconnected_callback.IsEmpty())
+    disconnected_callback_.Set(isolate, disconnected_callback);
+  if (!adopted_callback.IsEmpty())
+    adopted_callback_.Set(isolate, adopted_callback);
+  if (!attribute_changed_callback.IsEmpty())
+    attribute_changed_callback_.Set(isolate, attribute_changed_callback);
+}
 
 static void DispatchErrorEvent(v8::Isolate* isolate,
                                v8::Local<v8::Value> exception,
@@ -165,6 +153,14 @@ static void DispatchErrorEvent(v8::Isolate* isolate,
       isolate, exception, constructor.As<v8::Function>()->GetScriptOrigin());
 }
 
+DEFINE_TRACE_WRAPPERS(ScriptCustomElementDefinition) {
+  visitor->TraceWrappers(constructor_);
+  visitor->TraceWrappers(connected_callback_);

[Michael Lippautz, 2017/04/19 08:49:12]

Continued from the declaration: 
  visitor->TraceWrappers(connected_callback.Cast<v8::Value>());

+  visitor->TraceWrappers(disconnected_callback_);
+  visitor->TraceWrappers(adopted_callback_);
+  visitor->TraceWrappers(attribute_changed_callback_);
+}
+
 HTMLElement* ScriptCustomElementDefinition::HandleCreateElementSyncException(
     Document& document,
     const QualifiedName& tag_name,
@@ -284,7 +280,7 @@ Element* ScriptCustomElementDefinition::CallConstructor() {
 
 v8::Local<v8::Object> ScriptCustomElementDefinition::Constructor() const {
   DCHECK(!constructor_.IsEmpty());
-  return constructor_.NewLocal(script_state_->GetIsolate());
+  return constructor_.NewLocal(script_state_->GetIsolate()).As<v8::Object>();

[Michael Lippautz, 2017/04/19 08:49:12]

This cast is not needed then. 

(And all below.)

 }
 
 // CustomElementDefinition
@@ -331,7 +327,8 @@ void ScriptCustomElementDefinition::RunConnectedCallback(Element* element) {
     return;
   ScriptState::Scope scope(script_state_.Get());
   v8::Isolate* isolate = script_state_->GetIsolate();
-  RunCallback(connected_callback_.NewLocal(isolate), element);
+  RunCallback(connected_callback_.NewLocal(isolate).As<v8::Function>(),
+              element);
 }
 
 void ScriptCustomElementDefinition::RunDisconnectedCallback(Element* element) {
@@ -339,7 +336,8 @@ void ScriptCustomElementDefinition::RunDisconnectedCallback(Element* element) {
     return;
   ScriptState::Scope scope(script_state_.Get());
   v8::Isolate* isolate = script_state_->GetIsolate();
-  RunCallback(disconnected_callback_.NewLocal(isolate), element);
+  RunCallback(disconnected_callback_.NewLocal(isolate).As<v8::Function>(),
+              element);
 }
 
 void ScriptCustomElementDefinition::RunAdoptedCallback(Element* element,
@@ -352,7 +350,7 @@ void ScriptCustomElementDefinition::RunAdoptedCallback(Element* element,
   v8::Local<v8::Value> argv[] = {
       ToV8(old_owner, script_state_->GetContext()->Global(), isolate),
       ToV8(new_owner, script_state_->GetContext()->Global(), isolate)};
-  RunCallback(adopted_callback_.NewLocal(isolate), element,
+  RunCallback(adopted_callback_.NewLocal(isolate).As<v8::Function>(), element,
               WTF_ARRAY_LENGTH(argv), argv);
 }
 
@@ -370,8 +368,8 @@ void ScriptCustomElementDefinition::RunAttributeChangedCallback(
       V8StringOrNull(isolate, new_value),
       V8StringOrNull(isolate, name.NamespaceURI()),
   };
-  RunCallback(attribute_changed_callback_.NewLocal(isolate), element,
-              WTF_ARRAY_LENGTH(argv), argv);
+  RunCallback(attribute_changed_callback_.NewLocal(isolate).As<v8::Function>(),
+              element, WTF_ARRAY_LENGTH(argv), argv);
 }
 
 }  // namespace blink