Make customElements.define faster

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

Index: third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
index 2068c1e092892d2c822519d979a903f8772370d3..7d5f9354e9e154d1f9ece145924e91aaea53d806 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
@@ -4,7 +4,7 @@
 
 #include "bindings/core/v8/ScriptCustomElementDefinitionBuilder.h"
 
-#include "bindings/core/v8/DOMWrapperWorld.h"
+#include <utility>
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/IDLTypes.h"
 #include "bindings/core/v8/NativeValueTraitsImpl.h"
@@ -17,25 +17,15 @@
 
 namespace blink {
 
-ScriptCustomElementDefinitionBuilder*
-    ScriptCustomElementDefinitionBuilder::stack_ = nullptr;
-
 ScriptCustomElementDefinitionBuilder::ScriptCustomElementDefinitionBuilder(
     ScriptState* script_state,
     CustomElementRegistry* registry,
     const ScriptValue& constructor,
     ExceptionState& exception_state)
-    : prev_(stack_),
-      script_state_(script_state),
+    : script_state_(script_state),
       registry_(registry),
       constructor_value_(constructor.V8Value()),
-      exception_state_(exception_state) {
-  stack_ = this;
-}
-
-ScriptCustomElementDefinitionBuilder::~ScriptCustomElementDefinitionBuilder() {
-  stack_ = prev_;
-}
+      exception_state_(exception_state) {}
 
 bool ScriptCustomElementDefinitionBuilder::CheckConstructorIntrinsics() {
   DCHECK(script_state_->World().IsMainWorld());
@@ -54,46 +44,39 @@ bool ScriptCustomElementDefinitionBuilder::CheckConstructorIntrinsics() {
 }
 
 bool ScriptCustomElementDefinitionBuilder::CheckConstructorNotRegistered() {
-  if (ScriptCustomElementDefinition::ForConstructor(script_state_.Get(),
-                                                    registry_, constructor_)) {
-    // Constructor is already registered.
-    exception_state_.ThrowDOMException(
-        kNotSupportedError,
-        "this constructor has already been used with this registry");
-    return false;
-  }
-  for (auto builder = prev_; builder; builder = builder->prev_) {
-    CHECK(!builder->constructor_.IsEmpty());
-    if (registry_ != builder->registry_ ||
-        constructor_ != builder->constructor_) {
-      continue;
-    }
-    exception_state_.ThrowDOMException(
-        kNotSupportedError,
-        "this constructor is already being defined in this registry");
-    return false;
+  if (!ScriptCustomElementDefinition::ForConstructor(script_state_.Get(),
+                                                     registry_, constructor_)) {
+    return true;
   }
-  return true;
+  // Constructor is already registered.
+  exception_state_.ThrowDOMException(
+      kNotSupportedError,
+      "this constructor has already been used with this registry");
+  return false;
 }
 
 bool ScriptCustomElementDefinitionBuilder::ValueForName(
+    v8::Isolate* isolate,
+    v8::Local<v8::Context>& context,
+    const v8::TryCatch& try_catch,
     const v8::Local<v8::Object>& object,
     const StringView& name,
     v8::Local<v8::Value>& value) const {
-  v8::Isolate* isolate = script_state_->GetIsolate();
-  v8::Local<v8::Context> context = script_state_->GetContext();
   v8::Local<v8::String> name_string = V8AtomicString(isolate, name);
-  v8::TryCatch try_catch(isolate);
   if (!object->Get(context, name_string).ToLocal(&value)) {
     exception_state_.RethrowV8Exception(try_catch.Exception());
     return false;
   }
-  return true;
+  return script_state_->ContextIsValid();

[haraken, 2017/04/28 08:49:30]

What is this change for?

 }
 
 bool ScriptCustomElementDefinitionBuilder::CheckPrototype() {
+  v8::Isolate* isolate = script_state_->GetIsolate();
+  v8::Local<v8::Context> context = script_state_->GetContext();
+  v8::TryCatch try_catch(isolate);

[haraken, 2017/04/28 08:49:30]

Are you intentionally moving the TryCatch scope to the caller sites?

   v8::Local<v8::Value> prototype_value;
-  if (!ValueForName(constructor_, "prototype", prototype_value))
+  if (!ValueForName(isolate, context, try_catch, constructor_, "prototype",
+                    prototype_value))
     return false;
   if (!prototype_value->IsObject()) {
     exception_state_.ThrowTypeError("constructor prototype is not an object");
@@ -102,16 +85,19 @@ bool ScriptCustomElementDefinitionBuilder::CheckPrototype() {
   prototype_ = prototype_value.As<v8::Object>();
   // If retrieving the prototype destroyed the context, indicate that
   // defining the element should not proceed.
-  return true;
+  return script_state_->ContextIsValid();
 }
 
 bool ScriptCustomElementDefinitionBuilder::CallableForName(
+    v8::Isolate* isolate,
+    v8::Local<v8::Context>& context,
+    const v8::TryCatch& try_catch,
     const StringView& name,
     v8::Local<v8::Function>& callback) const {
   v8::Local<v8::Value> value;
-  if (!ValueForName(prototype_, name, value))
+  if (!ValueForName(isolate, context, try_catch, prototype_, name, value))
     return false;
-  // "undefined" means "omitted", so return true.
+  // "undefined" means "omitted" which is valid.
   if (value->IsUndefined())
     return true;
   if (!value->IsFunction()) {
@@ -123,16 +109,19 @@ bool ScriptCustomElementDefinitionBuilder::CallableForName(
   return true;
 }
 
-bool ScriptCustomElementDefinitionBuilder::RetrieveObservedAttributes() {
+bool ScriptCustomElementDefinitionBuilder::RetrieveObservedAttributes(
+    v8::Isolate* isolate,
+    v8::Local<v8::Context>& context,
+    const v8::TryCatch& try_catch) {
   v8::Local<v8::Value> observed_attributes_value;
-  if (!ValueForName(constructor_, "observedAttributes",
-                    observed_attributes_value))
+  if (!ValueForName(isolate, context, try_catch, constructor_,
+                    "observedAttributes", observed_attributes_value))
     return false;
   if (observed_attributes_value->IsUndefined())
     return true;
   Vector<String> list = NativeValueTraits<IDLSequence<IDLString>>::NativeValue(
-      script_state_->GetIsolate(), observed_attributes_value, exception_state_);
-  if (exception_state_.HadException())
+      isolate, observed_attributes_value, exception_state_);
+  if (exception_state_.HadException() || !script_state_->ContextIsValid())
     return false;
   if (list.IsEmpty())
     return true;
@@ -145,21 +134,29 @@ bool ScriptCustomElementDefinitionBuilder::RetrieveObservedAttributes() {
 bool ScriptCustomElementDefinitionBuilder::RememberOriginalProperties() {
   // Spec requires to use values of these properties at the point
   // CustomElementDefinition is built, even if JS changes them afterwards.
-  return CallableForName("connectedCallback", connected_callback_) &&
-         CallableForName("disconnectedCallback", disconnected_callback_) &&
-         CallableForName("adoptedCallback", adopted_callback_) &&
-         CallableForName("attributeChangedCallback",
+  v8::Isolate* isolate = script_state_->GetIsolate();
+  v8::Local<v8::Context> context = script_state_->GetContext();
+  v8::TryCatch try_catch(isolate);
+  return CallableForName(isolate, context, try_catch, "connectedCallback",
+                         connected_callback_) &&
+         CallableForName(isolate, context, try_catch, "disconnectedCallback",
+                         disconnected_callback_) &&
+         CallableForName(isolate, context, try_catch, "adoptedCallback",
+                         adopted_callback_) &&
+         CallableForName(isolate, context, try_catch,
+                         "attributeChangedCallback",
                          attribute_changed_callback_) &&
          (attribute_changed_callback_.IsEmpty() ||
-          RetrieveObservedAttributes());
+          RetrieveObservedAttributes(isolate, context, try_catch));
 }
 
 CustomElementDefinition* ScriptCustomElementDefinitionBuilder::Build(
-    const CustomElementDescriptor& descriptor) {
+    const CustomElementDescriptor& descriptor,
+    CustomElementDefinition::Id id) {
   return ScriptCustomElementDefinition::Create(
-      script_state_.Get(), registry_, descriptor, constructor_,
+      script_state_.Get(), registry_, descriptor, id, constructor_,
       connected_callback_, disconnected_callback_, adopted_callback_,
-      attribute_changed_callback_, observed_attributes_);
+      attribute_changed_callback_, std::move(observed_attributes_));
 }
 
 }  // namespace blink