Add the check for reentrancy to CustomElementRegistry
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Aug 2016 19:31:11 +0000 (19:31 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Aug 2016 19:31:11 +0000 (19:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161423

Reviewed by Antti Koivisto.

Source/WebCore:

Added the "element definition is running" flag to JSCustomElementRegistry:
https://html.spec.whatwg.org/multipage/scripting.html#element-definition-is-running

And added an exception for when this flag is set during JSCustomElementRegistry::define:
https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementregistry-define

Tests: fast/custom-elements/CustomElementRegistry.html

* bindings/js/JSCustomElementRegistryCustom.cpp:
(WebCore::JSCustomElementRegistry::define): Throw NotSupportedError when m_elementDefinitionIsRunning is true.
* dom/CustomElementRegistry.h:
(WebCore::CustomElementRegistry::elementDefinitionIsRunning): Added.

LayoutTests:

Add test cases for reentrancy during customElements.define.

* fast/custom-elements/CustomElementRegistry-expected.txt:
* fast/custom-elements/CustomElementRegistry.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205261 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/custom-elements/CustomElementRegistry-expected.txt
LayoutTests/fast/custom-elements/CustomElementRegistry.html
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCustomElementRegistryCustom.cpp
Source/WebCore/dom/CustomElementRegistry.h

index fce8839..eac44ea 100644 (file)
@@ -1,3 +1,15 @@
+2016-08-31  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add the check for reentrancy to CustomElementRegistry
+        https://bugs.webkit.org/show_bug.cgi?id=161423
+
+        Reviewed by Antti Koivisto.
+
+        Add test cases for reentrancy during customElements.define.
+
+        * fast/custom-elements/CustomElementRegistry-expected.txt:
+        * fast/custom-elements/CustomElementRegistry.html:
+
 2016-08-31  Chris Dumez  <cdumez@apple.com>
 
         Object.getPrototypeOf() should return null cross-origin
index 92bc20f..cd06614 100644 (file)
@@ -1,9 +1,14 @@
 
 PASS CustomElementRegistry interface must have define as a method 
+PASS customElements.define must throw when the element interface is not a constructor 
 PASS customElements.define must throw with an invalid name 
 PASS customElements.define must throw when there is already a custom element of the same name 
-PASS customElements.define must throw when there is already a custom element with the same class 
-PASS customElements.define must throw when the element interface is not a constructor 
+PASS customElements.define must throw a NotSupportedError when there is already a custom element with the same class 
+PASS customElements.define must throw a NotSupportedError when element definition is running flag is set 
+PASS customElements.define must check IsConstructor on the constructor before checking the element definition is running flag 
+PASS customElements.define must validate the custom element name before checking the element definition is running flag 
+PASS customElements.define must not throw when defining another custom element in a different global object during Get(constructor, "prototype") 
+PASS Custom Elements: CustomElementRegistry interface 
 PASS customElements.define must get "prototype" property of the constructor 
 PASS customElements.define must rethrow an exception thrown while getting "prototype" property of the constructor 
 PASS customElements.define must throw when "prototype" property of the constructor is not an object 
index f1e924d..b170786 100644 (file)
@@ -18,6 +18,17 @@ test(function () {
 }, 'CustomElementRegistry interface must have define as a method');
 
 test(function () {
+    assert_throws({'name': 'TypeError'}, function () { customElements.define('badname', 1); },
+        'customElements.define must throw a TypeError when the element interface is a number');
+    assert_throws({'name': 'TypeError'}, function () { customElements.define('badname', '123'); },
+        'customElements.define must throw a TypeError when the element interface is a string');
+    assert_throws({'name': 'TypeError'}, function () { customElements.define('badname', {}); },
+        'customElements.define must throw a TypeError when the element interface is an object');
+    assert_throws({'name': 'TypeError'}, function () { customElements.define('badname', []); },
+        'customElements.define must throw a TypeError when the element interface is an array');
+}, 'customElements.define must throw when the element interface is not a constructor');
+
+test(function () {
     class MyCustomElement extends HTMLElement {};
 
     assert_throws({'name': 'SyntaxError'}, function () { customElements.define(null, MyCustomElement); },
@@ -49,11 +60,19 @@ test(function () {
 
 test(function () {
     class SomeCustomElement extends HTMLElement {};
-    class OtherCustomElement extends HTMLElement {};
+
+    var calls = [];
+    var OtherCustomElement = new Proxy(class extends HTMLElement {}, {
+        get: function (target, name) {
+            calls.push(name);
+            return target[name];
+        }
+    })
 
     customElements.define('some-custom-element', SomeCustomElement);
     assert_throws({'name': 'NotSupportedError'}, function () { customElements.define('some-custom-element', OtherCustomElement); },
         'customElements.define must throw a NotSupportedError if the specified tag name is already used');
+    assert_array_equals(calls, [], 'customElements.define must validate the custom element name before getting the prototype of the constructor');
 
 }, 'customElements.define must throw when there is already a custom element of the same name');
 
@@ -64,18 +83,111 @@ test(function () {
     assert_throws({'name': 'NotSupportedError'}, function () { customElements.define('some-other-element', AnotherCustomElement); },
         'customElements.define must throw a NotSupportedError if the specified class already defines an element');
 
-}, 'customElements.define must throw when there is already a custom element with the same class');
+}, 'customElements.define must throw a NotSupportedError when there is already a custom element with the same class');
 
 test(function () {
-    assert_throws({'name': 'TypeError'}, function () { customElements.define('invalid-element', 1); },
-        'customElements.define must throw a TypeError when the element interface is a number');
-    assert_throws({'name': 'TypeError'}, function () { customElements.define('invalid-element', '123'); },
-        'customElements.define must throw a TypeError when the element interface is a string');
-    assert_throws({'name': 'TypeError'}, function () { customElements.define('invalid-element', {}); },
-        'customElements.define must throw a TypeError when the element interface is an object');
-    assert_throws({'name': 'TypeError'}, function () { customElements.define('invalid-element', []); },
-        'customElements.define must throw a TypeError when the element interface is an array');
-}, 'customElements.define must throw when the element interface is not a constructor');
+    var outerCalls = [];
+    var OuterCustomElement = new Proxy(class extends HTMLElement { }, {
+        get: function (target, name) {
+            outerCalls.push(name);
+            customElements.define('inner-custom-element', InnerCustomElement);
+            return target[name];
+        }
+    });
+    var innerCalls = [];
+    var InnerCustomElement = new Proxy(class extends HTMLElement { }, {
+        get: function (target, name) {
+            outerCalls.push(name);
+            return target[name];
+        }
+    });
+
+    assert_throws({'name': 'NotSupportedError'}, function () { customElements.define('outer-custom-element', OuterCustomElement); },
+        'customElements.define must throw a NotSupportedError if the specified class already defines an element');
+    assert_array_equals(outerCalls, ['prototype'], 'customElements.define must get "prototype"');
+    assert_array_equals(innerCalls, [],
+        'customElements.define must throw a NotSupportedError when element definition is running flag is set'
+        + ' before getting the prototype of the constructor');
+
+}, 'customElements.define must throw a NotSupportedError when element definition is running flag is set');
+
+test(function () {
+    var calls = [];
+    var ElementWithBadInnerConstructor = new Proxy(class extends HTMLElement { }, {
+        get: function (target, name) {
+            calls.push(name);
+            customElements.define('inner-custom-element', 1);
+            return target[name];
+        }
+    });
+
+    assert_throws({'name': 'TypeError'}, function () {
+        customElements.define('element-with-bad-inner-constructor', ElementWithBadInnerConstructor);
+    }, 'customElements.define must throw a NotSupportedError if IsConstructor(constructor) is false');
+
+    assert_array_equals(calls, ['prototype'], 'customElements.define must get "prototype"');
+}, 'customElements.define must check IsConstructor on the constructor before checking the element definition is running flag');
+
+test(function () {
+    var calls = [];
+    var ElementWithBadInnerName = new Proxy(class extends HTMLElement { }, {
+        get: function (target, name) {
+            calls.push(name);
+            customElements.define('badname', class extends HTMLElement {});
+            return target[name];
+        }
+    });
+
+    assert_throws({'name': 'SyntaxError'}, function () {
+        customElements.define('element-with-bad-inner-name', ElementWithBadInnerName);
+    }, 'customElements.define must throw a SyntaxError if the specified name is not a valid custom element name');
+
+    assert_array_equals(calls, ['prototype'], 'customElements.define must get "prototype"');
+}, 'customElements.define must validate the custom element name before checking the element definition is running flag');
+
+(function () {
+    var testCase = async_test('customElements.define must not throw'
+        +' when defining another custom element in a different global object during Get(constructor, "prototype")', {timeout: 100});
+
+    var iframe = document.createElement('iframe');
+    iframe.onload = function () {
+        testCase.step(function () {
+            var InnerCustomElement = class extends iframe.contentWindow.HTMLElement {};
+            var calls = [];
+            var proxy = new Proxy(class extends HTMLElement { }, {
+                get: function (target, name) {
+                    calls.push(name);
+                    iframe.contentWindow.customElements.define('another-custom-element', InnerCustomElement);
+                    return target[name];
+                }
+            })
+            customElements.define('element-with-inner-element-define', proxy);
+            assert_array_equals(calls, ['prototype'], 'customElements.define must get "prototype"');
+            assert_true(iframe.contentDocument.createElement('another-custom-element') instanceof InnerCustomElement);
+        });
+        document.body.removeChild(iframe);
+        testCase.done();
+    }
+
+    document.body.appendChild(iframe);
+})();
+
+test(function () {
+    var calls = [];
+    var ElementWithBadInnerName = new Proxy(class extends HTMLElement { }, {
+        get: function (target, name) {
+            calls.push(name);
+            customElements.define('badname', class extends HTMLElement {});
+            return target[name];
+        }
+    });
+
+    assert_throws({'name': 'SyntaxError'}, function () {
+        customElements.define('element-with-bad-inner-name', ElementWithBadInnerName);
+    }, 'customElements.define must throw a SyntaxError if the specified name is not a valid custom element name');
+
+    assert_array_equals(calls, ['prototype'], 'customElements.define must get "prototype"');
+}, '');
 
 test(function () {
     var calls = [];
index ad94520..5078142 100644 (file)
@@ -1,3 +1,23 @@
+2016-08-31  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add the check for reentrancy to CustomElementRegistry
+        https://bugs.webkit.org/show_bug.cgi?id=161423
+
+        Reviewed by Antti Koivisto.
+
+        Added the "element definition is running" flag to JSCustomElementRegistry:
+        https://html.spec.whatwg.org/multipage/scripting.html#element-definition-is-running
+
+        And added an exception for when this flag is set during JSCustomElementRegistry::define:
+        https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementregistry-define
+
+        Tests: fast/custom-elements/CustomElementRegistry.html
+
+        * bindings/js/JSCustomElementRegistryCustom.cpp:
+        (WebCore::JSCustomElementRegistry::define): Throw NotSupportedError when m_elementDefinitionIsRunning is true.
+        * dom/CustomElementRegistry.h:
+        (WebCore::CustomElementRegistry::elementDefinitionIsRunning): Added.
+
 2016-08-30  Ryosuke Niwa  <rniwa@webkit.org>
 
         Avoid using strong reference in JSDOMPromiseā€™s DeferredWrapper
index 8f731b6..1fb6f27 100644 (file)
@@ -92,6 +92,13 @@ JSValue JSCustomElementRegistry::define(ExecState& state)
     // https://github.com/w3c/webcomponents/issues/545
 
     CustomElementRegistry& registry = wrapped();
+
+    if (registry.elementDefinitionIsRunning()) {
+        throwNotSupportedError(state, scope, ASCIILiteral("Cannot define a custom element while defining another custom element"));
+        return jsUndefined();
+    }
+    TemporaryChange<bool> change(registry.elementDefinitionIsRunning(), true);
+
     if (registry.findInterface(localName)) {
         throwNotSupportedError(state, scope, ASCIILiteral("Cannot define multiple custom elements with the same tag name"));
         return jsUndefined();
index 00353d6..8b103c7 100644 (file)
@@ -29,6 +29,7 @@
 
 #include "QualifiedName.h"
 #include <wtf/HashMap.h>
+#include <wtf/TemporaryChange.h>
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/AtomicStringHash.h>
 
@@ -41,6 +42,7 @@ class JSValue;
 
 namespace WebCore {
 
+class CustomElementRegistry;
 class Element;
 class JSCustomElementInterface;
 class QualifiedName;
@@ -53,6 +55,8 @@ public:
     void addElementDefinition(Ref<JSCustomElementInterface>&&);
     void addUpgradeCandidate(Element&);
 
+    bool& elementDefinitionIsRunning() { return m_elementDefinitionIsRunning; }
+
     JSCustomElementInterface* findInterface(const QualifiedName&) const;
     JSCustomElementInterface* findInterface(const AtomicString&) const;
     JSCustomElementInterface* findInterface(const JSC::JSObject*) const;
@@ -66,6 +70,10 @@ private:
     HashMap<AtomicString, Vector<RefPtr<Element>>> m_upgradeCandidatesMap;
     HashMap<AtomicString, Ref<JSCustomElementInterface>> m_nameMap;
     HashMap<const JSC::JSObject*, JSCustomElementInterface*> m_constructorMap;
+
+    bool m_elementDefinitionIsRunning { false };
+
+    friend class ElementDefinitionIsRunningTemporaryChange;
 };
 
 }