Upgrading custom element should enqueue attributeChanged and connected callbacks
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Oct 2016 08:15:52 +0000 (08:15 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Oct 2016 08:15:52 +0000 (08:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163840

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaselined the tests as more test cases are passing now.

* web-platform-tests/custom-elements/reactions/Node-expected.txt:
* web-platform-tests/custom-elements/reactions/Range-expected.txt:

Source/WebCore:

When upgrading a custom element, enqueue attributeChanged and connectedCallbacks as needed as specified
in step 3 and 4 of: https://html.spec.whatwg.org/multipage/scripting.html#concept-upgrade-an-element

Test: fast/custom-elements/upgrading-enqueue-reactions.html

* bindings/js/JSCustomElementInterface.cpp:
(WebCore::JSCustomElementInterface::upgradeElement): Enqueue
* dom/CustomElementReactionQueue.cpp:
(WebCore::CustomElementReactionQueueItem::invoke): Don't invoke callbacks when the custom element had
failed to upgrade.
(WebCore::CustomElementReactionQueue::enqueuePostUpgradeReactions): Added.
(WebCore::CustomElementReactionQueue::invokeAll): Upgrading a custom element may enqueue more reactions.
Keep invoking reactions until the queue becomes empty.
* dom/CustomElementReactionQueue.h:
* dom/Range.idl: Added a forgotten CEReactions here.

LayoutTests:

Added a W3C style testharness.js test for making sure upgrading custom custom elements
would enqueue attributedChanged and connected reactions.

* fast/custom-elements/upgrading-enqueue-reactions-expected.txt: Added.
* fast/custom-elements/upgrading-enqueue-reactions.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/custom-elements/upgrading-enqueue-reactions-expected.txt [new file with mode: 0644]
LayoutTests/fast/custom-elements/upgrading-enqueue-reactions.html [new file with mode: 0644]
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/custom-elements/reactions/Node-expected.txt
LayoutTests/imported/w3c/web-platform-tests/custom-elements/reactions/Range-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCustomElementInterface.cpp
Source/WebCore/dom/CustomElementReactionQueue.cpp
Source/WebCore/dom/CustomElementReactionQueue.h
Source/WebCore/dom/Range.idl

index 40059a9..496bf1c 100644 (file)
@@ -1,3 +1,16 @@
+2016-10-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Upgrading custom element should enqueue attributeChanged and connected callbacks
+        https://bugs.webkit.org/show_bug.cgi?id=163840
+
+        Reviewed by Darin Adler.
+
+        Added a W3C style testharness.js test for making sure upgrading custom custom elements
+        would enqueue attributedChanged and connected reactions.
+
+        * fast/custom-elements/upgrading-enqueue-reactions-expected.txt: Added.
+        * fast/custom-elements/upgrading-enqueue-reactions.html: Added.
+
 2016-10-21  Ryosuke Niwa  <rniwa@webkit.org>
 
         Update custom elements tests
diff --git a/LayoutTests/fast/custom-elements/upgrading-enqueue-reactions-expected.txt b/LayoutTests/fast/custom-elements/upgrading-enqueue-reactions-expected.txt
new file mode 100644 (file)
index 0000000..78e03e4
--- /dev/null
@@ -0,0 +1,8 @@
+CONSOLE MESSAGE: line 157: Exception thrown as a part of test
+
+PASS Upgrading a custom element must enqueue attributeChangedCallback on each attribute 
+PASS Upgrading a custom element not must enqueue attributeChangedCallback on unobserved attributes 
+PASS Upgrading a custom element must enqueue connectedCallback if the element in the document 
+PASS Upgrading a custom element must enqueue attributeChangedCallback before connectedCallback 
+PASS Upgrading a custom element must not invoke attributeChangedCallback and connectedCallback when the element failed to upgrade 
+
diff --git a/LayoutTests/fast/custom-elements/upgrading-enqueue-reactions.html b/LayoutTests/fast/custom-elements/upgrading-enqueue-reactions.html
new file mode 100644 (file)
index 0000000..45e4316
--- /dev/null
@@ -0,0 +1,176 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Custom Elements: CEReactions on Attr interface</title>
+<meta name="author" title="Ryosuke Niwa" href="mailto:rniwa@webkit.org">
+<meta name="assert" content="value of Attr interface must have CEReactions">
+<meta name="help" content="https://dom.spec.whatwg.org/#node">
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../imported/w3c/web-platform-tests/custom-elements/resources/custom-elements-helpers.js"></script>
+</head>
+<body>
+<div id="log"></div>
+<script>
+setup({allow_uncaught_exception:true});
+
+function create_constructor_log(element) {
+    return {type: 'constructed', element: element};
+}
+
+function assert_constructor_log_entry(log, element) {
+    assert_equals(log.type, 'constructed');
+    assert_equals(log.element, element);
+}
+
+function create_connected_callback_log(element) {
+    return {type: 'connected', element: element};
+}
+
+function assert_connected_log_entry(log, element) {
+    assert_equals(log.type, 'connected');
+    assert_equals(log.element, element);
+}
+
+test_with_window(function (contentWindow) {
+    const contentDocument = contentWindow.document;
+    contentDocument.write('<test-element id="some" title="This is a test">');
+
+    const undefinedElement = contentDocument.querySelector('test-element');
+    assert_equals(Object.getPrototypeOf(undefinedElement), contentWindow.HTMLElement.prototype);
+
+    let log = [];
+    class TestElement extends contentWindow.HTMLElement {
+        constructor() {
+            super();
+            log.push(create_constructor_log(this));
+        }
+        attributeChangedCallback(...args) {
+            log.push(create_attribute_changed_callback_log(this, ...args));
+        }
+        static get observedAttributes() { return ['id', 'title']; }
+    }
+    contentWindow.customElements.define('test-element', TestElement);
+    assert_equals(Object.getPrototypeOf(undefinedElement), TestElement.prototype);
+
+    assert_equals(log.length, 3);
+    assert_constructor_log_entry(log[0], undefinedElement);
+    assert_attribute_log_entry(log[1], {name: 'id', oldValue: null, newValue: 'some', namespace: null});
+    assert_attribute_log_entry(log[2], {name: 'title', oldValue: null, newValue: 'This is a test', namespace: null});
+}, 'Upgrading a custom element must enqueue attributeChangedCallback on each attribute');
+
+test_with_window(function (contentWindow) {
+    const contentDocument = contentWindow.document;
+    contentDocument.write('<test-element id="some" title="This is a test" class="foo">');
+
+    const undefinedElement = contentDocument.querySelector('test-element');
+    assert_equals(Object.getPrototypeOf(undefinedElement), contentWindow.HTMLElement.prototype);
+
+    let log = [];
+    class TestElement extends contentWindow.HTMLElement {
+        constructor() {
+            super();
+            log.push(create_constructor_log(this));
+        }
+        attributeChangedCallback(...args) {
+            log.push(create_attribute_changed_callback_log(this, ...args));
+        }
+        static get observedAttributes() { return ['class', 'id']; }
+    }
+    contentWindow.customElements.define('test-element', TestElement);
+    assert_equals(Object.getPrototypeOf(undefinedElement), TestElement.prototype);
+
+    assert_equals(log.length, 3);
+    assert_constructor_log_entry(log[0], undefinedElement);
+    assert_attribute_log_entry(log[1], {name: 'id', oldValue: null, newValue: 'some', namespace: null});
+    assert_attribute_log_entry(log[2], {name: 'class', oldValue: null, newValue: 'foo', namespace: null});
+}, 'Upgrading a custom element not must enqueue attributeChangedCallback on unobserved attributes');
+
+test_with_window(function (contentWindow) {
+    const contentDocument = contentWindow.document;
+    contentDocument.write('<test-element id="some" title="This is a test" class="foo">');
+
+    const undefinedElement = contentDocument.querySelector('test-element');
+    assert_equals(Object.getPrototypeOf(undefinedElement), contentWindow.HTMLElement.prototype);
+
+    let log = [];
+    class TestElement extends contentWindow.HTMLElement {
+        constructor() {
+            super();
+            log.push(create_constructor_log(this));
+        }
+        connectedCallback(...args) {
+            log.push(create_connected_callback_log(this, ...args));
+        }
+    }
+    contentWindow.customElements.define('test-element', TestElement);
+    assert_equals(Object.getPrototypeOf(undefinedElement), TestElement.prototype);
+
+    assert_equals(log.length, 2);
+    assert_constructor_log_entry(log[0], undefinedElement);
+    assert_connected_log_entry(log[1], undefinedElement);
+}, 'Upgrading a custom element must enqueue connectedCallback if the element in the document');
+
+test_with_window(function (contentWindow) {
+    const contentDocument = contentWindow.document;
+    contentDocument.write('<test-element id="some" title="This is a test" class="foo">');
+
+    const undefinedElement = contentDocument.querySelector('test-element');
+    assert_equals(Object.getPrototypeOf(undefinedElement), contentWindow.HTMLElement.prototype);
+
+    let log = [];
+    class TestElement extends contentWindow.HTMLElement {
+        constructor() {
+            super();
+            log.push(create_constructor_log(this));
+        }
+        connectedCallback(...args) {
+            log.push(create_connected_callback_log(this, ...args));
+        }
+        attributeChangedCallback(...args) {
+            log.push(create_attribute_changed_callback_log(this, ...args));
+        }
+        static get observedAttributes() { return ['class', 'id']; }
+    }
+    contentWindow.customElements.define('test-element', TestElement);
+    assert_equals(Object.getPrototypeOf(undefinedElement), TestElement.prototype);
+
+    assert_equals(log.length, 4);
+    assert_constructor_log_entry(log[0], undefinedElement);
+    assert_attribute_log_entry(log[1], {name: 'id', oldValue: null, newValue: 'some', namespace: null});
+    assert_attribute_log_entry(log[2], {name: 'class', oldValue: null, newValue: 'foo', namespace: null});
+    assert_connected_log_entry(log[3], undefinedElement);
+}, 'Upgrading a custom element must enqueue attributeChangedCallback before connectedCallback');
+
+test_with_window(function (contentWindow) {
+    const contentDocument = contentWindow.document;
+    contentDocument.write('<test-element id="some" title="This is a test" class="foo">');
+
+    const undefinedElement = contentDocument.querySelector('test-element');
+    assert_equals(Object.getPrototypeOf(undefinedElement), contentWindow.HTMLElement.prototype);
+
+    let log = [];
+    class TestElement extends contentWindow.HTMLElement {
+        constructor() {
+            super();
+            log.push(create_constructor_log(this));
+            throw 'Exception thrown as a part of test';
+        }
+        connectedCallback(...args) {
+            log.push(create_connected_callback_log(this, ...args));
+        }
+        attributeChangedCallback(...args) {
+            log.push(create_attribute_changed_callback_log(this, ...args));
+        }
+        static get observedAttributes() { return ['class', 'id']; }
+    }
+    contentWindow.customElements.define('test-element', TestElement);
+    assert_equals(Object.getPrototypeOf(undefinedElement), TestElement.prototype);
+
+    assert_equals(log.length, 1);
+    assert_constructor_log_entry(log[0], undefinedElement);
+}, 'Upgrading a custom element must not invoke attributeChangedCallback and connectedCallback when the element failed to upgrade');
+
+</script>
+</body>
+</html>
index dc22377..5a284be 100644 (file)
@@ -1,3 +1,15 @@
+2016-10-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Upgrading custom element should enqueue attributeChanged and connected callbacks
+        https://bugs.webkit.org/show_bug.cgi?id=163840
+
+        Reviewed by Darin Adler.
+
+        Rebaselined the tests as more test cases are passing now.
+
+        * web-platform-tests/custom-elements/reactions/Node-expected.txt:
+        * web-platform-tests/custom-elements/reactions/Range-expected.txt:
+
 2016-10-21  Ryosuke Niwa  <rniwa@webkit.org>
 
         Update custom elements tests
index a58769e..fce63d0 100644 (file)
@@ -3,9 +3,9 @@ PASS nodeValue on Node must enqueue an attributeChanged reaction when replacing
 PASS nodeValue on Node must not enqueue an attributeChanged reaction when replacing an existing unobserved attribute 
 FAIL textContent on Node must enqueue an attributeChanged reaction when replacing an existing attribute assert_array_equals: lengths differ, expected 1 got 2
 PASS textContent on Node must not enqueue an attributeChanged reaction when replacing an existing unobserved attribute 
-FAIL cloneNode on Node must enqueue an attributeChanged reaction when cloning an element with an observed attribute assert_array_equals: lengths differ, expected 2 got 1
+PASS cloneNode on Node must enqueue an attributeChanged reaction when cloning an element with an observed attribute 
 PASS cloneNode on Node must not enqueue an attributeChanged reaction when cloning an element with an unobserved attribute 
-FAIL cloneNode on Node must enqueue an attributeChanged reaction when cloning an element only for observed attributes assert_array_equals: lengths differ, expected 3 got 1
+PASS cloneNode on Node must enqueue an attributeChanged reaction when cloning an element only for observed attributes 
 PASS insertBefore on ChildNode must enqueue a connected reaction 
 PASS insertBefore on ChildNode must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document 
 PASS appendChild on ChildNode must enqueue a connected reaction 
index 41efd1d..4663be7 100644 (file)
@@ -1,12 +1,12 @@
 
 PASS deleteContents on Range must enqueue a disconnected reaction 
 PASS extractContents on Range must enqueue a disconnected reaction 
-FAIL cloneContents on Range must enqueue an attributeChanged reaction when cloning an element with an observed attribute assert_array_equals: lengths differ, expected 2 got 1
+PASS cloneContents on Range must enqueue an attributeChanged reaction when cloning an element with an observed attribute 
 PASS cloneContents on Range must not enqueue an attributeChanged reaction when cloning an element with an unobserved attribute 
-FAIL cloneContents on Range must enqueue an attributeChanged reaction when cloning an element only for observed attributes assert_array_equals: lengths differ, expected 3 got 1
+PASS cloneContents on Range must enqueue an attributeChanged reaction when cloning an element only for observed attributes 
 PASS insertNode on Range must enqueue a connected reaction 
 PASS insertNode on Range must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document 
 PASS surroundContents on Range must enqueue a connected reaction 
 PASS surroundContents on Range must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document 
-FAIL createContextualFragment on Range must construct a custom element assert_array_equals: lengths differ, expected 2 got 1
+PASS createContextualFragment on Range must construct a custom element 
 
index 6da67ec..21983b9 100644 (file)
@@ -1,3 +1,26 @@
+2016-10-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Upgrading custom element should enqueue attributeChanged and connected callbacks
+        https://bugs.webkit.org/show_bug.cgi?id=163840
+
+        Reviewed by Darin Adler.
+
+        When upgrading a custom element, enqueue attributeChanged and connectedCallbacks as needed as specified
+        in step 3 and 4 of: https://html.spec.whatwg.org/multipage/scripting.html#concept-upgrade-an-element
+
+        Test: fast/custom-elements/upgrading-enqueue-reactions.html
+
+        * bindings/js/JSCustomElementInterface.cpp:
+        (WebCore::JSCustomElementInterface::upgradeElement): Enqueue 
+        * dom/CustomElementReactionQueue.cpp:
+        (WebCore::CustomElementReactionQueueItem::invoke): Don't invoke callbacks when the custom element had
+        failed to upgrade.
+        (WebCore::CustomElementReactionQueue::enqueuePostUpgradeReactions): Added.
+        (WebCore::CustomElementReactionQueue::invokeAll): Upgrading a custom element may enqueue more reactions.
+        Keep invoking reactions until the queue becomes empty.
+        * dom/CustomElementReactionQueue.h:
+        * dom/Range.idl: Added a forgotten CEReactions here.
+
 2016-10-21  David Kilzer  <ddkilzer@apple.com>
 
         Bug 163762: IntSize::area() should used checked arithmetic
index 8d39389..ac452d9 100644 (file)
@@ -182,6 +182,8 @@ void JSCustomElementInterface::upgradeElement(Element& element)
         return;
     }
 
+    CustomElementReactionQueue::enqueuePostUpgradeReactions(element, *this);
+
     m_constructionStack.append(&element);
 
     MarkedArgumentBuffer args;
index 6d11538..67df9ef 100644 (file)
@@ -75,6 +75,8 @@ public:
 
     void invoke()
     {
+        if (m_element->isFailedCustomElement())
+            return;
         switch (m_type) {
         case Type::ElementUpgrade:
             m_interface->upgradeElement(m_element.get());
@@ -188,12 +190,34 @@ void CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded(Element
         queue->m_items.append({element, *elementInterface, attributeName, oldValue, newValue});
 }
 
+void CustomElementReactionQueue::enqueuePostUpgradeReactions(Element& element, JSCustomElementInterface& elementInterface)
+{
+    if (!element.hasAttributes() && !element.inDocument())
+        return;
+
+    auto* queue = CustomElementReactionStack::ensureCurrentQueue();
+    if (!queue)
+        return;
+
+    if (element.hasAttributes()) {
+        for (auto& attribute : element.attributesIterator()) {
+            if (elementInterface.observesAttribute(attribute.localName()))
+                queue->m_items.append({element, elementInterface, attribute.name(), nullAtom, attribute.value()});
+        }
+    }
+
+    if (element.inDocument() && elementInterface.hasConnectedCallback())
+        queue->m_items.append({CustomElementReactionQueueItem::Type::Connected, element, elementInterface});
+}
+
 void CustomElementReactionQueue::invokeAll()
 {
-    Vector<CustomElementReactionQueueItem> items;
-    items.swap(m_items);
-    for (auto& item : items)
-        item.invoke();
+    // FIXME: This queue needs to be per element.
+    while (!m_items.isEmpty()) {
+        Vector<CustomElementReactionQueueItem> items = WTFMove(m_items);
+        for (auto& item : items)
+            item.invoke();
+    }
 }
 
 CustomElementReactionQueue* CustomElementReactionStack::ensureCurrentQueue()
index cc38103..77f0607 100644 (file)
@@ -51,6 +51,7 @@ public:
     static void enqueueDisconnectedCallbackIfNeeded(Element&);
     static void enqueueAdoptedCallbackIfNeeded(Element&, Document& oldDocument, Document& newDocument);
     static void enqueueAttributeChangedCallbackIfNeeded(Element&, const QualifiedName&, const AtomicString& oldValue, const AtomicString& newValue);
+    static void enqueuePostUpgradeReactions(Element&, JSCustomElementInterface&);
 
     void invokeAll();
 
index 783a01e..a568168 100644 (file)
@@ -67,7 +67,7 @@
     ClientRectList getClientRects();
     ClientRect getBoundingClientRect();
 
-    [MayThrowLegacyException, NewObject] DocumentFragment createContextualFragment(DOMString html);
+    [CEReactions, MayThrowLegacyException, NewObject] DocumentFragment createContextualFragment(DOMString html);
 
     [MayThrowLegacyException] short compareNode(Node refNode);