Recursively defining custom elements should not lead to redefinition

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 39da9922a20331c868b3889186b2f20796ec9a0a..3b7fa34a6f59ad09df8b6e5f1a935f8cfc7fa108 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
@@ -11,6 +11,8 @@
 #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 {
@@ -24,9 +26,20 @@ ScriptCustomElementDefinitionBuilder::ScriptCustomElementDefinitionBuilder(
     , 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());
@@ -46,6 +59,7 @@ bool ScriptCustomElementDefinitionBuilder::checkConstructorIntrinsics()
 
 bool ScriptCustomElementDefinitionBuilder::checkConstructorNotRegistered()
 {
+    // Check existing constructors
     if (ScriptCustomElementDefinition::forConstructor(
         m_scriptState.get(),
         m_registry,
@@ -57,6 +71,18 @@ bool ScriptCustomElementDefinitionBuilder::checkConstructorNotRegistered()
             "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;
 }
 
@@ -93,4 +119,20 @@ CustomElementDefinition* ScriptCustomElementDefinitionBuilder::build(
         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