⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 266269 in webkit


Ignore:
Timestamp:
Aug 27, 2020, 7:55:21 PM (6 years ago)
Author:
rniwa@webkit.org
Message:

Prevent infinite recursion when upgrading custom elements
​https://bugs.webkit.org/show_bug.cgi?id=206605

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

Rebaselined the test now that one more test case is passing.

  • web-platform-tests/custom-elements/upgrading-expected.txt:

Source/WebCore:

This patch updates our implementation of the concept to upgrade an element [1] and related algorithms
to match the latest HTML5 specification. In particular, it incorporates the algorithmic change [2] to
prevent infinite recursion an element is re-inserted into a document tree inside its constructor.

The key code change is in JSCustomElementInterface::upgradeElement where this patch adds an early exit
when custom element is not "undefined" or "uncustomized" and the custom element state is set to "failed"
immediately before invoking the constructor. The rest of code changes deals with this "failed" state
appearing during upgrades and updates various debug assertions.

[1] ​https://html.spec.whatwg.org/multipage/custom-elements.html#concept-upgrade-an-element
[2] ​https://github.com/whatwg/html/pull/5126
[3] ​https://html.spec.whatwg.org/multipage/custom-elements.html#concept-try-upgrade

Test: imported/w3c/web-platform-tests/custom-elements/upgrading.html

  • bindings/js/JSCustomElementInterface.cpp:

(WebCore::JSCustomElementInterface::constructElementWithFallback):
(WebCore::JSCustomElementInterface::upgradeElement): Implements the new behavior. Note that we still
need to clear the queue where we used to set the custom element state to "failed" to avoid memory leaks.

  • dom/CustomElementReactionQueue.cpp:

(WebCore::CustomElementReactionQueue::hasJustUpgradeReaction const): Added.
(WebCore::CustomElementReactionQueue::enqueueElementUpgrade): Enqueue the element to the element queue
even if it had already been scheduled to upgrade previously. This makes the innermost attempt to upgrade
to succeed instead of the outermost. Also updated debug assertions.
(WebCore::CustomElementReactionQueue::tryToUpgradeElement): Renamed from enqueueElementUpgradeIfDefined
to match the spec's name [3]. Unlike the concept in the spec, this function doesn't get called when
the custom element state of the elemnt is either "undefined" or "uncustomized" to avoid unnecessary work.

  • dom/CustomElementReactionQueue.h:

(WebCore::CustomElementReactionQueue::isEmpty const): Added.

  • dom/CustomElementRegistry.cpp:

(WebCore::upgradeElementsInShadowIncludingDescendants):
(WebCore::CustomElementRegistry::upgrade):

  • dom/Document.cpp:

(WebCore::createFallbackHTMLElement): Call setIsCustomElementUpgradeCandidate on a newly created since
we can no longer update node flags in enqueueToUpgrade

  • dom/Element.cpp:

(WebCore::Element::insertedIntoAncestor):
(WebCore::Element::setIsFailedCustomElement): Removed the unused function argument.
(WebCore::Element::setIsFailedCustomElementWithoutClearingReactionQueue): Extracted from
setIsFailedCustomElement.
(WebCore::Element::clearReactionQueueFromFailedCustomElement): Ditto.
(WebCore::Element::enqueueToUpgrade): No longer updates node flags as this would clear "failed" state
from a custom element which is currently being upgraded and cause all sorts of issues.
(WebCore::Element::reactionQueue const): Updated debug assertions.

  • dom/Element.h:

LayoutTests:

Removed the crash expectation from a test now that it's passing.

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r266264 r266269  
     12020-08-27  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Prevent infinite recursion when upgrading custom elements
     4        https://bugs.webkit.org/show_bug.cgi?id=206605
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Removed the crash expectation from a test now that it's passing.
     9
     10        * TestExpectations:
     11
    1122020-08-27  Alexey Shvayka  <shvaikalesh@gmail.com>
    213

  • trunk/LayoutTests/TestExpectations

    r266186 r266269  
    568568
    569569# Newly imported WPT tests that are crashing.
    570 [ Debug ] imported/w3c/web-platform-tests/custom-elements/upgrading.html [ Crash ]
    571570imported/w3c/web-platform-tests/html/semantics/embedded-content/the-embed-element/embed-represent-nothing-04.html [ ImageOnlyFailure Crash ]
    572571

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r266261 r266269  
     12020-08-27  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Prevent infinite recursion when upgrading custom elements
     4        https://bugs.webkit.org/show_bug.cgi?id=206605
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Rebaselined the test now that one more test case is passing.
     9
     10        * web-platform-tests/custom-elements/upgrading-expected.txt:
     11
    1122020-08-27  Chris Dumez  <cdumez@apple.com>
    213

  • trunk/LayoutTests/imported/w3c/web-platform-tests/custom-elements/upgrading-expected.txt

    r264020 r266269  
    2727FAIL If definition's disable shadow is true and element's shadow root is non-null, then throw a "NotSupportedError" DOMException. assert_false: Upgrading should fail. expected false got true
    2828PASS Infinite constructor recursion with upgrade(this) should not be possible
    29 FAIL Infinite constructor recursion with appendChild should not be possible assert_array_equals: expected property 0 to be Element node <infinite-cloning-element-2 id="b"></infinite-cloning-ele... but got Element node <infinite-cloning-element-2 id="a"></infinite-cloning-ele... (expected array [Element node <infinite-cloning-element-2 id="b"></infinite-cloning-ele..., "begin"] got [Element node <infinite-cloning-element-2 id="a"></infinite-cloning-ele..., "end"])
     29PASS Infinite constructor recursion with appendChild should not be possible
    3030 

  • trunk/Source/WebCore/ChangeLog

    r266268 r266269  
     12020-08-27  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Prevent infinite recursion when upgrading custom elements
     4        https://bugs.webkit.org/show_bug.cgi?id=206605
     5
     6        Reviewed by Antti Koivisto.
     7
     8        This patch updates our implementation of the concept to upgrade an element [1] and related algorithms
     9        to match the latest HTML5 specification. In particular, it incorporates the algorithmic change [2] to
     10        prevent infinite recursion an element is re-inserted into a document tree inside its constructor.
     11
     12        The key code change is in JSCustomElementInterface::upgradeElement where this patch adds an early exit
     13        when custom element is not "undefined" or "uncustomized" and the custom element state is set to "failed"
     14        immediately before invoking the constructor. The rest of code changes deals with this "failed" state
     15        appearing during upgrades and updates various debug assertions.
     16
     17        [1] https://html.spec.whatwg.org/multipage/custom-elements.html#concept-upgrade-an-element
     18        [2] https://github.com/whatwg/html/pull/5126
     19        [3] https://html.spec.whatwg.org/multipage/custom-elements.html#concept-try-upgrade
     20
     21        Test: imported/w3c/web-platform-tests/custom-elements/upgrading.html
     22
     23        * bindings/js/JSCustomElementInterface.cpp:
     24        (WebCore::JSCustomElementInterface::constructElementWithFallback):
     25        (WebCore::JSCustomElementInterface::upgradeElement): Implements the new behavior. Note that we still
     26        need to clear the queue where we used to set the custom element state to "failed" to avoid memory leaks.
     27        * dom/CustomElementReactionQueue.cpp:
     28        (WebCore::CustomElementReactionQueue::hasJustUpgradeReaction const): Added.
     29        (WebCore::CustomElementReactionQueue::enqueueElementUpgrade): Enqueue the element to the element queue
     30        even if it had already been scheduled to upgrade previously. This makes the innermost attempt to upgrade
     31        to succeed instead of the outermost. Also updated debug assertions.
     32        (WebCore::CustomElementReactionQueue::tryToUpgradeElement): Renamed from enqueueElementUpgradeIfDefined
     33        to match the spec's name [3]. Unlike the concept in the spec, this function doesn't get called when
     34        the custom element state of the elemnt is either "undefined" or "uncustomized" to avoid unnecessary work.
     35        * dom/CustomElementReactionQueue.h:
     36        (WebCore::CustomElementReactionQueue::isEmpty const): Added.
     37        * dom/CustomElementRegistry.cpp:
     38        (WebCore::upgradeElementsInShadowIncludingDescendants):
     39        (WebCore::CustomElementRegistry::upgrade):
     40        * dom/Document.cpp:
     41        (WebCore::createFallbackHTMLElement): Call setIsCustomElementUpgradeCandidate on a newly created since
     42        we can no longer update node flags in enqueueToUpgrade
     43        * dom/Element.cpp:
     44        (WebCore::Element::insertedIntoAncestor):
     45        (WebCore::Element::setIsFailedCustomElement): Removed the unused function argument.
     46        (WebCore::Element::setIsFailedCustomElementWithoutClearingReactionQueue): Extracted from
     47        setIsFailedCustomElement.
     48        (WebCore::Element::clearReactionQueueFromFailedCustomElement): Ditto.
     49        (WebCore::Element::enqueueToUpgrade): No longer updates node flags as this would clear "failed" state
     50        from a custom element which is currently being upgraded and cause all sorts of issues.
     51        (WebCore::Element::reactionQueue const): Updated debug assertions.
     52        * dom/Element.h:
     53
    1542020-08-27  John Wilander  <wilander@apple.com>
    255

  • trunk/Source/WebCore/bindings/js/JSCustomElementInterface.cpp

    r260744 r266269  
    6565    auto element = HTMLUnknownElement::create(QualifiedName(nullAtom(), localName, HTMLNames::xhtmlNamespaceURI), document);
    6666    element->setIsCustomElementUpgradeCandidate();
    67     element->setIsFailedCustomElement(*this);
     67    element->setIsFailedCustomElement();
    6868
    6969    return element;
    … …  
    8080    auto element = HTMLUnknownElement::create(name, document);
    8181    element->setIsCustomElementUpgradeCandidate();
    82     element->setIsFailedCustomElement(*this);
     82    element->setIsFailedCustomElement();
    8383
    8484    return element;
    … …  
    163163}
    164164
     165// https://html.spec.whatwg.org/multipage/custom-elements.html#concept-upgrade-an-element
    165166void JSCustomElementInterface::upgradeElement(Element& element)
    166167{
    167168    ASSERT(element.tagQName() == name());
     169
     170    if (element.isDefinedCustomElement() || element.isFailedCustomElement())
     171        return; // If element's custom element state is not "undefined" or "uncustomized", then return.
     172
    168173    ASSERT(element.isCustomElementUpgradeCandidate());
    169174    if (!canInvokeCallback())
    … …  
    193198
    194199    CustomElementReactionQueue::enqueuePostUpgradeReactions(element);
     200
     201    // Unlike spec, set element's custom element state to "failed" after enqueueing post-upgrade reactions
     202    // to avoid hitting debug assertions in enqueuePostUpgradeReactions.
     203    element.setIsFailedCustomElementWithoutClearingReactionQueue();
    195204
    196205    m_constructionStack.append(&element);
    … …  
    205214
    206215    if (UNLIKELY(scope.exception())) {
    207         element.setIsFailedCustomElement(*this);
     216        element.clearReactionQueueFromFailedCustomElement();
    208217        reportException(lexicalGlobalObject, scope.exception());
    209218        return;
    … …  
    212221    Element* wrappedElement = JSElement::toWrapped(vm, returnedElement);
    213222    if (!wrappedElement || wrappedElement != &element) {
    214         element.setIsFailedCustomElement(*this);
     223        element.clearReactionQueueFromFailedCustomElement();
    215224        reportException(lexicalGlobalObject, createDOMException(lexicalGlobalObject, TypeError, "Custom element constructor returned a wrong element"));
    216225        return;

  • trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp

    r254087 r266269  
    119119}
    120120
     121#if ASSERT_ENABLED
     122bool CustomElementReactionQueue::hasJustUpgradeReaction() const
     123{
     124    return m_items.size() == 1 && m_items[0].type() == CustomElementReactionQueueItem::Type::ElementUpgrade;
     125}
     126#endif
     127
    121128void CustomElementReactionQueue::enqueueElementUpgrade(Element& element, bool alreadyScheduledToUpgrade)
    122129{
    … …  
    124131    ASSERT(element.reactionQueue());
    125132    auto& queue = *element.reactionQueue();
    126     if (alreadyScheduledToUpgrade) {
    127         ASSERT(queue.m_items.size() == 1);
    128         ASSERT(queue.m_items[0].type() == CustomElementReactionQueueItem::Type::ElementUpgrade);
    129     } else {
     133    if (alreadyScheduledToUpgrade)
     134        ASSERT(queue.hasJustUpgradeReaction());
     135    else
    130136        queue.m_items.append({CustomElementReactionQueueItem::Type::ElementUpgrade});
    131         enqueueElementOnAppropriateElementQueue(element);
    132     }
    133 }
    134 
    135 void CustomElementReactionQueue::enqueueElementUpgradeIfDefined(Element& element)
     137    enqueueElementOnAppropriateElementQueue(element);
     138}
     139
     140// https://html.spec.whatwg.org/multipage/custom-elements.html#concept-try-upgrade
     141void CustomElementReactionQueue::tryToUpgradeElement(Element& element)
    136142{
    137143    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());

  • trunk/Source/WebCore/dom/CustomElementReactionQueue.h

    r254087 r266269  
    7171
    7272    static void enqueueElementUpgrade(Element&, bool alreadyScheduledToUpgrade);
    73     static void enqueueElementUpgradeIfDefined(Element&);
     73    static void tryToUpgradeElement(Element&);
    7474    static void enqueueConnectedCallbackIfNeeded(Element&);
    7575    static void enqueueDisconnectedCallbackIfNeeded(Element&);
    … …  
    8181    void invokeAll(Element&);
    8282    void clear();
     83    bool isEmpty() const { return m_items.isEmpty(); }
     84#if ASSERT_ENABLED
     85    bool hasJustUpgradeReaction() const;
     86#endif
    8387
    8488    static void processBackupQueue(CustomElementQueue&);

  • trunk/Source/WebCore/dom/CustomElementRegistry.cpp

    r266157 r266269  
    118118    for (auto& element : descendantsOfType<Element>(root)) {
    119119        if (element.isCustomElementUpgradeCandidate())
    120             CustomElementReactionQueue::enqueueElementUpgradeIfDefined(element);
     120            CustomElementReactionQueue::tryToUpgradeElement(element);
    121121        if (auto* shadowRoot = element.shadowRoot())
    122122            upgradeElementsInShadowIncludingDescendants(*shadowRoot);
    … …  
    130130
    131131    if (is<Element>(root) && downcast<Element>(root).isCustomElementUpgradeCandidate())
    132         CustomElementReactionQueue::enqueueElementUpgradeIfDefined(downcast<Element>(root));
     132        CustomElementReactionQueue::tryToUpgradeElement(downcast<Element>(root));
    133133
    134134    upgradeElementsInShadowIncludingDescendants(downcast<ContainerNode>(root));

  • trunk/Source/WebCore/dom/Document.cpp

    r265782 r266269  
    11521152            if (auto* elementInterface = registry->findInterface(name)) {
    11531153                auto element = HTMLElement::create(name, document);
     1154                element->setIsCustomElementUpgradeCandidate();
    11541155                element->enqueueToUpgrade(*elementInterface);
    11551156                return element;

  • trunk/Source/WebCore/dom/Element.cpp

    r265820 r266269  
    21742174        if (UNLIKELY(isCustomElementUpgradeCandidate())) {
    21752175            ASSERT(isConnected());
    2176             CustomElementReactionQueue::enqueueElementUpgradeIfDefined(*this);
     2176            CustomElementReactionQueue::tryToUpgradeElement(*this);
    21772177        }
    21782178        if (UNLIKELY(isDefinedCustomElement()))
    … …  
    24142414}
    24152415
    2416 void Element::setIsFailedCustomElement(JSCustomElementInterface&)
     2416void Element::setIsFailedCustomElement()
     2417{
     2418    setIsFailedCustomElementWithoutClearingReactionQueue();
     2419    clearReactionQueueFromFailedCustomElement();
     2420}
     2421
     2422void Element::setIsFailedCustomElementWithoutClearingReactionQueue()
    24172423{
    24182424    ASSERT(isUndefinedCustomElement());
    24192425    ASSERT(getFlag(IsEditingTextOrUndefinedCustomElementFlag));
    24202426    clearFlag(IsCustomElement);
    2421 
     2427    InspectorInstrumentation::didChangeCustomElementState(*this);
     2428}
     2429
     2430void Element::clearReactionQueueFromFailedCustomElement()
     2431{
     2432    ASSERT(isFailedCustomElement());
    24222433    if (hasRareData()) {
    24232434        // Clear the queue instead of deleting it since this function can be called inside CustomElementReactionQueue::invokeAll during upgrades.
    … …  
    24252436            queue->clear();
    24262437    }
    2427     InspectorInstrumentation::didChangeCustomElementState(*this);
    24282438}
    24292439
    … …  
    24382448void Element::enqueueToUpgrade(JSCustomElementInterface& elementInterface)
    24392449{
     2450    ASSERT(isCustomElementUpgradeCandidate());
    24402451    ASSERT(!isDefinedCustomElement() && !isFailedCustomElement());
    2441     setFlag(IsCustomElement);
    2442     setFlag(IsEditingTextOrUndefinedCustomElementFlag);
    2443     InspectorInstrumentation::didChangeCustomElementState(*this);
    2444 
    24452452    auto& data = ensureElementRareData();
    24462453    bool alreadyScheduledToUpgrade = data.customElementReactionQueue();
    … …  
    24522459CustomElementReactionQueue* Element::reactionQueue() const
    24532460{
    2454     ASSERT(isDefinedCustomElement() || isCustomElementUpgradeCandidate());
     2461#if ASSERT_ENABLED
     2462    if (isFailedCustomElement()) {
     2463        auto* queue = elementRareData()->customElementReactionQueue();
     2464        ASSERT(queue);
     2465        ASSERT(queue->isEmpty() || queue->hasJustUpgradeReaction());
     2466    } else
     2467        ASSERT(isDefinedCustomElement() || isCustomElementUpgradeCandidate());
     2468#endif
    24552469    if (!hasRareData())
    24562470        return nullptr;

  • trunk/Source/WebCore/dom/Element.h

    r265092 r266269  
    304304
    305305    void setIsDefinedCustomElement(JSCustomElementInterface&);
    306     void setIsFailedCustomElement(JSCustomElementInterface&);
     306    void setIsFailedCustomElement();
     307    void setIsFailedCustomElementWithoutClearingReactionQueue();
     308    void clearReactionQueueFromFailedCustomElement();
    307309    void setIsCustomElementUpgradeCandidate();
    308310    void enqueueToUpgrade(JSCustomElementInterface&);
Note: See TracChangeset for help on using the changeset viewer.