Recursively defining custom elements should not lead to redefinition

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/ScriptCustomElementDefinitionBuilder.h"
 
 #include "bindings/core/v8/DOMWrapperWorld.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptCustomElementDefinition.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/ScriptValue.h"
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8BindingMacros.h"
+#include "bindings/core/v8/V8CustomElementsRegistry.h"
+#include "bindings/core/v8/V8PrivateProperty.h"
 #include "core/dom/ExceptionCode.h"
 
 namespace blink {
 
 ScriptCustomElementDefinitionBuilder::ScriptCustomElementDefinitionBuilder(
     ScriptState* scriptState,
     CustomElementsRegistry* registry,
     const ScriptValue& constructor,
     ExceptionState& exceptionState)
     : m_scriptState(scriptState)
     , m_registry(registry)
     , m_constructorValue(constructor.v8Value())
     , m_exceptionState(exceptionState)
+    , m_didStartDefiningConstructor(false)
 {
 }
 
+ScriptCustomElementDefinitionBuilder::~ScriptCustomElementDefinitionBuilder()
+{
+    if (m_didStartDefiningConstructor) {
+        bool didDelete = v8CallOrCrash(constructorsBeingDefined()->Delete(
+            m_scriptState->context(),
+            m_constructor));
+        CHECK(didDelete);
+    }
+}
+
 bool ScriptCustomElementDefinitionBuilder::checkConstructorIntrinsics()
 {
     DCHECK(m_scriptState->world().isMainWorld());
 
     // The signature of CustomElementsRegistry.define says this is a
     // Function
     // https://html.spec.whatwg.org/multipage/scripting.html#customelementsregistry
     CHECK(m_constructorValue->IsFunction());
     m_constructor = m_constructorValue.As<v8::Object>();
     if (!m_constructor->IsConstructor()) {
         m_exceptionState.throwTypeError(
             "constructor argument is not a constructor");
         return false;
     }
     return true;
 }
 
 bool ScriptCustomElementDefinitionBuilder::checkConstructorNotRegistered()
 {
+    // Check existing constructors
     if (ScriptCustomElementDefinition::forConstructor(
         m_scriptState.get(),
         m_registry,
         m_constructor)) {
 
         // Constructor is already registered.
         m_exceptionState.throwDOMException(
             NotSupportedError,
             "this constructor has already been used with this registry");
         return false;
     }
+
+    // Check constructors that are being processed by builders right now.
+    v8::Local<v8::Set> constructors = constructorsBeingDefined();
+    if (v8CallOrCrash(

[haraken, 2016/06/02 02:29:36]

Would you help me understand why it's not enough to check the
m_didStartDefiningConstructor flag to detect if you're in a recursion or not?
Why do we need to store a reference from constructors to m_constructor?

+        constructors->Has(m_scriptState->context(), m_constructor))) {
+        m_exceptionState.throwDOMException(
+            NotSupportedError,
+            "this constructor is already being defined in this registry");
+        return false;
+    }
+    m_didStartDefiningConstructor = true;
+    v8CallOrCrash(constructors->Add(m_scriptState->context(), m_constructor));
     return true;
 }
 
 bool ScriptCustomElementDefinitionBuilder::checkPrototype()
 {
     v8::Isolate* isolate = m_scriptState->isolate();
     v8::Local<v8::Context> context = m_scriptState->context();
     v8::Local<v8::String> prototypeString =
         v8AtomicString(isolate, "prototype");
     v8::Local<v8::Value> prototypeValue;
@@ skipping 16 matching lines @@
     const CustomElementDescriptor& descriptor)
 {
     return ScriptCustomElementDefinition::create(
         m_scriptState.get(),
         m_registry,
         descriptor,
         m_constructor,
         m_prototype);
 }
 
+v8::Local<v8::Set> ScriptCustomElementDefinitionBuilder
+    ::constructorsBeingDefined()
+{
+    V8PrivateProperty::Symbol symbol =
+        V8PrivateProperty::getCustomElementsRegistryConstructorsBeingDefined(
+            m_scriptState->isolate());
+    v8::Local<v8::Object> wrapper =
+        toV8(m_registry.get(), m_scriptState.get()).As<v8::Object>();
+    v8::Local<v8::Value> set = symbol.get(m_scriptState->context(), wrapper);
+    if (set.IsEmpty()) {
+        set = v8::Set::New(m_scriptState->isolate());
+        symbol.set(m_scriptState->context(), wrapper, set);
+    }
+    return set.As<v8::Set>();
+}
+
 } // namespace blink